From: Pedro Alves Date: Tue, 29 Mar 2022 12:32:48 +0000 (+0100) Subject: gdbserver: Eliminate prepare_to_access_memory X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=8e347faf8f1556a0f1afc33bd53099ec5f2f8efe;p=binutils-gdb.git gdbserver: Eliminate prepare_to_access_memory Given: - The prepare_to_access_memory machinery was added for non-stop mode. - Only Linux supports non-stop. - Linux no longer needs the prepare_to_access_memory machinery. In fact, after the previous patch, linux_process_target::prepare_to_access_memory became a nop. Thus, prepare_to_access_memory can go away, simplifying core GDBserver code. Change-Id: I93ac8bfe66bd61c3d1c4a0e7d419335163120ecf --- diff --git a/gdbserver/mem-break.cc b/gdbserver/mem-break.cc index 5f5cdb18797..72ce8c8a5cb 100644 --- a/gdbserver/mem-break.cc +++ b/gdbserver/mem-break.cc @@ -1000,13 +1000,19 @@ z_type_supported (char z_type) failure returns NULL and sets *ERR to either -1 for error, or 1 if Z_TYPE breakpoints are not supported on this target. */ -static struct gdb_breakpoint * -set_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int kind, int *err) +struct gdb_breakpoint * +set_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind, int *err) { struct gdb_breakpoint *bp; enum bkpt_type type; enum raw_bkpt_type raw_type; + if (!z_type_supported (z_type)) + { + *err = 1; + return nullptr; + } + /* If we see GDB inserting a second code breakpoint at the same address, then either: GDB is updating the breakpoint's conditions or commands; or, the first breakpoint must have disappeared due @@ -1074,110 +1080,31 @@ set_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int kind, int *err) kind, NULL, err); } -static int -check_gdb_bp_preconditions (char z_type, int *err) -{ - /* As software/memory breakpoints work by poking at memory, we need - to prepare to access memory. If that operation fails, we need to - return error. Seeing an error, if this is the first breakpoint - of that type that GDB tries to insert, GDB would then assume the - breakpoint type is supported, but it may actually not be. So we - need to check whether the type is supported at all before - preparing to access memory. */ - if (!z_type_supported (z_type)) - { - *err = 1; - return 0; - } - - return 1; -} - -/* See mem-break.h. This is a wrapper for set_gdb_breakpoint_1 that - knows to prepare to access memory for Z0 breakpoints. */ - -struct gdb_breakpoint * -set_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind, int *err) -{ - struct gdb_breakpoint *bp; - - if (!check_gdb_bp_preconditions (z_type, err)) - return NULL; - - /* If inserting a software/memory breakpoint, need to prepare to - access memory. */ - if (z_type == Z_PACKET_SW_BP) - { - if (prepare_to_access_memory () != 0) - { - *err = -1; - return NULL; - } - } - - bp = set_gdb_breakpoint_1 (z_type, addr, kind, err); - - if (z_type == Z_PACKET_SW_BP) - done_accessing_memory (); - - return bp; -} - /* Delete a GDB breakpoint of type Z_TYPE and kind KIND previously inserted at ADDR with set_gdb_breakpoint_at. Returns 0 on success, -1 on error, and 1 if Z_TYPE breakpoints are not supported on this target. */ -static int -delete_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int kind) +int +delete_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind) { - struct gdb_breakpoint *bp; - int err; + if (!z_type_supported (z_type)) + return 1; - bp = find_gdb_breakpoint (z_type, addr, kind); + gdb_breakpoint *bp = find_gdb_breakpoint (z_type, addr, kind); if (bp == NULL) return -1; /* Before deleting the breakpoint, make sure to free its condition and command lists. */ clear_breakpoint_conditions_and_commands (bp); - err = delete_breakpoint ((struct breakpoint *) bp); + int err = delete_breakpoint ((struct breakpoint *) bp); if (err != 0) return -1; return 0; } -/* See mem-break.h. This is a wrapper for delete_gdb_breakpoint that - knows to prepare to access memory for Z0 breakpoints. */ - -int -delete_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind) -{ - int ret; - - if (!check_gdb_bp_preconditions (z_type, &ret)) - return ret; - - /* If inserting a software/memory breakpoint, need to prepare to - access memory. */ - if (z_type == Z_PACKET_SW_BP) - { - int err; - - err = prepare_to_access_memory (); - if (err != 0) - return -1; - } - - ret = delete_gdb_breakpoint_1 (z_type, addr, kind); - - if (z_type == Z_PACKET_SW_BP) - done_accessing_memory (); - - return ret; -} - /* Clear all conditions associated with a breakpoint. */ static void diff --git a/gdbserver/server.cc b/gdbserver/server.cc index e43a40a9b1f..24925cbbb5f 100644 --- a/gdbserver/server.cc +++ b/gdbserver/server.cc @@ -1067,19 +1067,12 @@ gdb_read_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len) /* (assume no half-trace half-real blocks for now) */ } - res = prepare_to_access_memory (); - if (res == 0) - { - if (set_desired_thread ()) - res = read_inferior_memory (memaddr, myaddr, len); - else - res = 1; - done_accessing_memory (); - - return res == 0 ? len : -1; - } + if (set_desired_thread ()) + res = read_inferior_memory (memaddr, myaddr, len); else - return -1; + res = 1; + + return res == 0 ? len : -1; } /* Write trace frame or inferior memory. Actually, writing to trace @@ -1095,15 +1088,10 @@ gdb_write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len) { int ret; - ret = prepare_to_access_memory (); - if (ret == 0) - { - if (set_desired_thread ()) - ret = target_write_memory (memaddr, myaddr, len); - else - ret = EIO; - done_accessing_memory (); - } + if (set_desired_thread ()) + ret = target_write_memory (memaddr, myaddr, len); + else + ret = EIO; return ret; } } diff --git a/gdbserver/target.cc b/gdbserver/target.cc index e9d1e1aa38c..883242377c0 100644 --- a/gdbserver/target.cc +++ b/gdbserver/target.cc @@ -39,88 +39,6 @@ set_desired_thread () return (current_thread != NULL); } -/* The thread that was current before prepare_to_access_memory was - called. done_accessing_memory uses this to restore the previous - selected thread. */ -static ptid_t prev_general_thread; - -/* See target.h. */ - -int -prepare_to_access_memory (void) -{ - client_state &cs = get_client_state (); - - /* The first thread found. */ - struct thread_info *first = NULL; - /* The first stopped thread found. */ - struct thread_info *stopped = NULL; - /* The current general thread, if found. */ - struct thread_info *current = NULL; - - /* Save the general thread value, since prepare_to_access_memory could change - it. */ - prev_general_thread = cs.general_thread; - - int res = the_target->prepare_to_access_memory (); - if (res != 0) - return res; - - for_each_thread (prev_general_thread.pid (), [&] (thread_info *thread) - { - if (mythread_alive (thread->id)) - { - if (stopped == NULL && the_target->supports_thread_stopped () - && target_thread_stopped (thread)) - stopped = thread; - - if (first == NULL) - first = thread; - - if (current == NULL && prev_general_thread == thread->id) - current = thread; - } - }); - - /* The thread we end up choosing. */ - struct thread_info *thread; - - /* Prefer a stopped thread. If none is found, try the current - thread. Otherwise, take the first thread in the process. If - none is found, undo the effects of - target->prepare_to_access_memory() and return error. */ - if (stopped != NULL) - thread = stopped; - else if (current != NULL) - thread = current; - else if (first != NULL) - thread = first; - else - { - done_accessing_memory (); - return 1; - } - - switch_to_thread (thread); - cs.general_thread = ptid_of (thread); - - return 0; -} - -/* See target.h. */ - -void -done_accessing_memory (void) -{ - client_state &cs = get_client_state (); - - the_target->done_accessing_memory (); - - /* Restore the previous selected thread. */ - cs.general_thread = prev_general_thread; - switch_to_thread (the_target, cs.general_thread); -} - int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len) { @@ -373,18 +291,6 @@ process_stratum_target::post_create_inferior () /* Nop. */ } -int -process_stratum_target::prepare_to_access_memory () -{ - return 0; -} - -void -process_stratum_target::done_accessing_memory () -{ - /* Nop. */ -} - void process_stratum_target::look_up_symbols () { diff --git a/gdbserver/target.h b/gdbserver/target.h index aaa9dab742c..f3172e2ed7e 100644 --- a/gdbserver/target.h +++ b/gdbserver/target.h @@ -141,21 +141,6 @@ public: If REGNO is -1, store all registers; otherwise, store at least REGNO. */ virtual void store_registers (regcache *regcache, int regno) = 0; - /* Prepare to read or write memory from the inferior process. - Targets use this to do what is necessary to get the state of the - inferior such that it is possible to access memory. - - This should generally only be called from client facing routines, - such as gdb_read_memory/gdb_write_memory, or the GDB breakpoint - insertion routine. - - Like `read_memory' and `write_memory' below, returns 0 on success - and errno on failure. */ - virtual int prepare_to_access_memory (); - - /* Undo the effects of prepare_to_access_memory. */ - virtual void done_accessing_memory (); - /* Read memory from the inferior process. This should generally be called through read_inferior_memory, which handles breakpoint shadowing. @@ -691,12 +676,6 @@ target_read_btrace_conf (struct btrace_target_info *tinfo, ptid_t mywait (ptid_t ptid, struct target_waitstatus *ourstatus, target_wait_flags options, int connected_wait); -/* Prepare to read or write memory from the inferior process. See the - corresponding process_stratum_target methods for more details. */ - -int prepare_to_access_memory (void); -void done_accessing_memory (void); - #define target_core_of_thread(ptid) \ the_target->core_of_thread (ptid) diff --git a/gdbserver/tracepoint.cc b/gdbserver/tracepoint.cc index 5459dc34cbb..18b2b0b3d77 100644 --- a/gdbserver/tracepoint.cc +++ b/gdbserver/tracepoint.cc @@ -2784,21 +2784,10 @@ cmd_qtenable_disable (char *own_buf, int enable) if (tp->type == fast_tracepoint || tp->type == static_tracepoint) { - int ret; int offset = offsetof (struct tracepoint, enabled); CORE_ADDR obj_addr = tp->obj_addr_on_target + offset; - ret = prepare_to_access_memory (); - if (ret) - { - trace_debug ("Failed to temporarily stop inferior threads"); - write_enn (own_buf); - return; - } - - ret = write_inferior_int8 (obj_addr, enable); - done_accessing_memory (); - + int ret = write_inferior_int8 (obj_addr, enable); if (ret) { trace_debug ("Cannot write enabled flag into "