public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Matthew Malcomson <hardenedapple@gmail.com>
To: gdb-patches@sourceware.org
Subject: [PATCH] fix bug with command `printf "%s\n", $_as_string($pc)`
Date: Sun, 19 Feb 2017 17:32:00 -0000	[thread overview]
Message-ID: <959cdc8e-1e54-a2e7-53d0-d80aaaea9ea8@gmail.com> (raw)

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

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))

             reply	other threads:[~2017-02-19 17:32 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-19 17:32 Matthew Malcomson [this message]
2017-02-20 18:20 ` Simon Marchi
2017-02-25 11:45   ` Matthew Malcomson
2017-02-25 18:33     ` Simon Marchi
2017-02-25 19:11       ` Matthew Malcomson
2017-02-25 21:33         ` Simon Marchi
2017-02-26 13:20           ` Matthew Malcomson
2017-02-26 14:44             ` Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=959cdc8e-1e54-a2e7-53d0-d80aaaea9ea8@gmail.com \
    --to=hardenedapple@gmail.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).