| Summary: | With mi-async on, -exec-continue outputs two (gdb) prompts | ||
|---|---|---|---|
| Product: | gdb | Reporter: | Simon Marchi <simark> |
| Component: | mi | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | pedro |
| Priority: | P2 | ||
| Version: | HEAD | ||
| Target Milestone: | --- | ||
| Host: | Target: | ||
| Build: | Last reconfirmed: | ||
Bisecting found this commit to have introduced the behavior: 198297aafb4f7a9717be8370581b048ae9107c14 Linux: make target_is_async_p return false when async is off Ok, from what I understand, in an ideal world, the first (gdb) wouldn't be there. It's printed here: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/mi/mi-interp.c;h=b37dc96ce4e0d4095a570b2e70ca360a64a21a58;hb=HEAD#l993 It seems like it's just there to keep an old behaviour, where doing -exec-continue in synchronous mode prints the prompt immediately, and a second time when the target stops. Setting aside backwards compatibility, gdb should only print the prompt when the target stops, since that's when it's ready to take new commands. Otherwise, it's just lying to the frontend. But in that case, we probably have to live with it. What I don't understand is that the condition evaluates to true here: 1001 if (!target_is_async_p () || sync_execution) 1002 fputs_unfiltered ("(gdb) \n", raw_stdout); When using mi-async on, sync_execution is 0, that's fine. However, target_is_async_p () returns false. I don't understand, because we are currently executing, so the linux native target is active, and it should be async by default (I didn't touch maint set target-async). I interrupted my debugged gdb at another time, while the inferior was executing, and then I could see that target_is_async_p would've returned true. Is it possible that when all threads are stopped, we close the linux_nat_event_pipe, only to re-open it when we resume? Anyway, it seems to me like this condition could only be if (sync_execution) Basically, if we are emulating a synchronous MI command, output the bogus (gdb). When using mi-async off, sync_execution will be false, so we will never output it. The result is this for mi-async off: -exec-run --start =breakpoint-created,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x00000000004004f1",func="main",file="test.c",fullname="/home/emaisin/build/binutils-gdb/gdb/test.c",line="2",thread-groups=["i1"],times="0",original-location="main"} =thread-group-started,id="i1",pid="1729" =thread-created,id="1",group-id="i1" =library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1" ^running *running,thread-id="all" (gdb) =library-loaded,id="/lib/x86_64-linux-gnu/libc.so.6",target-name="/lib/x86_64-linux-gnu/libc.so.6",host-name="/lib/x86_64-linux-gnu/libc.so.6",symbols-loaded="0",thread-group="i1" =breakpoint-modified,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x00000000004004f1",func="main",file="test.c",fullname="/home/emaisin/build/binutils-gdb/gdb/test.c",line="2",thread-groups=["i1"],times="1",original-location="main"} ~"\n" ~"Temporary breakpoint 1, main () at test.c:2\n" ~"2\t\tfor (;;);\n" *stopped,reason="breakpoint-hit",disp="del",bkptno="1",frame={addr="0x00000000004004f1",func="main",args=[],file="test.c",fullname="/home/emaisin/build/binutils-gdb/gdb/test.c",line="2"},thread-id="1",stopped-threads="all",core="2" =breakpoint-deleted,id="1" (gdb) -exec-continue ^running *running,thread-id="all" (gdb) ^C~"\nProgram" ~" received signal SIGINT, Interrupt.\n" ~"main () at test.c:2\n" ~"2\t\tfor (;;);\n" *stopped,reason="signal-received",signal-name="SIGINT",signal-meaning="Interrupt",frame={addr="0x00000000004004f1",func="main",args=[],file="test.c",fullname="/home/emaisin/build/binutils-gdb/gdb/test.c",line="2"},thread-id="1",stopped-threads="all",core="1" (gdb) Notice the (gdb) when -exec-run starts executing, and the other when the target stops (breakpoint at main). Same for the -exec-continue. That's the old (buggy but backwards compatible) behaviour. And with mi-async on: -exec-run --start =breakpoint-created,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x00000000004004f1",func="main",file="test.c",fullname="/home/emaisin/build/binutils-gdb/gdb/test.c",line="2",thread-groups=["i1"],times="0",original-location="main"} =thread-group-started,id="i1",pid="1788" =thread-created,id="1",group-id="i1" =library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-group="i1" ^running *running,thread-id="all" (gdb) =library-loaded,id="/lib/x86_64-linux-gnu/libc.so.6",target-name="/lib/x86_64-linux-gnu/libc.so.6",host-name="/lib/x86_64-linux-gnu/libc.so.6",symbols-loaded="0",thread-group="i1" =breakpoint-modified,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x00000000004004f1",func="main",file="test.c",fullname="/home/emaisin/build/binutils-gdb/gdb/test.c",line="2",thread-groups=["i1"],times="1",original-location="main"} ~"\n" ~"Temporary breakpoint 1, main () at test.c:2\n" ~"2\t\tfor (;;);\n" *stopped,reason="breakpoint-hit",disp="del",bkptno="1",frame={addr="0x00000000004004f1",func="main",args=[],file="test.c",fullname="/home/emaisin/build/binutils-gdb/gdb/test.c",line="2"},thread-id="1",stopped-threads="all",core="0" =breakpoint-deleted,id="1" -exec-continue ^running *running,thread-id="all" (gdb) ^C&"Quit\n" -exec-interrupt ^done (gdb) ~"\nProgram" ~" received signal SIGINT, Interrupt.\n" ~"main () at test.c:2\n" ~"2\t\tfor (;;);\n" *stopped,reason="signal-received",signal-name="SIGINT",signal-meaning="Interrupt",frame={addr="0x00000000004004f1",func="main",args=[],file="test.c",fullname="/home/emaisin/build/binutils-gdb/gdb/test.c",line="2"},thread-id="1",stopped-threads="all",core="3" Notice the single (gdb) after -exec-run. This is right because gdb is truly ready to receive new MI commands at this point. There is no (gdb) when the targets stops at main, because gdb was already ready. Same goes for -exec-continue, there is a single (gdb). Finally, there is a last one when gdb has executed -exec-interrupt. I haven't done some extensive testing though, this is just the result of some quick debugging. Don't forget that -exec-run is always synchronous, due to PR 18077. We should probably fix that. > Is it possible that when all threads are stopped, we close the
> linux_nat_event_pipe, only to re-open it when we resume?
We do, in both cases with in linux_nat_async. We close it with the target_async all in inf-loop.c:inferior_event_handler when the program stops, and reopen with the target_async call in linux_nat_resume when resuming.
> Anyway, it seems to me like this condition could only be > > if (sync_execution) > > Basically, if we are emulating a synchronous MI command, output the bogus > (gdb). When using mi-async off, sync_execution will be false, so we will > never output it. The result is this for mi-async off: I think you meant "When using mi-async on, sync_execution will be false". But when "maint set target-async off" (i.e., on targets that don't do async), sync_execution will be off, even with sync commands. This patch from the console series replaces that with a single enum that works the same for async and sync targets, so probably fixes this: https://sourceware.org/ml/gdb-patches/2016-02/msg00080.html (haven't tried yet) Though I think an easier and safe-for-7.11.1-even fix would be to call target_can_async instead of target_is_async here. (In reply to Pedro Alves from comment #3) > Don't forget that -exec-run is always synchronous, due to PR 18077. We > should probably fix that. Right, but we can probably treat it as a separate problem? Certainly. Was just pointing it out to avoid taking conclusions out of what -exec-run currently outputs. (In reply to Pedro Alves from comment #5) > > Anyway, it seems to me like this condition could only be > > > > if (sync_execution) > > > > Basically, if we are emulating a synchronous MI command, output the bogus > > (gdb). When using mi-async off, sync_execution will be false, so we will > > never output it. The result is this for mi-async off: > > I think you meant "When using mi-async on, sync_execution will be false". Oops, yes. > But when "maint set target-async off" (i.e., on targets that don't do > async), sync_execution will be off, even with sync commands. I didn't think about that. Does that mean that having mi-async on is impossible with sync targets? That would make sense, since when the target is busy waiting on the target, it can't process MI commands. > This patch from the console series replaces that with a single enum that > works the same for async and sync targets, so probably fixes this: > > https://sourceware.org/ml/gdb-patches/2016-02/msg00080.html > > (haven't tried yet) I tested with your console series (both palves/console-submit and palves/console-v3), and it indeed works fine on that. You are always two steps ahead of the rest of the world! > Though I think an easier and safe-for-7.11.1-even fix would be to call > target_can_async instead of target_is_async here. Is it a good idea to change such a behavior on a .1 release though? If a front-end has to do something special to cope with the additional prompt on 7.11 (which I doubt, since we didn't have any bug reports from front-end developers, but still), it would make things even more complicated for them if 7.11.1 had a different behavior than 7.11. Given that it's a mostly harmless bug, I suggest we leave all 7.11 gdbs behaving the same, and fix it for 7.12. WDYT? I just see it as a regression / bug. Seems better to me to contain the chances of frontends relying on the spurious prompt sooner than later. But I definitely won't insist. The master branch has been updated by Simon Marchi <simark@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=28addb40c77db5a5873172b62b6b7b43e5e05014 commit 28addb40c77db5a5873172b62b6b7b43e5e05014 Author: Simon Marchi <simon.marchi@ericsson.com> Date: Tue May 17 17:07:20 2016 -0400 Fix double prompt output after run control MI commands with mi-async on (PR 20045) When you use a run control command (-exec-run, -exec-continue, -exec-next, ...) with mi-async on, an extra (gdb) prompt is displayed: -exec-continue ^running *running,thread-id="all" (gdb) (gdb) It doesn't seem to be a big problem for front-ends, since this behavior started in gdb 7.9 and we haven't heard anything about that. However, it caused me some trouble while writing a test for PR 20039 [1]. The problem comes from an extra (gdb) prompt that we write when running in mi-async off mode to emulate a past buggy behavior. When executing a run control command synchronously, previous gdbs always printed a prompt right away, even though they are not ready to accept new MI commands until the target stops. Only at this time should they display a prompt. But to keep backwards compatibility apparently, we print it anyway. Since commit 198297aaf, the condition that decides whether we should print that "bogus" prompt or not has become true, even when running with mi-async on. Since we already print a prompt at the end of the asynchronous command execution, it results in two prompts for one command. The proposed fix is to call target_can_async_p instead of target_is_async_p, to make the condition: if (!target_can_async_p () || sync_execution) ... show prompt ... That shows the prompt if we are emulating a synchronous command on top of an asynchronous target (sync_execution) or if the target simply can't run asynchronously (!target_can_async_p ()). Note that this code is changed and this bug fixed by Pedro's separate console series, but I think it would be nice to have it fixed in the mean time. I ran the gdb.mi directory of the testsuite with mi-async on and off, I didn't see any regressions. gdb/ChangeLog: * mi/mi-main.c (mi_on_resume): Call target_can_async_p instead of target_is_async_p. [1] https://sourceware.org/ml/gdb-patches/2016-05/msg00075.html The master branch has been updated by Simon Marchi <simark@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9e8f9b05add4517189d7724ff3ed7c16f7b04daf commit 9e8f9b05add4517189d7724ff3ed7c16f7b04daf Author: Simon Marchi <simon.marchi@ericsson.com> Date: Wed May 18 10:13:12 2016 -0400 Add mi-threads-interrupt.exp test (PR 20039) Add a new test for PR 20039. The test spawns new threads, then tries to interrupt, continue, and interrupt again. This use case was fixed by commit 5fe966540d6b748f825774868463003700f0c878 in master, but gdb 7.11 is affected (so if you try it on the gdb-7.11-branch right now, the test will fail). New in v2, the test now handles mi-async on mode properly. The failure was specific to mi-async off, but I don't think it's bad to test the same thing under async on mode. I added a little hack when running in async mode to work around bug 20045. I also removed one continue/interrupt pair, as a single one was enough to trigger the problem. gdb/testsuite/ChangeLog: * gdb.mi/mi-threads-interrupt.c: New file. * gdb.mi/mi-threads-interrupt.exp: New file. The gdb-7.11-branch branch has been updated by Simon Marchi <simark@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f0a8d0dc70eddbf1e323e8c07f5092ff5e327548 commit f0a8d0dc70eddbf1e323e8c07f5092ff5e327548 Author: Simon Marchi <simon.marchi@ericsson.com> Date: Tue May 17 17:07:20 2016 -0400 Fix double prompt output after run control MI commands with mi-async on (PR 20045) When you use a run control command (-exec-run, -exec-continue, -exec-next, ...) with mi-async on, an extra (gdb) prompt is displayed: -exec-continue ^running *running,thread-id="all" (gdb) (gdb) It doesn't seem to be a big problem for front-ends, since this behavior started in gdb 7.9 and we haven't heard anything about that. However, it caused me some trouble while writing a test for PR 20039 [1]. The problem comes from an extra (gdb) prompt that we write when running in mi-async off mode to emulate a past buggy behavior. When executing a run control command synchronously, previous gdbs always printed a prompt right away, even though they are not ready to accept new MI commands until the target stops. Only at this time should they display a prompt. But to keep backwards compatibility apparently, we print it anyway. Since commit 198297aaf, the condition that decides whether we should print that "bogus" prompt or not has become true, even when running with mi-async on. Since we already print a prompt at the end of the asynchronous command execution, it results in two prompts for one command. The proposed fix is to call target_can_async_p instead of target_is_async_p, to make the condition: if (!target_can_async_p () || sync_execution) ... show prompt ... That shows the prompt if we are emulating a synchronous command on top of an asynchronous target (sync_execution) or if the target simply can't run asynchronously (!target_can_async_p ()). Note that this code is changed and this bug fixed by Pedro's separate console series, but I think it would be nice to have it fixed in the mean time. I ran the gdb.mi directory of the testsuite with mi-async on and off, I didn't see any regressions. gdb/ChangeLog: * mi/mi-main.c (mi_on_resume): Call target_can_async_p instead of target_is_async_p. [1] https://sourceware.org/ml/gdb-patches/2016-05/msg00075.html The gdb-7.11-branch branch has been updated by Simon Marchi <simark@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=cf2cd51217c0b99f5370708cc3932c12a1f4edad commit cf2cd51217c0b99f5370708cc3932c12a1f4edad Author: Simon Marchi <simon.marchi@ericsson.com> Date: Wed May 18 10:13:12 2016 -0400 Add mi-threads-interrupt.exp test (PR 20039) Add a new test for PR 20039. The test spawns new threads, then tries to interrupt, continue, and interrupt again. This use case was fixed by commit 5fe966540d6b748f825774868463003700f0c878 in master, but gdb 7.11 is affected (so if you try it on the gdb-7.11-branch right now, the test will fail). New in v2, the test now handles mi-async on mode properly. The failure was specific to mi-async off, but I don't think it's bad to test the same thing under async on mode. I added a little hack when running in async mode to work around bug 20045. I also removed one continue/interrupt pair, as a single one was enough to trigger the problem. gdb/testsuite/ChangeLog: * gdb.mi/mi-threads-interrupt.c: New file. * gdb.mi/mi-threads-interrupt.exp: New file. Fix pushed. |
Test program: int main() { for (;;); return 0; } Build: $ gcc test.c -g3 -O0 -o test Start GDB: $./gdb -nx -i mi -ex "set mi-async on" test Commands: -exec-run --start -exec-continue GDB will output two prompts: -exec-continue ^running *running,thread-id="all" (gdb) (gdb) while we would expect to have just one. This problem wasn't there with gdb 7.8, but was there in 7.9.