Re: [PATCH] gdb, infrun: do not discard a step-completed pending waitstatus

[Guinevere Larsen <guinevere@redhat.com>]

On 9/21/26 8:42 AM, Metzger, Markus T wrote:
>> On 9/15/26 11:14 AM, Markus Metzger wrote:
>>> Consider a scenario with two breakpoints on adjacent instructions:
>>>
>>>       bp1 at 0xf00
>>>       bp2 at 0xf01
>>>
>>> as well as two threads in all-stop-on-top-of-non-stop mode.
>>>
>>> Assume that threads A hits bp1 and we report the breakpoint hit to the
>>> user.  When the user continues, we start a step-over for thread A at 0xf00.
>>>
>>> Assume that thread B now hits bp1 and we report the breakpoint hit to the
>>> user.  We stop all threads to report the event.  Meanwhile, the step-over
>>> of thread A completes, so we save the pending waitstatus (stop_pc=0xf01,
>>> currently_stepping=1) of thread A.
>>>
>>> When the user continues, clear_proceed_status_thread() discards the
>>> pending step completed waitstatus of thread A, and proceed() starts
>>> another step-over for thread A at 0xf01.
>>>
>>> We skip bp2 for thread A.
>>>
>>> Remove the code in clear_proceed_status_thread() that discards a step
>>> completed waitstatus and let it get handled normally.
>> Hi! Thanks for working on this.
>>
>> I have a question about the situation in general. From what I
>> understand, the original intent of the code is that, if a user has asked
>> for a step, but we hit something more important (like watchpoint or
>> breakpoint), then we shouldn't mention the step. With this change, a
>> user will now see the step once they resume from the breakpoint, if I
>> understand the code correctly.
> If I understand correctly, the scenario is that the user steps thread A,
> the step completes, but GDB picks another event from thread B, say,
> a breakpoint hit.  The user then continues, and GDB processes the saved
> step completed event from thread A.
>
> In clear_proceed_status_thread(), we set control.step_range_end = 0
> for thread A when we continue thread B from the breakpoint.
> When processing the saved step completed event, process_event_stop_test()
> continues the thread:
>
>    if (ecs->event_thread->control.step_range_end == 0)
>      {
>        infrun_debug_printf ("no stepping, continue");
>        /* Likewise if we aren't even stepping.  */
>        keep_going (ecs);
>        return;
>      }
>
>
> If we instead switch to thread A and then step thread A, we get an extra
> stop event.  When source stepping, we'd simply keep stepping and the
> user wouldn't notice the difference.  When stepping a single instruction,
> however, we'd stop without making progress.
>
> And if we switch to thread A, insert a breakpoint at the current location,
> and continue, we start a step-over for that breakpoint, but then interpret
> the pending event as result of this step-over and report the breakpoint.
>
> This is different from current GDB behavior.
>
> If that breakpoint had already been there and thread A hit it, but GDB picked
> the event of thread B, we'd not discard the pending event of thread A, so
> when we switch to thread A and continue, we report the breakpoint.
>
> The behavior differs whether we stopped at a breakpoint or stepped onto it.
> In the latter case, we might skip the breakpoint.  We also wouldn't call
> finish_step_over() and we wouldn't clean up displaced stepping state.
> Breakpoint actions wouldn't trigger, either.
>
> I'm not sure what would happen if a thread was interrupted and stopped
> right at a breakpoint location.  I'd expect the breakpoint to fire when that
> thread is resumed, or, for the current thread, the interrupt being re-interpreted
> as a breakpoint hit.  Just like a step-completed onto a breakpoint location
> is reported as breakpoint hit when done by the current thread.
>
> We need to handle the step completed event and finish the step-over.
> It is also good that we now report a breakpoint we stepped onto reliably,
> whether this is done by the current thread or some other thread.
>
> We may need some special case to not report a newly inserted breakpoint
> at the current location after switching to a thread that completed a step
> in the background when resuming that thread.  And for the stepi case.
>
> Both are more niche than the bug this patch is fixing.

Right, this makes sense. Yeah, I agree that this is more niche than the 
bug you're dealing with, but I think it would be nice to document that 
this behavior change is known so that if we get a bug about this in the 
future, we know what's going on.

I don't know enough to review the code but I ran the test and I see it 
fixes the issue, so feel free to add my test tag

Tested-By: Guinevere Larsen <guinevere@redhat.com>

-- 
Cheers,
Guinevere Larsen
it/its
she/her (deprecated)

>
> Markus.
> ________________________________________
> Intel Deutschland GmbH
>
> Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
>
> Tel: +49 (89) 99143-0
>
> www.intel.de
>
> Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman
>
> Chairperson of the Supervisory Board: Sonja Pierer
>
> Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>