Re: [python][patch] Preserve string length, and embedded-nulls in pretty-print string emission.

[Phil Muldoon <pmuldoon@redhat.com>]

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

On 06/16/2009 07:07 PM, Tom Tromey wrote:
> Phil>  I decided to take a crack at the mechanics of this change this
> Phil>  morning. This produces the following hunks (in rough). I can see how
> Phil>  just borrowing the underlying data from the PyObject avoids an
> Phil>  allocation (the results of PyString_AsString must not be deallocated,
> Phil>  etc). Is this what you had in mind?
>    

This patch scraps the first value-only approach and as discussed 
completely implements the string in PyObject approach. Now strings are 
kept in a PyObject (along with their associated length data) until they 
are printed, or used in some other way.


What do you think?

Regards

Phil


2009-06-18 Phil Muldoon <pmuldoon@redhat.com>

* python/lib/gdb/libstdcxx/v6/printers.py
(StdStringPrinter.to_string): Extract length from header. Use in
string extraction.
* python/python-internal.h (apply_varobj_pretty_printer): Update
definition.
(python_string_to_target_python_string): Add definition.
* python/python-utils.c (unicode_to_encoded_python_string)
(unicode_to_target_python_string)
(python_string_to_target_python_string): New Functions.
* python/python.c : (print_string_repr): Refactor to logic to account for
PyObject returned strings.
(pretty_print_one_value): Likewise
(apply_varobj_pretty_printer): Likewise.
* varobj.c (value_get_print_value): Refactor logic to account for
PyObject returned strings.

Testsuite ChangeLog:

2009-06-18 Phil Muldoon <pmuldoon@redhat.com>

* gdb.python/python-prettyprint.c: Add counted null string
structure.
* gdb.python/python-prettyprint.exp: Print null string. Test for
embedded nulls.
* gdb.python/python-prettyprint.py (pp_ns): New
Function.


[-- Attachment #2: strings_as_pyobjects.patch --]
[-- Type: text/plain, Size: 14181 bytes --]

diff --git a/gdb/python/lib/gdb/libstdcxx/v6/printers.py b/gdb/python/lib/gdb/libstdcxx/v6/printers.py
index 2bd2593..6bf4f3b 100644
--- a/gdb/python/lib/gdb/libstdcxx/v6/printers.py
+++ b/gdb/python/lib/gdb/libstdcxx/v6/printers.py
@@ -468,7 +468,17 @@ class StdStringPrinter:
             encoding = gdb.parameter('target-charset')
         elif isinstance(encoding, WideEncoding):
             encoding = encoding.value
-        return self.val['_M_dataplus']['_M_p'].string(encoding)
+        type = self.val.type
+        if type.code == gdb.TYPE_CODE_REF:
+            type = type.target ()
+
+        ptr = self.val ['_M_dataplus']['_M_p']
+        realtype = type.unqualified ().strip_typedefs ()
+        reptype = gdb.lookup_type (str (realtype) + '::_Rep').pointer ()
+        header = ptr.cast(reptype) - 1
+        len = header.dereference ()['_M_length']
+
+        return self.val['_M_dataplus']['_M_p'].string (encoding, length = len)
 
     def display_hint (self):
         return 'string'
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 29e55dd..1d90722 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -169,6 +169,7 @@ void gdbpy_print_stack (void);
 PyObject *python_string_to_unicode (PyObject *obj);
 char *unicode_to_target_string (PyObject *unicode_str);
 char *python_string_to_target_string (PyObject *obj);
+PyObject *python_string_to_target_python_string (PyObject *obj);
 char *python_string_to_host_string (PyObject *obj);
 PyObject *target_string_to_unicode (const gdb_byte *str, int length);
 int gdbpy_is_string (PyObject *obj);
@@ -177,8 +178,8 @@ int gdbpy_is_value_object (PyObject *obj);
 
 /* Note that these are declared here, and not in python.h with the
    other pretty-printer functions, because they refer to PyObject.  */
-char *apply_varobj_pretty_printer (PyObject *print_obj,
-				   struct value **replacement);
+PyObject *apply_varobj_pretty_printer (PyObject *print_obj,
+				       struct value **replacement);
 PyObject *gdbpy_get_varobj_pretty_printer (struct value *value);
 PyObject *gdbpy_instantiate_printer (PyObject *cons, PyObject *value);
 char *gdbpy_get_display_hint (PyObject *printer);
diff --git a/gdb/python/python-utils.c b/gdb/python/python-utils.c
index f9c9486..45c74a5 100644
--- a/gdb/python/python-utils.c
+++ b/gdb/python/python-utils.c
@@ -122,6 +122,23 @@ unicode_to_encoded_string (PyObject *unicode_str, const char *charset)
   return result;
 }
 
+/* Returns a PyObject with the contents of the given unicode string
+   object converted to a named charset.  If an error occurs during
+   the conversion, NULL will be returned and a python exception will
+   be set.  */
+static PyObject *
+unicode_to_encoded_python_string (PyObject *unicode_str, const char *charset)
+{
+  PyObject *string;
+
+  /* Translate string to named charset.  */
+  string = PyUnicode_AsEncodedString (unicode_str, charset, NULL);
+  if (string == NULL)
+    return NULL;
+
+  return string;
+}
+
 /* Returns a newly allocated string with the contents of the given unicode
    string object converted to the target's charset.  If an error occurs during
    the conversion, NULL will be returned and a python exception will be set.
@@ -133,6 +150,16 @@ unicode_to_target_string (PyObject *unicode_str)
   return unicode_to_encoded_string (unicode_str, target_charset ());
 }
 
+/* Returns a PyObject with the contents of the given unicode string
+   object converted to the target's charset.  If an error occurs
+   during the conversion, NULL will be returned and a python exception
+   will be set.  */
+PyObject *
+unicode_to_target_python_string (PyObject *unicode_str)
+{
+  return unicode_to_encoded_python_string (unicode_str, target_charset ());
+}
+
 /* Converts a python string (8-bit or unicode) to a target string in
    the target's charset.  Returns NULL on error, with a python exception set.
 
@@ -152,6 +179,24 @@ python_string_to_target_string (PyObject *obj)
   return result;
 }
 
+/* Converts a python string (8-bit or unicode) to a target string in the
+   target's charset.  Returns NULL on error, with a python exception
+   set.  */
+PyObject *
+python_string_to_target_python_string (PyObject *obj)
+{
+  PyObject *str;
+  PyObject *result;
+
+  str = python_string_to_unicode (obj);
+  if (str == NULL)
+    return NULL;
+
+  result = unicode_to_target_python_string (str);
+  Py_DECREF (str);
+  return result;
+}
+
 /* Converts a python string (8-bit or unicode) to a target string in
    the host's charset.  Returns NULL on error, with a python exception set.
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index e97bef8..d13952f 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -826,41 +826,36 @@ find_pretty_printer (PyObject *value)
   return function;
 }
 
-/* Pretty-print a single value, via the printer object PRINTER.  If
-   the function returns a string, an xmalloc()d copy is returned.
-   Otherwise, if the function returns a value, a *OUT_VALUE is set to
-   the value, and NULL is returned.  On error, *OUT_VALUE is set to
-   NULL and NULL is returned.  */
-static char *
+/* Pretty-print a single value, via the printer object PRINTER.
+   If the function returns a string, a PyObject containing the string
+   is returned.  Otherwise, if the function returns a value,
+   *OUT_VALUE is set to the value, and NULL is returned.  On error,
+   *OUT_VALUE is set to NULL, and NULL is returned.  */
+static PyObject *
 pretty_print_one_value (PyObject *printer, struct value **out_value)
 {
-  char *output = NULL;
   volatile struct gdb_exception except;
+  PyObject *result = NULL;
 
   TRY_CATCH (except, RETURN_MASK_ALL)
     {
-      PyObject *result;
-
       result = PyObject_CallMethodObjArgs (printer, gdbpy_to_string_cst, NULL);
       if (result)
 	{
-	  if (gdbpy_is_string (result))
-	    output = python_string_to_host_string (result);
-	  else if (PyObject_TypeCheck (result, &value_object_type))
+	  if (! gdbpy_is_string (result))
 	    {
-	      /* If we just call convert_value_from_python for this
-		 type, we won't know who owns the result.  For this
-		 one case we need to copy the resulting value.  */
-	      struct value *v = value_object_to_value (result);
-	      *out_value = value_copy (v);
+	      *out_value = convert_value_from_python (result);
+	      Py_DECREF (result);
+	      result = NULL;
+	      if (PyErr_Occurred ())
+		*out_value = NULL;
 	    }
-	  else
-	    *out_value = convert_value_from_python (result);
-	  Py_DECREF (result);
 	}
+      else
+	*out_value = NULL;
     }
 
-  return output;
+  return result;
 }
 
 /* Instantiate a pretty-printer given a constructor, CONS, and a
@@ -906,18 +901,26 @@ print_string_repr (PyObject *printer, const char *hint,
 		   const struct value_print_options *options,
 		   const struct language_defn *language)
 {
-  char *output;
   struct value *replacement = NULL;
+  PyObject *py_str = NULL;
 
-  output = pretty_print_one_value (printer, &replacement);
-  if (output)
+  py_str = pretty_print_one_value (printer, &replacement);
+  if (py_str)
     {
-      if (hint && !strcmp (hint, "string"))
-	LA_PRINT_STRING (stream, (gdb_byte *) output, strlen (output),
-			 1, 0, options);
-      else
-	fputs_filtered (output, stream);
-      xfree (output);
+      PyObject *string = python_string_to_target_python_string (py_str);
+      if (string)
+	{
+	  gdb_byte *output = PyString_AsString (string);
+	  int len = PyString_Size (string) + 1;
+
+	  if (hint && !strcmp (hint, "string"))
+	    LA_PRINT_STRING (stream, output,
+			     len, 1, 0, options);
+	  else
+	    fputs_filtered (output, stream);
+	  Py_DECREF (string);
+	}
+      Py_DECREF (py_str);
     }
   else if (replacement)
     common_val_print (replacement, stream, recurse, options, language);
@@ -1232,28 +1235,31 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
   return result;
 }
 
-/* Apply a pretty-printer for the varobj code.  PRINTER_OBJ is the
-   print object.  It must have a 'to_string' method (but this is
-   checked by varobj, not here) which takes no arguments and
-   returns a string.  This function returns an xmalloc()d string if
-   the printer returns a string.  The printer may return a replacement
-   value instead; in this case *REPLACEMENT is set to the replacement
-   value, and this function returns NULL.  On error, *REPLACEMENT is
-   set to NULL and this function also returns NULL.  */
-char *
+ /* Apply a pretty-printer for the varobj code.  PRINTER_OBJ is the
+    print object.  It must have a 'to_string' method (but this is
+    checked by varobj, not here) which takes no arguments and
+    returns a string.  The printer will return a value and in the case
+    of a Python string being returned, this function will return a
+    PyObject containing the string.  For any other type, *REPLACEMENT is
+    set to the replacement value and this function returns NULL.  On
+    error, *REPLACEMENT is set to NULL and this function also returns
+    NULL.  */
+PyObject *
 apply_varobj_pretty_printer (PyObject *printer_obj,
 			     struct value **replacement)
 {
-  char *result;
+  int size = 0;
   PyGILState_STATE state = PyGILState_Ensure ();
+  PyObject *py_str = NULL;
 
   *replacement = NULL;
-  result = pretty_print_one_value (printer_obj, replacement);
-  if (result == NULL);
+  py_str  = pretty_print_one_value (printer_obj, replacement);
+
+  if (replacement == NULL && py_str == NULL)
     gdbpy_print_stack ();
   PyGILState_Release (state);
 
-  return result;
+  return py_str;
 }
 
 /* Find a pretty-printer object for the varobj module.  Returns a new
diff --git a/gdb/testsuite/gdb.python/python-prettyprint.c b/gdb/testsuite/gdb.python/python-prettyprint.c
index 0d9110d..5cc35be 100644
--- a/gdb/testsuite/gdb.python/python-prettyprint.c
+++ b/gdb/testsuite/gdb.python/python-prettyprint.c
@@ -29,6 +29,11 @@ struct ss
   struct s b;
 };
 
+struct ns {
+  const char *null_str;
+  int length;
+};
+
 #ifdef __cplusplus
 struct S : public s {
   int zs;
@@ -166,6 +171,10 @@ main ()
   init_ss(ssa+1, 5, 6);
   memset (&nullstr, 0, sizeof nullstr);
 
+  struct ns  ns;
+  ns.null_str = "embedded\0null\0string";
+  ns.length = 20;
+
 #ifdef __cplusplus
   S cps;
 
diff --git a/gdb/testsuite/gdb.python/python-prettyprint.exp b/gdb/testsuite/gdb.python/python-prettyprint.exp
index 47d2fa8..b2dc85d 100644
--- a/gdb/testsuite/gdb.python/python-prettyprint.exp
+++ b/gdb/testsuite/gdb.python/python-prettyprint.exp
@@ -75,6 +75,7 @@ proc run_lang_tests {lang} {
 	gdb_test "print ref" "= a=<15> b=< a=<8> b=<$hex>>"
 	gdb_test "print derived" \
 	    " = \{.*<Vbase1> = pp class name: Vbase1.*<Vbase2> = \{.*<VirtualTest> = pp value variable is: 1,.*members of Vbase2:.*_vptr.Vbase2 = $hex.*<Vbase3> = \{.*members of Vbase3.*members of Derived:.*value = 2.*"
+	gdb_test "print ns " "\"embedded\\\\000null\\\\000string\""
     }
 
     gdb_test "print x" " = $hex \"this is x\""
diff --git a/gdb/testsuite/gdb.python/python-prettyprint.py b/gdb/testsuite/gdb.python/python-prettyprint.py
index 82e5331..c3e0dc4 100644
--- a/gdb/testsuite/gdb.python/python-prettyprint.py
+++ b/gdb/testsuite/gdb.python/python-prettyprint.py
@@ -99,6 +99,19 @@ class pp_nullstr:
     def to_string(self):
         return self.val['s'].string(gdb.parameter('target-charset'))
 
+class pp_ns:
+    "Print a std::basic_string of some kind"
+
+    def __init__(self, val):
+        self.val = val
+
+    def to_string(self):
+        len = self.val['length']
+        return self.val['null_str'].string (gdb.parameter ('target-charset'), length = len)
+
+    def display_hint (self):
+        return 'string'
+
 def lookup_function (val):
     "Look-up and return a pretty-printer that can print val."
 
@@ -155,6 +168,8 @@ def register_pretty_printers ():
     pretty_printers_dict[re.compile ('^string_repr$')] = string_print
     pretty_printers_dict[re.compile ('^container$')] = ContainerPrinter
     
+    pretty_printers_dict[re.compile ('^struct ns$')]  = pp_ns
+    pretty_printers_dict[re.compile ('^ns$')]  = pp_ns
 pretty_printers_dict = {}
 
 register_pretty_printers ()
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 49b4a43..75f3f17 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -2250,8 +2250,9 @@ value_get_print_value (struct value *value, enum varobj_display_formats format,
   long dummy;
   struct ui_file *stb;
   struct cleanup *old_chain;
-  char *thevalue = NULL;
+  gdb_byte *thevalue = NULL;
   struct value_print_options opts;
+  int len = 0;
 
   if (value == NULL)
     return NULL;
@@ -2265,6 +2266,7 @@ value_get_print_value (struct value *value, enum varobj_display_formats format,
 	char *hint;
 	struct value *replacement;
 	int string_print = 0;
+	PyObject *output = NULL;
 
 	hint = gdbpy_get_display_hint (value_formatter);
 	if (hint)
@@ -2274,15 +2276,27 @@ value_get_print_value (struct value *value, enum varobj_display_formats format,
 	    xfree (hint);
 	  }
 
-	thevalue = apply_varobj_pretty_printer (value_formatter,
-						&replacement);
-	if (thevalue && !string_print)
+	output = apply_varobj_pretty_printer (value_formatter, &replacement);
+
+	if (output)
 	  {
-	    PyGILState_Release (state);
-	    return thevalue;
+	    PyObject *py_str = python_string_to_target_python_string (output);
+	    if (py_str)
+	      {
+		char *s = PyString_AsString (py_str);
+		len = PyString_Size (py_str) + 1;
+		thevalue = xmemdup (s, len, len);
+		Py_DECREF (py_str);
+	      }
+	    Py_DECREF (output);
 	  }
 	if (replacement)
 	  value = replacement;
+	if (thevalue && !string_print)
+         {
+           PyGILState_Release (state);
+           return thevalue;
+         }
       }
     PyGILState_Release (state);
   }
@@ -2297,8 +2311,7 @@ value_get_print_value (struct value *value, enum varobj_display_formats format,
   if (thevalue)
     {
       make_cleanup (xfree, thevalue);
-      LA_PRINT_STRING (stb, (gdb_byte *) thevalue, strlen (thevalue),
-		       1, 0, &opts);
+      LA_PRINT_STRING (stb, thevalue, len, 1, 0, &opts);
     }
   else
     common_val_print (value, stb, 0, &opts, current_language);