* [PATCH] Fix gdb/coffread.c build on 32bit architectures [not found] <3b0896a6-04d4-4c7c-ac32-9ae78acdb66c@redhat.com/> @ 2023-08-25 21:16 ` Mark Wielaard 2023-08-25 21:19 ` Keith Seitz 2023-08-28 14:08 ` Tom Tromey 0 siblings, 2 replies; 9+ messages in thread From: Mark Wielaard @ 2023-08-25 21:16 UTC (permalink / raw) To: gdb-patches; +Cc: Keith Seitz, Mark Wielaard The getsymname function tries to emit an error using %ld for an uintptr_t argument. Use PRIxPTR instead. Which works on any architecture for uintptr_t. --- gdb/coffread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdb/coffread.c b/gdb/coffread.c index ae7632d49cb..c609c963453 100644 --- a/gdb/coffread.c +++ b/gdb/coffread.c @@ -1325,7 +1325,7 @@ getsymname (struct internal_syment *symbol_entry) if (symbol_entry->_n._n_n._n_zeroes == 0) { if (symbol_entry->_n._n_n._n_offset > stringtab_length) - error (_("COFF Error: string table offset (%ld) outside string table (length %ld)"), + error (_("COFF Error: string table offset (%" PRIxPTR ") outside string table (length %ld)"), symbol_entry->_n._n_n._n_offset, stringtab_length); result = stringtab + symbol_entry->_n._n_n._n_offset; } -- 2.39.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix gdb/coffread.c build on 32bit architectures 2023-08-25 21:16 ` [PATCH] Fix gdb/coffread.c build on 32bit architectures Mark Wielaard @ 2023-08-25 21:19 ` Keith Seitz 2023-08-28 14:08 ` Tom Tromey 1 sibling, 0 replies; 9+ messages in thread From: Keith Seitz @ 2023-08-25 21:19 UTC (permalink / raw) To: Mark Wielaard, gdb-patches On 8/25/23 14:16, Mark Wielaard wrote: > The getsymname function tries to emit an error using %ld for an > uintptr_t argument. Use PRIxPTR instead. Which works on any architecture > for uintptr_t. Doh! Thank you for catching that! I should think this qualifies under the "obvious" rule... Keith ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix gdb/coffread.c build on 32bit architectures 2023-08-25 21:16 ` [PATCH] Fix gdb/coffread.c build on 32bit architectures Mark Wielaard 2023-08-25 21:19 ` Keith Seitz @ 2023-08-28 14:08 ` Tom Tromey 2023-08-28 14:20 ` Keith Seitz 2023-08-28 14:32 ` Mark Wielaard 1 sibling, 2 replies; 9+ messages in thread From: Tom Tromey @ 2023-08-28 14:08 UTC (permalink / raw) To: Mark Wielaard; +Cc: gdb-patches, Keith Seitz >>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes: Hi Mark. Thanks for the patch. Mark> + error (_("COFF Error: string table offset (%" PRIxPTR ") outside string table (length %ld)"), gdb doesn't use these PRI macros. Instead you'd write something like: error ("...%s...", hex_string (value)) Tom ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix gdb/coffread.c build on 32bit architectures 2023-08-28 14:08 ` Tom Tromey @ 2023-08-28 14:20 ` Keith Seitz 2023-08-28 14:27 ` Keith Seitz 2023-08-28 14:32 ` Mark Wielaard 1 sibling, 1 reply; 9+ messages in thread From: Keith Seitz @ 2023-08-28 14:20 UTC (permalink / raw) To: Tom Tromey, Mark Wielaard; +Cc: gdb-patches On 8/28/23 07:08, Tom Tromey wrote: >>>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes: > > Hi Mark. Thanks for the patch. > > Mark> + error (_("COFF Error: string table offset (%" PRIxPTR ") outside string table (length %ld)"), > > gdb doesn't use these PRI macros. Instead you'd write something like: > > error ("...%s...", hex_string (value)) Yeah, I remembered this over the weekend. I also think this isn't exactly the nicest construct for i18n. May I assume that converting to hex_string for printing these offsets is obvious or pre-approved? Keith ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix gdb/coffread.c build on 32bit architectures 2023-08-28 14:20 ` Keith Seitz @ 2023-08-28 14:27 ` Keith Seitz 0 siblings, 0 replies; 9+ messages in thread From: Keith Seitz @ 2023-08-28 14:27 UTC (permalink / raw) To: Tom Tromey, Mark Wielaard; +Cc: gdb-patches On 8/28/23 07:20, Keith Seitz via Gdb-patches wrote: > > May I assume that converting to hex_string for printing these offsets is obvious > or pre-approved? Or to be more explicit (pardon me -- ECAFFEINE): Use hex_string to output hexidecimal values Late last week, I committed a patch that used %ld to output the value of some offsets in coffread.c. This caused a build failure on 32-bit hardware, and Mark Wielaard offered up a quick fix by using PRIxPTR. As mentioned by Tom Tromey, the correct way to handle this is by using a utility function such as hex_string. This patch converts the two %ld formats into %s w/hex_string instead. --- gdb/coffread.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gdb/coffread.c b/gdb/coffread.c index c609c963453..3caa7eb3175 100644 --- a/gdb/coffread.c +++ b/gdb/coffread.c @@ -1325,8 +1325,8 @@ getsymname (struct internal_syment *symbol_entry) if (symbol_entry->_n._n_n._n_zeroes == 0) { if (symbol_entry->_n._n_n._n_offset > stringtab_length) - error (_("COFF Error: string table offset (%" PRIxPTR ") outside string table (length %ld)"), - symbol_entry->_n._n_n._n_offset, stringtab_length); + error (_("COFF Error: string table offset (%s) outside string table (length %s)"), + hex_string (symbol_entry->_n._n_n._n_offset), hex_string (stringtab_length)); result = stringtab + symbol_entry->_n._n_n._n_offset; } else -- 2.41.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix gdb/coffread.c build on 32bit architectures 2023-08-28 14:08 ` Tom Tromey 2023-08-28 14:20 ` Keith Seitz @ 2023-08-28 14:32 ` Mark Wielaard 2023-08-28 14:41 ` Andreas Schwab ` (2 more replies) 1 sibling, 3 replies; 9+ messages in thread From: Mark Wielaard @ 2023-08-28 14:32 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches, Keith Seitz [-- Attachment #1: Type: text/plain, Size: 730 bytes --] On Mon, 2023-08-28 at 08:08 -0600, Tom Tromey wrote: > > > > > > "Mark" == Mark Wielaard <mark@klomp.org> writes: > > Hi Mark. Thanks for the patch. > > Mark> + error (_("COFF Error: string table offset (%" PRIxPTR ") outside string table (length %ld)"), > > gdb doesn't use these PRI macros. Not always, but there are various uses in the code base. e.g. gdb/dwarf2/cooked-index.c uses gdb_printf (" DIE offset: 0x%" PRIx64 "\n", ... As do gdb/bt-utils.c, gdb/btrace.c, gdb/netbsd-nat.c. And bfd, opcodes, sim, etc. do use it. > Instead you'd write something like: > > error ("...%s...", hex_string (value)) OK, the attached seems to work on both 32 and 64 bit systems. Cheers, Mark [-- Attachment #2: 0001-Use-hex_string-in-gdb-coffread.c-instead-of-PRIxPTR.patch --] [-- Type: text/x-patch, Size: 1123 bytes --] From 3aaddda2c6e3679579c455e7a71981ffe1724206 Mon Sep 17 00:00:00 2001 From: Mark Wielaard <mark@klomp.org> Date: Mon, 28 Aug 2023 16:30:14 +0200 Subject: [PATCH] Use hex_string in gdb/coffread.c instead of PRIxPTR The getsymname function uses PRIxPTR to print and uintptr_t value in an error message. Use hex_string instead. --- gdb/coffread.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gdb/coffread.c b/gdb/coffread.c index c609c963453..c2fe9fa1761 100644 --- a/gdb/coffread.c +++ b/gdb/coffread.c @@ -1325,8 +1325,8 @@ getsymname (struct internal_syment *symbol_entry) if (symbol_entry->_n._n_n._n_zeroes == 0) { if (symbol_entry->_n._n_n._n_offset > stringtab_length) - error (_("COFF Error: string table offset (%" PRIxPTR ") outside string table (length %ld)"), - symbol_entry->_n._n_n._n_offset, stringtab_length); + error (_("COFF Error: string table offset (%s) outside string table (length %ld)"), + hex_string (symbol_entry->_n._n_n._n_offset), stringtab_length); result = stringtab + symbol_entry->_n._n_n._n_offset; } else -- 2.41.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix gdb/coffread.c build on 32bit architectures 2023-08-28 14:32 ` Mark Wielaard @ 2023-08-28 14:41 ` Andreas Schwab 2023-08-28 16:31 ` Tom Tromey 2023-08-28 16:33 ` Tom Tromey 2 siblings, 0 replies; 9+ messages in thread From: Andreas Schwab @ 2023-08-28 14:41 UTC (permalink / raw) To: Mark Wielaard; +Cc: Tom Tromey, gdb-patches On Aug 28 2023, Mark Wielaard wrote: > e.g. gdb/dwarf2/cooked-index.c uses > gdb_printf (" DIE offset: 0x%" PRIx64 "\n", ... There's actually sect_offset_str for this. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix gdb/coffread.c build on 32bit architectures 2023-08-28 14:32 ` Mark Wielaard 2023-08-28 14:41 ` Andreas Schwab @ 2023-08-28 16:31 ` Tom Tromey 2023-08-28 16:33 ` Tom Tromey 2 siblings, 0 replies; 9+ messages in thread From: Tom Tromey @ 2023-08-28 16:31 UTC (permalink / raw) To: Mark Wielaard; +Cc: Tom Tromey, gdb-patches >>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes: Mark> OK, the attached seems to work on both 32 and 64 bit systems. Thanks. Approved-By: Tom Tromey <tom@tromey.com> Tom ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix gdb/coffread.c build on 32bit architectures 2023-08-28 14:32 ` Mark Wielaard 2023-08-28 14:41 ` Andreas Schwab 2023-08-28 16:31 ` Tom Tromey @ 2023-08-28 16:33 ` Tom Tromey 2 siblings, 0 replies; 9+ messages in thread From: Tom Tromey @ 2023-08-28 16:33 UTC (permalink / raw) To: Mark Wielaard; +Cc: Tom Tromey, gdb-patches >>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes: Mark> + error (_("COFF Error: string table offset (%" PRIxPTR ") outside string table (length %ld)"), >> gdb doesn't use these PRI macros. Mark> Not always, but there are various uses in the code base. Yeah. In gdb it's typical for things to creep in, but the existence of something doesn't always make it ok. Tom ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-08-28 16:33 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <3b0896a6-04d4-4c7c-ac32-9ae78acdb66c@redhat.com/> 2023-08-25 21:16 ` [PATCH] Fix gdb/coffread.c build on 32bit architectures Mark Wielaard 2023-08-25 21:19 ` Keith Seitz 2023-08-28 14:08 ` Tom Tromey 2023-08-28 14:20 ` Keith Seitz 2023-08-28 14:27 ` Keith Seitz 2023-08-28 14:32 ` Mark Wielaard 2023-08-28 14:41 ` Andreas Schwab 2023-08-28 16:31 ` Tom Tromey 2023-08-28 16:33 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).