From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1551) id 1A5743851163; Tue, 28 Jun 2022 17:12:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1A5743851163 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Pedro Alves To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdb+gdbserver/Linux: avoid reading registers while going through shell X-Act-Checkin: binutils-gdb X-Git-Author: Pedro Alves X-Git-Refname: refs/heads/master X-Git-Oldrev: 9117c7b452ef76304f4394a97c887d0c4af439f5 X-Git-Newrev: a9deee17d33235eeba401eebed55c85a742e3358 Message-Id: <20220628171214.1A5743851163@sourceware.org> Date: Tue, 28 Jun 2022 17:12:14 +0000 (GMT) X-BeenThere: gdb-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Jun 2022 17:12:14 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Da9deee17d332= 35eeba401eebed55c85a742e3358 commit a9deee17d33235eeba401eebed55c85a742e3358 Author: Pedro Alves Date: Mon Jun 27 20:41:50 2022 +0100 gdb+gdbserver/Linux: avoid reading registers while going through shell =20 For every stop, Linux GDB and GDBserver save the stopped thread's PC, in lwp->stop_pc. This is done in save_stop_reason, in both gdb/linux-nat.c and gdbserver/linux-low.cc. However, while we're going through the shell after "run", in startup_inferior, we shouldn't be reading registers, as we haven't yet determined the target's architecture -- the shell's architecture may not even be the same as the final inferior's. =20 In gdb/linux-nat.c, lwp->stop_pc is only needed when the thread has stopped for a breakpoint, and since when going through the shell, no breakpoint is going to hit, we could simply teach save_stop_reason to only record the stop pc when the thread stopped for a breakpoint. =20 However, in gdbserver/linux-low.cc, lwp->stop_pc is used in more cases than breakpoint hits (e.g., it's used in tracepoints & the "while-stepping" feature). =20 So to avoid GDB vs GDBserver divergence, we apply the same approach to both implementations. =20 We set a flag in the inferior (process in GDBserver) whenever it is being nursed through the shell, and when that flag is set, save_stop_reason bails out early. While going through the shell, we'll only ever get process exits (normal or signalled), random signals, and exec events, so nothing is lost. =20 Change-Id: If0f01831514d3a74d17efd102875de7d2c6401ad Diff: --- gdb/fork-child.c | 3 +++ gdb/inferior.h | 7 +++++++ gdb/linux-nat.c | 4 ++++ gdbserver/fork-child.cc | 5 +++++ gdbserver/inferiors.h | 7 +++++++ gdbserver/linux-low.cc | 16 ++++++++++++---- 6 files changed, 38 insertions(+), 4 deletions(-) diff --git a/gdb/fork-child.c b/gdb/fork-child.c index 89003fa617d..66b523e118d 100644 --- a/gdb/fork-child.c +++ b/gdb/fork-child.c @@ -126,6 +126,9 @@ gdb_startup_inferior (pid_t pid, int num_traps) inferior *inf =3D current_inferior (); process_stratum_target *proc_target =3D inf->process_target (); =20 + scoped_restore save_starting_up + =3D make_scoped_restore (&inf->starting_up, true); + ptid_t ptid =3D startup_inferior (proc_target, pid, num_traps, NULL, NUL= L); =20 /* Mark all threads non-executing. */ diff --git a/gdb/inferior.h b/gdb/inferior.h index f6e26a32feb..c376d780de0 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -551,6 +551,13 @@ public: architecture/description. */ bool needs_setup =3D false; =20 + /* True if the inferior is starting up (inside startup_inferior), + and we're nursing it along (through the shell) until it is ready + to execute its first instruction. Until that is done, we must + not access inferior memory or registers, as we haven't determined + the target architecture/description. */ + bool starting_up =3D false; + /* True when we are reading the library list of the inferior during an attach or handling a fork child. */ bool in_initial_library_scan =3D false; diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index b9164e621db..0a93ab5c6ae 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -2539,6 +2539,10 @@ save_stop_reason (struct lwp_info *lp) if (!linux_target->low_status_is_event (lp->status)) return; =20 + inferior *inf =3D find_inferior_ptid (linux_target, lp->ptid); + if (inf->starting_up) + return; + regcache =3D get_thread_regcache (linux_target, lp->ptid); gdbarch =3D regcache->arch (); =20 diff --git a/gdbserver/fork-child.cc b/gdbserver/fork-child.cc index 96dd4d009ab..7ea66f2a435 100644 --- a/gdbserver/fork-child.cc +++ b/gdbserver/fork-child.cc @@ -18,6 +18,7 @@ =20 #include "server.h" #include "gdbsupport/job-control.h" +#include "gdbsupport/scoped_restore.h" #include "nat/fork-inferior.h" #ifdef HAVE_SIGNAL_H #include @@ -103,6 +104,10 @@ post_fork_inferior (int pid, const char *program) atexit (restore_old_foreground_pgrp); #endif =20 + process_info *proc =3D find_process_pid (pid); + scoped_restore save_starting_up + =3D make_scoped_restore (&proc->starting_up, true); + startup_inferior (the_target, pid, START_INFERIOR_TRAPS_EXPECTED, &cs.last_status, &cs.last_ptid); diff --git a/gdbserver/inferiors.h b/gdbserver/inferiors.h index f3ba4d82f71..6de746cb228 100644 --- a/gdbserver/inferiors.h +++ b/gdbserver/inferiors.h @@ -75,6 +75,13 @@ struct process_info =20 /* Flag to mark that the DLL list has changed. */ bool dlls_changed =3D false; + + /* True if the inferior is starting up (inside startup_inferior), + and we're nursing it along (through the shell) until it is ready + to execute its first instruction. Until that is done, we must + not access inferior memory or registers, as we haven't determined + the target architecture/description. */ + bool starting_up =3D false; }; =20 /* Get the pid of PROC. */ diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc index 8b8614f6ed4..2f71360d3bd 100644 --- a/gdbserver/linux-low.cc +++ b/gdbserver/linux-low.cc @@ -747,8 +747,8 @@ linux_process_target::handle_extended_wait (lwp_info **= orig_event_lwp, CORE_ADDR linux_process_target::get_pc (lwp_info *lwp) { - struct regcache *regcache; - CORE_ADDR pc; + process_info *proc =3D get_thread_process (get_lwp_thread (lwp)); + gdb_assert (!proc->starting_up); =20 if (!low_supports_breakpoints ()) return 0; @@ -756,8 +756,8 @@ linux_process_target::get_pc (lwp_info *lwp) scoped_restore_current_thread restore_thread; switch_to_thread (get_lwp_thread (lwp)); =20 - regcache =3D get_thread_regcache (current_thread, 1); - pc =3D low_get_pc (regcache); + struct regcache *regcache =3D get_thread_regcache (current_thread, 1); + CORE_ADDR pc =3D low_get_pc (regcache); =20 threads_debug_printf ("pc is 0x%lx", (long) pc); =20 @@ -797,6 +797,14 @@ linux_process_target::save_stop_reason (lwp_info *lwp) if (!low_supports_breakpoints ()) return false; =20 + process_info *proc =3D get_thread_process (get_lwp_thread (lwp)); + if (proc->starting_up) + { + /* Claim we have the stop PC so that the caller doesn't try to + fetch it itself. */ + return true; + } + pc =3D get_pc (lwp); sw_breakpoint_pc =3D pc - low_decr_pc_after_break ();