Fix build failure for 32-bit targets

Message ID 20211001115002.1681134-1-luis.machado@linaro.org
State New
Headers show
Series
  • Fix build failure for 32-bit targets
Related show

Commit Message

Mike Frysinger via Gdb-patches Oct. 1, 2021, 11:50 a.m.
When building master GDB, I ran into the following:

binutils-gdb/gdb/bt-utils.c: In function ‘int libbacktrace_print(void*, uintptr_t, const char*, int, const char*)’:
binutils-gdb/gdb/bt-utils.c:93:44: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uintptr_t {aka unsigned int}’ [-Werror=format=]
   snprintf (buf, sizeof (buf), "0x%lx ", pc);

Fix this by using phex and %s as opposed to 0x%lx.
---
 gdb/bt-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.25.1

Comments

Mike Frysinger via Gdb-patches Oct. 1, 2021, 3:25 p.m. | #1
On 01 Oct 2021 08:50, Luis Machado via Gdb-patches wrote:
> When building master GDB, I ran into the following:

> 

> binutils-gdb/gdb/bt-utils.c: In function ‘int libbacktrace_print(void*, uintptr_t, const char*, int, const char*)’:

> binutils-gdb/gdb/bt-utils.c:93:44: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uintptr_t {aka unsigned int}’ [-Werror=format=]

>    snprintf (buf, sizeof (buf), "0x%lx ", pc);

> 

> Fix this by using phex and %s as opposed to 0x%lx.


wouldn't PRIxPTR fix it and be simpler ?
-mike
Mike Frysinger via Gdb-patches Oct. 1, 2021, 3:53 p.m. | #2
On 10/1/21 12:25 PM, Mike Frysinger wrote:
> On 01 Oct 2021 08:50, Luis Machado via Gdb-patches wrote:

>> When building master GDB, I ran into the following:

>>

>> binutils-gdb/gdb/bt-utils.c: In function ‘int libbacktrace_print(void*, uintptr_t, const char*, int, const char*)’:

>> binutils-gdb/gdb/bt-utils.c:93:44: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uintptr_t {aka unsigned int}’ [-Werror=format=]

>>     snprintf (buf, sizeof (buf), "0x%lx ", pc);

>>

>> Fix this by using phex and %s as opposed to 0x%lx.

> 

> wouldn't PRIxPTR fix it and be simpler ?

> -mike

> 


I'm happy either way, but GDB uses phex/phex_nz/pulongest much more often.

grep "phex" gdb -R | wc -l
143

grep "PRIx" gdb -R | wc -l
25
Simon Marchi Oct. 1, 2021, 4:08 p.m. | #3
On 2021-10-01 11:53 a.m., Luis Machado via Gdb-patches wrote:
> On 10/1/21 12:25 PM, Mike Frysinger wrote:

>> On 01 Oct 2021 08:50, Luis Machado via Gdb-patches wrote:

>>> When building master GDB, I ran into the following:

>>>

>>> binutils-gdb/gdb/bt-utils.c: In function ‘int libbacktrace_print(void*, uintptr_t, const char*, int, const char*)’:

>>> binutils-gdb/gdb/bt-utils.c:93:44: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uintptr_t {aka unsigned int}’ [-Werror=format=]

>>>     snprintf (buf, sizeof (buf), "0x%lx ", pc);

>>>

>>> Fix this by using phex and %s as opposed to 0x%lx.

>>

>> wouldn't PRIxPTR fix it and be simpler ?

>> -mike

>>

>

> I'm happy either way, but GDB uses phex/phex_nz/pulongest much more often.

>

> grep "phex" gdb -R | wc -l

> 143

>

> grep "PRIx" gdb -R | wc -l

> 25


That's probably because GDB (for historical reasons I suppose) uses
these LONGEST/ULONGEST types, so we can't use standard things like
PRIx64 directly.  Although we could also define, say, PRIxLONGEST and
PRIxULONGEST and use them in format strings.

But here, since we want to print a uintptr_t, I would also go for
PRIxPTR, it's standard after all:

  https://en.cppreference.com/w/cpp/types/integer

Simon
Simon Marchi Oct. 1, 2021, 4:11 p.m. | #4
On 2021-10-01 12:08 p.m., Simon Marchi wrote:
> On 2021-10-01 11:53 a.m., Luis Machado via Gdb-patches wrote:

>> On 10/1/21 12:25 PM, Mike Frysinger wrote:

>>> On 01 Oct 2021 08:50, Luis Machado via Gdb-patches wrote:

>>>> When building master GDB, I ran into the following:

>>>>

>>>> binutils-gdb/gdb/bt-utils.c: In function ‘int libbacktrace_print(void*, uintptr_t, const char*, int, const char*)’:

>>>> binutils-gdb/gdb/bt-utils.c:93:44: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uintptr_t {aka unsigned int}’ [-Werror=format=]

>>>>     snprintf (buf, sizeof (buf), "0x%lx ", pc);

>>>>

>>>> Fix this by using phex and %s as opposed to 0x%lx.

>>>

>>> wouldn't PRIxPTR fix it and be simpler ?

>>> -mike

>>>

>>

>> I'm happy either way, but GDB uses phex/phex_nz/pulongest much more often.

>>

>> grep "phex" gdb -R | wc -l

>> 143

>>

>> grep "PRIx" gdb -R | wc -l

>> 25

> 

> That's probably because GDB (for historical reasons I suppose) uses

> these LONGEST/ULONGEST types, so we can't use standard things like

> PRIx64 directly.  Although we could also define, say, PRIxLONGEST and

> PRIxULONGEST and use them in format strings.

> 

> But here, since we want to print a uintptr_t, I would also go for

> PRIxPTR, it's standard after all:

> 

>   https://en.cppreference.com/w/cpp/types/integer

> 

> Simon


Or, maybe %p since the goal is to print a pointer for the local host, but
I suppose the value would have to be cast to void*.

Simon
Mike Frysinger via Gdb-patches Oct. 1, 2021, 4:32 p.m. | #5
On 10/1/21 1:08 PM, Simon Marchi wrote:
> On 2021-10-01 11:53 a.m., Luis Machado via Gdb-patches wrote:

>> On 10/1/21 12:25 PM, Mike Frysinger wrote:

>>> On 01 Oct 2021 08:50, Luis Machado via Gdb-patches wrote:

>>>> When building master GDB, I ran into the following:

>>>>

>>>> binutils-gdb/gdb/bt-utils.c: In function ‘int libbacktrace_print(void*, uintptr_t, const char*, int, const char*)’:

>>>> binutils-gdb/gdb/bt-utils.c:93:44: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uintptr_t {aka unsigned int}’ [-Werror=format=]

>>>>      snprintf (buf, sizeof (buf), "0x%lx ", pc);

>>>>

>>>> Fix this by using phex and %s as opposed to 0x%lx.

>>>

>>> wouldn't PRIxPTR fix it and be simpler ?

>>> -mike

>>>

>>

>> I'm happy either way, but GDB uses phex/phex_nz/pulongest much more often.

>>

>> grep "phex" gdb -R | wc -l

>> 143

>>

>> grep "PRIx" gdb -R | wc -l

>> 25

> 

> That's probably because GDB (for historical reasons I suppose) uses

> these LONGEST/ULONGEST types, so we can't use standard things like

> PRIx64 directly.  Although we could also define, say, PRIxLONGEST and

> PRIxULONGEST and use them in format strings.

> 

> But here, since we want to print a uintptr_t, I would also go for

> PRIxPTR, it's standard after all:


I pushed it with this format. Thanks!

Patch

diff --git a/gdb/bt-utils.c b/gdb/bt-utils.c
index 79e6e090d42..51791ccb9f6 100644
--- a/gdb/bt-utils.c
+++ b/gdb/bt-utils.c
@@ -90,7 +90,7 @@  libbacktrace_print (void *data, uintptr_t pc, const char *filename,
      files.  We are also careful to ensure we don't overflow this buffer.  */
   char buf[20];
 
-  snprintf (buf, sizeof (buf), "0x%lx ", pc);
+  snprintf (buf, sizeof (buf), "%s ", phex (pc, sizeof (pc));
   buf[sizeof (buf) - 1] = '\0';
   sig_write (buf);
   sig_write (function == nullptr ? "???" : function);