From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f43.google.com (mail-wr1-f43.google.com [209.85.221.43]) by sourceware.org (Postfix) with ESMTPS id 24FC2388883E for ; Wed, 30 Mar 2022 12:43:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 24FC2388883E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f43.google.com with SMTP id h23so29099232wrb.8 for ; Wed, 30 Mar 2022 05:43:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=QTyU2YhXrJbf2L38Xtnx/j7VckVhs0BFmzP/AgaFNdM=; b=efzWnrX+Ve6RMByKB1JMT4hS2PWWF/dO4qeqjmK7R1f/XSV0YetYm6eWsihuOLHoxi Z4iigh4fkJ0CaSzUQFRvzrEaBI01aZ3P1C9luPsW7m2qoAjP/ylFivOczxY7xtknxZZU +Fm0Ys6eoL+FithIXrXSJ8BUPapJQoKzvWyjv5c8xT7aCkcUUp5Leiv5NE1grWeRGBuU mD2YwoG9uy6imQgxDXw+pt7hElGif/mLYYyvlvRv51I82Dd4/o8d9MeLb74PcEWdBfFy Fhz8BP7ogwXjnrFg/i/FjrW7zZB9XGxkolJC/d1bqi2TmzwKAM5GHUqsXG3+gczvfPyR iR5g== X-Gm-Message-State: AOAM532borNe3SUqpwd2xxZFtYmnx0gS4UwPKwQaOGDlVtsU0m+BT+ue jD/bVxLWEER/gLnovQwvuiePfbnbjng= X-Google-Smtp-Source: ABdhPJxq5fajt02U65S83OMEVSmhgBiwH0B+uld5PXBp9VQBmfzTIIhXegbhDOEo4RRQUd6O6PbdMA== X-Received: by 2002:adf:e444:0:b0:203:d6f4:3cc4 with SMTP id t4-20020adfe444000000b00203d6f43cc4mr36554825wrm.493.1648644209123; Wed, 30 Mar 2022 05:43:29 -0700 (PDT) Received: from localhost ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id l15-20020a05600c1d0f00b0038c8ff8e708sm4710318wms.13.2022.03.30.05.43.27 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 30 Mar 2022 05:43:28 -0700 (PDT) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 3/3] gdbserver: Eliminate prepare_to_access_memory Date: Wed, 30 Mar 2022 13:43:19 +0100 Message-Id: <20220330124319.2804582-4-pedro@palves.net> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20220330124319.2804582-1-pedro@palves.net> References: <20220330124319.2804582-1-pedro@palves.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-9.5 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 30 Mar 2022 12:43:33 -0000 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, target_ops::prepare_to_access_memory became a nop. Thus, prepare_to_access_memory can go away, simplifying core GDBserver code. Change-Id: I93ac8bfe66bd61c3d1c4a0e7d419335163120ecf --- gdbserver/mem-break.cc | 101 ++++++---------------------------------- gdbserver/server.cc | 30 ++++-------- gdbserver/target.cc | 94 ------------------------------------- gdbserver/target.h | 21 --------- gdbserver/tracepoint.cc | 13 +----- 5 files changed, 24 insertions(+), 235 deletions(-) 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 60d0c2be17a..64b8bb34b9a 100644 --- a/gdbserver/server.cc +++ b/gdbserver/server.cc @@ -1071,19 +1071,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 @@ -1099,15 +1092,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 5009146d663..ddc0a08dd6d 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) { @@ -360,18 +278,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 @@ class process_stratum_target 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 " -- 2.26.2