[PATCHv2 5/6] gdb: add assert in remote_target::wait relating to async being off

[Tom de Vries]

On 11/25/21 2:46 PM, Andrew Burgess wrote:
> * Tom de Vries <tdevries@suse.de> [2021-11-25 12:36:07 +0100]:
> 
>> On 11/25/21 11:04 AM, Andrew Burgess via Gdb-patches wrote:
>>> * Simon Marchi <simark@simark.ca> [2021-11-24 16:23:30 -0500]:
>>>
>>>> On 2021-11-24 7:22 a.m., Andrew Burgess via Gdb-patches wrote:
>>>>> While working on another patch I ended up in a situation where I had
>>>>> async mode disabled (with 'maint set target-async off'), but the async
>>>>> event token got marked anyway.
>>>>>
>>>>> In this situation GDB was continually calling into
>>>>> remote_target::wait, however, the async token would never become
>>>>> unmarked as the unmarking is guarded by target_is_async_p.
>>>>>
>>>>> We could just unconditionally unmark the token, but that would feel
>>>>> like just ignoring a bug, so, instead, lets assert that if
>>>>> !target_is_async_p, then the async token should not be marked.
>>>>>
>>>>> This assertion would have caught my earlier mistake.
>>>>>
>>>>> There should be no user visible changes with this commit.
>>>>> ---
>>>>>  gdb/remote.c | 6 +++++-
>>>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/gdb/remote.c b/gdb/remote.c
>>>>> index 25a4d3cab6e..da8ed81ba78 100644
>>>>> --- a/gdb/remote.c
>>>>> +++ b/gdb/remote.c
>>>>> @@ -8309,9 +8309,13 @@ remote_target::wait (ptid_t ptid, struct target_waitstatus *status,
>>>>>    remote_state *rs = get_remote_state ();
>>>>>  
>>>>>    /* Start by clearing the flag that asks for our wait method to be called,
>>>>> -     we'll mark it again at the end if needed.  */
>>>>> +     we'll mark it again at the end if needed.  If the target is not in
>>>>> +     async mode then the async token should not be marked.  */
>>>>>    if (target_is_async_p ())
>>>>>      clear_async_event_handler (rs->remote_async_inferior_event_token);
>>>>> +  else
>>>>> +    gdb_assert (!async_event_handler_marked
>>>>> +		(rs->remote_async_inferior_event_token));
>>>>>  
>>>>>    ptid_t event_ptid;
>>>>>  
>>>>> -- 
>>>>> 2.25.4
>>>>>
>>>>
>>>> LGTM.
>>>>
>>>> I think the series can be merged at least up to here, I think these are
>>>> good cleanups.
>>>
>>> Thanks, I made the change you suggested about one target_can_async_p
>>> function calling the other, and pushed patches 1 to 5.
>>>
>>
>> I'm running into the assert:
>> https://sourceware.org/bugzilla/show_bug.cgi?id=28627 .
> 
> Sorry about that.  I've reverted this patch, and closed the bug.
> 

Np :)

Thanks for the quick fix.
- Tom

> The cause of the assert is fixed by patch #6 in this series, so I
> guess I'll remerge this patch after patch #6 lands.
> 
> Thanks,
> Andrew
>