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

[Luis Machado <luis.machado@arm.com>]

On 5/4/22 10:45, Pedro Alves wrote:
> On 2022-05-04 10:42, Luis Machado wrote:
>> On 5/4/22 10:11, Luis Machado via Gdb-patches wrote:
>>> On 5/3/22 15:24, Pedro Alves wrote:
>>>> On 2022-04-19 23:47, Pedro Alves wrote:
>>>>> The recent change to make GDBserver always access memory via
>>>>> /proc/pid/mem caused a regression in
>>>>> gdb.threads/access-mem-running-thread-exit.exp that I somehow missed.
>>>>> This is actually a pre-existing GDBserver issue being now exposed.
>>>>>
>>>>> Patch #2 fixes the GDBserver bug.
>>>>>
>>>>> Patch #1 fixes the gdb.threads/access-mem-running-thread-exit.exp
>>>>> testcase itself -- it doesn't run properly against
>>>>> --target_board=native-extended-gdbserver today.
>>>>>
>>>>
>>>> I'm pushing this in.
>>>
>>> Just a heads-up, this seems to have regressed a few gdb.multi/*.exp tests for aarch64-linux.
>>>
>>> I see 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.
>>>
>>> I haven't investigated this yet.
>>
>> Ok. I tracked this down to gdbserver crashing when trying to fetch the register cache using a nullptr for the thread pointer. This happens when trying to read memory, then the backend goes to try to figure out if it is 64-bit and then eventually tries to fetch the regcache with current_thread (nullptr).
>>
>> I'm guessing we should really be using the process pointer if there isn't a valid thread pointer to fetch the register data.
>>
> 
> 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.