fix bug with command `printf "%s\n", $_as_string($pc)`

Message ID 959cdc8e-1e54-a2e7-53d0-d80aaaea9ea8@gmail.com
State New, archived
Headers

Commit Message

Matthew Malcomson Feb. 19, 2017, 5:32 p.m. UTC
  Automatic conversion of python string to gdb value leaves off the NULL byte.

This doesn't matter with `print $_as_string($pc)` because the print 
command uses `value_print()` which limits how many characters are 
printed based on the length of the array.

The command `printf "%s\n", $_as_string($pc)` instead writes the value 
into the inferior with `value_coerce_to_target()` and then treats the 
string as a NULL terminated string in the inferior. As the value created 
from `$_as_string($pc)` is not NULL terminated, this sometimes ends up 
printing extra characters (depending on where the inferior allocated the 
memory for this string).

`printf "%s\n", "Some string"` doesn't have this problem because the gdb 
value type created here includes the NULL terminating byte, as is 
ensured in `evaluate_subexp_c()` file and line c-lang.c:677 (at the time 
of writing this), commented as `/* Write the terminating character.  */`.


There are two other places where a terminating NULL byte is not included 
when calling `value_cstring()`.

When converting from a guile string into a gdb internal value structure. 
This doesn't cause any observable bugs because the string can't be 
stored in a variable (as there's no equivalent of the python 
`gdb.Function` class) and hence can't be passed to the `printf` command. 
There appears to have been some thought about terminating NULL bytes 
commented above `gdbscm_scm_to_string()`. Given there are no observable 
bugs and there has already been some thought about the NULL byte in this 
area I've not changed anything there.

In `value_of_internalvar()` when the variable is of kind 
`INTERNALVAR_STRING`. There are only two internal variables of this 
kind, `$trace_func` and `$trace_file`. The info documentation says that 
`$trace_file` is not suitable for use in `printf`, and I believe the 
same should be said about `$trace_func`. Both are prohibited by the 
check on `get_traceframe_number()` in `call_function_by_hand_dummy()` 
that is called from `value_coerce_to_target()` during printf on string 
variables. I have made a slight change to the documentation here.


------------------------------------------------

CHANGELOG:

2017-02-19  Matthew Malcomson <hardenedapple@gmail.com>

     * python/py-value.c (convert_value_from_python): Include NULL 
terminator in result.
     doc/gdb.texinfo ($trace_func): Mention this value can't be used 
with printf.

-------------------------------------------------

PATCH:

commit 5a0ceb374621de8eb2264c6b9ae92cf936a66f6b
Author: Matthew Malcomson <hardenedapple@gmail.com>
Date:   Sun Feb 19 14:35:09 2017 +0000

     convert_value_from_python include terminating NULL

     When converting python strings to internal gdb Value strings, the NULL
     byte was initially left out, this can result in extra data from the
     inferior being printed when the resulting value is used with
     printf "%s\n", value
  

Comments

Simon Marchi Feb. 20, 2017, 6:19 p.m. UTC | #1
On 2017-02-19 12:32, Matthew Malcomson wrote:
> diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
> index eb3d307b19..c786f68865 100644
> --- a/gdb/python/py-value.c
> +++ b/gdb/python/py-value.c
> @@ -1615,7 +1615,7 @@ convert_value_from_python (PyObject *obj)
>        gdb::unique_xmalloc_ptr<char> s
>          = python_string_to_target_string (obj);
>        if (s != NULL)
> -        value = value_cstring (s.get (), strlen (s.get ()),
> +        value = value_cstring (s.get (), strlen (s.get ()) + 1,
>                     builtin_type_pychar);
>      }
>        else if (PyObject_TypeCheck (obj, &value_object_type))

This fix looks good to me.

One test (py-mi.exp) needs to be updated though.  You can run all the 
Python-related tests using:

   $ make check TESTS="gdb.python/*.exp"

Normally, the Python tests all pass reliably, unlike some other parts of 
the testsuite.

It might also be good to improve gdb.python/py-as-string.exp to include 
a test for this bug.

Thanks,

Simon
  

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c465dc2f9f..5fb34853f1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -13645,8 +13645,8 @@  The source file for the current trace snapshot.
  The name of the function containing @code{$tracepoint}.
  @end table

-Note: @code{$trace_file} is not suitable for use in @code{printf},
-use @code{output} instead.
+Note: @code{$trace_file} and @code{$trace_func} are not suitable for use in
+@code{printf}, use @code{output} instead.

  Here's a simple example of using these convenience variables for
  stepping through all the trace snapshots and printing some of their
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index eb3d307b19..c786f68865 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1615,7 +1615,7 @@  convert_value_from_python (PyObject *obj)
        gdb::unique_xmalloc_ptr<char> s
          = python_string_to_target_string (obj);
        if (s != NULL)
-        value = value_cstring (s.get (), strlen (s.get ()),
+        value = value_cstring (s.get (), strlen (s.get ()) + 1,
                     builtin_type_pychar);
      }
        else if (PyObject_TypeCheck (obj, &value_object_type))