Re: [PATCH][gdb] Fix assert in remote_async_get_pending_events_handler

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-04-22 7:03 a.m., Tom de Vries wrote:
> On 4/22/21 12:19 PM, Andrew Burgess wrote:
>> * Tom de Vries <tdevries@suse.de> [2021-04-22 10:51:29 +0200]:
>>
>>> Hi,
>>>
>>> Occassionally I run into the following assert:
>>> ...
>>> (gdb) PASS: gdb.multi/multi-target-continue.exp: inferior 5
>>> Remote debugging from host ::1, port 49990^M
>>> Process multi-target-continue created; pid = 31241^M
>>> src/gdb/remote-notif.c:113: internal-error: \
>>>   void remote_async_get_pending_events_handler(gdb_client_data): \
>>>   Assertion `target_is_non_stop_p ()' failed.^M
>>> ...
>>>
>>> The assert checks target_is_non_stop_p, which is related to the current
>>> target.
>>>
>>> Fix this by changing the assert such that it checks non-stopness related to
>>> the event it's handling.
>>>
>>> Tested on x86_64-linux.
>>>
>>> Any comments?
>>
>> This seems fine to me.  I wonder though if you considered converting
>> target_is_non_stop_p into a member function on target_ops?
>> If we did
>> then we would avoid having to switch targets just to ask this
>> question.  All of the helper functions that target_is_non_stop_p calls
>> are already available as member functions so there would be no
>> additional changes needed I think.
>>
> 
> Um, I'm the one who ran into the problem, Simon is the one who came up
> with the fix, so I guess this is a question for him.  I'm afraid I'm not
> familiar with this code at all.

This was proposed in the bug as well:

  https://sourceware.org/bugzilla/show_bug.cgi?id=27710#c16

I am not against it, but I think Tom's patch is OK, given it follows
current practices.

Simon