Re: [PATCH 0/2] Fix gdbserver/linux memory access regression

[Pedro Alves <pedro@palves.net>]

On 2022-05-04 10:52, Luis Machado wrote:
> On 5/4/22 10:45, Pedro Alves wrote:

>> Can you show a backtrace?  If this is when reading memory, what code cares whether it's 64-bit?  Reading memory
>> out of /proc/pid/mem should not care about that.
> 
> Here it is:
> 
> #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 need to fetch the register cache, and we do so through a thread point, which is now nullptr.
> 

Thanks.  I believe the patch below should fix that particular instance.

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);
    }
...

There may be other spots that require similar treatments.

From 28f784b14bcd5b435de0a764a6a1e11df5e131c9 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 4 May 2022 11:09:07 +0100
Subject: [PATCH] fix

Change-Id: Ibc809d7345e70a2f058b522bdc5cdbdca97e2cdc
---
 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 0091f998c63..c947f2bcac1 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

base-commit: 7f8acedeebe295fc8cc1d11ed971cbfc1942618c
-- 
2.36.0