Fix GDBserver Aarch64 Linux regression
authorPedro Alves <pedro@palves.net>
Wed, 4 May 2022 10:09:07 +0000 (11:09 +0100)
committerPedro Alves <pedro@palves.net>
Wed, 4 May 2022 13:42:58 +0000 (14:42 +0100)
commit5890af36e5112bcbb8d7555e63570f68466e6944
treeae9cc11becac91df69c2abf40b3db53e997cfc5d
parent901e4e8d5c4f1d32c50e4a31b228bc7ef4210775
Fix GDBserver Aarch64 Linux regression

Luis noticed that the recent changes to gdbserver to make it track
process and threads independently regressed a few gdb.multi/*.exp
tests for aarch64-linux.

We started seeing the following internal error for
gdb.multi/multi-target-continue.exp for example:

 Starting program: binutils-gdb/gdb/testsuite/outputs/gdb.multi/multi-target-continue/multi-target-continue ^M
 Error in re-setting breakpoint 2: Remote connection closed^M
 ../../../repos/binutils-gdb/gdb/thread.c:85: internal-error: inferior_thread: Assertion `current_thread_ != nullptr' failed.^M
 A problem internal to GDB has been detected,^M
 further debugging may prove unreliable.

A backtrace looks like:

 #0  thread_regcache_data (thread=thread@entry=0x0) at ../../../repos/binutils-gdb/gdbserver/inferiors.cc:120
 #1  0x0000aaaaaaabf0e8 in get_thread_regcache (thread=0x0, fetch=fetch@entry=0) at ../../../repos/binutils-gdb/gdbserver/regcache.cc:31
 #2  0x0000aaaaaaad785c in is_64bit_tdesc () at ../../../repos/binutils-gdb/gdbserver/linux-aarch64-low.cc:194
 #3  0x0000aaaaaaad8a48 in aarch64_target::sw_breakpoint_from_kind (this=<optimized out>, kind=4, size=0xffffffffef04) at ../../../repos/binutils-gdb/gdbserver/linux-aarch64-low.cc:3226
 #4  0x0000aaaaaaabe220 in bp_size (bp=0xaaaaaab6f3d0) at ../../../repos/binutils-gdb/gdbserver/mem-break.cc:226
 #5  check_mem_read (mem_addr=187649984471104, buf=buf@entry=0xaaaaaab625d0 "\006", mem_len=mem_len@entry=56) at ../../../repos/binutils-gdb/gdbserver/mem-break.cc:1862
 #6  0x0000aaaaaaacc660 in read_inferior_memory (memaddr=<optimized out>, myaddr=0xaaaaaab625d0 "\006", len=56) at ../../../repos/binutils-gdb/gdbserver/target.cc:93
 #7  0x0000aaaaaaac3d9c in gdb_read_memory (len=56, myaddr=0xaaaaaab625d0 "\006", memaddr=187649984471104) at ../../../repos/binutils-gdb/gdbserver/server.cc:1071
 #8  gdb_read_memory (memaddr=187649984471104, myaddr=0xaaaaaab625d0 "\006", len=56) at ../../../repos/binutils-gdb/gdbserver/server.cc:1048
 #9  0x0000aaaaaaac82a4 in process_serial_event () at ../../../repos/binutils-gdb/gdbserver/server.cc:4307
 #10 handle_serial_event (err=<optimized out>, client_data=<optimized out>) at ../../../repos/binutils-gdb/gdbserver/server.cc:4520
 #11 0x0000aaaaaaafbcd0 in gdb_wait_for_event (block=block@entry=1) at ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:700
 #12 0x0000aaaaaaafc0b0 in gdb_wait_for_event (block=1) at ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:596
 #13 gdb_do_one_event () at ../../../repos/binutils-gdb/gdbsupport/event-loop.cc:237
 #14 0x0000aaaaaaacacb0 in start_event_loop () at ../../../repos/binutils-gdb/gdbserver/server.cc:3518
 #15 captured_main (argc=4, argv=<optimized out>) at ../../../repos/binutils-gdb/gdbserver/server.cc:3998
 #16 0x0000aaaaaaab66dc in main (argc=<optimized out>, argv=<optimized out>) at ../../../repos/binutils-gdb/gdbserver/server.cc:4084

This sequence of functions is invoked due to a series of conditions:

 1 - The probe-based breakpoint mechanism failed (for some reason) so ...

 2 - ... gdbserver has to know what type of architecture it is dealing
     with so it can pick the right breakpoint kind, so it wants to
     check if we have a 64-bit target.

 3 - To determine the size of a register, we currently fetch the
     current thread's register cache, and the current thread pointer
     is now nullptr.

In #3, the current thread is nullptr because gdb_read_memory clears it
on purpose, via set_desired_process, exactly to expose code relying on
the current thread when it shouldn't.  It was always possible to end
up in this situation (when the current thread exits), but it was
harder to reproduce before.

This commit fixes it by tweaking is_64bit_tdesc to look at the current
process's tdesc instead of the current thread's tdesc.

Note that the thread's tdesc is itself filled from the process's
tdesc, so this should be equivalent:

 struct regcache *
 get_thread_regcache (struct thread_info *thread, int fetch)
 {
   struct regcache *regcache;

   regcache = thread_regcache_data (thread);

 ...
   if (regcache == NULL)
     {
       struct process_info *proc = get_thread_process (thread);

       gdb_assert (proc->tdesc != NULL);

       regcache = new_register_cache (proc->tdesc);
       set_thread_regcache_data (thread, regcache);
     }
 ...

Change-Id: Ibc809d7345e70a2f058b522bdc5cdbdca97e2cdc
gdbserver/linux-aarch64-low.cc