Toolchain/binutils-gdb
1
0
Fork 0
forked from Imagelibrary/binutils-gdb
Code Pull Requests Activity

Move "tee" building down to interpreter::set_logging_proc

Browse Source 
This patch gets rid of this hack in mi_set_logging:

      /* The tee created already is based on gdb_stdout, which for MI
	 is a console and so we end up in an infinite loop of console
	 writing to ui_file writing to console etc.  So discard the
	 existing tee (it hasn't been used yet, and MI won't ever use
	 it), and create one based on raw_stdout instead.  */

By pushing down responsibility for the tee creation to the
interpreter.  I.e., pushing the CLI bits out of handle_redirections
down to the CLI interpreter's set_logging_proc method.

This fixes a few leaks that I spotted, and then confirmed with
"valgrind --leak-check=full":

[...]
  ==21429== 56 (32 direct, 24 indirect) bytes in 1 blocks are definitely lost in loss record 30,243 of 34,980
  ==21429==    at 0x4C29216: operator new(unsigned long) (vg_replace_malloc.c:334)
  ==21429==    by 0x62D9A9: mi_set_logging(interp*, int, ui_file*, ui_file*) (mi-interp.c:1395)
  ==21429==    by 0x810B8A: current_interp_set_logging(int, ui_file*, ui_file*) (interps.c:360)
  ==21429==    by 0x61C537: handle_redirections(int) (cli-logging.c:162)
  ==21429==    by 0x61C6EC: set_logging_on(char*, int) (cli-logging.c:190)
  ==21429==    by 0x6163BE: do_cfunc(cmd_list_element*, char*, int) (cli-decode.c:105)
  ==21429==    by 0x6193C1: cmd_func(cmd_list_element*, char*, int) (cli-decode.c:1913)
  ==21429==    by 0x8DB790: execute_command(char*, int) (top.c:674)
  ==21429==    by 0x632AE6: mi_execute_cli_command(char const*, int, char const*) (mi-main.c:2343)
  ==21429==    by 0x6329BA: mi_cmd_execute(mi_parse*) (mi-main.c:2306)
  ==21429==    by 0x631E19: captured_mi_execute_command(ui_out*, mi_parse*) (mi-main.c:1998)
  ==21429==    by 0x632389: mi_execute_command(char const*, int) (mi-main.c:2163)
  ==21429==
[...]
  ==26635== 24 bytes in 1 blocks are definitely lost in loss record 20,740 of 34,995
  ==26635==    at 0x4C29216: operator new(unsigned long) (vg_replace_malloc.c:334)
  ==26635==    by 0x61C355: handle_redirections(int) (cli-logging.c:131)
  ==26635==    by 0x61C6EC: set_logging_on(char*, int) (cli-logging.c:190)
  ==26635==    by 0x6163BE: do_cfunc(cmd_list_element*, char*, int) (cli-decode.c:105)
  ==26635==    by 0x6193C1: cmd_func(cmd_list_element*, char*, int) (cli-decode.c:1913)
  ==26635==    by 0x8DB7BC: execute_command(char*, int) (top.c:674)
  ==26635==    by 0x7B9132: command_handler(char*) (event-top.c:590)
  ==26635==    by 0x7B94F7: command_line_handler(char*) (event-top.c:780)
  ==26635==    by 0x7B8ABB: gdb_rl_callback_handler(char*) (event-top.c:213)
  ==26635==    by 0x933CE9: rl_callback_read_char (callback.c:220)
  ==26635==    by 0x7B89ED: gdb_rl_callback_read_char_wrapper_noexcept() (event-top.c:175)
  ==26635==    by 0x7B8A49: gdb_rl_callback_read_char_wrapper(void*) (event-top.c:192)

One is fixed by transfering ownership of the log file to the tee.  In
pseudo-patch, since the code was moved at the same time:

 -     out = new tee_file (curr_output, false, logfile.get (), false);
 +     out = new tee_file (curr_output, false, logfile.get (), true);

The other is this bit in mi_set_logging:

    else
      {
 +      delete mi->raw_stdout;

I tried to split the leak fixes to a smaller preparatory patch, but
that was difficult exactly because of the tee hack in
handle_redirections -> mi_set_logging.

gdb/ChangeLog:
2017-02-02  Pedro Alves  <palves@redhat.com>

	* cli/cli-interp.c (struct saved_output_files, saved_output):
	Moved from cli/cli-logging.c.
	(cli_set_logging): New function.
	(cli_interp_procs): Install cli_set_logging.
	* cli/cli-interp.h (make_logging_output, cli_set_logging):
	Declare.
	* cli/cli-logging.c (struct saved_output_files, saved_output):
	Moved to cli/cli-interp.c.
	(pop_output_files): Don't save outputs here.
	(make_logging_output): New function.
	(handle_redirections): Don't build tee nor save previous outputs
	here.
	* interps.c (current_interp_set_logging): Change prototype.
	Assume there's always a set_logging_proc method installed.
	* interps.h (interp_set_logging_ftype): Change prototype.
	(current_interp_set_logging): Change prototype and adjust comment.
	* mi/mi-interp.c (mi_set_logging): Change protototype.  Adjust to
	use make_logging_output.
	* tui/tui-interp.c (tui_interp_procs): Install cli_set_logging.
This commit is contained in:
Pedro Alves
2017-02-02 22:00:43 +00:00
parent 5be5dbf0ce
commit 616268b639
8 changed files with 154 additions and 101 deletions
Download Patch File Download Diff File Expand all files Collapse all files

90
gdb/cli/cli-logging.c
View File

@@ -22,17 +22,6 @@
#include "ui-out.h"
#include "interps.h"
/* These hold the pushed copies of the gdb output files.
If NULL then nothing has yet been pushed. */
struct saved_output_files
{
struct ui_file *out;
struct ui_file *err;
struct ui_file *log;
struct ui_file *targ;
struct ui_file *targerr;
};
static struct saved_output_files saved_output;
static char *saved_filename;
static char *logging_filename;
@@ -90,37 +79,35 @@ show_logging_redirect (struct ui_file *file, int from_tty,
static void
pop_output_files (void)
{
if (current_interp_set_logging (0, NULL, NULL) == 0)
{
/* Only delete one of the files -- they are all set to the same
value. */
delete gdb_stdout;
gdb_stdout = saved_output.out;
gdb_stderr = saved_output.err;
gdb_stdlog = saved_output.log;
gdb_stdtarg = saved_output.targ;
gdb_stdtargerr = saved_output.targerr;
}
saved_output.out = NULL;
saved_output.err = NULL;
saved_output.log = NULL;
saved_output.targ = NULL;
saved_output.targerr = NULL;
current_interp_set_logging (NULL, false);
/* Stay consistent with handle_redirections. */
if (!current_uiout->is_mi_like_p ())
current_uiout->redirect (NULL);
}
/* See cli-interp.h. */
ui_file *
make_logging_output (ui_file *curr_output, ui_file_up logfile,
bool logging_redirect)
{
if (logging_redirect)
return logfile.release ();
else
{
/* Note that the "tee" takes ownership of the log file. */
ui_file *out = new tee_file (curr_output, false,
logfile.get (), true);
logfile.release ();
return out;
}
}
/* This is a helper for the `set logging' command. */
static void
handle_redirections (int from_tty)
{
ui_file_up output;
ui_file_up no_redirect_file;
if (saved_filename != NULL)
{
fprintf_unfiltered (gdb_stdout, "Already logging to %s.\n",
@@ -133,46 +120,27 @@ handle_redirections (int from_tty)
perror_with_name (_("set logging"));
/* Redirects everything to gdb_stdout while this is running. */
if (!logging_redirect)
if (from_tty)
{
no_redirect_file = std::move (log);
output.reset (new tee_file (gdb_stdout, 0, no_redirect_file.get (), 0));
if (from_tty)
if (!logging_redirect)
fprintf_unfiltered (gdb_stdout, "Copying output to %s.\n",
logging_filename);
}
else
{
output = std::move (log);
if (from_tty)
else
fprintf_unfiltered (gdb_stdout, "Redirecting output to %s.\n",
logging_filename);
}
saved_filename = xstrdup (logging_filename);
saved_output.out = gdb_stdout;
saved_output.err = gdb_stderr;
saved_output.log = gdb_stdlog;
saved_output.targ = gdb_stdtarg;
saved_output.targerr = gdb_stdtargerr;
/* Let the interpreter do anything it needs. */
if (current_interp_set_logging (1, output.get (),
no_redirect_file.get ()) == 0)
{
gdb_stdout = output.get ();
gdb_stdlog = output.get ();
gdb_stderr = output.get ();
gdb_stdtarg = output.get ();
gdb_stdtargerr = output.get ();
}
current_interp_set_logging (std::move (log), logging_redirect);
output.release ();
no_redirect_file.release ();
/* Don't do the redirect for MI, it confuses MI's ui-out scheme. */
/* Redirect the current ui-out object's output to the log. Use
gdb_stdout, not log, since the interpreter may have created a tee
that wraps the log. Don't do the redirect for MI, it confuses
MI's ui-out scheme. Note that we may get here with MI as current
interpreter, but with the current ui_out as a CLI ui_out, with
'-interpreter-exec console "set logging on"'. */
if (!current_uiout->is_mi_like_p ())
current_uiout->redirect (gdb_stdout);
}