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]

FYI: fix PR 11948 - lazier lazy strings


I'm checking this in.

Right now gdb will eagerly read all of the memory referenced by a Python
lazy string.  This can make gdb crash if the string is long enough.

I couldn't think of a reasonable way to write a regression test for
this.  The issue is that the inferior must have enough valid memory to
make the allocation in gdb blow up.  This can happen with real programs,
but it didn't seem like a good idea to try to make a test case allocate
a huge amount of memory.

Built and regtested on x86-64 (compile farm).

Note that we still can have this same problem when printing ordinary
values (i.e., not via Python).  And, gdb can still die even with this
patch if the user sets the print limit high enough.

The solution to these problems is somewhat more complicated.  I think we
need to ditch val_print and then make the string and array printers take
input iterators to avoid allocating too much memory at once.

Tom

2010-10-15  Tom Tromey  <tromey@redhat.com>

	PR python/11948:
	* varobj.c (value_get_print_value): Use val_print_string to print
	lazy strings.
	* python/py-prettyprint.c (print_string_repr): Use
	val_print_string to print lazy strings.  Fix cleanup logic.
	(print_children): Likewise.
	* python/python-internal.h (gdbpy_extract_lazy_string): Update.
	* python/py-lazy-string.c (gdbpy_extract_lazy_string): Rewrite.
	Change return type to 'void', add 'addr' argument.
	* value.h (val_print_string): Update.
	* valprint.c (val_print_string): Add 'encoding' argument.
	* printcmd.c (print_formatted): Update.
	* p-valprint.c (pascal_val_print): Update.
	* m2-valprint.c (print_unpacked_pointer): Update.
	(m2_print_array_contents): Likewise.
	* jv-valprint.c (java_value_print): Update.
	* f-valprint.c (f_val_print): Update.
	* c-valprint.c (c_val_print): Update.
	* auxv.c (fprint_target_auxv): Update.

Index: auxv.c
===================================================================
RCS file: /cvs/src/src/gdb/auxv.c,v
retrieving revision 1.29
diff -u -r1.29 auxv.c
--- auxv.c	5 Jul 2010 18:00:39 -0000	1.29
+++ auxv.c	15 Oct 2010 18:42:14 -0000
@@ -401,7 +401,7 @@
 	    if (opts.addressprint)
 	      fprintf_filtered (file, "%s", paddress (target_gdbarch, val));
 	    val_print_string (builtin_type (target_gdbarch)->builtin_char,
-			      val, -1, file, &opts);
+			      NULL, val, -1, file, &opts);
 	    fprintf_filtered (file, "\n");
 	  }
 	  break;
Index: c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.73
diff -u -r1.73 c-valprint.c
--- c-valprint.c	6 Oct 2010 08:44:14 -0000	1.73
+++ c-valprint.c	15 Oct 2010 18:42:14 -0000
@@ -289,7 +289,7 @@
 	  if (c_textual_element_type (unresolved_elttype, options->format)
 	      && addr != 0)
 	    {
-	      i = val_print_string (unresolved_elttype, addr, -1, stream,
+	      i = val_print_string (unresolved_elttype, NULL, addr, -1, stream,
 				    options);
 	    }
 	  else if (cp_is_vtbl_member (type))
Index: f-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/f-valprint.c,v
retrieving revision 1.59
diff -u -r1.59 f-valprint.c
--- f-valprint.c	21 Jun 2010 18:01:50 -0000	1.59
+++ f-valprint.c	15 Oct 2010 18:42:14 -0000
@@ -299,8 +299,8 @@
 	      && TYPE_CODE (elttype) == TYPE_CODE_INT
 	      && (options->format == 0 || options->format == 's')
 	      && addr != 0)
-	    i = val_print_string (TYPE_TARGET_TYPE (type), addr, -1, stream,
-				  options);
+	    i = val_print_string (TYPE_TARGET_TYPE (type), NULL, addr, -1,
+				  stream, options);
 
 	  /* Return number of characters printed, including the terminating
 	     '\0' if we reached the end.  val_print_string takes care including
Index: jv-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/jv-valprint.c,v
retrieving revision 1.45
diff -u -r1.45 jv-valprint.c
--- jv-valprint.c	11 Jun 2010 15:36:04 -0000	1.45
+++ jv-valprint.c	15 Oct 2010 18:42:14 -0000
@@ -239,7 +239,8 @@
       value_free_to_mark (mark);	/* Release unnecessary values */
 
       char_type = builtin_java_type (gdbarch)->builtin_char;
-      val_print_string (char_type, data + boffset, count, stream, options);
+      val_print_string (char_type, NULL, data + boffset, count, stream,
+			options);
 
       return 0;
     }
Index: m2-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/m2-valprint.c,v
retrieving revision 1.30
diff -u -r1.30 m2-valprint.c
--- m2-valprint.c	21 Jun 2010 18:01:51 -0000	1.30
+++ m2-valprint.c	15 Oct 2010 18:42:14 -0000
@@ -235,7 +235,7 @@
       && TYPE_CODE (elttype) == TYPE_CODE_INT
       && (options->format == 0 || options->format == 's')
       && addr != 0)
-    return val_print_string (TYPE_TARGET_TYPE (type), addr, -1,
+    return val_print_string (TYPE_TARGET_TYPE (type), NULL, addr, -1,
 			     stream, options);
   
   return 0;
@@ -296,7 +296,7 @@
 	   || ((current_language->la_language == language_m2)
 	       && (TYPE_CODE (type) == TYPE_CODE_CHAR)))
 	  && (options->format == 0 || options->format == 's'))
-	val_print_string (type, address, len+1, stream, options);
+	val_print_string (type, NULL, address, len+1, stream, options);
       else
 	{
 	  fprintf_filtered (stream, "{");
Index: p-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/p-valprint.c,v
retrieving revision 1.76
diff -u -r1.76 p-valprint.c
--- p-valprint.c	21 Jun 2010 18:01:51 -0000	1.76
+++ p-valprint.c	15 Oct 2010 18:42:14 -0000
@@ -183,7 +183,7 @@
 	  && addr != 0)
 	{
 	  /* no wide string yet */
-	  i = val_print_string (elttype, addr, -1, stream, options);
+	  i = val_print_string (elttype, NULL, addr, -1, stream, options);
 	}
       /* also for pointers to pascal strings */
       /* Note: this is Free Pascal specific:
@@ -202,7 +202,9 @@
 	  string_length = extract_unsigned_integer (buffer, length_size,
 						    byte_order);
 	  xfree (buffer);
-	  i = val_print_string (char_type ,addr + string_pos, string_length, stream, options);
+	  i = val_print_string (char_type, NULL,
+				addr + string_pos, string_length,
+				stream, options);
 	}
       else if (pascal_object_is_vtbl_member (type))
 	{
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.182
diff -u -r1.182 printcmd.c
--- printcmd.c	25 Jun 2010 15:13:52 -0000	1.182
+++ printcmd.c	15 Oct 2010 18:42:15 -0000
@@ -298,7 +298,7 @@
 	    struct type *elttype = value_type (val);
 
 	    next_address = (value_address (val)
-			    + val_print_string (elttype,
+			    + val_print_string (elttype, NULL,
 						value_address (val), -1,
 						stream, options) * len);
 	  }
Index: valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/valprint.c,v
retrieving revision 1.95
diff -u -r1.95 valprint.c
--- valprint.c	11 Jun 2010 15:36:05 -0000	1.95
+++ valprint.c	15 Oct 2010 18:42:15 -0000
@@ -1414,10 +1414,13 @@
    characters, of WIDTH bytes a piece, to STREAM.  If LEN is -1, printing
    stops at the first null byte, otherwise printing proceeds (including null
    bytes) until either print_max or LEN characters have been printed,
-   whichever is smaller.  */
+   whichever is smaller.  ENCODING is the name of the string's
+   encoding.  It can be NULL, in which case the target encoding is
+   assumed.  */
 
 int
-val_print_string (struct type *elttype, CORE_ADDR addr, int len,
+val_print_string (struct type *elttype, const char *encoding,
+		  CORE_ADDR addr, int len,
 		  struct ui_file *stream,
 		  const struct value_print_options *options)
 {
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.161
diff -u -r1.161 value.h
--- value.h	7 Jul 2010 16:15:18 -0000	1.161
+++ value.h	15 Oct 2010 18:42:15 -0000
@@ -687,7 +687,8 @@
 			     const struct value_print_options *options,
 			     const struct language_defn *language);
 
-extern int val_print_string (struct type *elttype, CORE_ADDR addr, int len,
+extern int val_print_string (struct type *elttype, const char *encoding,
+			     CORE_ADDR addr, int len,
 			     struct ui_file *stream,
 			     const struct value_print_options *options);
 
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.161
diff -u -r1.161 varobj.c
--- varobj.c	13 Oct 2010 13:24:39 -0000	1.161
+++ varobj.c	15 Oct 2010 18:42:15 -0000
@@ -2479,13 +2479,15 @@
 		       struct varobj *var)
 {
   struct ui_file *stb;
-  struct cleanup *old_chain;
+  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
   gdb_byte *thevalue = NULL;
   struct value_print_options opts;
   struct type *type = NULL;
   long len = 0;
   char *encoding = NULL;
   struct gdbarch *gdbarch = NULL;
+  CORE_ADDR str_addr;
+  int string_print = 0;
 
   if (value == NULL)
     return NULL;
@@ -2493,9 +2495,10 @@
   gdbarch = get_type_arch (value_type (value));
 #if HAVE_PYTHON
   {
-    struct cleanup *back_to = varobj_ensure_python_env (var);
     PyObject *value_formatter = var->pretty_printer;
 
+    varobj_ensure_python_env (var);
+
     if (value_formatter)
       {
 	/* First check to see if we have any children at all.  If so,
@@ -2507,7 +2510,6 @@
 	  {
 	    char *hint;
 	    struct value *replacement;
-	    int string_print = 0;
 	    PyObject *output = NULL;
 
 	    hint = gdbpy_get_display_hint (value_formatter);
@@ -2522,10 +2524,13 @@
 						  &replacement);
 	    if (output)
 	      {
+		make_cleanup_py_decref (output);
+
 		if (gdbpy_is_lazy_string (output))
 		  {
-		    thevalue = gdbpy_extract_lazy_string (output, &type,
-							  &len, &encoding);
+		    gdbpy_extract_lazy_string (output, &str_addr, &type,
+					       &len, &encoding);
+		    make_cleanup (free_current_contents, &encoding);
 		    string_print = 1;
 		  }
 		else
@@ -2541,38 +2546,36 @@
 			thevalue = xmemdup (s, len + 1, len + 1);
 			type = builtin_type (gdbarch)->builtin_char;
 			Py_DECREF (py_str);
+
+			if (!string_print)
+			  {
+			    do_cleanups (old_chain);
+			    return thevalue;
+			  }
+
+			make_cleanup (xfree, thevalue);
 		      }
 		    else
 		      gdbpy_print_stack ();
 		  }
-		Py_DECREF (output);
-	      }
-	    if (thevalue && !string_print)
-	      {
-		do_cleanups (back_to);
-		xfree (encoding);
-		return thevalue;
 	      }
 	    if (replacement)
 	      value = replacement;
 	  }
       }
-    do_cleanups (back_to);
   }
 #endif
 
   stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
+  make_cleanup_ui_file_delete (stb);
 
   get_formatted_print_options (&opts, format_code[(int) format]);
   opts.deref_ref = 0;
   opts.raw = 1;
   if (thevalue)
-    {
-      make_cleanup (xfree, thevalue);
-      make_cleanup (xfree, encoding);
-      LA_PRINT_STRING (stb, type, thevalue, len, encoding, 0, &opts);
-    }
+    LA_PRINT_STRING (stb, type, thevalue, len, encoding, 0, &opts);
+  else if (string_print)
+    val_print_string (type, encoding, str_addr, len, stb, &opts);
   else
     common_val_print (value, stb, 0, &opts, current_language);
   thevalue = ui_file_xstrdup (stb, NULL);
Index: python/py-lazy-string.c
===================================================================
RCS file: /cvs/src/src/gdb/python/py-lazy-string.c,v
retrieving revision 1.6
diff -u -r1.6 py-lazy-string.c
--- python/py-lazy-string.c	17 May 2010 21:23:25 -0000	1.6
+++ python/py-lazy-string.c	15 Oct 2010 18:42:15 -0000
@@ -24,6 +24,7 @@
 #include "exceptions.h"
 #include "valprint.h"
 #include "language.h"
+#include "gdb_assert.h"
 
 typedef struct {
   PyObject_HEAD
@@ -169,86 +170,26 @@
   return PyObject_TypeCheck (result, &lazy_string_object_type);
 }
 
-/* Extract and return the actual string from the lazy string object
-   STRING.  Addtionally, the string type is written to *STR_TYPE, the
-   string length is written to *LENGTH, and the string encoding is
-   written to *ENCODING.  On error, NULL is returned.  The caller is
-   responsible for freeing the returned buffer.  */
-gdb_byte *
-gdbpy_extract_lazy_string (PyObject *string, struct type **str_type,
-		     long *length, char **encoding)
-{
-  int width;
-  int bytes_read;
-  gdb_byte *buffer = NULL;
-  int errcode = 0;
-  CORE_ADDR addr;
-  struct gdbarch *gdbarch;
-  enum bfd_endian byte_order;
-  PyObject *py_len = NULL, *py_encoding = NULL; 
-  PyObject *py_addr = NULL, *py_type = NULL;
-  volatile struct gdb_exception except;
-
-  py_len = PyObject_GetAttrString (string, "length");
-  py_encoding = PyObject_GetAttrString (string, "encoding");
-  py_addr = PyObject_GetAttrString (string, "address");
-  py_type = PyObject_GetAttrString (string, "type");
-
-  /* A NULL encoding, length, address or type is not ok.  */
-  if (!py_len || !py_encoding || !py_addr || !py_type)
-    goto error;
-
-  *length = PyLong_AsLong (py_len);
-  addr = PyLong_AsUnsignedLongLong (py_addr);
-
-  /* If the user supplies Py_None an encoding, set encoding to NULL.
-     This will trigger the resulting LA_PRINT_CALL to automatically
-     select an encoding.  */
-  if (py_encoding == Py_None)
-    *encoding = NULL;
-  else
-    *encoding = xstrdup (PyString_AsString (py_encoding));
-
-  *str_type = type_object_to_type (py_type);
-  gdbarch = get_type_arch (*str_type);
-  byte_order = gdbarch_byte_order (gdbarch);
-  width = TYPE_LENGTH (*str_type);
-
-  TRY_CATCH (except, RETURN_MASK_ALL)
-    {
-      errcode = read_string (addr, *length, width,
-			     *length, byte_order, &buffer,
-			     &bytes_read);
-    }
-  if (except.reason < 0)
-    {
-      PyErr_Format (except.reason == RETURN_QUIT			\
-		    ? PyExc_KeyboardInterrupt : PyExc_RuntimeError,	\
-		    "%s", except.message);				\
-      goto error;
+/* Extract the parameters from the lazy string object STRING.
+   ENCODING will either be set to NULL, or will be allocated with
+   xmalloc, in which case the callers is responsible for freeing
+   it.  */
 
-    }
+void
+gdbpy_extract_lazy_string (PyObject *string, CORE_ADDR *addr,
+			   struct type **str_type,
+			   long *length, char **encoding)
+{
+  lazy_string_object *lazy;
 
-  if (errcode)
-    goto error;
+  gdb_assert (gdbpy_is_lazy_string (string));
 
-  *length = bytes_read / width;
+  lazy = (lazy_string_object *) string;
 
-  Py_DECREF (py_encoding);
-  Py_DECREF (py_len);
-  Py_DECREF (py_addr);
-  Py_DECREF (py_type);
-  return buffer;
-
- error:
-  Py_XDECREF (py_encoding);
-  Py_XDECREF (py_len);
-  Py_XDECREF (py_addr);
-  Py_XDECREF (py_type);
-  xfree (buffer);
-  *length = 0;
-  *str_type = NULL;
-  return NULL;
+  *addr = lazy->address;
+  *str_type = lazy->type;
+  *length = lazy->length;
+  *encoding = lazy->encoding ? xstrdup (lazy->encoding) : NULL;
 }
 
 
Index: python/py-prettyprint.c
===================================================================
RCS file: /cvs/src/src/gdb/python/py-prettyprint.c,v
retrieving revision 1.15
diff -u -r1.15 py-prettyprint.c
--- python/py-prettyprint.c	13 Oct 2010 13:24:39 -0000	1.15
+++ python/py-prettyprint.c	15 Oct 2010 18:42:15 -0000
@@ -275,53 +275,51 @@
   py_str = pretty_print_one_value (printer, &replacement);
   if (py_str)
     {
+      struct cleanup *cleanup = make_cleanup_py_decref (py_str);
+
       if (py_str == Py_None)
 	is_py_none = 1;
-      else
+      else if (gdbpy_is_lazy_string (py_str))
 	{
-	  gdb_byte *output = NULL;
+	  CORE_ADDR addr;
 	  long length;
 	  struct type *type;
 	  char *encoding = NULL;
-	  PyObject *string = NULL;
-	  int is_lazy;
 
-	  is_lazy = gdbpy_is_lazy_string (py_str);
-	  if (is_lazy)
-	    output = gdbpy_extract_lazy_string (py_str, &type, &length, &encoding);
-	  else
-	    {
-	      string = python_string_to_target_python_string (py_str);
-	      if (string)
-		{
-		  output = PyString_AsString (string);
-		  length = PyString_Size (string);
-		  type = builtin_type (gdbarch)->builtin_char;
-		}
-	      else
-		gdbpy_print_stack ();
-	      
-	    }
-	
-	  if (output)
+	  make_cleanup (free_current_contents, &encoding);
+	  gdbpy_extract_lazy_string (py_str, &addr, &type,
+				     &length, &encoding);
+
+	  val_print_string (type, encoding, addr, (int) length,
+			    stream, options);
+	}
+      else
+	{
+	  PyObject *string;
+
+	  string = python_string_to_target_python_string (py_str);
+	  if (string)
 	    {
-	      if (is_lazy || (hint && !strcmp (hint, "string")))
-		LA_PRINT_STRING (stream, type, output, length, encoding,
+	      gdb_byte *output;
+	      long length;
+	      struct type *type;
+
+	      make_cleanup_py_decref (string);
+	      output = PyString_AsString (string);
+	      length = PyString_Size (string);
+	      type = builtin_type (gdbarch)->builtin_char;
+
+	      if (hint && !strcmp (hint, "string"))
+		LA_PRINT_STRING (stream, type, output, length, NULL,
 				 0, options);
 	      else
 		fputs_filtered (output, stream);
 	    }
 	  else
 	    gdbpy_print_stack ();
-	  
-	  if (string)
-	    Py_DECREF (string);
-	  else
-	    xfree (output);
-      
-	  xfree (encoding);
-	  Py_DECREF (py_str);
 	}
+
+      do_cleanups (cleanup);
     }
   else if (replacement)
     {
@@ -548,32 +546,31 @@
 	  fputs_filtered (" = ", stream);
 	}
 
-      if (gdbpy_is_lazy_string (py_v) || gdbpy_is_string (py_v))
+      if (gdbpy_is_lazy_string (py_v))
 	{
-	  gdb_byte *output = NULL;
+	  CORE_ADDR addr;
+	  struct type *type;
+	  long length;
+	  char *encoding = NULL;
 
-	  if (gdbpy_is_lazy_string (py_v))
-	    {
-	      struct type *type;
-	      long length;
-	      char *encoding = NULL;
+	  make_cleanup (free_current_contents, &encoding);
+	  gdbpy_extract_lazy_string (py_v, &addr, &type, &length, &encoding);
 
-	      output = gdbpy_extract_lazy_string (py_v, &type,
-						  &length, &encoding);
-	      if (!output)
-		gdbpy_print_stack ();
-	      LA_PRINT_STRING (stream, type, output, length, encoding,
-			       0, options);
-	      xfree (encoding);
-	      xfree (output);
-	    }
+	  val_print_string (type, encoding, addr, (int) length, stream,
+			    options);
+
+	  do_cleanups (inner_cleanup);
+	}
+      else if (gdbpy_is_string (py_v))
+	{
+	  gdb_byte *output;
+
+	  output = python_string_to_host_string (py_v);
+	  if (!output)
+	    gdbpy_print_stack ();
 	  else
 	    {
-	      output = python_string_to_host_string (py_v);
-	      if (!output)
-		gdbpy_print_stack ();
-	      else
-		fputs_filtered (output, stream);
+	      fputs_filtered (output, stream);
 	      xfree (output);
 	    }
 	}
Index: python/python-internal.h
===================================================================
RCS file: /cvs/src/src/gdb/python/python-internal.h,v
retrieving revision 1.32
diff -u -r1.32 python-internal.h
--- python/python-internal.h	13 Aug 2010 20:50:47 -0000	1.32
+++ python/python-internal.h	15 Oct 2010 18:42:15 -0000
@@ -209,9 +209,9 @@
 char *gdbpy_exception_to_string (PyObject *ptype, PyObject *pvalue);
 
 int gdbpy_is_lazy_string (PyObject *result);
-gdb_byte *gdbpy_extract_lazy_string (PyObject *string,
-				     struct type **str_type, 
-				     long *length, char **encoding);
+void gdbpy_extract_lazy_string (PyObject *string, CORE_ADDR *addr,
+				struct type **str_type, 
+				long *length, char **encoding);
 
 int gdbpy_is_value_object (PyObject *obj);
 


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