[binutils-gdb] Fix GDBserver Aarch64 Linux regression

[binutils-gdb] Fix GDBserver Aarch64 Linux regression

[Pedro Alves]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=5890af36e5112bcbb8d7555e63570f68466e6944

commit 5890af36e5112bcbb8d7555e63570f68466e6944
Author: Pedro Alves <pedro@palves.net>
Date:   Wed May 4 11:09:07 2022 +0100

    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

Diff:
---
 gdbserver/linux-aarch64-low.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
index c924821c25c..d1e7acb7b4a 100644
--- a/gdbserver/linux-aarch64-low.cc
+++ b/gdbserver/linux-aarch64-low.cc
@@ -191,9 +191,9 @@ struct arch_process_info
 static int
 is_64bit_tdesc (void)
 {
-  struct regcache *regcache = get_thread_regcache (current_thread, 0);
-
-  return register_size (regcache->tdesc, 0) == 8;
+  /* We may not have a current thread at this point, so go straight to
+     the process's target description.  */
+  return register_size (current_process ()->tdesc) == 8;
 }
 
 static void