Re: [PATCH 01/30] gdb/mi: fix ^running record with multiple MI interpreters

[Simon Marchi <simark@simark.ca>]

On 5/2/23 16:49, Simon Marchi via Gdb-patches wrote:
> I stumbled on the mi_proceeded and running_result_record_printed
> globals, which are shared by all MI interpreter instances (it's unlikely
> that people use multiple MI interpreter instances, but it's possible).
> After poking at it, I found this bug:
> 
> 1. Start GDB in MI mode
> 2. Add a second MI interpreter with the new-ui command
> 3. Use -exec-run on the second interpreter
> 
> This is the output I get on the first interpreter:
> 
>     =thread-group-added,id="i1"
>     ~"Reading symbols from a.out...\n"
>     ~"New UI allocated\n"
>     (gdb)
>     =thread-group-started,id="i1",pid="94718"
>     =thread-created,id="1",group-id="i1"
>     ^running
>     *running,thread-id="all"
> 
> And this is the output I get on the second intepreter:
> 
>     =thread-group-added,id="i1"
>     (gdb)
>     -exec-run
>     =thread-group-started,id="i1",pid="94718"
>     =thread-created,id="1",group-id="i1"
>     *running,thread-id="all"
> 
> The problem here is that the `^running` reply to the -exec-run command
> is printed on the wrong UI.  It is printed on the first one, it should
> be printed on the second (the one on which we sent the -exec-run).
> 
> What happens under the hood is that captured_mi_execute_command, while
> executing a command for the second intepreter, clears the
> running_result_record_printed and mi_proceeded globals.
> mi_about_to_proceed then sets mi_proceeded.  Then, mi_on_resume_1 gets
> called for the first intepreter first.  Since the
> 
>     !running_result_record_printed && mi_proceeded
> 
> condition is true, it prints a ^running, and sets
> running_result_record_printed.  When mi_on_resume_1 gets called for the
> second interpreter, running_result_record_printed is already set, so
> ^running is not printed there.
> 
> It took me a while to understand the relationship between these two
> variables.  I think that in the end, this is what we want to track:
> 
>  1. When executing an MI command, take note if that command causes a
>     "proceed".  This is done in mi_about_to_proceed.
>  2. In mi_on_resume_1, if the command indeed caused a "proceed", we want
>     to output a ^running record.  And we want to remember that we did,
>     because...
>  3. Back in captured_mi_execute_command, if we did not output a
>     ^running, we want to output a ^done.
> 
> Moving those two variables to the mi_interp struture appears to fix it.
> Only for the interpreter doing the -exec-run command does the
> running_result_record_printed flag get cleared, and therefore only or
> that one does the ^running record get printed.
> 
> Add a new test for this, that does pretty much what the reproducer above
> shows.  Without the fix, the test fails because
> mi_send_resuming_command_raw never sees the ^running record.
> 
> Change-Id: I63ea30e6cb61a8e1dd5ef03377e6003381a9209b

I pushed just this patch on its own.  The rest of the series conflicts
in non-trivial ways with new stuff in master.

Simon