Description of problem: gdb.execute([...], to_string=True) should not print to stdout/stderr. It does print something, somehow to stdout/stderr. Version-Release number of selected component (if applicable): gdb-7.1.90.20100806-10.fc14.i686 How reproducible: Always. Steps to Reproduce: (gdb) python >help_out = gdb.execute("help", False, to_string=True) >print "help_out:" >print help_out >end Actual results: Aliases of other commandsMaking program stop at certain pointsExamining dataSpecifying and examining filesMaintenance commandsObscure featuresRunning the programExamining the stackStatus inquiriesSupport facilitiesTracing of program execution without stopping the programUser-defined commandshelp_out: List of classes of commands: aliases -- breakpoints -- data -- files -- internals -- obscure -- running -- stack -- status -- support -- tracepoints -- user-defined -- Type "help" followed by a class name for a list of commands in that class. Type "help all" for the list of all commands. Type "help" followed by command name for full documentation. Type "apropos word" to search for commands related to "word". Command name abbreviations are allowed if unambiguous. (gdb) Expected results: help_out: List of classes of commands: aliases -- Aliases of other commands breakpoints -- Making program stop at certain points data -- Examining data files -- Specifying and examining files internals -- Maintenance commands obscure -- Obscure features running -- Running the program stack -- Examining the stack status -- Status inquiries support -- Support facilities tracepoints -- Tracing of program execution without stopping the program user-defined -- User-defined commands Type "help" followed by a class name for a list of commands in that class. Type "help all" for the list of all commands. Type "help" followed by command name for full documentation. Type "apropos word" to search for commands related to "word". Command name abbreviations are allowed if unambiguous. (gdb) Additional info: Note how only a part of the output (everything following " -- " in the output) is not printed "to_string" bot to stdout/stderr. I have no idea what's going on yet, so I can't suggest a patch.
0) For what it's worth, it seems gdb/cli/cli-decode.c:print_help_for_command() is relevant here. static void print_help_for_command (struct cmd_list_element *c, char *prefix, int recurse, struct ui_file *stream) { fprintf_filtered (stream, "%s%s -- ", prefix, c->name); print_doc_line (stream, c->doc); fputs_filtered ("\n", stream); [...]; } Note that the lines still printed by gdb here, and not sent "to_string", are generated with print_doc_line(). 1) gdb/cli/cli-decode.c:print_doc_line() doesn't actually use the stream argument and apparently prints using the (new for me) ui_out stuff: ui_out_text (uiout, line_buffer); My first guess is the ui_out stuff ignores the python command tricks with stdout/stderr. Sadly, the ui_out stuff is opaque to me, so I couldn't pinpoint this further.
uiout should be also redirected. Why it isn't is a question/bug.
Created attachment 441315 [details] make uiout do what gdb.execute([...]), to_string=True) expects This patch is in the "hit the engine until it starts running" category, but it seems to do the right thing. Let's just call it a RFC.
There is rather ui_out_redirect(), see handle_redirections().
(In reply to comment #4) > There is rather ui_out_redirect(), see handle_redirections(). That was a bit terse. I didn't understand the reference to handle_redirections(). Anyhow, this seems to work too (please note that it's very lightly tested). Is this what was hinted at? diff --git a/gdb/top.c b/gdb/top.c index b29e68d..038c916 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -481,11 +481,13 @@ execute_command_to_string (char *p, int from_tty) gdb_stdout = str_file; gdb_stderr = str_file; + ui_out_redirect (uiout, str_file); execute_command (p, from_tty); retval = ui_file_xstrdup (str_file, NULL); + ui_out_redirect (uiout, NULL); do_cleanups (cleanup); return retval;
(In reply to comment #5) > Is this what was hinted at? Generally yes. > @@ -481,11 +481,13 @@ execute_command_to_string (char *p, int from_tty) > > gdb_stdout = str_file; > gdb_stderr = str_file; > + ui_out_redirect (uiout, str_file); > > execute_command (p, from_tty); > > retval = ui_file_xstrdup (str_file, NULL); > > + ui_out_redirect (uiout, NULL); > do_cleanups (cleanup); There is a problem. If execute_command() throws an exception uiout remains incorrectly redirected. There should be registered a cleanup function that gets executed (either during an exception or during do_cleanups (cleanup);). These exceptions are specific to GDB (not C++) which is a bit inconvenient.
(In reply to comment #6) > There is a problem. If execute_command() throws an exception uiout remains > incorrectly redirected. There should be registered a cleanup function that > gets executed (either during an exception or during do_cleanups (cleanup);). You mean something like this: diff --git a/gdb/top.c b/gdb/top.c index b29e68d..d94c662 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -458,6 +458,12 @@ execute_command (char *p, int from_tty) } } +static void +do_ui_out_redirect (void *dummy) +{ + ui_out_redirect (uiout, NULL); +}; + /* Run execute_command for P and FROM_TTY. Capture its output into the returned string, do not display it to the screen. BATCH_FLAG will be temporarily set to true. */ @@ -477,10 +483,14 @@ execute_command_to_string (char *p, int from_tty) make_cleanup_restore_ui_file (&gdb_stdout); make_cleanup_restore_ui_file (&gdb_stderr); + make_cleanup_restore_ui_file (&gdb_stdtarg); make_cleanup_ui_file_delete (str_file); gdb_stdout = str_file; gdb_stderr = str_file; + gdb_stdtarg = str_file; + ui_out_redirect (uiout, str_file); + make_cleanup (do_ui_out_redirect, 0); execute_command (p, from_tty); (Note that this also redirects gdb_stdtarg to str_file, which helps with the monitor command. Trying to send the output of the monitor command to a python string, while debugging a remote target, actually is what triggered me to do all this.)
(In reply to comment #7) > +static void > +do_ui_out_redirect (void *dummy) > +{ > + ui_out_redirect (uiout, NULL); > +}; The original redirection may not be to NULL but to some other file when `set logging' is active. You should restore the original value, not to just set it to NULL. Also this function should be present in utils.c where all the cleanup stubs are present. > make_cleanup_restore_ui_file (&gdb_stdout); > make_cleanup_restore_ui_file (&gdb_stderr); > + make_cleanup_restore_ui_file (&gdb_stdtarg); + > (Note that this also redirects gdb_stdtarg to str_file, which helps with the > monitor command. Trying to send the output of the monitor command to a python > string, while debugging a remote target, actually is what triggered me to do > all this.) Shouldn't be also gdb_stdlog redirected? (I am not sure now but I ask, at least it could be commented why not). This is not a requirement for the patch, just if you would like it complete. > + make_cleanup (do_ui_out_redirect, 0); This should be also moved to the stub in utils.c BTW this patch is getting larger than trivial, do you have a signed FSF GDB copyright assignment? If not please contact Tom Tromey <tromey at redhat.com>. The patch will need to be posted to <gdb-patches>. I can check it more during weekend otherwise.
(In reply to comment #8) > BTW this patch is getting larger than trivial, do you have a signed FSF GDB > copyright assignment? If not please contact Tom Tromey <tromey at redhat.com>. No, I don't. I'm happy to do that, but I'm also happy to see a patch submitted on your name (as I'm just following the directions you gave me). > The patch will need to be posted to <gdb-patches>. > > I can check it more during weekend otherwise. I suppose we finalize the patch here, before sending it to that list. Is that right?
(In reply to comment #9) > I suppose we finalize the patch here, before sending it to that list. Is that > right? I do not mind, I usually post an upstream post link to the RH Bug anyway.
(In reply to comment #8) > The original redirection may not be to NULL but to some other file > when `set logging' is active. You should restore the original value, not to > just set it to NULL. 0) There seem to be three places where a pair of complementing calls to ui_out_redirect() is used: gdb/breakpoint.c: save_breakpoints() gdb/python/py-breakpoint.c: bppy_get_commands() gdb/cli/cli-logging.c: set_logging_on() handle_redirections() set_logging_off() pop_output_files() Doesn't the same problem occur with the interaction of save_breakpoints(), bppy_get_commands(), and set_logging_on and set_logging_off? 1) I'd think gdb/cli-out.c:cli_redirect() could manipulate a (limited) stack of streams in ui_out->data instead of the current pair of ui_out->data-stream and ui_out->data->original_stream to handle this problem. But how many levels of redirection are actually possible?
(In reply to comment #8) > Shouldn't be also gdb_stdlog redirected? > (I am not sure now but I ask, at least it could be commented why not). > > This is not a requirement for the patch, just if you would like it complete. I wouldn't know. I know next to nothing about the logging system. I never ran into a problem with the python command and gdb_stdlog so I haven't looked at it yet. By the way, there's also gdb_stdtargerr. But that's new for me too.
Created attachment 442390 [details] updated patch, taken into account comments from comment #8 0) Another try, with a rather large patch. 1) Patch highlights: - allows for a trivial stack of four outstreams (ie, three redirects). Four is an arbitrary cutoff; - that stack is implemented with cli_stream_push(), cli_stream_pop(), and changes to struct cli_ui_out_data; - adds a new cleanup: make_cleanup_redirect_ui_out(). That cleanup doesn't blindly pop off the last outstream, but has a simple test to see if an outstream needs to be popped off (eg, it tries to do the right thing even if invoked before the outstream actually was redirected in the first place); - now only four lines are changed in gdb/top.c:execute_command_to_string(), but rather a lot had to be changed in related areas to get there. 2) Comments appreciated.
Created attachment 442392 [details] include _all_ needed changes 0) Yet another try, with an even larger patch (which should actually be complete). 1) Patch highlights: - allows for a trivial stack of four outstreams (ie, three redirects). Four is an arbitrary cutoff; - that stack is implemented with cli_stream_push(), cli_stream_pop(), and changes to struct cli_ui_out_data; - adds ui_out_can_redirect(), a small helper function; - adds a new cleanup: make_cleanup_redirect_ui_out(). That cleanup doesn't blindly pop off the last outstream, but has a simple test to see if an outstream needs to be popped off (eg, it tries to do the right thing even if invoked before the outstream actually was redirected in the first place); - now only four lines are changed in gdb/top.c:execute_command_to_string(), but rather a lot had to be changed in related areas to get there. 2) Comments appreciated.
(In reply to comment #14) > - allows for a trivial stack of four outstreams (ie, three redirects). Four is > an arbitrary cutoff; I do not find such limitation needed/useful/good. > 2) Comments appreciated. Bugzilla is not a place for patches discussion. gdb-patches is. Going to post there something.
(In reply to comment #15) > Going to post there something. If you mean to say that you're going to post a solution on that list, please feel free to CC me.
pre-requisite fix: [patch] Fix nesting of ui_out_redirect http://sourceware.org/ml/gdb-patches/2010-09/msg00120.html
The fix: [patch] Fix uiout for execute_command_to_string http://sourceware.org/ml/gdb-patches/2010-09/msg00131.html I haven't seen a real world reason for ui_out_can_redirect(). If you think it should be there follow up upstream, please.
Upstream check-in: http://sourceware.org/ml/gdb-cvs/2010-09/msg00080.html
gdb-7.2-4.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/gdb-7.2-4.fc14
gdb-7.2-4.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report.