[RFA] Fix leak in most gdb.mi tests.

[RFA] Fix leak in most gdb.mi tests.

[Philippe Waroquiers]

Valgrind detects a leak on the input_buffer in most (all?) gdb.mi tests,
e.g. gdb.mi/mi2-var-child.exp:

  ==27567== 23,120 bytes in 313 blocks are definitely lost in loss record 3,149 of 3,200
  ==27567==    at 0x4C2E2B3: realloc (vg_replace_malloc.c:836)
  ==27567==    by 0x403D1C: xrealloc (common-utils.c:62)
  ==27567==    by 0x402DD3: buffer_grow(buffer*, char const*, unsigned long) [clone .part.1] (buffer.c:40)
  ==27567==    by 0x49CECA: buffer_grow_char (buffer.h:40)
  ==27567==    by 0x49CECA: gdb_readline_no_editing_callback(void*) (event-top.c:847)
  ==27567==    by 0x49D27F: stdin_event_handler(int, void*) (event-top.c:511)
  ==27567==    by 0x49C0CC: gdb_wait_for_event(int) (event-loop.c:859)
  ==27567==    by 0x49C203: gdb_do_one_event() [clone .part.4] (event-loop.c:347)
  ==27567==    by 0x49C394: gdb_do_one_event (common-exceptions.h:219)
  ==27567==    by 0x49C394: start_event_loop() (event-loop.c:371)
  ==27567==    by 0x532687: captured_command_loop() (main.c:330)
  ==27567==    by 0x53367C: captured_main (main.c:1177)
  ==27567==    by 0x53367C: gdb_main(captured_main_args*) (main.c:1193)
  ==27567==    by 0x2841A7: main (gdb.c:32)

Fix the leak by freeing in mi_execute_command_input_handler
the command allocated in gdb_readline_no_editing_callback.

Tested on amd64, native and under valgrind.

gdb/ChangeLog
2018-12-29  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* mi/mi-interp.c (mi_execute_command_input_handler): xfree cmd,
	allocated in gdb_readline_no_editing_callback.
---
 gdb/mi/mi-interp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 9a317bc0ec..d2a46106a1 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -310,6 +310,9 @@ mi_execute_command_input_handler (char *cmd)
      stops.  */
   if (ui->prompt_state == PROMPT_NEEDED)
     display_mi_prompt (mi);
+
+  /* Allocated in gdb_readline_no_editing_callback.  */
+  xfree (cmd);
 }
 
 void
-- 
2.19.2

Re: [RFA] Fix leak in most gdb.mi tests.

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> Fix the leak by freeing in mi_execute_command_input_handler
Philippe> the command allocated in gdb_readline_no_editing_callback.

Philippe> Tested on amd64, native and under valgrind.

Philippe> gdb/ChangeLog
Philippe> 2018-12-29  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* mi/mi-interp.c (mi_execute_command_input_handler): xfree cmd,
Philippe> 	allocated in gdb_readline_no_editing_callback.

Thanks.

Would you mind adding to the comment comment above struct ui::input_handler
(in top.h) to explain that input_handler takes ownership of the string?

This all seems somewhat ugly to me.  Perhaps ideally the caller should
manage the memory and input_handler should take a const char *?  But not
your problem, fixing the leak is enough.

This is ok with the comment tweak.

Tom

Re: [RFA] Fix leak in most gdb.mi tests.

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> This all seems somewhat ugly to me.  Perhaps ideally the caller should
Tom> manage the memory and input_handler should take a const char *?  But not
Tom> your problem, fixing the leak is enough.

I poked at this and it isn't trivial due to gdb_readline_wrapper_line.

Maybe input_handler could accept a unique_xmalloc_ptr<char>&& instead.
That would at least clarify the ownership semantics.

Tom

Re: [RFA] Fix leak in most gdb.mi tests.

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> Maybe input_handler could accept a unique_xmalloc_ptr<char>&& instead.
Tom> That would at least clarify the ownership semantics.

What do you think of this?

Tom

commit fe70f92b8e3c0af79d041b9004b9abbeed27374d
Author: Tom Tromey <tom@tromey.com>
Date:   Sat Dec 29 12:42:18 2018 -0700

    Change input_handler to take a unique_xmalloc_ptr
    
    This changes ui::input_handler to take a unique_xmalloc_ptr.  This
    clarifies the ownership transfer of input_handler's argument.
    
    gdb/ChangeLog
    2018-12-29  Tom Tromey  <tom@tromey.com>
    
            * event-top.h (command_line_handler): Update.
            * top.c (class gdb_readline_wrapper_cleanup) <m_handler_orig>:
            Update.
            (gdb_readline_wrapper_line): Update.
            * top.h (struct ui) <input_handler>: Take a unique_xmalloc_ptr.
            (handle_line_of_input): Update.
            * event-top.c: Update.
            (gdb_readline_no_editing_callback): Update.
            (command_line_handler): Take a unique_xmalloc_ptr.
            (handle_line_of_input): Take a const char *.
            (command_line_append_input_line): Take a const char *.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5ed91a84fc..56d86acee3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,17 @@
+2018-12-29  Tom Tromey  <tom@tromey.com>
+
+	* event-top.h (command_line_handler): Update.
+	* top.c (class gdb_readline_wrapper_cleanup) <m_handler_orig>:
+	Update.
+	(gdb_readline_wrapper_line): Update.
+	* top.h (struct ui) <input_handler>: Take a unique_xmalloc_ptr.
+	(handle_line_of_input): Update.
+	* event-top.c: Update.
+	(gdb_readline_no_editing_callback): Update.
+	(command_line_handler): Take a unique_xmalloc_ptr.
+	(handle_line_of_input): Take a const char *.
+	(command_line_append_input_line): Take a const char *.
+
 2018-12-28  Tom Tromey  <tom@tromey.com>
 
 	* source-cache.c (get_language_name): Conditionally compile.
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 5852089f09..fd7c869bc0 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -210,7 +210,7 @@ gdb_rl_callback_handler (char *rl) noexcept
 
   TRY
     {
-      ui->input_handler (rl);
+      ui->input_handler (gdb::unique_xmalloc_ptr<char> (rl));
     }
   CATCH (ex, RETURN_MASK_ALL)
     {
@@ -594,7 +594,7 @@ command_handler (const char *command)
    line ends in a backslash).  Takes ownership of RL.  */
 
 static char *
-command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)
+command_line_append_input_line (struct buffer *cmd_line_buffer, const char *rl)
 {
   char *cmd;
   size_t len;
@@ -615,9 +615,6 @@ command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)
       cmd = cmd_line_buffer->buffer;
     }
 
-  /* Allocated in readline.  */
-  xfree (rl);
-
   return cmd;
 }
 
@@ -643,7 +640,8 @@ command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)
 
 char *
 handle_line_of_input (struct buffer *cmd_line_buffer,
-		      char *rl, int repeat, const char *annotation_suffix)
+		      const char *rl, int repeat,
+		      const char *annotation_suffix)
 {
   struct ui *ui = current_ui;
   int from_tty = ui->instream == ui->stdin_stream;
@@ -746,13 +744,13 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
    function.  */
 
 void
-command_line_handler (char *rl)
+command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
 {
   struct buffer *line_buffer = get_command_line_buffer ();
   struct ui *ui = current_ui;
   char *cmd;
 
-  cmd = handle_line_of_input (line_buffer, rl, 1, "prompt");
+  cmd = handle_line_of_input (line_buffer, rl.get (), 1, "prompt");
   if (cmd == (char *) EOF)
     {
       /* stdin closed.  The connection with the terminal is gone.
@@ -846,7 +844,7 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
 
   buffer_grow_char (&line_buffer, '\0');
   result = buffer_finish (&line_buffer);
-  ui->input_handler (result);
+  ui->input_handler (gdb::unique_xmalloc_ptr<char> (result));
 }
 \f
 
diff --git a/gdb/event-top.h b/gdb/event-top.h
index a77303e5c3..1164f69071 100644
--- a/gdb/event-top.h
+++ b/gdb/event-top.h
@@ -35,7 +35,7 @@ extern void gdb_disable_readline (void);
 extern void async_init_signals (void);
 extern void change_line_handler (int);
 
-extern void command_line_handler (char *rl);
+extern void command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl);
 extern void command_handler (const char *command);
 
 #ifdef SIGTSTP
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 9a317bc0ec..fd446c20e6 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -43,7 +43,8 @@
    interpreter.  */
 
 static void mi_execute_command_wrapper (const char *cmd);
-static void mi_execute_command_input_handler (char *cmd);
+static void mi_execute_command_input_handler
+  (gdb::unique_xmalloc_ptr<char> &&cmd);
 
 /* These are hooks that we put in place while doing interpreter_exec
    so we can report interesting things that happened "behind the MI's
@@ -294,14 +295,14 @@ mi_on_sync_execution_done (void)
 /* mi_execute_command_wrapper wrapper suitable for INPUT_HANDLER.  */
 
 static void
-mi_execute_command_input_handler (char *cmd)
+mi_execute_command_input_handler (gdb::unique_xmalloc_ptr<char> &&cmd)
 {
   struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
   struct ui *ui = current_ui;
 
   ui->prompt_state = PROMPT_NEEDED;
 
-  mi_execute_command_wrapper (cmd);
+  mi_execute_command_wrapper (cmd.get ());
 
   /* Print a prompt, indicating we're ready for further input, unless
      we just started a synchronous command.  In that case, we're about
diff --git a/gdb/top.c b/gdb/top.c
index 4bcb4e28fb..8b3fc5ee9a 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -900,10 +900,10 @@ gdb_in_secondary_prompt_p (struct ui *ui)
    text.  */
 
 static void
-gdb_readline_wrapper_line (char *line)
+gdb_readline_wrapper_line (gdb::unique_xmalloc_ptr<char> &&line)
 {
   gdb_assert (!gdb_readline_wrapper_done);
-  gdb_readline_wrapper_result = line;
+  gdb_readline_wrapper_result = line.release ();
   gdb_readline_wrapper_done = 1;
 
   /* Prevent operate-and-get-next from acting too early.  */
@@ -972,7 +972,7 @@ public:
 
 private:
 
-  void (*m_handler_orig) (char *);
+  void (*m_handler_orig) (gdb::unique_xmalloc_ptr<char> &&);
   int m_already_prompted_orig;
 
   /* Whether the target was async.  */
diff --git a/gdb/top.h b/gdb/top.h
index b34defa1f2..4d11f3443b 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -81,7 +81,7 @@ struct ui
 
   /* The function to invoke when a complete line of input is ready for
      processing.  */
-  void (*input_handler) (char *);
+  void (*input_handler) (gdb::unique_xmalloc_ptr<char> &&);
 
   /* True if this UI is using the readline library for command
      editing; false if using GDB's own simple readline emulation, with
@@ -297,7 +297,7 @@ extern void show_history (const char *, int);
 extern void set_verbose (const char *, int, struct cmd_list_element *);
 
 extern char *handle_line_of_input (struct buffer *cmd_line_buffer,
-				   char *rl, int repeat,
+				   const char *rl, int repeat,
 				   const char *annotation_suffix);
 
 #endif

Re: [RFA] Fix leak in most gdb.mi tests.

[Philippe Waroquiers]

On Sat, 2018-12-29 at 12:44 -0700, Tom Tromey wrote:
> > > > > > "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
> Tom> Maybe input_handler could accept a unique_xmalloc_ptr<char>&& instead.
> Tom> That would at least clarify the ownership semantics.
> 
> What do you think of this?

The patch looks nice to me.  I re-run all gdb.mi tests under valgrind,
and no errors/no leak of the line_buffer.
A minor comment below.

Thanks

Philippe
> 
> Tom
> 
> commit fe70f92b8e3c0af79d041b9004b9abbeed27374d
> Author: Tom Tromey <tom@tromey.com>
> Date:   Sat Dec 29 12:42:18 2018 -0700
> 
>     Change input_handler to take a unique_xmalloc_ptr
>     
>     This changes ui::input_handler to take a unique_xmalloc_ptr.  This
>     clarifies the ownership transfer of input_handler's argument.
>     
>     gdb/ChangeLog
>     2018-12-29  Tom Tromey  <tom@tromey.com>
>     
>             * event-top.h (command_line_handler): Update.
>             * top.c (class gdb_readline_wrapper_cleanup) <m_handler_orig>:
>             Update.
>             (gdb_readline_wrapper_line): Update.
>             * top.h (struct ui) <input_handler>: Take a unique_xmalloc_ptr.
>             (handle_line_of_input): Update.
>             * event-top.c: Update.
>             (gdb_readline_no_editing_callback): Update.
>             (command_line_handler): Take a unique_xmalloc_ptr.
>             (handle_line_of_input): Take a const char *.
>             (command_line_append_input_line): Take a const char *.
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 5ed91a84fc..56d86acee3 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,17 @@
> +2018-12-29  Tom Tromey  <tom@tromey.com>
> +
> +	* event-top.h (command_line_handler): Update.
> +	* top.c (class gdb_readline_wrapper_cleanup) <m_handler_orig>:
> +	Update.
> +	(gdb_readline_wrapper_line): Update.
> +	* top.h (struct ui) <input_handler>: Take a unique_xmalloc_ptr.
> +	(handle_line_of_input): Update.
> +	* event-top.c: Update.
> +	(gdb_readline_no_editing_callback): Update.
> +	(command_line_handler): Take a unique_xmalloc_ptr.
> +	(handle_line_of_input): Take a const char *.
> +	(command_line_append_input_line): Take a const char *.
> +
>  2018-12-28  Tom Tromey  <tom@tromey.com>
>  
>  	* source-cache.c (get_language_name): Conditionally compile.
> diff --git a/gdb/event-top.c b/gdb/event-top.c
> index 5852089f09..fd7c869bc0 100644
> --- a/gdb/event-top.c
> +++ b/gdb/event-top.c
> @@ -210,7 +210,7 @@ gdb_rl_callback_handler (char *rl) noexcept
>  
>    TRY
>      {
> -      ui->input_handler (rl);
> +      ui->input_handler (gdb::unique_xmalloc_ptr<char> (rl));
>      }
>    CATCH (ex, RETURN_MASK_ALL)
>      {
> @@ -594,7 +594,7 @@ command_handler (const char *command)
>     line ends in a backslash).  Takes ownership of RL.  */
Remove the part of the comment about ownership ?

>  
>  static char *
> -command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)
> +command_line_append_input_line (struct buffer *cmd_line_buffer, const char *rl)
>  {
>    char *cmd;
>    size_t len;
> @@ -615,9 +615,6 @@ command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)
>        cmd = cmd_line_buffer->buffer;
>      }
>  
> -  /* Allocated in readline.  */
> -  xfree (rl);
> -
>    return cmd;
>  }
>  
> @@ -643,7 +640,8 @@ command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)
>  
>  char *
>  handle_line_of_input (struct buffer *cmd_line_buffer,
> -		      char *rl, int repeat, const char *annotation_suffix)
> +		      const char *rl, int repeat,
> +		      const char *annotation_suffix)
>  {
>    struct ui *ui = current_ui;
>    int from_tty = ui->instream == ui->stdin_stream;
> @@ -746,13 +744,13 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
>     function.  */
>  
>  void
> -command_line_handler (char *rl)
> +command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
>  {
>    struct buffer *line_buffer = get_command_line_buffer ();
>    struct ui *ui = current_ui;
>    char *cmd;
>  
> -  cmd = handle_line_of_input (line_buffer, rl, 1, "prompt");
> +  cmd = handle_line_of_input (line_buffer, rl.get (), 1, "prompt");
>    if (cmd == (char *) EOF)
>      {
>        /* stdin closed.  The connection with the terminal is gone.
> @@ -846,7 +844,7 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
>  
>    buffer_grow_char (&line_buffer, '\0');
>    result = buffer_finish (&line_buffer);
> -  ui->input_handler (result);
> +  ui->input_handler (gdb::unique_xmalloc_ptr<char> (result));
>  }
>  \f
>  
> diff --git a/gdb/event-top.h b/gdb/event-top.h
> index a77303e5c3..1164f69071 100644
> --- a/gdb/event-top.h
> +++ b/gdb/event-top.h
> @@ -35,7 +35,7 @@ extern void gdb_disable_readline (void);
>  extern void async_init_signals (void);
>  extern void change_line_handler (int);
>  
> -extern void command_line_handler (char *rl);
> +extern void command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl);
>  extern void command_handler (const char *command);
>  
>  #ifdef SIGTSTP
> diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
> index 9a317bc0ec..fd446c20e6 100644
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -43,7 +43,8 @@
>     interpreter.  */
>  
>  static void mi_execute_command_wrapper (const char *cmd);
> -static void mi_execute_command_input_handler (char *cmd);
> +static void mi_execute_command_input_handler
> +  (gdb::unique_xmalloc_ptr<char> &&cmd);
>  
>  /* These are hooks that we put in place while doing interpreter_exec
>     so we can report interesting things that happened "behind the MI's
> @@ -294,14 +295,14 @@ mi_on_sync_execution_done (void)
>  /* mi_execute_command_wrapper wrapper suitable for INPUT_HANDLER.  */
>  
>  static void
> -mi_execute_command_input_handler (char *cmd)
> +mi_execute_command_input_handler (gdb::unique_xmalloc_ptr<char> &&cmd)
>  {
>    struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
>    struct ui *ui = current_ui;
>  
>    ui->prompt_state = PROMPT_NEEDED;
>  
> -  mi_execute_command_wrapper (cmd);
> +  mi_execute_command_wrapper (cmd.get ());
>  
>    /* Print a prompt, indicating we're ready for further input, unless
>       we just started a synchronous command.  In that case, we're about
> diff --git a/gdb/top.c b/gdb/top.c
> index 4bcb4e28fb..8b3fc5ee9a 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -900,10 +900,10 @@ gdb_in_secondary_prompt_p (struct ui *ui)
>     text.  */
>  
>  static void
> -gdb_readline_wrapper_line (char *line)
> +gdb_readline_wrapper_line (gdb::unique_xmalloc_ptr<char> &&line)
>  {
>    gdb_assert (!gdb_readline_wrapper_done);
> -  gdb_readline_wrapper_result = line;
> +  gdb_readline_wrapper_result = line.release ();
>    gdb_readline_wrapper_done = 1;
>  
>    /* Prevent operate-and-get-next from acting too early.  */
> @@ -972,7 +972,7 @@ public:
>  
>  private:
>  
> -  void (*m_handler_orig) (char *);
> +  void (*m_handler_orig) (gdb::unique_xmalloc_ptr<char> &&);
>    int m_already_prompted_orig;
>  
>    /* Whether the target was async.  */
> diff --git a/gdb/top.h b/gdb/top.h
> index b34defa1f2..4d11f3443b 100644
> --- a/gdb/top.h
> +++ b/gdb/top.h
> @@ -81,7 +81,7 @@ struct ui
>  
>    /* The function to invoke when a complete line of input is ready for
>       processing.  */
> -  void (*input_handler) (char *);
> +  void (*input_handler) (gdb::unique_xmalloc_ptr<char> &&);
>  
>    /* True if this UI is using the readline library for command
>       editing; false if using GDB's own simple readline emulation, with
> @@ -297,7 +297,7 @@ extern void show_history (const char *, int);
>  extern void set_verbose (const char *, int, struct cmd_list_element *);
>  
>  extern char *handle_line_of_input (struct buffer *cmd_line_buffer,
> -				   char *rl, int repeat,
> +				   const char *rl, int repeat,
>  				   const char *annotation_suffix);
>  
>  #endif

Re: [RFA] Fix leak in most gdb.mi tests.

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> The patch looks nice to me.  I re-run all gdb.mi tests under valgrind,
Philippe> and no errors/no leak of the line_buffer.
Philippe> A minor comment below.

Thanks for taking a look.

>> @@ -594,7 +594,7 @@ command_handler (const char *command)
>> line ends in a backslash).  Takes ownership of RL.  */

Philippe> Remove the part of the comment about ownership ?

I'm checking it in with this change.

Tom

Re: [RFA] Fix leak in most gdb.mi tests.

[Philippe Waroquiers]

On Sun, 2018-12-30 at 08:54 -0700, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> 
> Philippe> The patch looks nice to me.  I re-run all gdb.mi tests under valgrind,
> Philippe> and no errors/no leak of the line_buffer.
Some more tests under valgrind have revealed some additional leaks
of line_buffer, fixed by the below patch, that calls xfree (rl).
The below patch does not put rl inside a gdb::unique_xmalloc_ptr<char>,
as rl is allocated locally, is then passed as a 'const char*'
and then xfree-d in the same block.

Ok to push ?
Or would it still be better to have a local gdb::unique_xmalloc_ptr<char> ?
Thanks
Philippe


From 421c709f519d05e92aef71170bf2a52c666e940d Mon Sep 17 00:00:00 2001
From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Date: Sun, 30 Dec 2018 20:41:49 +0100
Subject: [PATCH] Add missing xfree following 'Change input_handler to take a
 unique_xmalloc_ptr'

Following the change of logic where the input_handler gets a
gdb::unique_xmalloc_ptr<char>, a call to readline directly
followed by a call to handle_line_of_input is missing a free,
and cause the below leak.

Adding an xfree solves the leak.

==16291== VALGRIND_GDB_ERROR_BEGIN
==16291== 64 bytes in 1 blocks are definitely lost in loss record 1,815 of 4,111
==16291==    at 0x4C2E2B3: realloc (vg_replace_malloc.c:836)
==16291==    by 0x41EB1C: xrealloc (common-utils.c:62)
==16291==    by 0x41DBD3: buffer_grow(buffer*, char const*, unsigned long) [clone .part.1]
(buffer.c:40)
==16291==    by 0x66E8FF: buffer_grow_char (buffer.h:40)
==16291==    by 0x66E8FF: gdb_readline_no_editing (top.c:798)
==16291==    by 0x66E8FF: command_line_input(char const*, char const*) (top.c:1249)
==16291==    by 0x66EBD8: read_command_file(_IO_FILE*) (top.c:421)
==16291==    by 0x412C0C: script_from_file(_IO_FILE*, char const*) (cli-script.c:1547)
==16291==    by 0x40BE90: source_script_from_stream (cli-cmds.c:569)
==16291==    by 0x40BE90: source_script_with_search(char const*, int, int) (cli-
cmds.c:606)
==16291==    by 0x54D567: catch_command_errors(void (*)(char const*, int), char const*,
int) (main.c:379)
==16291==    by 0x54EA84: captured_main_1 (main.c:994)
==16291==    by 0x54EA84: captured_main (main.c:1167)
==16291==    by 0x54EA84: gdb_main(captured_main_args*) (main.c:1193)
==16291==    by 0x29DA27: main (gdb.c:32)
==16291==
==16291== VALGRIND_GDB_ERROR_END

gdb/ChangeLog
2018-12-30  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* top.c (command_line_input): Call xfree (rl) as this is allocated
	by readline, and not released by handle_line_of_input.
---
 gdb/top.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gdb/top.c b/gdb/top.c
index 8b3fc5ee9a..1dd2135736 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1251,6 +1251,7 @@ command_line_input (const char *prompt_arg, const char
*annotation_suffix)
 
       cmd = handle_line_of_input (&cmd_line_buffer, rl,
 				  0, annotation_suffix);
+      xfree (rl); /* Allocated by readline.  */
       if (cmd == (char *) EOF)
 	{
 	  cmd = NULL;
-- 
2.19.2

Re: [RFA] Fix leak in most gdb.mi tests.

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> The below patch does not put rl inside a gdb::unique_xmalloc_ptr<char>,
Philippe> as rl is allocated locally, is then passed as a 'const char*'
Philippe> and then xfree-d in the same block.

Philippe> Ok to push ?
Philippe> Or would it still be better to have a local gdb::unique_xmalloc_ptr<char> ?

I think it's better to use a unique_xmalloc_ptr, in case something can
throw, either now or in the future.

Tom