[patch] Fix completion memory double-free

[patch] Fix completion memory double-free

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 135 bytes --]

Hi,

p values[0].nonex.<tab>
causes a double-free (testcase included) when linked with `-lmcheck'.

Approval requested.


Regards,
Jan

[-- Attachment #2: gdb-completer.patch --]
[-- Type: text/plain, Size: 2253 bytes --]

gdb/
2008-12-20  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix memory double-free.
	* completer.c (line_completion_function): Clear LIST after called xfree.

gdb/testsuite/
2008-12-20  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/completion.exp (Completing non-existing component): New test.

--- ./gdb/completer.c	24 Nov 2008 17:05:43 -0000	1.28
+++ ./gdb/completer.c	19 Dec 2008 23:59:36 -0000
@@ -752,8 +752,10 @@ line_completion_function (const char *te
       if (list)
 	{
 	  /* Free the storage used by LIST, but not by the strings inside.
-	     This is because rl_complete_internal () frees the strings.  */
+	     This is because rl_complete_internal () frees the strings.
+	     As complete_line may abort by calling `error' clear LIST now.  */
 	  xfree (list);
+	  list = NULL;
 	}
       index = 0;
       list = complete_line (text, line_buffer, point);
--- ./gdb/testsuite/gdb.base/completion.exp	22 Oct 2008 19:46:13 -0000	1.34
+++ ./gdb/testsuite/gdb.base/completion.exp	20 Dec 2008 00:18:50 -0000
@@ -805,6 +805,26 @@ gdb_expect  {
         timeout         { fail "(timeout) complete 'set follow-fork-mode'" }
         }
 
+send_gdb "p values\[0\].nonex.\t"
+sleep 1
+gdb_expect  {
+        -re "Type struct some_struct has no component named nonex.\r\n$gdb_prompt $"\
+            { pass "Completing non-existing component" }
+        -re ".*$gdb_prompt $"       { fail "Completing non-existing component" }
+        timeout         { fail "(timeout) Completing non-existing component" }
+        eof             { fail "(eof) Completing non-existing component #2" }
+        }
+# Double memory freeing gets found only on the second run:
+send_gdb "p values\[0\].nonex.\t"
+sleep 1
+gdb_expect  {
+        -re "Type struct some_struct has no component named nonex.\r\n$gdb_prompt $"\
+            { pass "Completing non-existing component #2" }
+        -re ".*$gdb_prompt $"       { fail "Completing non-existing component #2" }
+        timeout         { fail "(timeout) Completing non-existing component #2" }
+        eof             { fail "(eof) Completing non-existing component #2" }
+        }
+
 # Restore globals modified in this test...
 if [info exists old_inputrc] {
     set env(INPUTRC) $old_inputrc

Re: [patch] Fix completion memory double-free

[Joel Brobecker]

> 2008-12-20  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Fix memory double-free.
> 	* completer.c (line_completion_function): Clear LIST after called xfree.

Approved.

> gdb/testsuite/
> 2008-12-20  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.base/completion.exp (Completing non-existing component): New test.

And approved.

I really wonder if the "sleep 1" is needed in this case. A 1sec sleeping
delay is a LOT of time :-(, particularly since we can't run more than
one testcase at a time.

Thanks for the fix!
-- 
Joel

Re: [patch] Fix completion memory double-free

[Jan Kratochvil]

On Mon, 22 Dec 2008 05:35:02 +0100, Joel Brobecker wrote:
> I really wonder if the "sleep 1" is needed in this case.

You are right, it isn't.  It was a copy-paste but in this case there is
fortunately the `$gdb_prompt $' terminator which does not need the sleep.
Removed `sleep 1'.

Committed.


Thanks,
Jan


gdb/
2008-12-22  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix memory double-free.
	* completer.c (line_completion_function): Clear LIST after called xfree.

gdb/testsuite/
2008-12-22  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/completion.exp (Completing non-existing component): New test.

===================================================================
RCS file: /cvs/src/src/gdb/completer.c,v
retrieving revision 1.28
retrieving revision 1.29
diff -u -r1.28 -r1.29
--- src/gdb/completer.c	2008/11/24 17:05:43	1.28
+++ src/gdb/completer.c	2008/12/22 13:19:29	1.29
@@ -752,8 +752,10 @@
       if (list)
 	{
 	  /* Free the storage used by LIST, but not by the strings inside.
-	     This is because rl_complete_internal () frees the strings.  */
+	     This is because rl_complete_internal () frees the strings.
+	     As complete_line may abort by calling `error' clear LIST now.  */
 	  xfree (list);
+	  list = NULL;
 	}
       index = 0;
       list = complete_line (text, line_buffer, point);
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/completion.exp,v
retrieving revision 1.34
retrieving revision 1.35
diff -u -r1.34 -r1.35
--- src/gdb/testsuite/gdb.base/completion.exp	2008/10/22 19:46:13	1.34
+++ src/gdb/testsuite/gdb.base/completion.exp	2008/12/22 13:19:30	1.35
@@ -805,6 +805,24 @@
         timeout         { fail "(timeout) complete 'set follow-fork-mode'" }
         }
 
+send_gdb "p values\[0\].nonex.\t"
+gdb_expect  {
+        -re "Type struct some_struct has no component named nonex.\r\n$gdb_prompt $"\
+            { pass "Completing non-existing component" }
+        -re ".*$gdb_prompt $"       { fail "Completing non-existing component" }
+        timeout         { fail "(timeout) Completing non-existing component" }
+        eof             { fail "(eof) Completing non-existing component #2" }
+        }
+# Double memory freeing gets found only on the second run:
+send_gdb "p values\[0\].nonex.\t"
+gdb_expect  {
+        -re "Type struct some_struct has no component named nonex.\r\n$gdb_prompt $"\
+            { pass "Completing non-existing component #2" }
+        -re ".*$gdb_prompt $"       { fail "Completing non-existing component #2" }
+        timeout         { fail "(timeout) Completing non-existing component #2" }
+        eof             { fail "(eof) Completing non-existing component #2" }
+        }
+
 # Restore globals modified in this test...
 if [info exists old_inputrc] {
     set env(INPUTRC) $old_inputrc