diff mbox

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

Message ID 1546205040.12900.14.camel@skynet.be
State New
Headers show

Commit Message

Philippe Waroquiers Dec. 30, 2018, 9:24 p.m. UTC
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(+)

 	  cmd = NULL;
-- 
2.19.2

Comments

Tom Tromey Dec. 30, 2018, 11:43 p.m. UTC | #1
>>>>> "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
diff mbox

Patch

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)
 	{