Message ID | gerrit.1572968811000.Ib6c44d88aa5bce265d757e4c0698881803dd186f@gnutoolchain-gerrit.osci.io |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
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
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
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
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
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
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 41daef6..d29fffe 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2019-11-05 Andrew Burgess <andrew.burgess@embecosm.com> + + * gdbsupport/format.c (format_pieces::format_pieces): Support + printf 'z' size modifier. + 2019-11-04 Christian Biesinger <cbiesinger@google.com> * psympriv.h: Add static_asserts for sizeof (general_symbol_info) diff --git a/gdb/gdbsupport/format.c b/gdb/gdbsupport/format.c index 1e80350..fea73c8 100644 --- a/gdb/gdbsupport/format.c +++ b/gdb/gdbsupport/format.c @@ -234,6 +234,9 @@ else seen_big_d = 1; } + /* For size_t or ssize_t. */ + else if (*f == 'z') + f++; switch (*f) {