[PATCH v5 06/15] do_target_wait_1: Clear TARGET_WNOHANG if the target isn't async.

[John Baldwin]

On 2/23/22 7:51 AM, Simon Marchi wrote:
> On 2022-02-22 20:40, Simon Marchi via Gdb-patches wrote:
>>
>>
>> On 2022-02-22 19:07, John Baldwin wrote:
>>> I have no idea why these tests didn't run in my VM.  If you revert just this
>>> commit and keep the rest of the series do you see any other regressions?
>>
>> If I revert this commit, the testsuite goes back to "no regressions" for me.
> 
> Here's what happens:
> 
> 1. On remote inferior was started, is now stopped.  It is not "async",
>     because when everything is stopped, the async fd is removed from the
>     event loop, but it "can async".
> 
> 2. We run a native inferior, the event loop gets woken up by the native
>     target's fd.
> 
> 3. In do_target_wait, we randomly choose an inferior to call target_wait
>     on first, it happens to be the remote inferior.
> 
> 4. Because the target is currently not "async", we clear
>     TARGET_WNOHANG, resulting in synchronous wait.  We therefore block
>     here:
> 
>    #0  0x00007fe9540dbb4d in select () from /usr/lib/libc.so.6
>    #1  0x000055fc7e821da7 in gdb_select (n=15, readfds=0x7ffdb77c1fb0, writefds=0x0, exceptfds=0x7ffdb77c2050, timeout=0x7ffdb77c1f90) at /home/simark/src/binutils-gdb/gdb/posix-hdep.c:31
>    #2  0x000055fc7ddef905 in interruptible_select (n=15, readfds=0x7ffdb77c1fb0, writefds=0x0, exceptfds=0x7ffdb77c2050, timeout=0x7ffdb77c1f90) at /home/simark/src/binutils-gdb/gdb/event-top.c:1134
>    #3  0x000055fc7eda58e4 in ser_base_wait_for (scb=0x6250002e4100, timeout=1) at /home/simark/src/binutils-gdb/gdb/ser-base.c:240
>    #4  0x000055fc7eda66ba in do_ser_base_readchar (scb=0x6250002e4100, timeout=-1) at /home/simark/src/binutils-gdb/gdb/ser-base.c:365
>    #5  0x000055fc7eda6ff6 in generic_readchar (scb=0x6250002e4100, timeout=-1, do_readchar=0x55fc7eda663c <do_ser_base_readchar(serial*, int)>) at /home/simark/src/binutils-gdb/gdb/ser-base.c:444
>    #6  0x000055fc7eda718a in ser_base_readchar (scb=0x6250002e4100, timeout=-1) at /home/simark/src/binutils-gdb/gdb/ser-base.c:471
>    #7  0x000055fc7edb1ecd in serial_readchar (scb=0x6250002e4100, timeout=-1) at /home/simark/src/binutils-gdb/gdb/serial.c:393
>    #8  0x000055fc7ec48b8f in remote_target::readchar (this=0x617000038780, timeout=-1) at /home/simark/src/binutils-gdb/gdb/remote.c:9446
>    #9  0x000055fc7ec4da82 in remote_target::getpkt_or_notif_sane_1 (this=0x617000038780, buf=0x6170000387a8, forever=1, expecting_notif=1, is_notif=0x7ffdb77c24f0) at /home/simark/src/binutils-gdb/gdb/remote.c:9928
>    #10 0x000055fc7ec4f045 in remote_target::getpkt_or_notif_sane (this=0x617000038780, buf=0x6170000387a8, forever=1, is_notif=0x7ffdb77c24f0) at /home/simark/src/binutils-gdb/gdb/remote.c:10037
>    #11 0x000055fc7ec354d4 in remote_target::wait_ns (this=0x617000038780, ptid=..., status=0x7ffdb77c33c8, options=...) at /home/simark/src/binutils-gdb/gdb/remote.c:8147
>    #12 0x000055fc7ec38aa1 in remote_target::wait (this=0x617000038780, ptid=..., status=0x7ffdb77c33c8, options=...) at /home/simark/src/binutils-gdb/gdb/remote.c:8337
>    #13 0x000055fc7f1409ce in target_wait (ptid=..., status=0x7ffdb77c33c8, options=...) at /home/simark/src/binutils-gdb/gdb/target.c:2612
>    #14 0x000055fc7e19da98 in do_target_wait_1 (inf=0x617000038080, ptid=..., status=0x7ffdb77c33c8, options=...) at /home/simark/src/binutils-gdb/gdb/infrun.c:3636
>    #15 0x000055fc7e19e26b in operator() (__closure=0x7ffdb77c2f90, inf=0x617000038080) at /home/simark/src/binutils-gdb/gdb/infrun.c:3697
>    #16 0x000055fc7e19f0c4 in do_target_wait (ecs=0x7ffdb77c33a0, options=...) at /home/simark/src/binutils-gdb/gdb/infrun.c:3716
>    #17 0x000055fc7e1a31f7 in fetch_inferior_event () at /home/simark/src/binutils-gdb/gdb/infrun.c:4061
> 
> Normally, the wait on the remote target would return nothing, so we
> would then wait on the native target and fetch its event.

Thanks for digging the details up.  I assumed it had something to do with
an initial misunderstanding I had which was that a target was always in
async mode vs only being in async mode when there are non-stopped
inferiors.  Can you please include these details in your commit reverting
this change?  (Or did you want me to push the revert?  I'm happy for you
to do so since you did the testing and investigation.)

-- 
John Baldwin