This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[RFC] Broken -thread-info output in non-stop mode.
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Cc: Vladimir Prus <vladimir at codesourcery dot com>, Marc Khouzam <marc dot khouzam at ericsson dot com>, laszlo dot benedek at ericsson dot com
- Date: Sun, 15 Mar 2009 18:25:13 +0000
- Subject: [RFC] Broken -thread-info output in non-stop mode.
Hi guys,
I've run across this broken MI output in non-stop mode:
-thread-info
=thread-created,id="2",group-id="32"
~"[New Thread 32.32801]\n"
^running
*running,thread-id="2"
(gdb)
Note that we have output ^running instead of ^done, and, that
the ",threads=..." part is completelly missing. I was expecting
something like this instead:
-thread-info
=thread-created,id="2",group-id="32"
*running,thread-id="2"
^done,threads=[{id="2",target-id="...",details="...",state="running"},{id="1",...,frame={...},state="stopped"}]
-thread-info, is calling target_find_new_threads, and ... that is
finding new threads. In non-stop mode, threads we find this way
have to be running, otherwise we'd have seen a stop reply for
them already, hence, we call set_running on them. Then, the problem
is that we decide to output ^running instead of ^done in the
mi_on_resume observer. This observer is called from inside the
threads.c:set_running function. So, we're here:
static void
mi_on_resume (ptid_t ptid)
{
/* To cater for older frontends, emit ^running, but do it only once
per each command. We do it here, since at this point we know
that the target was successfully resumed, and in non-async mode,
we won't return back to MI interpreter code until the target
is done running, so delaying the output of "^running" until then
will make it impossible for frontend to know what's going on.
In future (MI3), we'll be outputting "^done" here. */
if (!running_result_record_printed)
{
if (current_token)
fputs_unfiltered (current_token, raw_stdout);
fputs_unfiltered ("^running\n", raw_stdout);
}
Coming from here:
#0 mi_on_resume (ptid={pid = 32, lwp = 0, tid = 32801})
at mi/mi-interp.c:389
#1 0x0805049a in observer_target_resumed_notification_stub (data=0x8093339, args_data=0xffffce30)
at ./observer.inc:422
#2 0x0804fd6b in generic_observer_notify (subject=0x83cb870, args=0xffffce30)
at gdb/observer.c:166
#3 0x08050524 in observer_notify_target_resumed (ptid={pid = 32, lwp = 0, tid = 32801}) at ./observer.inc:447
#4 0x080fadf4 in set_running (ptid={pid = 32, lwp = 0, tid = 32801}, running=1)
at gdb/thread.c:524
#5 0x0806c5de in remote_add_thread (currthread={pid = 32, lwp = 0, tid = 32801}, running=1)
at gdb/remote.c:1154
#5 0x0806c5de in remote_notice_new_inferior (currthread={pid = 32, lwp = 0, tid = 32801}, running=1)
at gdb/remote.c:1213
#6 0x0806dd41 in remote_threads_info ()
at remote.c:2247
#7 0x080fb406 in print_thread_info (uiout=0x83c3670, requested_thread=-1, pid=-1)
at gdb/thread.c:712
#8 0x08093d60 in mi_cmd_thread_info (command=0x84ba8e0 "thread-info", argv=0x83dc4a8, argc=0)
at mi/mi-main.c:347
The "^running" output was moved to the observer by this change:
http://sourceware.org/ml/gdb-patches/2008-06/msg00247.html
I've been trying to find a way to fix this without changing the
MI output, but it is tricky at best. In sync mode, we can not delay
printing the "^running" bit until the current command is done, as that
would be too late --- that's the main reason it is done on the
observer. In async mode, however, I think we can, by instead of
outputting "^running" immediatly in the observer, setting a flag claiming
any thread was resumed, and then when the command finishes, we output
the "^running" instead of "^done". In async mode, the MI resumption commands
are always asynchronous, so when the command is finished the target is still
running, unlike in sync mode. See patch below --- although it becomes a
misnomer, I'm reusing the "running_result_record_printed" flag for
exactly this. This somewhat fixes the broken output I shown at the top
of this email, but, it still leaves a "^running" where I don't
think we want it at all, see:
-thread-info
=thread-created,id="2",group-id="32"
*running,thread-id="2"
^running,threads=[{id="2",target-id="...",details="...",state="running"},{id="1",...,frame={...},state="stopped"}]
^^^^^^^^
For -thread-info, that should always be "^done", I believe.
"^done" [ "," results ]
The synchronous operation was successful, results are the return values.
"^running"
The asynchronous operation was successfully started. The target is running.
I kind of only expect ^running on resumption commands (-exec-run,
-exec-step, -exec-next, etc.). The docs mention "asynchronous
operation", although we do output it for synchronous resumptions ...
( My interpretation of the docs is that in SYNC mode, -exec-step
should output a ^done, when the step is complete, and the target
stops; while in ASYNC mode, -exec-step would output an
immediate ^running. But, that's not how it works... anyway,
moving on. )
Any suggestions on this? One way I was thinking was to split the
"this thread is running because we just resumed it"
from "this thread was already running" set_running calls, and pass
that information down to the observer, but, I'm not sure that will
fix it completelly. In multi-process, we may even find a new
process when we do a -thread-info, and we may end up doing an internal
stop/resume bit, thus breaking the output again.
My next bet is to still do what the patch below does, but in addition,
never output "^running" if the command was a "-thread-info", similarly
to the hack in place that checks for "target-select". Or the other way
around, white list which commands want ^running instead of ^done --- AFAIK,
only the resumption commands are expected to output ^running: -exec-run,
-exec-step, -exec-next, etc. For MI3, when we completelly get rid of "^running",
the hack could go away. This would leave CLI commands issued from
MI to solve --- "info threads" would still output ^running...
It is a problem? I'm guessing it would be, but I'm not sure.
The patch at:
http://sourceware.org/ml/gdb-patches/2008-06/msg00247.html
... claimed that it is a bonus. Is it really a bonus, in that would
any frontend really expect it? Given that CLI commands didn't output
^running for close to 10 years, how bad would it be if we reverted
back to not doing it again, at least for async targets?
Any other suggestions?
--
Pedro Alves
---
gdb/mi/mi-interp.c | 2 +-
gdb/mi/mi-main.c | 23 +++++++++++++++++++++--
2 files changed, 22 insertions(+), 3 deletions(-)
Index: src/gdb/mi/mi-interp.c
===================================================================
--- src.orig/gdb/mi/mi-interp.c 2009-03-15 17:11:00.000000000 +0000
+++ src/gdb/mi/mi-interp.c 2009-03-15 17:30:58.000000000 +0000
@@ -378,7 +378,7 @@ mi_on_resume (ptid_t ptid)
will make it impossible for frontend to know what's going on.
In future (MI3), we'll be outputting "^done" here. */
- if (!running_result_record_printed)
+ if (!running_result_record_printed && !target_is_async_p ())
{
if (current_token)
fputs_unfiltered (current_token, raw_stdout);
Index: src/gdb/mi/mi-main.c
===================================================================
--- src.orig/gdb/mi/mi-main.c 2009-03-15 17:11:00.000000000 +0000
+++ src/gdb/mi/mi-main.c 2009-03-15 17:37:16.000000000 +0000
@@ -1165,7 +1165,18 @@ captured_mi_execute_command (struct ui_o
to directly use the mi_interp's uiout, since the command could
have reset the interpreter, in which case the current uiout
will most likely crash in the mi_out_* routines. */
- if (!running_result_record_printed)
+ if (running_result_record_printed && target_is_async_p ())
+ {
+ fputs_unfiltered (context->token, raw_stdout);
+ /* There's no particularly good reason why target-connect results
+ in not ^done. Should kill ^connected for MI3. */
+ fputs_unfiltered (strcmp (context->command, "target-select") == 0
+ ? "^connected" : "^running", raw_stdout);
+ mi_out_put (uiout, raw_stdout);
+ mi_out_rewind (uiout);
+ fputs_unfiltered ("\n", raw_stdout);
+ }
+ else if (!running_result_record_printed)
{
fputs_unfiltered (context->token, raw_stdout);
/* There's no particularly good reason why target-connect results
@@ -1203,7 +1214,15 @@ captured_mi_execute_command (struct ui_o
|| current_interp_named_p (INTERP_MI2)
|| current_interp_named_p (INTERP_MI3))
{
- if (!running_result_record_printed)
+ if (running_result_record_printed && target_is_async_p ())
+ {
+ fputs_unfiltered (context->token, raw_stdout);
+ fputs_unfiltered ("^running\n", raw_stdout);
+ mi_out_put (uiout, raw_stdout);
+ mi_out_rewind (uiout);
+ fputs_unfiltered ("\n", raw_stdout);
+ }
+ else if (!running_result_record_printed)
{
fputs_unfiltered (context->token, raw_stdout);
fputs_unfiltered ("^done", raw_stdout);