Message ID | gerrit.1572968811000.Ib6c44d88aa5bce265d757e4c0698881803dd186f@gnutoolchain-gerrit.osci.io |
---|---|
State | New, archived |
Headers |
Received: (qmail 106536 invoked by alias); 5 Nov 2019 15:46:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 106523 invoked by uid 89); 5 Nov 2019 15:46:58 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3 autolearn=ham version=3.3.1 spammy=H*R:D*sourceware.org X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 05 Nov 2019 15:46:56 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id 5A72F20456; Tue, 5 Nov 2019 10:46:55 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id ADCF420266 for <gdb-patches@sourceware.org>; Tue, 5 Nov 2019 10:46:51 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id 9266925B28 for <gdb-patches@sourceware.org>; Tue, 5 Nov 2019 10:46:51 -0500 (EST) X-Gerrit-PatchSet: 1 Date: Tue, 5 Nov 2019 10:46:51 -0500 From: "Andrew Burgess (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io> To: gdb-patches@sourceware.org Message-ID: <gerrit.1572968811000.Ib6c44d88aa5bce265d757e4c0698881803dd186f@gnutoolchain-gerrit.osci.io> Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange Subject: [review] gdb: Support printf 'z' size modifier X-Gerrit-Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f X-Gerrit-Change-Number: 511 X-Gerrit-ChangeURL: <https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511> X-Gerrit-Commit: 4f23383e8e03e2dda242ae719402884bea64e28e References: <gerrit.1572968811000.Ib6c44d88aa5bce265d757e4c0698881803dd186f@gnutoolchain-gerrit.osci.io> Reply-To: andrew.burgess@embecosm.com, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Disposition: inline User-Agent: Gerrit/3.0.3-75-g9005159e5d Content-Type: text/plain; charset=UTF-8 |
Commit Message
Simon Marchi (Code Review)
Nov. 5, 2019, 3:46 p.m. UTC
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've not added a test for this as I ran into the issue when trying to use some 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 Processing actual line 5: file 1, address 0x4004a0, is_stmt 0, discrim 0 Processing actual line 5: file 1, address 0x4004a6, is_stmt 0, discrim 0 Processing actual line 15: file 1, address 0x4003b0, is_stmt 1, discrim 0 Processing actual line 16: file 1, address 0x4003b0, is_stmt 1, discrim 0 Processing actual line 10: file 1, address 0x4003b0, is_stmt 1, discrim 0 Processing actual line 10: file 1, address 0x4003b0, is_stmt 0, discrim 0 Processing actual line 10: file 1, address 0x4003b7, is_stmt 0, discrim 0 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 Recording line 3, file test.c, address 0x4004a0 Processing actual line 4: file 1, address 0x4004a0, is_stmt 1, discrim 0 Recording line 4, file test.c, address 0x4004a0 Processing actual line 5: file 1, address 0x4004a0, is_stmt 0, discrim 0 Processing actual line 5: file 1, address 0x4004a6, is_stmt 0, discrim 0 Processing actual line 15: file 1, address 0x4003b0, is_stmt 1, discrim 0 Recording line 15, file test.c, address 0x4003b0 Processing actual line 16: file 1, address 0x4003b0, is_stmt 1, discrim 0 Recording line 16, file test.c, address 0x4003b0 Processing actual line 10: file 1, address 0x4003b0, is_stmt 1, discrim 0 Recording line 10, file test.c, address 0x4003b0 Processing actual line 10: file 1, address 0x4003b0, is_stmt 0, discrim 0 Processing actual line 10: file 1, address 0x4003b7, is_stmt 0, discrim 0 (gdb) gdb/ChangeLog: * gdbsupport/format.c (format_pieces::format_pieces): Support printf 'z' size modifier. Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f --- M gdb/ChangeLog M gdb/gdbsupport/format.c 2 files changed, 8 insertions(+), 0 deletions(-)
Comments
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.
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.
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.
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.
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
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) {