This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[patch] fixing leaking struct value* in py-value.c


Given:

$ cat simple.c
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>

struct blah {
 char test[2];
 int i;
 float none;
} *list_of_blahs;

int main(){
 list_of_blahs = malloc(sizeof(struct blah) * 2000000);
 printf("pid: %d\n", getpid());
 while(1){
   sleep(1);
 }
}


$ cat simple.py
list = gdb.parse_and_eval('list_of_blahs')
end = list + 1000000

while list < end:
  list['i']
  list += 1


$ gcc -o simple -ggdb simple.c
$ ./simple &
[1] 16572
$ gdb -ex 'py execfile("./simple.py")' ./simple 16572

The gdb process will leak a large amount of memory (>512mb RSS) while
executing the python loop. Some (but not all) of this memory is
returned after the loop completes and free_all_values() is called for
the next command. The following patch against gdb master
keeps the memory usage constant during the loop.

I ran `make check` before and after the patch on a "Linux 2.6.32.3 #3
SMP Thu Feb 4 23:11:14 UTC 2010 x86_64 GNU/Linux" and there were no
regressions.

See also:
  http://sourceware.org/bugzilla/show_bug.cgi?id=10896
  http://sourceware.org/ml/archer/2010-q1/msg00009.html


diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5ef6eb20a..80c17da 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2010-11-04  Aman Gupta  <aman@tmm1.net>
+	* value.c (release_value): Return 0 on success.
+	(release_value_or_incref): New function, incref value if already
+	released.
+	* python/py-value.c (valpy_new, value_to_value_object): Use
+	release_value_or_incref() instead of value_incref() to prevent leaks.
+	(valpy_richcompare, valpy_binop, valpy_getitem): Call value_mark()
+	before calling into gdb, and make sure to value_free_to_mark() before
+	returning or raising any exceptions.
+
 2010-11-04  Hui Zhu  <teawater@gmail.com>

 	* tracepoint.c (remote_trace_set_readonly_regions): Change lma to vma.
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 4445655..f3c6309 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -150,7 +150,7 @@ valpy_new (PyTypeObject *subtype, PyObject *args,
PyObject *keywords)
     }

   value_obj->value = value;
-  value_incref (value);
+  release_value_or_incref (value);
   value_obj->address = NULL;
   value_obj->type = NULL;
   value_obj->dynamic_type = NULL;
@@ -445,8 +445,10 @@ valpy_getitem (PyObject *self, PyObject *key)
 {
   value_object *self_value = (value_object *) self;
   char *field = NULL;
+  PyObject *res_obj = NULL;
   struct value *res_val = NULL;
   volatile struct gdb_exception except;
+  struct value *mark = value_mark ();

   if (gdbpy_is_string (key))
     {
@@ -486,9 +488,15 @@ valpy_getitem (PyObject *self, PyObject *key)
     }

   xfree (field);
+  if (except.reason < 0)
+    value_free_to_mark (mark);
   GDB_PY_HANDLE_EXCEPTION (except);

-  return res_val ? value_to_value_object (res_val) : NULL;
+  if (res_val)
+    res_obj = value_to_value_object (res_val);
+
+  value_free_to_mark (mark);
+  return res_obj;
 }

 static int
@@ -625,8 +633,10 @@ enum valpy_opcode
 static PyObject *
 valpy_binop (enum valpy_opcode opcode, PyObject *self, PyObject *other)
 {
+  PyObject *res_obj = NULL;
   struct value *res_val = NULL;	  /* Initialize to appease gcc warning.  */
   volatile struct gdb_exception except;
+  struct value *mark = value_mark ();

   TRY_CATCH (except, RETURN_MASK_ALL)
     {
@@ -717,9 +727,16 @@ valpy_binop (enum valpy_opcode opcode, PyObject
*self, PyObject *other)
 	  break;
 	}
     }
+
+  if (except.reason < 0)
+    value_free_to_mark (mark);
   GDB_PY_HANDLE_EXCEPTION (except);

-  return res_val ? value_to_value_object (res_val) : NULL;
+  if (res_val)
+    res_obj = value_to_value_object (res_val);
+
+  value_free_to_mark (mark);
+  return res_obj;
 }

 static PyObject *
@@ -881,6 +898,7 @@ valpy_richcompare (PyObject *self, PyObject *other, int op)
   int result = 0;
   struct value *value_other;
   volatile struct gdb_exception except;
+  struct value *mark = value_mark ();

   if (other == Py_None)
     /* Comparing with None is special.  From what I can tell, in Python
@@ -939,6 +957,7 @@ valpy_richcompare (PyObject *self, PyObject *other, int op)
 	  break;
       }
     }
+  value_free_to_mark (mark);
   GDB_PY_HANDLE_EXCEPTION (except);

   /* In this case, the Python exception has already been set.  */
@@ -1062,7 +1081,7 @@ value_to_value_object (struct value *val)
   if (val_obj != NULL)
     {
       val_obj->value = val;
-      value_incref (val);
+      release_value_or_incref (val);
       val_obj->address = NULL;
       val_obj->type = NULL;
       val_obj->dynamic_type = NULL;
diff --git a/gdb/value.c b/gdb/value.c
index 381318b..805dcf0 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -737,9 +737,10 @@ free_value_chain (struct value *v)
 }

 /* Remove VAL from the chain all_values
-   so it will not be freed automatically.  */
+   so it will not be freed automatically.
+   Result of 0 indicates the value was found and released. */

-void
+int
 release_value (struct value *val)
 {
   struct value *v;
@@ -748,7 +749,7 @@ release_value (struct value *val)
     {
       all_values = val->next;
       val->next = NULL;
-      return;
+      return 0;
     }

   for (v = all_values; v; v = v->next)
@@ -757,12 +758,25 @@ release_value (struct value *val)
 	{
 	  v->next = val->next;
 	  val->next = NULL;
-	  break;
+	  return 0;
 	}
     }
+
+  return -1;
+}
+
+/* Release VAL or increment its reference count if
+   it was released already */
+
+void
+release_value_or_incref (struct value *val)
+{
+  if (release_value (val) != 0)
+    value_incref (val);
 }

 /* Release all values up to mark  */
+
 struct value *
 value_release_to_mark (struct value *mark)
 {
diff --git a/gdb/value.h b/gdb/value.h
index 543290a..e7e00d2 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -641,7 +641,9 @@ extern void free_all_values (void);

 extern void free_value_chain (struct value *v);

-extern void release_value (struct value *val);
+extern int release_value (struct value *val);
+
+extern void release_value_or_incref (struct value *val);

 extern int record_latest_value (struct value *val);

Attachment: python-value-leak.patch
Description: Binary data


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]