From cfa8e270c9dce83e89b03bc5d793835027731891 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 7 Jun 2021 11:03:04 -0400 Subject: [PATCH] gdb: set only inferior_ptid in sparc_{fetch,store}_inferior_registers The past commit d1e93af64a6b ("gdb: set current thread in sparc_{fetch,collect}_inferior_registers (PR gdb/27147)") changed sparc_fetch_inferior_registers and sparc_store_inferior_registers to look up the thread corresponding to the regcache's ptid and make it the current thread. The reason being that down the call chain, some functions (like sparc_supply_rwindow) can do some memory reads or write, through target_read_memory/target_write_memory, and those rely on the current global context. There is one small problem with this approach: when debugging a multi-threaded program, the regcache for a new thread is created just before the corresponding thread_info is created. In fact, the regcache is created somewhere during the call to thread_from_lwp, which is responsible for creating the thread_info: #8 0x0000010000ab9968 in internal_error (file=0x10000bfca20 "/home/simark/src/binutils-gdb/gdb/thread.c", line=1346, fmt=0x10000bfc918 "%s: Assertion `%s' failed.") at /home/simark/src/binutils-gdb/gdbsupport/errors.cc:55 #9 0x0000010000827f3c in switch_to_thread (thr=0x0) at /home/simark/src/binutils-gdb/gdb/thread.c:1346 #10 0x0000010000753444 in sparc_fetch_inferior_registers (proc_target=0x10000fa8cb0 , regcache=0x10000ff03c0, regnum=-1) at /home/simark/src/binutils-gdb/gdb/sparc-nat.c:175 #11 0x000001000075b908 in sparc64_linux_nat_target::fetch_registers (this=0x10000fa8cb0 , regcache=0x10000ff03c0, regnum=-1) at /home/simark/src/binutils-gdb/gdb/sparc64-linux-nat.c:38 #12 0x00000100007fe6f4 in target_ops::fetch_registers (this=0x10000f7feb0 , arg0=0x10000ff03c0, arg1=-1) at /home/simark/src/binutils-gdb/gdb/target-delegates.c:496 #13 0x00000100008162a0 in target_fetch_registers (regcache=0x10000ff03c0, regno=-1) at /home/simark/src/binutils-gdb/gdb/target.c:3287 #14 0x000001000060a4bc in ps_lgetregs (ph=0x10001264368, lwpid=458727, gregset=0x7feff97d388) at /home/simark/src/binutils-gdb/gdb/proc-service.c:158 #15 0xffff800103e32420 in __td_ta_lookup_th_unique (ta_arg=0x100012d7080, lwpid=, th=0x7feff97d7c8) at td_ta_map_lwp2thr.c:119 #16 0xffff800103e32604 in td_ta_map_lwp2thr (ta_arg=0x100012d7080, lwpid=, th=0x7feff97d7c8) at td_ta_map_lwp2thr.c:207 #17 0x000001000051fee8 in thread_from_lwp (stopped=0x100011a3650, ptid=...) at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:415 #18 0x0000010000520150 in thread_db_notice_clone (parent=..., child=...) at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:446 #19 0x00000100005068a8 in linux_handle_extended_wait (lp=0x10001230700, status=4479) at /home/simark/src/binutils-gdb/gdb/linux-nat.c:1978 #20 0x000001000050a278 in linux_nat_filter_event (lwpid=458724, status=198015) at /home/simark/src/binutils-gdb/gdb/linux-nat.c:2913 #21 0x000001000050b818 in linux_nat_wait_1 (ptid=..., ourstatus=0x7feff97e8d0, target_options=...) at /home/simark/src/binutils-gdb/gdb/linux-nat.c:3194 #22 0x000001000050ca4c in linux_nat_target::wait (this=0x10000fa8cb0 , ptid=..., ourstatus=0x7feff97e8d0, target_options=...) at /home/simark/src/binutils-gdb/gdb/linux-nat.c:3432 #23 0x00000100005237ec in thread_db_target::wait (this=0x10000f7feb0 , ptid=..., ourstatus=0x7feff97e8d0, options=...) at /home/simark/src/binutils-gdb/gdb/linux-thread-db.c:1379 #24 0x00000100007fa668 in target_wait (ptid=..., status=0x7feff97e8d0, options=...) at /home/simark/src/binutils-gdb/gdb/target.c:2000 #25 0x00000100004adb0c in do_target_wait_1 (inf=0x10001173170, ptid=..., status=0x7feff97e8d0, options=...) at /home/simark/src/binutils-gdb/gdb/infrun.c:3464 #26 0x00000100004add48 in operator() (__closure=0x7feff97e658, inf=0x10001173170) at /home/simark/src/binutils-gdb/gdb/infrun.c:3527 #27 0x00000100004ae15c in do_target_wait (wait_ptid=..., ecs=0x7feff97e8a8, options=...) at /home/simark/src/binutils-gdb/gdb/infrun.c:3540 #28 0x00000100004af254 in fetch_inferior_event () at /home/simark/src/binutils-gdb/gdb/infrun.c:3880 #29 0x0000010000486ef8 in inferior_event_handler (event_type=INF_REG_EVENT) at /home/simark/src/binutils-gdb/gdb/inf-loop.c:42 The problem is that while sparc_fetch_inferior_registers runs and is asked to read the registers of a given ptid, there isn't a thread_info with that ptid yet. So, find_thread_ptid returns nullptr, and switch_to_thread gives an internal error. Fix this by only setting inferior_ptid, instead of the whole global context, as switch_to_thread does. This is sufficient for target_read_memory / target_write_memory to work down the line. Ideally, it would be nice to be able to pass the ptid down the whole call chain and to target_read_memory / target_write_memory, so that this setting of inferior_ptid would not be necessary. But this is not going to happen soon. This fixes running a multi-threaded program, which would hit the internal error show in the call stack above. gdb/ChangeLog: PR gdb/27899 * sparc-nat.c (sparc_fetch_inferior_registers): Set inferior_ptid instead of using switch_to_thread. (sparc_store_inferior_registers): Likewise. Change-Id: I0b6ddb3af9b11f67b10ee46a734fb82ecc6462d5 --- gdb/ChangeLog | 7 +++++++ gdb/sparc-nat.c | 10 ++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index dcb97ae0461..32108023aa2 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2021-06-07 Simon Marchi + + PR gdb/27899 + * sparc-nat.c (sparc_fetch_inferior_registers): Set + inferior_ptid instead of using switch_to_thread. + (sparc_store_inferior_registers): Likewise. + 2021-06-05 Bernd Edlinger * compile/compile.c (scoped_ignore_sigpipe): New helper class. diff --git a/gdb/sparc-nat.c b/gdb/sparc-nat.c index fa3b32cee18..7f09a60420d 100644 --- a/gdb/sparc-nat.c +++ b/gdb/sparc-nat.c @@ -170,9 +170,8 @@ sparc_fetch_inferior_registers (process_stratum_target *proc_target, /* Deep down, sparc_supply_rwindow reads memory, so needs the global thread context to be set. */ - thread_info *thread = find_thread_ptid (proc_target, ptid); - scoped_restore_current_thread restore_thread; - switch_to_thread (thread); + scoped_restore restore_inferior_ptid + = make_scoped_restore (&inferior_ptid, ptid); sparc_supply_gregset (sparc_gregmap, regcache, -1, ®s); if (regnum != -1) @@ -219,9 +218,8 @@ sparc_store_inferior_registers (process_stratum_target *proc_target, /* Deep down, sparc_collect_rwindow writes memory, so needs the global thread context to be set. */ - thread_info *thread = find_thread_ptid (proc_target, ptid); - scoped_restore_current_thread restore_thread; - switch_to_thread (thread); + scoped_restore restore_inferior_ptid + = make_scoped_restore (&inferior_ptid, ptid); sparc_collect_rwindow (regcache, sp, regnum); } -- 2.39.2