* [review] gdb: Support printf 'z' size modifier
2019-11-05 15:46 [review] gdb: Support printf 'z' size modifier Andrew Burgess (Code Review)
@ 2019-11-05 16:22 ` Tom Tromey (Code Review)
2019-11-05 16:31 ` Simon Marchi (Code Review)
` (6 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Tom Tromey (Code Review) @ 2019-11-05 16:22 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
Tom Tromey has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511
......................................................................
Patch Set 1:
(1 comment)
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1/gdb/gdbsupport/format.c
File gdb/gdbsupport/format.c:
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1/gdb/gdbsupport/format.c@237
PS1, Line 237:
23 | format_pieces::format_pieces (const char **arg, bool gdb_extensions)
| ...
232 | seen_double_big_d = 1;
233 | }
234 | else
235 | seen_big_d = 1;
236 | }
237 > /* For size_t or ssize_t. */
238 > else if (*f == 'z')
239 > f++;
240 |
241 | switch (*f)
242 | {
243 | case 'u':
244 | if (seen_hash)
This seems necessary but perhaps not sufficient -- nothing
seems to use the 'z' flag to change the size of the expected
argument.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f
Gerrit-Change-Number: 511
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Tue, 05 Nov 2019 16:22:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 19+ messages in thread* [review] gdb: Support printf 'z' size modifier
2019-11-05 15:46 [review] gdb: Support printf 'z' size modifier Andrew Burgess (Code Review)
2019-11-05 16:22 ` Tom Tromey (Code Review)
@ 2019-11-05 16:31 ` Simon Marchi (Code Review)
2019-11-06 0:18 ` Pedro Alves (Code Review)
` (5 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Simon Marchi (Code Review) @ 2019-11-05 16:31 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches; +Cc: Tom Tromey
Simon Marchi has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511
......................................................................
Patch Set 1:
(1 comment)
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1//COMMIT_MSG
Commit Message:
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1//COMMIT_MSG@13
PS1, Line 13:
8 |
9 | The gdb format mechanism doesn't currently support the 'z' size
10 | modifier, there are a few places in GDB where this is used. Instead
11 | of removing these uses lets just add support to GDB for using 'z'.
12 |
13 > I've not added a test for this as I ran into the issue when trying to
14 > use some debug output. Before this commit:
15 |
16 | (gdb) set debug dwarf-line 9
17 | (gdb) file test
18 | Reading symbols from test...
19 | Unrecognized format specifier 'z' in printf
It would seem easy enough to add an entry in test_format_specifier, in unittests/format_pieces-selftests.c, I think it would be a good idea to add it.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f
Gerrit-Change-Number: 511
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Tue, 05 Nov 2019 16:31:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 19+ messages in thread* [review] gdb: Support printf 'z' size modifier
2019-11-05 15:46 [review] gdb: Support printf 'z' size modifier Andrew Burgess (Code Review)
2019-11-05 16:22 ` Tom Tromey (Code Review)
2019-11-05 16:31 ` Simon Marchi (Code Review)
@ 2019-11-06 0:18 ` Pedro Alves (Code Review)
2019-11-07 10:59 ` [review v2] " Andrew Burgess (Code Review)
` (4 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves (Code Review) @ 2019-11-06 0:18 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches; +Cc: Simon Marchi, Tom Tromey
Pedro Alves has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511
......................................................................
Patch Set 1:
(1 comment)
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1//COMMIT_MSG
Commit Message:
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1//COMMIT_MSG@11
PS1, Line 11:
6 |
7 | gdb: Support printf 'z' size modifier
8 |
9 | The gdb format mechanism doesn't currently support the 'z' size
10 | modifier, there are a few places in GDB where this is used. Instead
11 > of removing these uses lets just add support to GDB for using 'z'.
12 |
13 | I've not added a test for this as I ran into the issue when trying to
14 | use some debug output. Before this commit:
15 |
16 | (gdb) set debug dwarf-line 9
Just FYI, we didn't use to allow 'z' since it is a C99 feature (and thus C++11), and older compilers didn't support it, namely the MSVC runtime used by mingw.org. I think that is passed us nowadays, and I think mingw nowadays has replacement printf that support C99 features. And if it doesn't likely gnulib is taking care of it.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f
Gerrit-Change-Number: 511
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Wed, 06 Nov 2019 00:18:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 19+ messages in thread* [review v2] gdb: Support printf 'z' size modifier
2019-11-05 15:46 [review] gdb: Support printf 'z' size modifier Andrew Burgess (Code Review)
` (2 preceding siblings ...)
2019-11-06 0:18 ` Pedro Alves (Code Review)
@ 2019-11-07 10:59 ` Andrew Burgess (Code Review)
2019-11-07 11:05 ` Andrew Burgess (Code Review)
` (3 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Andrew Burgess (Code Review) @ 2019-11-07 10:59 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves, Tom Tromey, Simon Marchi
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511
......................................................................
gdb: Support printf 'z' size modifier
The gdb format mechanism doesn't currently support the 'z' size
modifier, there are a few places in GDB where this is used. Instead
of removing these uses lets just add support to GDB for using 'z'.
I found this issue when trying to use some of the debug output.
Before this commit:
(gdb) set debug dwarf-line 9
(gdb) file test
Reading symbols from test...
Unrecognized format specifier 'z' in printf
(No debugging symbols found in test)
(gdb)
After this commit:
(gdb) set debug dwarf-line 9
(gdb) file test
Reading symbols from test...
Adding dir 1: /usr/include
Adding file 1: test.c
Adding file 2: stdc-predef.h
Processing actual line 3: file 1, address 0x4004a0, is_stmt 1, discrim 0
Processing actual line 4: file 1, address 0x4004a0, is_stmt 1, discrim 0
.... lots of debug output ...
Processing actual line 10: file 1, address 0x4003b7, is_stmt 0, discrim 0
(gdb)
I've added a self test to cover the integer format size modifiers,
including the 'z' modifier.
gdb/ChangeLog:
* gdbsupport/format.c (format_pieces::format_pieces): Support
printf 'z' size modifier.
* gdbsupport/format.h (enum argclass): Add size_t_arg.
* printcmd.c (ui_printf): Handle size_t_arg.
* ui-out.c (ui_out::vmessage): Likewise.
* unittests/format_pieces-selftests.c (test_format_int_sizes): New
function.
(run_tests): Call test_format_int_sizes.
gdb/gdbserver/ChangeLog:
* ax.c (ax_printf): Handle size_t_arg.
Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f
---
M gdb/ChangeLog
M gdb/gdbserver/ChangeLog
M gdb/gdbserver/ax.c
M gdb/gdbsupport/format.c
M gdb/gdbsupport/format.h
M gdb/printcmd.c
M gdb/ui-out.c
M gdb/unittests/format_pieces-selftests.c
8 files changed, 106 insertions(+), 2 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 41daef6..860037f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2019-11-05 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * gdbsupport/format.c (format_pieces::format_pieces): Support
+ printf 'z' size modifier.
+ * gdbsupport/format.h (enum argclass): Add size_t_arg.
+ * printcmd.c (ui_printf): Handle size_t_arg.
+ * ui-out.c (ui_out::vmessage): Likewise.
+ * unittests/format_pieces-selftests.c (test_format_int_sizes): New
+ function.
+ (run_tests): Call test_format_int_sizes.
+
2019-11-04 Christian Biesinger <cbiesinger@google.com>
* psympriv.h: Add static_asserts for sizeof (general_symbol_info)
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 35684db..a69a320 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,7 @@
+2019-11-07 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * ax.c (ax_printf): Handle size_t_arg.
+
2019-11-01 Christian Biesinger <cbiesinger@google.com>
* configure: Regenerate.
diff --git a/gdb/gdbserver/ax.c b/gdb/gdbserver/ax.c
index 2c096ef..21fde5e 100644
--- a/gdb/gdbserver/ax.c
+++ b/gdb/gdbserver/ax.c
@@ -904,6 +904,14 @@
break;
}
+ case size_t_arg:
+ {
+ size_t val = args[i];
+
+ printf (current_substring, val);
+ break;
+ }
+
case literal_piece:
/* Print a portion of the format string that has no
directives. Note that this will not include any
diff --git a/gdb/gdbsupport/format.c b/gdb/gdbsupport/format.c
index 1e80350..2e2d90a 100644
--- a/gdb/gdbsupport/format.c
+++ b/gdb/gdbsupport/format.c
@@ -123,6 +123,7 @@
int seen_space = 0, seen_plus = 0;
int seen_big_l = 0, seen_h = 0, seen_big_h = 0;
int seen_big_d = 0, seen_double_big_d = 0;
+ int seen_size_t = 0;
int bad = 0;
int n_int_args = 0;
@@ -234,6 +235,12 @@
else
seen_big_d = 1;
}
+ /* For size_t or ssize_t. */
+ else if (*f == 'z')
+ {
+ seen_size_t = 1;
+ f++;
+ }
switch (*f)
{
@@ -251,7 +258,9 @@
case 'd':
case 'i':
- if (lcount == 0)
+ if (seen_size_t)
+ this_argclass = size_t_arg;
+ else if (lcount == 0)
this_argclass = int_arg;
else if (lcount == 1)
this_argclass = long_arg;
diff --git a/gdb/gdbsupport/format.h b/gdb/gdbsupport/format.h
index e2a47ba..07374db 100644
--- a/gdb/gdbsupport/format.h
+++ b/gdb/gdbsupport/format.h
@@ -38,7 +38,7 @@
enum argclass
{
literal_piece,
- int_arg, long_arg, long_long_arg, ptr_arg,
+ int_arg, long_arg, long_long_arg, size_t_arg, ptr_arg,
string_arg, wide_string_arg, wide_char_arg,
double_arg, long_double_arg,
dec32float_arg, dec64float_arg, dec128float_arg
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 27aaf7a..323a5d0 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2663,6 +2663,16 @@
DIAGNOSTIC_POP
break;
}
+ case size_t_arg:
+ {
+ size_t val = value_as_long (val_args[i]);
+
+ DIAGNOSTIC_PUSH
+ DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
+ fprintf_filtered (stream, current_substring, val);
+ DIAGNOSTIC_POP
+ break;
+ }
/* Handles floating-point values. */
case double_arg:
case long_double_arg:
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 6b0b5ac..80845f4 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -688,6 +688,24 @@
}
}
break;
+ case size_t_arg:
+ {
+ size_t val = va_arg (args, size_t);
+ switch (piece.n_int_args)
+ {
+ case 0:
+ call_do_message (style, current_substring, val);
+ break;
+ case 1:
+ call_do_message (style, current_substring, intvals[0], val);
+ break;
+ case 2:
+ call_do_message (style, current_substring,
+ intvals[0], intvals[1], val);
+ break;
+ }
+ }
+ break;
case double_arg:
call_do_message (style, current_substring, va_arg (args, double));
break;
diff --git a/gdb/unittests/format_pieces-selftests.c b/gdb/unittests/format_pieces-selftests.c
index ed83d96..3971201 100644
--- a/gdb/unittests/format_pieces-selftests.c
+++ b/gdb/unittests/format_pieces-selftests.c
@@ -76,12 +76,56 @@
}, true);
}
+/* Test the different size modifiers that can be applied to an integer
+ argument. Test with different integer format specifiers too. */
+
+static void
+test_format_int_sizes ()
+{
+ check ("Hello\\t %hu%lu%llu%zu", /* ARI: %ll */
+ {
+ format_piece ("Hello\t ", literal_piece, 0),
+ format_piece ("%hu", int_arg, 0),
+ format_piece ("%lu", long_arg, 0),
+ format_piece ("%llu", long_long_arg, 0), /* ARI: %ll */
+ format_piece ("%zu", size_t_arg, 0)
+ });
+
+ check ("Hello\\t %hx%lx%llx%zx", /* ARI: %ll */
+ {
+ format_piece ("Hello\t ", literal_piece, 0),
+ format_piece ("%hx", int_arg, 0),
+ format_piece ("%lx", long_arg, 0),
+ format_piece ("%llx", long_long_arg, 0), /* ARI: %ll */
+ format_piece ("%zx", size_t_arg, 0)
+ });
+
+ check ("Hello\\t %ho%lo%llo%zo", /* ARI: %ll */
+ {
+ format_piece ("Hello\t ", literal_piece, 0),
+ format_piece ("%ho", int_arg, 0),
+ format_piece ("%lo", long_arg, 0),
+ format_piece ("%llo", long_long_arg, 0), /* ARI: %ll */
+ format_piece ("%zo", size_t_arg, 0)
+ });
+
+ check ("Hello\\t %hd%ld%lld%zd", /* ARI: %ll */
+ {
+ format_piece ("Hello\t ", literal_piece, 0),
+ format_piece ("%hd", int_arg, 0),
+ format_piece ("%ld", long_arg, 0),
+ format_piece ("%lld", long_long_arg, 0), /* ARI: %ll */
+ format_piece ("%zd", size_t_arg, 0)
+ });
+}
+
static void
run_tests ()
{
test_escape_sequences ();
test_format_specifier ();
test_gdb_formats ();
+ test_format_int_sizes ();
}
} /* namespace format_pieces */
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f
Gerrit-Change-Number: 511
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset
^ permalink raw reply [flat|nested] 19+ messages in thread* [review v2] gdb: Support printf 'z' size modifier
2019-11-05 15:46 [review] gdb: Support printf 'z' size modifier Andrew Burgess (Code Review)
` (3 preceding siblings ...)
2019-11-07 10:59 ` [review v2] " Andrew Burgess (Code Review)
@ 2019-11-07 11:05 ` Andrew Burgess (Code Review)
2019-11-12 20:19 ` Kevin Buettner (Code Review)
` (2 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Andrew Burgess (Code Review) @ 2019-11-07 11:05 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves, Simon Marchi, Tom Tromey
Andrew Burgess has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511
......................................................................
Patch Set 2:
(3 comments)
New version should hopefully correctly handle the argument as a different size and includes some selftests.
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1//COMMIT_MSG
Commit Message:
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1//COMMIT_MSG@11
PS1, Line 11:
6 |
7 | gdb: Support printf 'z' size modifier
8 |
9 | The gdb format mechanism doesn't currently support the 'z' size
10 | modifier, there are a few places in GDB where this is used. Instead
11 > of removing these uses lets just add support to GDB for using 'z'.
12 |
13 | I've not added a test for this as I ran into the issue when trying to
14 | use some debug output. Before this commit:
15 |
16 | (gdb) set debug dwarf-line 9
> Just FYI, we didn't use to allow 'z' since it is a C99 feature (and thus C++11), and older compilers […]
I did consider just replacing all uses of %z with something else and casting the argument, there aren't that many uses so the patch would be pretty similar in size to what I've now posted. Let me know if you'd prefer this approach.
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1//COMMIT_MSG@13
PS1, Line 13:
8 |
9 | The gdb format mechanism doesn't currently support the 'z' size
10 | modifier, there are a few places in GDB where this is used. Instead
11 | of removing these uses lets just add support to GDB for using 'z'.
12 |
13 > I've not added a test for this as I ran into the issue when trying to
14 > use some debug output. Before this commit:
15 |
16 | (gdb) set debug dwarf-line 9
17 | (gdb) file test
18 | Reading symbols from test...
19 | Unrecognized format specifier 'z' in printf
> It would seem easy enough to add an entry in test_format_specifier, in unittests/format_pieces-selft […]
Done.
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1/gdb/gdbsupport/format.c
File gdb/gdbsupport/format.c:
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1/gdb/gdbsupport/format.c@237
PS1, Line 237:
23 | format_pieces::format_pieces (const char **arg, bool gdb_extensions)
| ...
232 | seen_double_big_d = 1;
233 | }
234 | else
235 | seen_big_d = 1;
236 | }
237 > /* For size_t or ssize_t. */
238 > else if (*f == 'z')
239 > f++;
240 |
241 | switch (*f)
242 | {
243 | case 'u':
244 | if (seen_hash)
> This seems necessary but perhaps not sufficient -- nothing […]
Thanks for this. I think I was fooled by it "just working" when treating the size_t as an int. I believe I've now handled this correctly.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f
Gerrit-Change-Number: 511
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Thu, 07 Nov 2019 11:05:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Comment-In-Reply-To: Tom Tromey <tromey@sourceware.org>
Comment-In-Reply-To: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 19+ messages in thread* [review v2] gdb: Support printf 'z' size modifier
2019-11-05 15:46 [review] gdb: Support printf 'z' size modifier Andrew Burgess (Code Review)
` (4 preceding siblings ...)
2019-11-07 11:05 ` Andrew Burgess (Code Review)
@ 2019-11-12 20:19 ` Kevin Buettner (Code Review)
2019-11-12 23:53 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-11-12 23:53 ` Sourceware to Gerrit sync (Code Review)
7 siblings, 0 replies; 19+ messages in thread
From: Kevin Buettner (Code Review) @ 2019-11-12 20:19 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
Cc: Joel Brobecker, Pedro Alves, Simon Marchi, Tom Tromey
Kevin Buettner has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511
......................................................................
Patch Set 2: Code-Review+2
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f
Gerrit-Change-Number: 511
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Tue, 12 Nov 2019 20:18:59 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 19+ messages in thread* [pushed] gdb: Support printf 'z' size modifier
2019-11-05 15:46 [review] gdb: Support printf 'z' size modifier Andrew Burgess (Code Review)
` (5 preceding siblings ...)
2019-11-12 20:19 ` Kevin Buettner (Code Review)
@ 2019-11-12 23:53 ` Sourceware to Gerrit sync (Code Review)
2019-11-14 12:55 ` Eli Zaretskii
2019-11-12 23:53 ` Sourceware to Gerrit sync (Code Review)
7 siblings, 1 reply; 19+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-11-12 23:53 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches
Cc: Kevin Buettner, Joel Brobecker, Pedro Alves, Simon Marchi, Tom Tromey
Sourceware to Gerrit sync has submitted this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511
......................................................................
gdb: Support printf 'z' size modifier
The gdb format mechanism doesn't currently support the 'z' size
modifier, there are a few places in GDB where this is used. Instead
of removing these uses lets just add support to GDB for using 'z'.
I found this issue when trying to use some of the debug output.
Before this commit:
(gdb) set debug dwarf-line 9
(gdb) file test
Reading symbols from test...
Unrecognized format specifier 'z' in printf
(No debugging symbols found in test)
(gdb)
After this commit:
(gdb) set debug dwarf-line 9
(gdb) file test
Reading symbols from test...
Adding dir 1: /usr/include
Adding file 1: test.c
Adding file 2: stdc-predef.h
Processing actual line 3: file 1, address 0x4004a0, is_stmt 1, discrim 0
Processing actual line 4: file 1, address 0x4004a0, is_stmt 1, discrim 0
.... lots of debug output ...
Processing actual line 10: file 1, address 0x4003b7, is_stmt 0, discrim 0
(gdb)
I've added a self test to cover the integer format size modifiers,
including the 'z' modifier.
gdb/ChangeLog:
* gdbsupport/format.c (format_pieces::format_pieces): Support
printf 'z' size modifier.
* gdbsupport/format.h (enum argclass): Add size_t_arg.
* printcmd.c (ui_printf): Handle size_t_arg.
* ui-out.c (ui_out::vmessage): Likewise.
* unittests/format_pieces-selftests.c (test_format_int_sizes): New
function.
(run_tests): Call test_format_int_sizes.
gdb/gdbserver/ChangeLog:
* ax.c (ax_printf): Handle size_t_arg.
Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f
---
M gdb/ChangeLog
M gdb/gdbserver/ChangeLog
M gdb/gdbserver/ax.c
M gdb/gdbsupport/format.c
M gdb/gdbsupport/format.h
M gdb/printcmd.c
M gdb/ui-out.c
M gdb/unittests/format_pieces-selftests.c
8 files changed, 106 insertions(+), 2 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 44ace04..4f40ea8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2019-11-12 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * gdbsupport/format.c (format_pieces::format_pieces): Support
+ printf 'z' size modifier.
+ * gdbsupport/format.h (enum argclass): Add size_t_arg.
+ * printcmd.c (ui_printf): Handle size_t_arg.
+ * ui-out.c (ui_out::vmessage): Likewise.
+ * unittests/format_pieces-selftests.c (test_format_int_sizes): New
+ function.
+ (run_tests): Call test_format_int_sizes.
+
2019-11-12 Christian Biesinger <cbiesinger@google.com>
* ada-exp.y (write_ambiguous_var): Update.
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 9d0afaa..e17a7ca 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,7 @@
+2019-11-12 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * ax.c (ax_printf): Handle size_t_arg.
+
2019-11-06 Christian Biesinger <cbiesinger@google.com>
* linux-tdep.c (linux_info_proc): Use strtok_r instead of strtok.
diff --git a/gdb/gdbserver/ax.c b/gdb/gdbserver/ax.c
index 2c096ef..21fde5e 100644
--- a/gdb/gdbserver/ax.c
+++ b/gdb/gdbserver/ax.c
@@ -904,6 +904,14 @@
break;
}
+ case size_t_arg:
+ {
+ size_t val = args[i];
+
+ printf (current_substring, val);
+ break;
+ }
+
case literal_piece:
/* Print a portion of the format string that has no
directives. Note that this will not include any
diff --git a/gdb/gdbsupport/format.c b/gdb/gdbsupport/format.c
index 1e80350..2e2d90a 100644
--- a/gdb/gdbsupport/format.c
+++ b/gdb/gdbsupport/format.c
@@ -123,6 +123,7 @@
int seen_space = 0, seen_plus = 0;
int seen_big_l = 0, seen_h = 0, seen_big_h = 0;
int seen_big_d = 0, seen_double_big_d = 0;
+ int seen_size_t = 0;
int bad = 0;
int n_int_args = 0;
@@ -234,6 +235,12 @@
else
seen_big_d = 1;
}
+ /* For size_t or ssize_t. */
+ else if (*f == 'z')
+ {
+ seen_size_t = 1;
+ f++;
+ }
switch (*f)
{
@@ -251,7 +258,9 @@
case 'd':
case 'i':
- if (lcount == 0)
+ if (seen_size_t)
+ this_argclass = size_t_arg;
+ else if (lcount == 0)
this_argclass = int_arg;
else if (lcount == 1)
this_argclass = long_arg;
diff --git a/gdb/gdbsupport/format.h b/gdb/gdbsupport/format.h
index e2a47ba..07374db 100644
--- a/gdb/gdbsupport/format.h
+++ b/gdb/gdbsupport/format.h
@@ -38,7 +38,7 @@
enum argclass
{
literal_piece,
- int_arg, long_arg, long_long_arg, ptr_arg,
+ int_arg, long_arg, long_long_arg, size_t_arg, ptr_arg,
string_arg, wide_string_arg, wide_char_arg,
double_arg, long_double_arg,
dec32float_arg, dec64float_arg, dec128float_arg
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 27aaf7a..323a5d0 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2663,6 +2663,16 @@
DIAGNOSTIC_POP
break;
}
+ case size_t_arg:
+ {
+ size_t val = value_as_long (val_args[i]);
+
+ DIAGNOSTIC_PUSH
+ DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
+ fprintf_filtered (stream, current_substring, val);
+ DIAGNOSTIC_POP
+ break;
+ }
/* Handles floating-point values. */
case double_arg:
case long_double_arg:
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 6b0b5ac..80845f4 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -688,6 +688,24 @@
}
}
break;
+ case size_t_arg:
+ {
+ size_t val = va_arg (args, size_t);
+ switch (piece.n_int_args)
+ {
+ case 0:
+ call_do_message (style, current_substring, val);
+ break;
+ case 1:
+ call_do_message (style, current_substring, intvals[0], val);
+ break;
+ case 2:
+ call_do_message (style, current_substring,
+ intvals[0], intvals[1], val);
+ break;
+ }
+ }
+ break;
case double_arg:
call_do_message (style, current_substring, va_arg (args, double));
break;
diff --git a/gdb/unittests/format_pieces-selftests.c b/gdb/unittests/format_pieces-selftests.c
index ed83d96..3971201 100644
--- a/gdb/unittests/format_pieces-selftests.c
+++ b/gdb/unittests/format_pieces-selftests.c
@@ -76,12 +76,56 @@
}, true);
}
+/* Test the different size modifiers that can be applied to an integer
+ argument. Test with different integer format specifiers too. */
+
+static void
+test_format_int_sizes ()
+{
+ check ("Hello\\t %hu%lu%llu%zu", /* ARI: %ll */
+ {
+ format_piece ("Hello\t ", literal_piece, 0),
+ format_piece ("%hu", int_arg, 0),
+ format_piece ("%lu", long_arg, 0),
+ format_piece ("%llu", long_long_arg, 0), /* ARI: %ll */
+ format_piece ("%zu", size_t_arg, 0)
+ });
+
+ check ("Hello\\t %hx%lx%llx%zx", /* ARI: %ll */
+ {
+ format_piece ("Hello\t ", literal_piece, 0),
+ format_piece ("%hx", int_arg, 0),
+ format_piece ("%lx", long_arg, 0),
+ format_piece ("%llx", long_long_arg, 0), /* ARI: %ll */
+ format_piece ("%zx", size_t_arg, 0)
+ });
+
+ check ("Hello\\t %ho%lo%llo%zo", /* ARI: %ll */
+ {
+ format_piece ("Hello\t ", literal_piece, 0),
+ format_piece ("%ho", int_arg, 0),
+ format_piece ("%lo", long_arg, 0),
+ format_piece ("%llo", long_long_arg, 0), /* ARI: %ll */
+ format_piece ("%zo", size_t_arg, 0)
+ });
+
+ check ("Hello\\t %hd%ld%lld%zd", /* ARI: %ll */
+ {
+ format_piece ("Hello\t ", literal_piece, 0),
+ format_piece ("%hd", int_arg, 0),
+ format_piece ("%ld", long_arg, 0),
+ format_piece ("%lld", long_long_arg, 0), /* ARI: %ll */
+ format_piece ("%zd", size_t_arg, 0)
+ });
+}
+
static void
run_tests ()
{
test_escape_sequences ();
test_format_specifier ();
test_gdb_formats ();
+ test_format_int_sizes ();
}
} /* namespace format_pieces */
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f
Gerrit-Change-Number: 511
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: merged
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [pushed] gdb: Support printf 'z' size modifier
2019-11-12 23:53 ` [pushed] " Sourceware to Gerrit sync (Code Review)
@ 2019-11-14 12:55 ` Eli Zaretskii
2019-11-14 16:49 ` Andrew Burgess
0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-11-14 12:55 UTC (permalink / raw)
To: noreply, simon.marchi, tromey, andrew.burgess, palves, brobecker,
kevinb, gdb-patches
> Date: Tue, 12 Nov 2019 18:53:15 -0500
> From: "Sourceware to Gerrit sync (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
> Cc: Kevin Buettner <kevinb@redhat.com>, Joel Brobecker <brobecker@adacore.com>, Pedro Alves <palves@redhat.com>, Simon Marchi <simon.marchi@polymtl.ca>, Tom Tromey <tromey@sourceware.org>
>
> gdb/ChangeLog:
>
> * gdbsupport/format.c (format_pieces::format_pieces): Support
> printf 'z' size modifier.
> * gdbsupport/format.h (enum argclass): Add size_t_arg.
> * printcmd.c (ui_printf): Handle size_t_arg.
> * ui-out.c (ui_out::vmessage): Likewise.
> * unittests/format_pieces-selftests.c (test_format_int_sizes): New
> function.
> (run_tests): Call test_format_int_sizes.
I believe this requires to use __USE_MINGW_ANSI_STDIO with the MinGW
builds, since %z is not universally supported by the Windows runtime.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [pushed] gdb: Support printf 'z' size modifier
2019-11-14 12:55 ` Eli Zaretskii
@ 2019-11-14 16:49 ` Andrew Burgess
2019-11-14 16:59 ` Eli Zaretskii
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Andrew Burgess @ 2019-11-14 16:49 UTC (permalink / raw)
To: Eli Zaretskii
Cc: noreply, simon.marchi, tromey, palves, brobecker, kevinb, gdb-patches
* Eli Zaretskii <eliz@gnu.org> [2019-11-14 14:54:50 +0200]:
> > Date: Tue, 12 Nov 2019 18:53:15 -0500
> > From: "Sourceware to Gerrit sync (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
> > Cc: Kevin Buettner <kevinb@redhat.com>, Joel Brobecker <brobecker@adacore.com>, Pedro Alves <palves@redhat.com>, Simon Marchi <simon.marchi@polymtl.ca>, Tom Tromey <tromey@sourceware.org>
> >
> > gdb/ChangeLog:
> >
> > * gdbsupport/format.c (format_pieces::format_pieces): Support
> > printf 'z' size modifier.
> > * gdbsupport/format.h (enum argclass): Add size_t_arg.
> > * printcmd.c (ui_printf): Handle size_t_arg.
> > * ui-out.c (ui_out::vmessage): Likewise.
> > * unittests/format_pieces-selftests.c (test_format_int_sizes): New
> > function.
> > (run_tests): Call test_format_int_sizes.
>
> I believe this requires to use __USE_MINGW_ANSI_STDIO with the MinGW
> builds, since %z is not universally supported by the Windows runtime.
I only stumbled onto this issue as I hit a use of %z (which wasn't
guarded with __USE_MINGW_ANSI_STDIO) and wanted it to work now that
these strings pass through GDB's formatting code.
I guess we're no worse off now than we were before, but obviously
that's not saying much if we were possibly broken before.
I guess there are a couple of solutions:
1. Remove all uses of %z from GDB, and back out the %z support, or
2. Have GDB translate %z into some other suitable format specifier
for targets where %z is not supported.
Below is a patch that tries to take the second approach.
Feedback / thoughts welcome.
Thanks,
Andrew
----
commit fb431811b59597b63c5bb9bcf7bf8559991c52e1
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date: Thu Nov 14 16:40:57 2019 +0000
gdb: Support for %z format on MinGW
Eli pointed out that the %z size specifier is not supported on all
versions of MinGW. This commit attempts to work around this by
translating %z into some other suitable format specifier. So
something like %zd will become either %d, %ld, or %lld depending on
whether the sizeof (size_t) matches the sizeof (int), sizeof (long),
or sizeof (long long).
For the long long case we might also translate to %I64d if
USE_PRINTF_I64 is true.
I don't have access to MinGW so this code is mostly untested - I did
remove the '#if defined __MINGW32__ ....' check and try using %z in
some formatted prints, this all seemed to work fine - on my machine
sizeof (size_t) == sizeof (long).
gdb/ChangeLog:
* gdbsupport/format.c (format_pieces::format_pieces): Translate %z
into some other suitable format size specifier if it is not
supported.
Change-Id: I20f5e4cb1a7ab88f00f5e42d3fd8ca4aef00993d
diff --git a/gdb/gdbsupport/format.c b/gdb/gdbsupport/format.c
index 2e2d90a9246..e316e840e22 100644
--- a/gdb/gdbsupport/format.c
+++ b/gdb/gdbsupport/format.c
@@ -375,6 +375,40 @@ format_pieces::format_pieces (const char **arg, bool gdb_extensions)
strcpy (current_substring + length_before_ls, "s");
current_substring += length_before_ls + 2;
}
+#if defined __MINGW32__ && !defined __USE_MINGW_ANSI_STDIO
+ else if (this_argclass == size_t_arg)
+ {
+ /* Some versions of MinGW don't support the %z size format, so
+ lets change to use some appropriate alternative. */
+ *current_substring++ = '%';
+ if (sizeof (size_t) == sizeof (int))
+ {
+ *current_substring++ = *(percent_loc + 2);
+ this_argclass = int_arg;
+ }
+ else if (sizeof (size_t) == sizeof (long))
+ {
+ *current_substring++ = 'l';
+ *current_substring++ = *(percent_loc + 2);
+ this_argclass = long_arg;
+ }
+ else if (sizeof (size_t) == sizeof (long long))
+ {
+ if (USE_PRINTF_I64)
+ {
+ strcpy (current_substring, "I64");
+ current_substring += 3;
+ }
+ else
+ {
+ strcpy (current_substring, "ll");
+ current_substring += 2;
+ }
+ *current_substring++ = *(percent_loc + 2);
+ this_argclass = long_long_arg;
+ }
+ }
+#endif /* defined __MINGW32__ && !defined __USE_MINGW_ANSI_STDIO */
else
{
strncpy (current_substring, percent_loc, f - percent_loc);
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [pushed] gdb: Support printf 'z' size modifier
2019-11-14 16:49 ` Andrew Burgess
@ 2019-11-14 16:59 ` Eli Zaretskii
2019-11-14 17:09 ` Simon Marchi
2019-11-14 17:06 ` Pedro Alves
2019-11-14 21:27 ` Tom Tromey
2 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-11-14 16:59 UTC (permalink / raw)
To: Andrew Burgess
Cc: noreply, simon.marchi, tromey, palves, brobecker, kevinb, gdb-patches
> Date: Thu, 14 Nov 2019 16:49:45 +0000
> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: noreply@gnutoolchain-gerrit.osci.io, simon.marchi@polymtl.ca,
> tromey@sourceware.org, palves@redhat.com, brobecker@adacore.com,
> kevinb@redhat.com, gdb-patches@sourceware.org
>
> I guess there are a couple of solutions:
>
> 1. Remove all uses of %z from GDB, and back out the %z support, or
>
> 2. Have GDB translate %z into some other suitable format specifier
> for targets where %z is not supported.
>
> Below is a patch that tries to take the second approach.
Hmm... isn't it better to simply add -D__USE_MINGW_ANSI_STDIO=1 to the
cpp flags when building with MinGW? I don't think there are versions
of MinGW that don't support that macro.
The effect of that macro is to link against a MinGW library that
provides replacement implementations for *printf functions, and the
replacements do support %z.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [pushed] gdb: Support printf 'z' size modifier
2019-11-14 16:59 ` Eli Zaretskii
@ 2019-11-14 17:09 ` Simon Marchi
2019-11-14 18:27 ` Eli Zaretskii
0 siblings, 1 reply; 19+ messages in thread
From: Simon Marchi @ 2019-11-14 17:09 UTC (permalink / raw)
To: Eli Zaretskii, Andrew Burgess
Cc: tromey, palves, brobecker, kevinb, gdb-patches
On 2019-11-14 11:59 a.m., Eli Zaretskii wrote:
>> Date: Thu, 14 Nov 2019 16:49:45 +0000
>> From: Andrew Burgess <andrew.burgess@embecosm.com>
>> Cc: noreply@gnutoolchain-gerrit.osci.io, simon.marchi@polymtl.ca,
>> tromey@sourceware.org, palves@redhat.com, brobecker@adacore.com,
>> kevinb@redhat.com, gdb-patches@sourceware.org
>>
>> I guess there are a couple of solutions:
>>
>> 1. Remove all uses of %z from GDB, and back out the %z support, or
>>
>> 2. Have GDB translate %z into some other suitable format specifier
>> for targets where %z is not supported.
>>
>> Below is a patch that tries to take the second approach.
>
> Hmm... isn't it better to simply add -D__USE_MINGW_ANSI_STDIO=1 to the
> cpp flags when building with MinGW? I don't think there are versions
> of MinGW that don't support that macro.
>
> The effect of that macro is to link against a MinGW library that
> provides replacement implementations for *printf functions, and the
> replacements do support %z.
>
Apaprently, the __USE_MINGW_ANSI_STDIO macro has been marked as deprecated (IIUC, to
incite people not to define it directly), and I found this thread [1] on the mingw-users
mailing list where you asked the maintainer to reconsider this decision.
What is the status on that? If defining __USE_MINGW_ANSI_STDIO is not the right way
of enabling this feature, then what is? What is the status of this?
Simon
[1] https://osdn.net/projects/mingw/lists/archive/users/2019-January/thread.html#199
Subject "Deprecation of __USE_MINGW_ANSI_STDIO"
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [pushed] gdb: Support printf 'z' size modifier
2019-11-14 17:09 ` Simon Marchi
@ 2019-11-14 18:27 ` Eli Zaretskii
2019-11-14 20:37 ` Pedro Alves
0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-11-14 18:27 UTC (permalink / raw)
To: Simon Marchi
Cc: andrew.burgess, tromey, palves, brobecker, kevinb, gdb-patches
> Cc: tromey@sourceware.org, palves@redhat.com, brobecker@adacore.com,
> kevinb@redhat.com, gdb-patches@sourceware.org
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Thu, 14 Nov 2019 12:09:20 -0500
>
> Apaprently, the __USE_MINGW_ANSI_STDIO macro has been marked as deprecated (IIUC, to
> incite people not to define it directly), and I found this thread [1] on the mingw-users
> mailing list where you asked the maintainer to reconsider this decision.
>
> What is the status on that? If defining __USE_MINGW_ANSI_STDIO is not the right way
> of enabling this feature, then what is? What is the status of this?
The "new" way is to use the __MINGW_FEATURES__ macro. But if you want
to do this in a way that will work with noth mingw.org's MinGW and
MinGW64, the __USE_MINGW_ANSI_STDIO is the way, and it is still
supported.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [pushed] gdb: Support printf 'z' size modifier
2019-11-14 18:27 ` Eli Zaretskii
@ 2019-11-14 20:37 ` Pedro Alves
2019-11-15 7:37 ` Eli Zaretskii
0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2019-11-14 20:37 UTC (permalink / raw)
To: Eli Zaretskii, Simon Marchi
Cc: andrew.burgess, tromey, brobecker, kevinb, gdb-patches
On 11/14/19 6:27 PM, Eli Zaretskii wrote:
>> Cc: tromey@sourceware.org, palves@redhat.com, brobecker@adacore.com,
>> kevinb@redhat.com, gdb-patches@sourceware.org
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>> Date: Thu, 14 Nov 2019 12:09:20 -0500
>>
>> Apaprently, the __USE_MINGW_ANSI_STDIO macro has been marked as deprecated (IIUC, to
>> incite people not to define it directly), and I found this thread [1] on the mingw-users
>> mailing list where you asked the maintainer to reconsider this decision.
>>
>> What is the status on that? If defining __USE_MINGW_ANSI_STDIO is not the right way
>> of enabling this feature, then what is? What is the status of this?
>
> The "new" way is to use the __MINGW_FEATURES__ macro. But if you want
> to do this in a way that will work with noth mingw.org's MinGW and
> MinGW64, the __USE_MINGW_ANSI_STDIO is the way, and it is still
> supported.
>
I think that the ideal place to do that would be in src/config/mh-mingw,
so that all projects in the tree, plus gcc, enabled it.
It's where we do -D__USE_MINGW_ACCESS.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [pushed] gdb: Support printf 'z' size modifier
2019-11-14 20:37 ` Pedro Alves
@ 2019-11-15 7:37 ` Eli Zaretskii
0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2019-11-15 7:37 UTC (permalink / raw)
To: Pedro Alves
Cc: simon.marchi, andrew.burgess, tromey, brobecker, kevinb, gdb-patches
> Cc: andrew.burgess@embecosm.com, tromey@sourceware.org,
> brobecker@adacore.com, kevinb@redhat.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 14 Nov 2019 20:37:15 +0000
>
> > The "new" way is to use the __MINGW_FEATURES__ macro. But if you want
> > to do this in a way that will work with noth mingw.org's MinGW and
> > MinGW64, the __USE_MINGW_ANSI_STDIO is the way, and it is still
> > supported.
> >
>
> I think that the ideal place to do that would be in src/config/mh-mingw,
> so that all projects in the tree, plus gcc, enabled it.
> It's where we do -D__USE_MINGW_ACCESS.
That'd be fine, thanks.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [pushed] gdb: Support printf 'z' size modifier
2019-11-14 16:49 ` Andrew Burgess
2019-11-14 16:59 ` Eli Zaretskii
@ 2019-11-14 17:06 ` Pedro Alves
2019-11-14 18:18 ` Eli Zaretskii
2019-11-14 21:27 ` Tom Tromey
2 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2019-11-14 17:06 UTC (permalink / raw)
To: Andrew Burgess, Eli Zaretskii
Cc: noreply, simon.marchi, tromey, brobecker, kevinb, gdb-patches
On 11/14/19 4:49 PM, Andrew Burgess wrote:
> * Eli Zaretskii <eliz@gnu.org> [2019-11-14 14:54:50 +0200]:
>
>> I believe this requires to use __USE_MINGW_ANSI_STDIO with the MinGW
>> builds, since %z is not universally supported by the Windows runtime.
>
> I only stumbled onto this issue as I hit a use of %z (which wasn't
> guarded with __USE_MINGW_ANSI_STDIO) and wanted it to work now that
> these strings pass through GDB's formatting code.
Isn't gnulib taking care of this, by either enforcing __USE_MINGW_ANSI_STDIO,
or by replacing printf with it's own implementation?
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [pushed] gdb: Support printf 'z' size modifier
2019-11-14 17:06 ` Pedro Alves
@ 2019-11-14 18:18 ` Eli Zaretskii
0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2019-11-14 18:18 UTC (permalink / raw)
To: Pedro Alves
Cc: andrew.burgess, noreply, simon.marchi, tromey, brobecker, kevinb,
gdb-patches
> Cc: noreply@gnutoolchain-gerrit.osci.io, simon.marchi@polymtl.ca,
> tromey@sourceware.org, brobecker@adacore.com, kevinb@redhat.com,
> gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 14 Nov 2019 17:06:20 +0000
>
> Isn't gnulib taking care of this, by either enforcing __USE_MINGW_ANSI_STDIO,
> or by replacing printf with it's own implementation?
Only if you import the printf module. Which I'm not sure we want,
because it's a MinGW only issue, and MinGW already has a solution.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [pushed] gdb: Support printf 'z' size modifier
2019-11-14 16:49 ` Andrew Burgess
2019-11-14 16:59 ` Eli Zaretskii
2019-11-14 17:06 ` Pedro Alves
@ 2019-11-14 21:27 ` Tom Tromey
2 siblings, 0 replies; 19+ messages in thread
From: Tom Tromey @ 2019-11-14 21:27 UTC (permalink / raw)
To: Andrew Burgess
Cc: Eli Zaretskii, simon.marchi, tromey, palves, brobecker, kevinb,
gdb-patches
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
Andrew> For the long long case we might also translate to %I64d if
Andrew> USE_PRINTF_I64 is true.
FWIW, today I debugged a crash on an internal test case that turned out
to happen because format_pieces does not recognize %I64d, but format.c
also already changes %lld to %I64d.
So, we need to handle something here. I am leaning toward just teaching
format.c about %I64d as well.
Tom
^ permalink raw reply [flat|nested] 19+ messages in thread
* [pushed] gdb: Support printf 'z' size modifier
2019-11-05 15:46 [review] gdb: Support printf 'z' size modifier Andrew Burgess (Code Review)
` (6 preceding siblings ...)
2019-11-12 23:53 ` [pushed] " Sourceware to Gerrit sync (Code Review)
@ 2019-11-12 23:53 ` Sourceware to Gerrit sync (Code Review)
7 siblings, 0 replies; 19+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-11-12 23:53 UTC (permalink / raw)
To: Andrew Burgess, Kevin Buettner, gdb-patches
Cc: Pedro Alves, Joel Brobecker, Tom Tromey, Simon Marchi
The original change was created by Andrew Burgess.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511
......................................................................
gdb: Support printf 'z' size modifier
The gdb format mechanism doesn't currently support the 'z' size
modifier, there are a few places in GDB where this is used. Instead
of removing these uses lets just add support to GDB for using 'z'.
I found this issue when trying to use some of the debug output.
Before this commit:
(gdb) set debug dwarf-line 9
(gdb) file test
Reading symbols from test...
Unrecognized format specifier 'z' in printf
(No debugging symbols found in test)
(gdb)
After this commit:
(gdb) set debug dwarf-line 9
(gdb) file test
Reading symbols from test...
Adding dir 1: /usr/include
Adding file 1: test.c
Adding file 2: stdc-predef.h
Processing actual line 3: file 1, address 0x4004a0, is_stmt 1, discrim 0
Processing actual line 4: file 1, address 0x4004a0, is_stmt 1, discrim 0
.... lots of debug output ...
Processing actual line 10: file 1, address 0x4003b7, is_stmt 0, discrim 0
(gdb)
I've added a self test to cover the integer format size modifiers,
including the 'z' modifier.
gdb/ChangeLog:
* gdbsupport/format.c (format_pieces::format_pieces): Support
printf 'z' size modifier.
* gdbsupport/format.h (enum argclass): Add size_t_arg.
* printcmd.c (ui_printf): Handle size_t_arg.
* ui-out.c (ui_out::vmessage): Likewise.
* unittests/format_pieces-selftests.c (test_format_int_sizes): New
function.
(run_tests): Call test_format_int_sizes.
gdb/gdbserver/ChangeLog:
* ax.c (ax_printf): Handle size_t_arg.
Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f
---
M gdb/ChangeLog
M gdb/gdbserver/ChangeLog
M gdb/gdbserver/ax.c
M gdb/gdbsupport/format.c
M gdb/gdbsupport/format.h
M gdb/printcmd.c
M gdb/ui-out.c
M gdb/unittests/format_pieces-selftests.c
8 files changed, 106 insertions(+), 2 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 44ace04..4f40ea8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2019-11-12 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * gdbsupport/format.c (format_pieces::format_pieces): Support
+ printf 'z' size modifier.
+ * gdbsupport/format.h (enum argclass): Add size_t_arg.
+ * printcmd.c (ui_printf): Handle size_t_arg.
+ * ui-out.c (ui_out::vmessage): Likewise.
+ * unittests/format_pieces-selftests.c (test_format_int_sizes): New
+ function.
+ (run_tests): Call test_format_int_sizes.
+
2019-11-12 Christian Biesinger <cbiesinger@google.com>
* ada-exp.y (write_ambiguous_var): Update.
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 9d0afaa..e17a7ca 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,7 @@
+2019-11-12 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * ax.c (ax_printf): Handle size_t_arg.
+
2019-11-06 Christian Biesinger <cbiesinger@google.com>
* linux-tdep.c (linux_info_proc): Use strtok_r instead of strtok.
diff --git a/gdb/gdbserver/ax.c b/gdb/gdbserver/ax.c
index 2c096ef..21fde5e 100644
--- a/gdb/gdbserver/ax.c
+++ b/gdb/gdbserver/ax.c
@@ -904,6 +904,14 @@
break;
}
+ case size_t_arg:
+ {
+ size_t val = args[i];
+
+ printf (current_substring, val);
+ break;
+ }
+
case literal_piece:
/* Print a portion of the format string that has no
directives. Note that this will not include any
diff --git a/gdb/gdbsupport/format.c b/gdb/gdbsupport/format.c
index 1e80350..2e2d90a 100644
--- a/gdb/gdbsupport/format.c
+++ b/gdb/gdbsupport/format.c
@@ -123,6 +123,7 @@
int seen_space = 0, seen_plus = 0;
int seen_big_l = 0, seen_h = 0, seen_big_h = 0;
int seen_big_d = 0, seen_double_big_d = 0;
+ int seen_size_t = 0;
int bad = 0;
int n_int_args = 0;
@@ -234,6 +235,12 @@
else
seen_big_d = 1;
}
+ /* For size_t or ssize_t. */
+ else if (*f == 'z')
+ {
+ seen_size_t = 1;
+ f++;
+ }
switch (*f)
{
@@ -251,7 +258,9 @@
case 'd':
case 'i':
- if (lcount == 0)
+ if (seen_size_t)
+ this_argclass = size_t_arg;
+ else if (lcount == 0)
this_argclass = int_arg;
else if (lcount == 1)
this_argclass = long_arg;
diff --git a/gdb/gdbsupport/format.h b/gdb/gdbsupport/format.h
index e2a47ba..07374db 100644
--- a/gdb/gdbsupport/format.h
+++ b/gdb/gdbsupport/format.h
@@ -38,7 +38,7 @@
enum argclass
{
literal_piece,
- int_arg, long_arg, long_long_arg, ptr_arg,
+ int_arg, long_arg, long_long_arg, size_t_arg, ptr_arg,
string_arg, wide_string_arg, wide_char_arg,
double_arg, long_double_arg,
dec32float_arg, dec64float_arg, dec128float_arg
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 27aaf7a..323a5d0 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2663,6 +2663,16 @@
DIAGNOSTIC_POP
break;
}
+ case size_t_arg:
+ {
+ size_t val = value_as_long (val_args[i]);
+
+ DIAGNOSTIC_PUSH
+ DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
+ fprintf_filtered (stream, current_substring, val);
+ DIAGNOSTIC_POP
+ break;
+ }
/* Handles floating-point values. */
case double_arg:
case long_double_arg:
diff --git a/gdb/ui-out.c b/gdb/ui-out.c
index 6b0b5ac..80845f4 100644
--- a/gdb/ui-out.c
+++ b/gdb/ui-out.c
@@ -688,6 +688,24 @@
}
}
break;
+ case size_t_arg:
+ {
+ size_t val = va_arg (args, size_t);
+ switch (piece.n_int_args)
+ {
+ case 0:
+ call_do_message (style, current_substring, val);
+ break;
+ case 1:
+ call_do_message (style, current_substring, intvals[0], val);
+ break;
+ case 2:
+ call_do_message (style, current_substring,
+ intvals[0], intvals[1], val);
+ break;
+ }
+ }
+ break;
case double_arg:
call_do_message (style, current_substring, va_arg (args, double));
break;
diff --git a/gdb/unittests/format_pieces-selftests.c b/gdb/unittests/format_pieces-selftests.c
index ed83d96..3971201 100644
--- a/gdb/unittests/format_pieces-selftests.c
+++ b/gdb/unittests/format_pieces-selftests.c
@@ -76,12 +76,56 @@
}, true);
}
+/* Test the different size modifiers that can be applied to an integer
+ argument. Test with different integer format specifiers too. */
+
+static void
+test_format_int_sizes ()
+{
+ check ("Hello\\t %hu%lu%llu%zu", /* ARI: %ll */
+ {
+ format_piece ("Hello\t ", literal_piece, 0),
+ format_piece ("%hu", int_arg, 0),
+ format_piece ("%lu", long_arg, 0),
+ format_piece ("%llu", long_long_arg, 0), /* ARI: %ll */
+ format_piece ("%zu", size_t_arg, 0)
+ });
+
+ check ("Hello\\t %hx%lx%llx%zx", /* ARI: %ll */
+ {
+ format_piece ("Hello\t ", literal_piece, 0),
+ format_piece ("%hx", int_arg, 0),
+ format_piece ("%lx", long_arg, 0),
+ format_piece ("%llx", long_long_arg, 0), /* ARI: %ll */
+ format_piece ("%zx", size_t_arg, 0)
+ });
+
+ check ("Hello\\t %ho%lo%llo%zo", /* ARI: %ll */
+ {
+ format_piece ("Hello\t ", literal_piece, 0),
+ format_piece ("%ho", int_arg, 0),
+ format_piece ("%lo", long_arg, 0),
+ format_piece ("%llo", long_long_arg, 0), /* ARI: %ll */
+ format_piece ("%zo", size_t_arg, 0)
+ });
+
+ check ("Hello\\t %hd%ld%lld%zd", /* ARI: %ll */
+ {
+ format_piece ("Hello\t ", literal_piece, 0),
+ format_piece ("%hd", int_arg, 0),
+ format_piece ("%ld", long_arg, 0),
+ format_piece ("%lld", long_long_arg, 0), /* ARI: %ll */
+ format_piece ("%zd", size_t_arg, 0)
+ });
+}
+
static void
run_tests ()
{
test_escape_sequences ();
test_format_specifier ();
test_gdb_formats ();
+ test_format_int_sizes ();
}
} /* namespace format_pieces */
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f
Gerrit-Change-Number: 511
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Kevin Buettner <kevinb@redhat.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: newpatchset
^ permalink raw reply [flat|nested] 19+ messages in thread