Re: [PATCH] Don't compare the pid returned from wait() against inferior_ptid.

[John Baldwin <jhb@FreeBSD.org>]

On 7/8/20 9:52 AM, Simon Marchi wrote:
> On 2020-07-08 12:46 p.m., John Baldwin wrote:
>> On 7/8/20 2:48 AM, Gary Benson wrote:
>>> Hi John,
>>>
>>> John Baldwin wrote:
>>>> inf_ptrace::wait() needs to discard termination events reported by
>>>> detached child processes.  Previously it compared the returned pid
>>>> against the pid in inferior_ptid to determine if a termination event
>>>> should be discarded or reported.  inferior_ptid is now null_ptid in
>>>> wait() target methods, so this was always failing and never
>>>> reporting exit events.  Instead, report termination events whose pid
>>>> matches any inferior belonging to the current target.
>>>>
>>>> gdb/ChangeLog:
>>>>
>>>> 	* inf-ptrace.c (inf_ptrace_target::wait): Don't compare against
>>>> 	inferior_ptid.
>>>
>>> Did the bug you've fixed cause any tests to fail?  And, if not, is
>>> this something it's possible to write a test for?
>>
>> It did cause regressions in some tests on FreeBSD.  I am still letting
>> a before-after testsuite run, but gdb.base/a2-run.exp at least failed
>> all tests due to this bug and is back to passing on FreeBSD with the fix.
> 
> Here's the context, for Gary and others:
> 
> https://sourceware.org/pipermail/gdb-patches/2020-July/170238.html
> 
> In the commit message, you should point out that commit xYZ caused some
> existing tests to fail, and that this patch makes them pass again.  This
> way it's implied that there doesn't need to be new tests added, and it
> gives a paper trail to understand why this fix has come to be needed.

So after some git bisecting on the a2-run.exp test, I found that this
actually broke in the multi-target commit.  This is ironic as I had tested
that change pre-commit and given Pedro patches to fix it on the FreeBSD
native target, but clearly I didn't test it well enough.

FYI, about 400 tests are fixed on FreeBSD after applying this patch
and some other failures go back to failing for an original reason
instead of timeouts / GDB internal errors.

How's this for an updated log:

    Don't compare the pid returned from wait() against inferior_ptid.
    
    inf_ptrace::wait() needs to discard termination events reported by
    detached child processes.  Previously it compared the returned pid
    against the pid in inferior_ptid to determine if a termination event
    should be discarded or reported.  The multi-target changes cleared
    inferior_ptid to null_ptid in wait() target methods, so this was
    always failing and never reporting exit events.  Instead, report
    termination events whose pid matches any inferior belonging to the
    current target.
    
    Several tests started failing on FreeBSD after the multi-target
    changes and pass again after this change.


-- 
John Baldwin