[RFA,3/3] Special case NULL when using printf's %s format
Commit Message
This changes the printf command's %s and %ls formats to special-case
NULL, and print "(null)" for these. This is PR cli/14977. This
behavior seems a bit friendlier; I was undecided on whether other
invalid pointers should be handled specially somehow, so for the time
being I've left those out.
gdb/ChangeLog
2018-02-14 Tom Tromey <tom@tromey.com>
PR cli/14977:
* printcmd.c (printf_c_string, printf_wide_c_string): Special case
for NULL.
gdb/gdbserver/ChangeLog
2018-02-14 Tom Tromey <tom@tromey.com>
PR cli/14977:
* ax.c (ax_printf): Special case for NULL.
gdb/testsuite/ChangeLog
2018-02-14 Tom Tromey <tom@tromey.com>
PR cli/14977:
* gdb.base/printcmds.exp (test_printf): Add printf test of %s with
a null pointer.
* gdb.base/wchar.exp: Likewise.
---
gdb/ChangeLog | 6 ++++++
gdb/gdbserver/ChangeLog | 5 +++++
gdb/gdbserver/ax.c | 5 +++++
gdb/printcmd.c | 10 ++++++++++
gdb/testsuite/ChangeLog | 7 +++++++
gdb/testsuite/gdb.base/printcmds.exp | 3 +++
gdb/testsuite/gdb.base/wchar.exp | 3 +++
7 files changed, 39 insertions(+)
Comments
On 02/15/2018 08:50 PM, Tom Tromey wrote:
> This changes the printf command's %s and %ls formats to special-case
> NULL, and print "(null)" for these. This is PR cli/14977. This
> behavior seems a bit friendlier; I was undecided on whether other
> invalid pointers should be handled specially somehow, so for the time
> being I've left those out.
A question here is what to do on targets that actually map things at
address zero. IIRC, some ARM chips do that. For such ports, I think you'd
want to be able to read/print that memory. I thought we had a gdbarch hook
for that, but I'm not finding it right now. Maybe I was thinking of
has_section_at_zero in dwarf2read.c.
Thanks,
Pedro Alves
On 02/16/2018 12:08 PM, Pedro Alves wrote:
> On 02/15/2018 08:50 PM, Tom Tromey wrote:
>> This changes the printf command's %s and %ls formats to special-case
>> NULL, and print "(null)" for these. This is PR cli/14977. This
>> behavior seems a bit friendlier; I was undecided on whether other
>> invalid pointers should be handled specially somehow, so for the time
>> being I've left those out.
>
> A question here is what to do on targets that actually map things at
> address zero. IIRC, some ARM chips do that. For such ports, I think you'd
> want to be able to read/print that memory. I thought we had a gdbarch hook
> for that, but I'm not finding it right now. Maybe I was thinking of
> has_section_at_zero in dwarf2read.c.
Then again, I don't think it's likely that programs map
strings at 0 (I think it's more like interrupt vectors), so for %s,
it's likely that nobody would miss printing 0 as a string.
So with that in mind, this is fine with me.
Thanks,
Pedro Alves
Pedro> Then again, I don't think it's likely that programs map
Pedro> strings at 0 (I think it's more like interrupt vectors), so for %s,
Pedro> it's likely that nobody would miss printing 0 as a string.
Another option would be to do a try-catch and only print "(null)" on
failure, if the pointer is 0.
Tom
@@ -847,6 +847,11 @@ ax_printf (CORE_ADDR fn, CORE_ADDR chan, const char *format,
int j;
tem = args[i];
+ if (tem == 0)
+ {
+ printf (current_substring, "(null)");
+ break;
+ }
/* This is a %s argument. Find the length of the string. */
for (j = 0;; j++)
@@ -2220,6 +2220,11 @@ printf_c_string (struct ui_file *stream, const char *format,
int j;
tem = value_as_address (value);
+ if (tem == 0)
+ {
+ fprintf_filtered (stream, format, "(null)");
+ return;
+ }
/* This is a %s argument. Find the length of the string. */
for (j = 0;; j++)
@@ -2260,6 +2265,11 @@ printf_wide_c_string (struct ui_file *stream, const char *format,
gdb_byte *buf = (gdb_byte *) alloca (wcwidth);
tem = value_as_address (value);
+ if (tem == 0)
+ {
+ fprintf_filtered (stream, format, "(null)");
+ return;
+ }
/* This is a %s argument. Find the length of the string. */
for (j = 0;; j += wcwidth)
@@ -780,6 +780,9 @@ proc test_printf {} {
# PR cli/19918.
gdb_test "printf \"%-16dq\\n\", 0" "0 q"
gdb_test "printf \"%-16pq\\n\", 0" "\\(nil\\) q"
+
+ # PR cli/14977.
+ gdb_test "printf \"%s\\n\", 0" "\\(null\\)"
}
#Test printing DFP values with printf
@@ -69,3 +69,6 @@ gdb_test "print repeat" "= L\"A$cent$cent\"\.\.\." \
gdb_test "print repeat_p" "= $hex L\"A$cent$cent\"\.\.\." \
"print repeat_p (print elements 3)"
+
+# From PR cli/14977, but here because it requires wchar_t.
+gdb_test "printf \"%ls\\n\", 0" "\\(null\\)"