Re: [PATCH 1/2] [gdb] Fix segfault in for_each_block, part 1

[Andrew Burgess <aburgess@redhat.com>]

Simon Marchi <simark@simark.ca> writes:

> On 11/4/23 11:57, Tom de Vries wrote:
>> When running test-case gdb.base/vfork-follow-parent.exp on powerpc64 (likewise
>> on s390x), I run into:
>> ...
>> (gdb) PASS: gdb.base/vfork-follow-parent.exp: \
>>   exec_file=vfork-follow-parent-exit: target-non-stop=on: non-stop=off: \
>>   resolution_method=schedule-multiple: print unblock_parent = 1
>> continue^M
>> Continuing.^M
>> Reading symbols from vfork-follow-parent-exit...^M
>> ^M
>> ^M
>> Fatal signal: Segmentation fault^M
>> ----- Backtrace -----^M
>> 0x1027d3e7 gdb_internal_backtrace_1^M
>>         src/gdb/bt-utils.c:122^M
>> 0x1027d54f _Z22gdb_internal_backtracev^M
>>         src/gdb/bt-utils.c:168^M
>> 0x1057643f handle_fatal_signal^M
>>         src/gdb/event-top.c:889^M
>> 0x10576677 handle_sigsegv^M
>>         src/gdb/event-top.c:962^M
>> 0x3fffa7610477 ???^M
>> 0x103f2144 for_each_block^M
>>         src/gdb/dcache.c:199^M
>> 0x103f235b _Z17dcache_invalidateP13dcache_struct^M
>>         src/gdb/dcache.c:251^M
>> 0x10bde8c7 _Z24target_dcache_invalidatev^M
>>         src/gdb/target-dcache.c:50^M
>> ...
>> or similar.
>> 
>> The root cause for the segmentation fault is that linux_is_uclinux gives an
>> incorrect result: it should always return false, given that we're running on a
>> regular linux system, but instead it returns first true, then false.
>> 
>> In more detail, the segmentation fault happens as follows:
>> - a program space with an address space is created
>> - a second program space is about to be created. maybe_new_address_space
>>   is called, and because linux_is_uclinux returns true, maybe_new_address_space
>>   returns false, and no new address space is created
>> - a second program space with the same address space is created
>> - a program space is deleted. Because linux_is_uclinux now returns false,
>>   gdbarch_has_shared_address_space (current_inferior ()->arch ()) returns
>>   false, and the address space is deleted
>> - when gdb uses the address space of the remaining program space, we run into
>>   the segfault, because the address space is deleted.
>> 
>> Hardcoding linux_is_uclinux to false makes the test-case pass.
>> 
>> We leave addressing the root cause for the following commit in this series.
>> 
>> For now, prevent the segmentation fault by making the address space a refcounted
>> object.
>> 
>> This was already suggested here [1]:
>> ...
>> A better solution might be to have the address spaces be reference counted
>> ...
>
> That reminded me of a patch I started to work on a while ago to remove
> program_space::aspace, which I have to get back to.  The rationale is
> that there might be a 1:N relation ship between one program space and
> many address spaces (see diagram in progspace.h), so having this one
> pointer does not represent well the relationship.  An inferior is always
> bound to a single address space, so my patch changes the code to use
> inferior::aspace instead.

I also started looking at this after our previous discussion on
address_space pointer sharing :)

>
> I think that even with my patch, we would still have the problem of
> knowing when to delete an address space.  When you have many inferiors
> sharing an address space, and you delete an inferior, how do you know if
> you should delete the address space?  Perhaps we can implement a method
> like program_space::empty, but for address_space.
>
> However, I still think that using refcounting for address spaces (and
> perhaps for program spaces too?) is a good solution.  It would spare us
> of having to deal with the logic of knowing when address space is no
> longer used.  Refcounting isn't always the right solution, but I don't
> see any downsides in this case.

Agreed.  The biggest yuck for me was finding the address_space to assign
to a new inferior when in the shared address_space case.  Currently, the
program_space serves as the authority that hands out the single shared
address_space.

With the address_space removed from the program_space, you now need to
find a sibling inferior and take a reference to their address space.
Not a show stopping issue by any means, I just never got around to
finishing this part of the task.

Thanks,
Andrew

>
> Implementation question: is there any reason you didn't choose
> std::shared_ptr?  For objects that are new'ed / delete'd, that seems
> like a good choice.
>
> Simon