Re: [PATCH] gdb/hurd: handle inferiors exiting

[Pedro Alves <pedro@palves.net>]

On 2022-01-06 17:35, John Baldwin wrote:
> On 1/6/22 7:41 AM, Andrew Burgess via Gdb-patches wrote:
>> While testing on GNU/Hurd (i386) I noticed that GDB crashes when an
>> inferior exits, with this error:
>>
>>    inferior.c:293: internal-error: inferior* find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0' failed.
>>
>> The problem appears to be in gnu_nat_target::wait.
>>
>> When GDB is waiting for any active inferior to do something
>> interesting, we pass in the null_ptid to indicate that the target
>> should wait on all inferiors.
>>
>> In gnu_nat_target::wait, when we get an exit event, we currently
>> return the passed in ptid (null_ptid) and there's a comment in the
>> code "let wait_for_inferior handle exit case".
>>
>> This may have been the case once upon a time, when GDB could only
>> handle one inferior at a time, but is certainly not the case any more,
>> we expect ::wait to return a valid ptid if it saw an event.
>>
>> I believe that the fix for this issue is pretty easy, when we see the
>> exit event, just return a ptid containing the process-id only.
>>
>> For testing, running something like gdb.base/break.exp is enough to
>> show the original problem, and this test now fully passes with this
>> patch applied.
>>
>> I've not run the full testsuite on GNU/Hurd as there appear to be lots
>> of other issues with this target that makes running the full testsuite
>> very painful, but I think this looks like a small easy improvement.
>>
>> Feedback welcome.
> 
> This looks fine to me.  The multi-target changes from Pedro stopped
> setting inferior_ptid for target ::wait methods and this is fallout
> from that I suspect.  

That is correct.  We set inferior_ptid to null_ptid before calling target_wait.
The event that ends up being reported is unrelated to the inferior/thread that was
selected before target_wait was called.  This change has caught a number
of spots in the target backends that assumed inferior_ptid was set to something.
While working on multi-target, I was running into a number of weird bugs because
of those, and the change to always clear inferior_ptid made them a lot more obvious.
This spot went unnoticed up till now.  

This is done by this bit in infrun.c:do_target_wait_1, btw:

  /* We know that we are looking for an event in the target of inferior
     INF, but we don't know which thread the event might come from.  As
     such we want to make sure that INFERIOR_PTID is reset so that none of
     the wait code relies on it - doing so is always a mistake.  */
  switch_to_inferior_no_thread (inf);


In particular, when wanting to wait for multiple ptids
> I think the core passes in minus_one_ptid or some other wildcard in 'ptid'
> rather than null_ptid, so that part of the log message might not be quite
> correct.
> 

You're right.