[review v2] gdb: Support printf 'z' size modifier

["Andrew Burgess (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>]

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