Skip to content
  • Pedro Alves's avatar
    Move "tee" building down to interpreter::set_logging_proc · 616268b6
    Pedro Alves authored
    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.
    616268b6