This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix printf of a convenience variable holding an inferior address
- From: Andrew Burgess <andrew dot burgess at embecosm dot com>
- To: Sergio Durigan Junior <sergiodj at redhat dot com>
- Cc: GDB Patches <gdb-patches at sourceware dot org>, Philippe Waroquiers <philippe dot waroquiers at skynet dot be>, Pedro Alves <palves at redhat dot com>
- Date: Tue, 3 Mar 2020 13:39:18 +0000
- Subject: Re: [PATCH] Fix printf of a convenience variable holding an inferior address
- References: <20190610211622.15237-1-philippe.waroquiers@skynet.be> <20200302024616.1049417-1-sergiodj@redhat.com>
* Sergio Durigan Junior <sergiodj@redhat.com> [2020-03-01 21:46:16 -0500]:
> Back at:
>
> commit 1f6f6e21fa86dc3411a6498608f32e9eb24b7851
> Author: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Mon Jun 10 21:41:51 2019 +0200
>
> Ensure GDB printf command can print convenience var strings without a target.
>
> GDB was extended in order to allow the printing of convenience
> variables that are strings without a target. However, this introduced
> a regression that hasn't been caught by our testsuite (because there
> were no tests for it).
>
> The problem happens when we try to print a convenience variable that
> holds the address of a string in the inferior. The following
> two-liners can reproduce the issue:
>
> $ echo -e 'int main(){const char a[]="test";return 0;}' | gcc -x c - -O0-g3
> $ ./gdb/gdb --data-directory ./gdb/data-directory -q ./a.out -ex 'start' -ex 'set $x = (const char *) (&a[0] + 2)' -ex 'printf "%s\n", $x'
>
> After some investigation, I found that the problem happens on
> printcmd.c:printf_c_string. In the case above, we're taking the first
> branch of the 'if' condition, which assumes that there will be a value
> to be printed at "value_contents (value)". There isn't. We actually
> need to obtain the address that the variable points to, and read the
> contents from memory.
>
> It seems to me that we should avoid this branch if the TYPE_CODE of
> "value_type (value)" is TYPE_CODE_PTR (i.e., a pointer to the
> inferior's memory). This is what this patch does.
>
> I took the liberty to extend the current testcase under
> gdb.base/printcmds.exp and create a test that exercises this scenario.
>
> No regressions have been found on Buildbot.
>
> gdb/ChangeLog:
> 2020-03-02 Sergio Durigan Junior <sergiodj@redhat.com>
>
> * printcmd.c (print_c_string): Check also for TYPE_CODE_PTR
> when verifying if dealing with a convenience variable.
>
> gdb/testsuite/ChangeLog:
> 2020-03-02 Sergio Durigan Junior <sergiodj@redhat.com>
>
> * gdb.base/printcmds.exp: Add test to verify printf of a
> variable holding an address.
LGTM.
Thanks,
Andrew
> ---
> gdb/printcmd.c | 3 ++-
> gdb/testsuite/gdb.base/printcmds.exp | 8 ++++++++
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index 797041484e..78d8d3d81e 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -2260,7 +2260,8 @@ printf_c_string (struct ui_file *stream, const char *format,
> {
> const gdb_byte *str;
>
> - if (VALUE_LVAL (value) == lval_internalvar
> + if (TYPE_CODE (value_type (value)) != TYPE_CODE_PTR
> + && VALUE_LVAL (value) == lval_internalvar
> && c_is_string_type_p (value_type (value)))
> {
> size_t len = TYPE_LENGTH (value_type (value));
> diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
> index bd2afc8696..c87a1517f0 100644
> --- a/gdb/testsuite/gdb.base/printcmds.exp
> +++ b/gdb/testsuite/gdb.base/printcmds.exp
> @@ -1039,6 +1039,14 @@ gdb_test_no_output "set may-call-functions off"
> test_printf_convenience_var "with target, may-call-functions off"
> gdb_test_no_output "set may-call-functions on"
>
> +# Test printf of a variable that holds the address to a substring in
> +# the inferior. This test will not work without a target.
> +gdb_test_no_output "set var \$test_substr = \(char \*\) \(&teststring\[0\] + 4\)" \
> + "set \$test_substr var"
> +gdb_test "printf \"test_substr val = %s\\n\", \$test_substr" \
> + "test_substr val = string contents" \
> + "print \$test_substr"
> +
> test_integer_literals_accepted
> test_integer_literals_rejected
> test_float_accepted
> --
> 2.24.1
>