[RFC] convert a host address to a string

[RFC] convert a host address to a string

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1142 bytes --]

The host_address_to_string function converts the host address to
a string using sprintf after having converted the address to an
unsigned long.

Unfortunately for us, on x86_64-windows, unsigned long is not big
enough to hold an address. My initial approach was to detect this case
by using a configure check which defines HOST_IS_LLP64 if sizeof (void*)
is 8 while sizeof (long) is 4.

Mark's reaction was that we should be able to use something more elegant.
One of the things we could do, perhaps, is use %p, which is mentioned by
the comment inside the function.  I checked the C90 draft, and %p is
definitely mentioned, so I suspect it's fine to use it assuming we
require C90. The problem is, are we going to break the build on some
targets if we do?

2009-01-07  Joel Brobecker  <brobecker@adacore.com>

        * configure.ac: Define HOST_IS_LLP64 if the host uses the LLP64 ABI.
        * configure, config.in: Regenerate.
        * utils.c (host_address_to_string): Use "long long" to print
        addresses rather than "long" if HOST_IS_LLP64 is defined.

Tested on x86-linux, x86-windows and x86_64-windows.

Thoughts?

-- 
Joel

[-- Attachment #2: print_addr2.diff --]
[-- Type: text/plain, Size: 1831 bytes --]

diff --git a/gdb/configure.ac b/gdb/configure.ac
index e970234..8774309 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1189,6 +1189,20 @@ if test $gdb_cv_printf_has_long_long = yes; then
             [Define to 1 if the "%ll" format works to print long longs.])
 fi
 
+# Check if the host is LLP64 (pointers are 64bits and "long" is only 32bits).
+
+AC_CACHE_CHECK([host is LLP64],
+               gdb_cv_host_is_llp64,
+               [AC_RUN_IFELSE([AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT],
+[[return !(sizeof (void *) == 8 && sizeof (long) == 4);]])],
+                              gdb_cv_host_is_llp64=yes,
+                              gdb_cv_host_is_llp64=no,
+                              gdb_cv_host_is_llp64=no)])
+if test $gdb_cv_host_is_llp64 = yes; then
+  AC_DEFINE(HOST_IS_LLP64, 1,
+            [Define to 1 if host is LLP64 (pointers is 64bits and "long" is32bits)])
+fi
+
 # Check if the compiler and runtime support printing decfloats.
 
 AC_CACHE_CHECK([for decfloat support in printf],
diff --git a/gdb/utils.c b/gdb/utils.c
index 2549a3c..d289509 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3066,9 +3066,15 @@ host_address_to_string (const void *addr)
   char *str = get_cell ();
 
   /* We could use the %p conversion specifier to sprintf if we had any
-     way of knowing whether this host supports it.  But the following
-     should work on the Alpha and on 32 bit machines.  */
+     way of knowing whether this host supports it.  */
+#ifdef HOST_IS_LLP64
+  /* On LLP64 hosts, longs are only 32bits whereas pointers are 64bit.
+     Use a "long long" to print our address.  */
+  sprintf (str, "0x%llx", (unsigned long long) addr);
+#else
+  /* The following should work on the Alpha and on 32 bit machines.  */
   sprintf (str, "0x%lx", (unsigned long) addr);
+#endif
   return str;
 }
 

Re: [RFC] convert a host address to a string

[Mark Kettenis]

> Date: Wed, 7 Jan 2009 16:19:08 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> 
> The host_address_to_string function converts the host address to
> a string using sprintf after having converted the address to an
> unsigned long.
> 
> Unfortunately for us, on x86_64-windows, unsigned long is not big
> enough to hold an address. My initial approach was to detect this case
> by using a configure check which defines HOST_IS_LLP64 if sizeof (void*)
> is 8 while sizeof (long) is 4.
> 
> Mark's reaction was that we should be able to use something more elegant.
> One of the things we could do, perhaps, is use %p, which is mentioned by
> the comment inside the function.  I checked the C90 draft, and %p is
> definitely mentioned, so I suspect it's fine to use it assuming we
> require C90. The problem is, are we going to break the build on some
> targets if we do?

Probably not, but there's a problem with %p.  While it is specified by
C90 and almost certainly implemented in the C library of all systems
we care about, it is implemented how exactly the pointer will be
printed.  On OpenBSD and Linux it is something like 0xNNNNNNNN, but
Solaris generates NNNNNNNN (without the initial 0x).  That's
undesirable I think.

An option would be to use the strategy used by phex_nz() to print host
addresses.  Or we could use PRINTF_HAS_LONG_LONG, and always use %llx
if it's available.

I'd really like to avoid introducing another macro dealing with
type-size issues if possible.  I especially dislike HOST_IS_LLP64
since I fear its existence encourages people to write unportable code.

Re: [RFC] convert a host address to a string

[Joel Brobecker]

> Probably not, but there's a problem with %p.  While it is specified by
> C90 and almost certainly implemented in the C library of all systems
> we care about, it is implemented how exactly the pointer will be
> printed.  On OpenBSD and Linux it is something like 0xNNNNNNNN, but
> Solaris generates NNNNNNNN (without the initial 0x).  That's
> undesirable I think.

I agree.

> An option would be to use the strategy used by phex_nz() to print host
> addresses.  Or we could use PRINTF_HAS_LONG_LONG, and always use %llx
> if it's available.

Unfortunately, I don't know how this could be made to work.
The problem is that GCC insists that the integer type that we
use to cast the host address to must have the same size.
At one point, hoping that GCC would kill the wrong branch,
I even tried:

  if (sizeof (void *) == sizeof (long))
    printf ("0x%lx", (long) address);
  else
    printf ("0x%llx", (long long) address);

But this didn't work, because GCC complained about the cast
in the "if" branch.

Actually, it's only after writing the entire email that I realized
that we have another option. See option (3) below.

> I'd really like to avoid introducing another macro dealing with
> type-size issues if possible.  I especially dislike HOST_IS_LLP64
> since I fear its existence encourages people to write unportable code.

I can see several solutions:

  1. Use %p. To overcome the problem with 0x, we could use
     two alternatives:

       a. Import printf from gnulib. I looked at this a while ago,
          for some other issue, and I immediately stopped, as it
          looked like it might be a lot of work to do so (printf
          doesn't come alone, there's a bunch of other routines
          that printf uses which we probably want).

       b. Strip the leading "0x" if %p already provides it. In other
          words:

            fprintf (buf, "0x%p", address);
            if (buf[2] == '0' && buf[3] == 'x')
              buf = buf + 2;
            return buf;

          There is no memory management issue in this case, because
          the buffer we return is more or less static. It's part
          of a bunch of buffers we cycle through each time we call
          this routine.  The caller never frees the memory we return.

  2. Avoid the HOST_IS_LLP64 macro, but still do something similar
     inside host_address_to_string. Something like:

       #if defined(WIN64_)
           fprintf (buf, "0x%llx", (unsigned long long) address);
       #else
           fprintf (buf, "0x%lx", (unsigned long) address);
       #endif

     This eliminates the likeliness of re-using the HOST_IS_LLP64
     macro to write non-portable code.

  3. Work through uintptr_t.

       #ifdef PRINTF_HAS_LONG_LONG
         fprintf (buf, "0x%llx", (unsigned long long) (uintptr_t) address);
       #else
         fprintf (buf, "0x%lx", (unsigned long) (uintptr_t) address);
       #endif

For completeness' sake, I also investigate the use of the PRIxPTR
macro, but we still have the problem of casting the address to
the right integer type: If PRIxPTR resolves to "lx", then we should
cast to "long", otherwise, we should cas to "long long".

I kinda like option 1b as being simple and avoiding the need to
cast the address to an integer.  Option (3) is my next favorite,
but I don't like the fact that we end up doing an unnecessary
integer promotion on the 32bit targets. Perhaps we could avoid that
using an extra "if (sizeof (void *) != sizeof (long))" but then
the code becomes increasingly complex. My next favorite would
probably be option 2 because I'm lazy, but it's really not elegant.
Option 1 looks like a fair amount of work, but would give us access
to a predicatable printf.

Thoughts?

-- 
Joel

Re: [RFC] convert a host address to a string

[Kai Tietz]

gdb-patches-owner@sourceware.org wrote on 08.01.2009 11:19:11:

> > Probably not, but there's a problem with %p.  While it is specified by
> > C90 and almost certainly implemented in the C library of all systems
> > we care about, it is implemented how exactly the pointer will be
> > printed.  On OpenBSD and Linux it is something like 0xNNNNNNNN, but
> > Solaris generates NNNNNNNN (without the initial 0x).  That's
> > undesirable I think.
> 
> I agree.
> 
> > An option would be to use the strategy used by phex_nz() to print host
> > addresses.  Or we could use PRINTF_HAS_LONG_LONG, and always use %llx
> > if it's available.
> 
> Unfortunately, I don't know how this could be made to work.
> The problem is that GCC insists that the integer type that we
> use to cast the host address to must have the same size.
> At one point, hoping that GCC would kill the wrong branch,
> I even tried:
> 
>   if (sizeof (void *) == sizeof (long))
>     printf ("0x%lx", (long) address);
>   else
>     printf ("0x%llx", (long long) address);
> 
> But this didn't work, because GCC complained about the cast
> in the "if" branch.
> 
> Actually, it's only after writing the entire email that I realized
> that we have another option. See option (3) below.
> 
> > I'd really like to avoid introducing another macro dealing with
> > type-size issues if possible.  I especially dislike HOST_IS_LLP64
> > since I fear its existence encourages people to write unportable code.
> 
> I can see several solutions:
> 
>   1. Use %p. To overcome the problem with 0x, we could use
>      two alternatives:
> 
>        a. Import printf from gnulib. I looked at this a while ago,
>           for some other issue, and I immediately stopped, as it
>           looked like it might be a lot of work to do so (printf
>           doesn't come alone, there's a bunch of other routines
>           that printf uses which we probably want).
> 
>        b. Strip the leading "0x" if %p already provides it. In other
>           words:
> 
>             fprintf (buf, "0x%p", address);
>             if (buf[2] == '0' && buf[3] == 'x')
>               buf = buf + 2;
>             return buf;
> 
>           There is no memory management issue in this case, because
>           the buffer we return is more or less static. It's part
>           of a bunch of buffers we cycle through each time we call
>           this routine.  The caller never frees the memory we return.
> 
>   2. Avoid the HOST_IS_LLP64 macro, but still do something similar
>      inside host_address_to_string. Something like:
> 
>        #if defined(WIN64_)
>            fprintf (buf, "0x%llx", (unsigned long long) address);
>        #else
>            fprintf (buf, "0x%lx", (unsigned long) address);
>        #endif
> 
>      This eliminates the likeliness of re-using the HOST_IS_LLP64
>      macro to write non-portable code.
> 
>   3. Work through uintptr_t.
> 
>        #ifdef PRINTF_HAS_LONG_LONG
>          fprintf (buf, "0x%llx", (unsigned long long) (uintptr_t) 
address);
>        #else
>          fprintf (buf, "0x%lx", (unsigned long) (uintptr_t) address);
>        #endif
> 
> For completeness' sake, I also investigate the use of the PRIxPTR
> macro, but we still have the problem of casting the address to
> the right integer type: If PRIxPTR resolves to "lx", then we should
> cast to "long", otherwise, we should cas to "long long".
> 
> I kinda like option 1b as being simple and avoiding the need to
> cast the address to an integer.  Option (3) is my next favorite,
> but I don't like the fact that we end up doing an unnecessary
> integer promotion on the 32bit targets. Perhaps we could avoid that
> using an extra "if (sizeof (void *) != sizeof (long))" but then
> the code becomes increasingly complex. My next favorite would
> probably be option 2 because I'm lazy, but it's really not elegant.
> Option 1 looks like a fair amount of work, but would give us access
> to a predicatable printf.
> 
> Thoughts?
> 
> -- 
> Joel
> 

Why not simply use stdint.h (gstdint.h) for this?

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Re: [RFC] convert a host address to a string

[Joel Brobecker]

> Why not simply use stdint.h (gstdint.h) for this?

Could you give more details? I'm not sure what you mean exactly.

Thanks,
-- 
Joel

Re: [RFC] convert a host address to a string

[Kai Tietz]

Joel Brobecker <brobecker@adacore.com> wrote on 08.01.2009 11:48:00:

> > Why not simply use stdint.h (gstdint.h) for this?
> 
> Could you give more details? I'm not sure what you mean exactly.
> 
> Thanks,
> -- 
> Joel
> 

By it you can use standard ISO type intptr_t. And I missed to add 
inttypes.h.
By this you could simply use PRIXPTR for printing hex pointer failues.

E.g.
 fprint (fp, "0x%" PRIXPTR, (intptr_t) address);

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Re: [RFC] convert a host address to a string

[Joel Brobecker]

> By it you can use standard ISO type intptr_t. And I missed to add 
> inttypes.h.
> By this you could simply use PRIXPTR for printing hex pointer failues.
> 
> E.g.
>  fprint (fp, "0x%" PRIXPTR, (intptr_t) address);

That's pretty clever.  Bad luck for us, however. The MinGW inttypes.h
defines PRIxPTR as "I64x". When I tried with an example, I got a warning
because intptr_t is "long long int" whereas "%I64x" requires an unsigned
int. Changing the cast to uintptr_t helps a little, but it's not enough
since "unsigned int" is definitely not large enough to hold a pointer!

I really should report that to the MinGW guys!

-- 
Joel

Re: [RFC] convert a host address to a string

[Kai Tietz]

Hi Joel,

gdb-patches-owner@sourceware.org wrote on 08.01.2009 12:24:48:

> > By it you can use standard ISO type intptr_t. And I missed to add 
> > inttypes.h.
> > By this you could simply use PRIXPTR for printing hex pointer failues.
> > 
> > E.g.
> >  fprint (fp, "0x%" PRIXPTR, (intptr_t) address);
> 
> That's pretty clever.  Bad luck for us, however. The MinGW inttypes.h
> defines PRIxPTR as "I64x". When I tried with an example, I got a warning
> because intptr_t is "long long int" whereas "%I64x" requires an unsigned
> int. Changing the cast to uintptr_t helps a little, but it's not enough
> since "unsigned int" is definitely not large enough to hold a pointer!
> 
> I really should report that to the MinGW guys!

Ok, I got it. But there is not much to do here. I am a bit curious about 
that x wants an unsigned scalar. I have to verify this.
The alternative for this, would be to cast to size_t in general.

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Re: [RFC] convert a host address to a string

[Mark Kettenis]

> From: Kai Tietz <Kai.Tietz@onevision.com>
> Date: Thu, 8 Jan 2009 12:01:56 +0100
> 
> Joel Brobecker <brobecker@adacore.com> wrote on 08.01.2009 11:48:00:
> 
> > > Why not simply use stdint.h (gstdint.h) for this?
> > 
> > Could you give more details? I'm not sure what you mean exactly.
> > 
> > Thanks,
> > -- 
> > Joel
> > 
> 
> By it you can use standard ISO type intptr_t. And I missed to add 
> inttypes.h.
> By this you could simply use PRIXPTR for printing hex pointer failues.
> 
> E.g.
>  fprint (fp, "0x%" PRIXPTR, (intptr_t) address);

Well, the PRIxxx macros are ISO C99 inventions, so we can't assume
they're generally available.  Guess that could be mitigated by
importing inttypes module from gnulib.

Re: [RFC] convert a host address to a string

[Joel Brobecker]

> Well, the PRIxxx macros are ISO C99 inventions, so we can't assume
> they're generally available.  Guess that could be mitigated by
> importing inttypes module from gnulib.

It's mentioned in the C90 draft document that I have. Has it been
removed from the final revision?

In any case, it doesn't work on Windows, because their definition
is screwed. How about some of my suggestions?

-- 
Joel

Re: [RFC] convert a host address to a string

[Kai Tietz]

Joel Brobecker <brobecker@adacore.com> wrote on 08.01.2009 13:54:30:

> > Well, the PRIxxx macros are ISO C99 inventions, so we can't assume
> > they're generally available.  Guess that could be mitigated by
> > importing inttypes module from gnulib.
> 
> It's mentioned in the C90 draft document that I have. Has it been
> removed from the final revision?
> 
> In any case, it doesn't work on Windows, because their definition
> is screwed. How about some of my suggestions?

hmm, I tested it for the w64 compiler and I got no warnings about this at 
all. Is gdb using some --std option? If so it could be that you are fooled 
by the ISO C message that I isn't an ISO formatter. This check can be 
turned off (as we did it for gcc 4.4) by the option 
-Wno-pedantic-ms-format.

Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Re: [RFC] convert a host address to a string

[Mark Kettenis]

> Date: Thu, 8 Jan 2009 16:54:30 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> 
> > Well, the PRIxxx macros are ISO C99 inventions, so we can't assume
> > they're generally available.  Guess that could be mitigated by
> > importing inttypes module from gnulib.
> 
> It's mentioned in the C90 draft document that I have. Has it been
> removed from the final revision?

Are you sure you're looking at a C90 draft and not a C99 draft?

The draft linked from the Wikipedia page:

    http://flash-gordon.me.uk/ansi.c.txt

doesn't mention <inttypes.h> or the PRIxPTR define.  I'm fairly sure
they're not part of C90, since the uintptr_t typedef isn't either.

> In any case, it doesn't work on Windows, because their definition
> is screwed. How about some of my suggestions?

Actually, I think their defenition is ok (but nonstandard).  IIRC,
%I64x is the way to print the __int64 type that Microsoft invented
before long long was standardized by C99.  So it seems the problem
here is with gcc.

I'll reply to your origional mail to discuss your alternative suggestions.

Re: [RFC] convert a host address to a string

[Mark Kettenis]

> Date: Thu, 8 Jan 2009 14:19:11 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> 
> > An option would be to use the strategy used by phex_nz() to print host
> > addresses.  Or we could use PRINTF_HAS_LONG_LONG, and always use %llx
> > if it's available.
> 
> Unfortunately, I don't know how this could be made to work.
> The problem is that GCC insists that the integer type that we
> use to cast the host address to must have the same size.
> At one point, hoping that GCC would kill the wrong branch,
> I even tried:
> 
>   if (sizeof (void *) == sizeof (long))
>     printf ("0x%lx", (long) address);
>   else
>     printf ("0x%llx", (long long) address);
> 
> But this didn't work, because GCC complained about the cast
> in the "if" branch.

Ah, GCC is being a bit too helpful here.  Damn!

> Actually, it's only after writing the entire email that I realized
> that we have another option. See option (3) below.
> 
> > I'd really like to avoid introducing another macro dealing with
> > type-size issues if possible.  I especially dislike HOST_IS_LLP64
> > since I fear its existence encourages people to write unportable code.
> 
> I can see several solutions:
> 
>   1. Use %p. To overcome the problem with 0x, we could use
>      two alternatives:
> 
>        a. Import printf from gnulib. I looked at this a while ago,
>           for some other issue, and I immediately stopped, as it
>           looked like it might be a lot of work to do so (printf
>           doesn't come alone, there's a bunch of other routines
>           that printf uses which we probably want).

I'm not very excited about this option.  And if the gnulib printf
doesn't actually implement the Microsoft-invented non-standard format
specifiers it may even cause us more grief.

>        b. Strip the leading "0x" if %p already provides it. In other
>           words:
> 
>             fprintf (buf, "0x%p", address);
>             if (buf[2] == '0' && buf[3] == 'x')
>               buf = buf + 2;
>             return buf;
> 
>           There is no memory management issue in this case, because
>           the buffer we return is more or less static. It's part
>           of a bunch of buffers we cycle through each time we call
>           this routine.  The caller never frees the memory we return.

Ugh, this is a bit ugly.  And we can't even be sure that there are
even more variations on the format that %p generates.  I wouldn't be
surprised at all if some platforms would use upper case for the hex
digits for example.

>   2. Avoid the HOST_IS_LLP64 macro, but still do something similar
>      inside host_address_to_string. Something like:
> 
>        #if defined(WIN64_)
>            fprintf (buf, "0x%llx", (unsigned long long) address);
>        #else
>            fprintf (buf, "0x%lx", (unsigned long) address);
>        #endif
> 
>      This eliminates the likeliness of re-using the HOST_IS_LLP64
>      macro to write non-portable code.

Not really excited about this one either.

>   3. Work through uintptr_t.
> 
>        #ifdef PRINTF_HAS_LONG_LONG
>          fprintf (buf, "0x%llx", (unsigned long long) (uintptr_t) address);
>        #else
>          fprintf (buf, "0x%lx", (unsigned long) (uintptr_t) address);
>        #endif

This wouldn't be the first place where we'd use a double cast in
connection with intptr_t/uintptr_t.  So I'd say that while this is a
bit ugly, it's certainly acceptable.  It's by far the simplest way to
fix things.

> I kinda like option 1b as being simple and avoiding the need to
> cast the address to an integer.  Option (3) is my next favorite,
> but I don't like the fact that we end up doing an unnecessary
> integer promotion on the 32bit targets.

I'm not really worried about the integer promotion.  Printing host
addresses is a fairly rare operation, and certainly not time critical.

Re: [RFC] convert a host address to a string

[Kai Tietz]

gdb-patches-owner@sourceware.org wrote on 08.01.2009 14:26:15:

> > Date: Thu, 8 Jan 2009 14:19:11 +0400
> > From: Joel Brobecker <brobecker@adacore.com>
> > 
> > > An option would be to use the strategy used by phex_nz() to print 
host
> > > addresses.  Or we could use PRINTF_HAS_LONG_LONG, and always use 
%llx
> > > if it's available.
> > 
> > Unfortunately, I don't know how this could be made to work.
> > The problem is that GCC insists that the integer type that we
> > use to cast the host address to must have the same size.
> > At one point, hoping that GCC would kill the wrong branch,
> > I even tried:
> > 
> >   if (sizeof (void *) == sizeof (long))
> >     printf ("0x%lx", (long) address);
> >   else
> >     printf ("0x%llx", (long long) address);
> > 
> > But this didn't work, because GCC complained about the cast
> > in the "if" branch.
> 
> Ah, GCC is being a bit too helpful here.  Damn!
> 
> > Actually, it's only after writing the entire email that I realized
> > that we have another option. See option (3) below.
> > 
> > > I'd really like to avoid introducing another macro dealing with
> > > type-size issues if possible.  I especially dislike HOST_IS_LLP64
> > > since I fear its existence encourages people to write unportable 
code.
> > 
> > I can see several solutions:
> > 
> >   1. Use %p. To overcome the problem with 0x, we could use
> >      two alternatives:
> > 
> >        a. Import printf from gnulib. I looked at this a while ago,
> >           for some other issue, and I immediately stopped, as it
> >           looked like it might be a lot of work to do so (printf
> >           doesn't come alone, there's a bunch of other routines
> >           that printf uses which we probably want).
> 
> I'm not very excited about this option.  And if the gnulib printf
> doesn't actually implement the Microsoft-invented non-standard format
> specifiers it may even cause us more grief.
> 
> >        b. Strip the leading "0x" if %p already provides it. In other
> >           words:
> > 
> >             fprintf (buf, "0x%p", address);
> >             if (buf[2] == '0' && buf[3] == 'x')
> >               buf = buf + 2;
> >             return buf;
> > 
> >           There is no memory management issue in this case, because
> >           the buffer we return is more or less static. It's part
> >           of a bunch of buffers we cycle through each time we call
> >           this routine.  The caller never frees the memory we return.
> 
> Ugh, this is a bit ugly.  And we can't even be sure that there are
> even more variations on the format that %p generates.  I wouldn't be
> surprised at all if some platforms would use upper case for the hex
> digits for example.
> 
> >   2. Avoid the HOST_IS_LLP64 macro, but still do something similar
> >      inside host_address_to_string. Something like:
> > 
> >        #if defined(WIN64_)
> >            fprintf (buf, "0x%llx", (unsigned long long) address);
> >        #else
> >            fprintf (buf, "0x%lx", (unsigned long) address);
> >        #endif
> > 
> >      This eliminates the likeliness of re-using the HOST_IS_LLP64
> >      macro to write non-portable code.
> 
> Not really excited about this one either.
> 
> >   3. Work through uintptr_t.
> > 
> >        #ifdef PRINTF_HAS_LONG_LONG
> >          fprintf (buf, "0x%llx", (unsigned long long) (uintptr_t) 
address);
> >        #else
> >          fprintf (buf, "0x%lx", (unsigned long) (uintptr_t) address);
> >        #endif
> 
> This wouldn't be the first place where we'd use a double cast in
> connection with intptr_t/uintptr_t.  So I'd say that while this is a
> bit ugly, it's certainly acceptable.  It's by far the simplest way to
> fix things.
> 
> > I kinda like option 1b as being simple and avoiding the need to
> > cast the address to an integer.  Option (3) is my next favorite,
> > but I don't like the fact that we end up doing an unnecessary
> > integer promotion on the 32bit targets.
> 
> I'm not really worried about the integer promotion.  Printing host
> addresses is a fairly rare operation, and certainly not time critical.
> 

Please be aware that %llx isn't valid for x86_64 windows (as it isn't for 
32-bit standard). The options 'I' or 'I64' have to be used for those 
targets (at least for _WIN64).

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Re: [RFC] convert a host address to a string

[Joel Brobecker]

> Please be aware that %llx isn't valid for x86_64 windows (as it isn't for 
> 32-bit standard). The options 'I' or 'I64' have to be used for those 
> targets (at least for _WIN64).

Can you be a little more specific?

I just checked the config.h file generated by configure on
x86_64-windows, and I found this:

    /* Define to 1 if the "%ll" format works to print long longs. */
    #define PRINTF_HAS_LONG_LONG 1

If this is wrong, we have yet another problem.

-- 
Joel

Re: [RFC] convert a host address to a string

[Kai Tietz]

Joel Brobecker <brobecker@adacore.com> wrote on 08.01.2009 14:42:45:

> > Please be aware that %llx isn't valid for x86_64 windows (as it isn't 
for 
> > 32-bit standard). The options 'I' or 'I64' have to be used for those 
> > targets (at least for _WIN64).
> 
> Can you be a little more specific?
> 
> I just checked the config.h file generated by configure on
> x86_64-windows, and I found this:
> 
>     /* Define to 1 if the "%ll" format works to print long longs. */
>     #define PRINTF_HAS_LONG_LONG 1
> 
> If this is wrong, we have yet another problem.

Yes, this is wrong, at least for w64. I used the w64 compiler by this and 
it returns me for the probing code an result of -1.

The issue here is that msvcrt doesn't supports (for 32-bit and for 64-bit) 
the %ll with specifier. For msvcrt80 (or so) they introduced it IIRC, but 
this version isn't used by mingw as default runtime (because it isn't part 
of the OS itself).

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Re: [RFC] convert a host address to a string

[Mark Kettenis]

> From: Kai Tietz <Kai.Tietz@onevision.com>
> Date: Thu, 8 Jan 2009 14:35:11 +0100
> 
> Please be aware that %llx isn't valid for x86_64 windows (as it isn't for 
> 32-bit standard). The options 'I' or 'I64' have to be used for those 
> targets (at least for _WIN64).

Are you serious?  Wow, that's unbelievable.  %llx is defined by ISO
C99, and you're saying that even the 64-bit editions of Windows that
were introduced several years after the standard was ratified (and
almost a decade after the first drafts were circulated) don't
implement that bit of the standard?

Re: [RFC] convert a host address to a string

[Kai Tietz]

Mark Kettenis <mark.kettenis@xs4all.nl> wrote on 08.01.2009 17:18:05:

> > From: Kai Tietz <Kai.Tietz@onevision.com>
> > Date: Thu, 8 Jan 2009 14:35:11 +0100
> > 
> > Please be aware that %llx isn't valid for x86_64 windows (as it isn't 
for 
> > 32-bit standard). The options 'I' or 'I64' have to be used for those 
> > targets (at least for _WIN64).
> 
> Are you serious?  Wow, that's unbelievable.  %llx is defined by ISO
> C99, and you're saying that even the 64-bit editions of Windows that
> were introduced several years after the standard was ratified (and
> almost a decade after the first drafts were circulated) don't
> implement that bit of the standard?
> 

Sadly, yes. IIRC they implemented it in their newer runtimes, but it 
didn't made it into msvcrt.dll even on 64-bit systems.

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Re: [RFC] convert a host address to a string

[Joel Brobecker]

Kai,

> > > Please be aware that %llx isn't valid for x86_64 windows (as it isn't 
> for 
> > > 32-bit standard). The options 'I' or 'I64' have to be used for
> > > those targets (at least for _WIN64).
> > 
> > Are you serious?  Wow, that's unbelievable.  %llx is defined by ISO
> > C99, and you're saying that even the 64-bit editions of Windows that
> > were introduced several years after the standard was ratified (and
> > almost a decade after the first drafts were circulated) don't
> > implement that bit of the standard?
> > 
> 
> Sadly, yes. IIRC they implemented it in their newer runtimes, but it 
> didn't made it into msvcrt.dll even on 64-bit systems.

I don't doubt that you are right, but I was wondering whether configure
was (apparently incorrectly) defining PRINTF_HAS_LONG_LONG.

  | #include <stdio.h>
  | 
  | int
  | main (void)
  | {
  |   long long addr = 0xdeadbeeffeedfaceLL;
  | 
  |   printf ("addr = 0x%llx.\n", addr);
  |   return 0;
  | }

I compiled the program above on our x86_64 Vista, and it compiled and
worked like a charm. The check in configure is actually of the same
nature.

On 32bit systems:
  - XP: PRINTF_HAS_LONG_LONG is not defined.
  - Vista: All works fine.

I don't have access to an x86_64 XP install, but I'm going to make
a guess that PRINTF_HAS_LONG_LONG is not defined there.

What I will do is send a patch that implements what Mark suggested.
This should work on all our supported platforms, with the addition
of x86_64 Vista, but to the exclusion of x86_64 XP. I am OK with
the idea of adding a extra #if WIN64_ branch that allows us to build
on x86_64 XP, but you'll have to convince others as well.

-- 
Joel

Re: [RFC] convert a host address to a string

[Kai Tietz]

Joel,

Joel Brobecker <brobecker@adacore.com> wrote on 09.01.2009 10:57:05:

> Kai,
> 
> > > > Please be aware that %llx isn't valid for x86_64 windows (as it 
isn't 
> > for 
> > > > 32-bit standard). The options 'I' or 'I64' have to be used for
> > > > those targets (at least for _WIN64).
> > > 
> > > Are you serious?  Wow, that's unbelievable.  %llx is defined by ISO
> > > C99, and you're saying that even the 64-bit editions of Windows that
> > > were introduced several years after the standard was ratified (and
> > > almost a decade after the first drafts were circulated) don't
> > > implement that bit of the standard?
> > > 
> > 
> > Sadly, yes. IIRC they implemented it in their newer runtimes, but it 
> > didn't made it into msvcrt.dll even on 64-bit systems.
> 
> I don't doubt that you are right, but I was wondering whether configure
> was (apparently incorrectly) defining PRINTF_HAS_LONG_LONG.
> 
>   | #include <stdio.h>
>   | 
>   | int
>   | main (void)
>   | {
>   |   long long addr = 0xdeadbeeffeedfaceLL;
>   | 
>   |   printf ("addr = 0x%llx.\n", addr);
>   |   return 0;
>   | }
> 
> I compiled the program above on our x86_64 Vista, and it compiled and
> worked like a charm. The check in configure is actually of the same
> nature.
> 
> On 32bit systems:
>   - XP: PRINTF_HAS_LONG_LONG is not defined.
>   - Vista: All works fine.
> 
> I don't have access to an x86_64 XP install, but I'm going to make
> a guess that PRINTF_HAS_LONG_LONG is not defined there.
> 
> What I will do is send a patch that implements what Mark suggested.
> This should work on all our supported platforms, with the addition
> of x86_64 Vista, but to the exclusion of x86_64 XP. I am OK with
> the idea of adding a extra #if WIN64_ branch that allows us to build
> on x86_64 XP, but you'll have to convince others as well.

That makes sense. I use at work XP64, and there is for sure no '%llx' 
support. I will cross check this this evening at home on Vista64.
The bad thing here is if you would enable -Wformat, you get by gcc a 
warning about the double-(el)l ...

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Re: [RFC] convert a host address to a string

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]

> >   3. Work through uintptr_t.
> > 
> >        #ifdef PRINTF_HAS_LONG_LONG
> >          fprintf (buf, "0x%llx", (unsigned long long) (uintptr_t) address);
> >        #else
> >          fprintf (buf, "0x%lx", (unsigned long) (uintptr_t) address);
> >        #endif
> 
> This wouldn't be the first place where we'd use a double cast in
> connection with intptr_t/uintptr_t.  So I'd say that while this is a
> bit ugly, it's certainly acceptable.  It's by far the simplest way to
> fix things.

Here is a new patch implementing this approach.  As I told Kai,
it's probably not going to work for x86_64/XP, but it works for
x86_64/Vista (I think Kai agreed to take on the job of improving
this approach to work on XP as well :-).

2009-01-09  Joel Brobecker  <brobecker@adacore.com>

        * utils.c (host_address_to_string): Cast the address to uintptr_t
        first to avoid a possible compilation warning about casting to
        an integer of the wrong size.  Then format the address using
        unsigned long long if supported by printf.  Otherwise, fallback
        on using usingned long, hoping that long is large enough to hold
        an address.

Tested on x86-linux. I also verified that it still builds on
x86_64 Vista, as well as x86/Windows XP (on XP, PRINTF_HAS_LONG_LONG
is undefined).

-- 
Joel

[-- Attachment #2: host_addr_to_str.diff --]
[-- Type: text/plain, Size: 2332 bytes --]

diff --git a/gdb/utils.c b/gdb/utils.c
index 9e2dfd7..dd606a2 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3071,10 +3071,44 @@ host_address_to_string (const void *addr)
 {
   char *str = get_cell ();
 
-  /* We could use the %p conversion specifier to sprintf if we had any
-     way of knowing whether this host supports it.  But the following
-     should work on the Alpha and on 32 bit machines.  */
-  sprintf (str, "0x%lx", (unsigned long) addr);
+  /* We do not use the %p conversion specifier, because the resulting
+     image can vary from implementation to implementation.  For instance,
+     some implementations format the pointer value with a leading "0x"
+     whereas others don't (Solaris, for instance).  Also, it is unspecified
+     whether the alphabetical digits are printed using uppercase letters
+     or not (in GDB, we want lowercase).
+
+     So we use the %x type instead.  This, however, introduces
+     a couple of issues:
+
+       1. The %x type expects an integer value, not a pointer.
+          So we first need to cast our pointer to an integer type
+          whose size is identical to the size of our pointer.
+          We use uintptr_t for that.
+
+       2. The %x type alone expects and int, which is not always
+          large enough to hold an address.  Usually, type "long"
+          has the same size as pointers, but certain ABIs define
+          the size of pointers to be larger than the size of long
+          (64bit Windows is one such example).
+
+          So, to be certain to have a type that's large enough
+          to hold an address, we need to use "long long".  But
+          the trick is that not all printf implementations support
+          the "ll" modifier.  On those platforms where the "ll"
+          modifier is not available, we'll assume that type "long"
+          is large enough to contain an address.
+
+          To make sure that the type we pass to sprintf matches
+          the type we specified in our expression, we perform
+          a second cast to "unsigned long long" if we used "%llx",
+          or "unsigned long" if we used "%lx".  */
+
+#ifdef PRINTF_HAS_LONG_LONG
+  sprintf (str, "0x%llx", (unsigned long long) (uintptr_t) addr);
+#else
+  sprintf (str, "0x%lx", (unsigned long) (uintptr_t) addr);
+#endif
   return str;
 }
 

Re: [RFC] convert a host address to a string

[Kai Tietz]

Hi Joel,

gdb-patches-owner@sourceware.org wrote on 09.01.2009 14:12:27:
> > >   3. Work through uintptr_t.
> > > 
> > >        #ifdef PRINTF_HAS_LONG_LONG
> > >          fprintf (buf, "0x%llx", (unsigned long long) 
> (uintptr_t) address);
> > >        #else
> > >          fprintf (buf, "0x%lx", (unsigned long) (uintptr_t) 
address);
> > >        #endif
> > 
> > This wouldn't be the first place where we'd use a double cast in
> > connection with intptr_t/uintptr_t.  So I'd say that while this is a
> > bit ugly, it's certainly acceptable.  It's by far the simplest way to
> > fix things.
> 
> Here is a new patch implementing this approach.  As I told Kai,
> it's probably not going to work for x86_64/XP, but it works for
> x86_64/Vista (I think Kai agreed to take on the job of improving
> this approach to work on XP as well :-).
> 
> 2009-01-09  Joel Brobecker  <brobecker@adacore.com>
> 
>         * utils.c (host_address_to_string): Cast the address to 
uintptr_t
>         first to avoid a possible compilation warning about casting to
>         an integer of the wrong size.  Then format the address using
>         unsigned long long if supported by printf.  Otherwise, fallback
>         on using usingned long, hoping that long is large enough to hold
>         an address.
> 
> Tested on x86-linux. I also verified that it still builds on
> x86_64 Vista, as well as x86/Windows XP (on XP, PRINTF_HAS_LONG_LONG
> is undefined).

I think it is a sub-optimal solution to have just support for Vista64, but 
not for XP. On a second thought, I remembered, that bfd does things right 
;) There is the macro sprintf_vma in bfd.h, which handles things pretty 
well and can be used here in utils.c, too.


Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

Re: [RFC] convert a host address to a string

[Joel Brobecker]

> I think it is a sub-optimal solution to have just support for Vista64, but 
> not for XP.

To me, the question is not about whether to support XP64 or not.
I agree it would be nice to support XP64 as well.  It's about who
has the time and energy to drive the discussion to find an accepted
solution.  I decided to drop XP64, because it's not in the list of
things I'm interested in while I'm sensing that it's going to take
a bit of effort to reach a consensus. You already made a very nice
contribution in the coff/pe reader, why not send another patch to
further improve host_address_to_string for XP64?

> On a second thought, I remembered, that bfd does things right ;) There
> is the macro sprintf_vma in bfd.h, which handles things pretty well
> and can be used here in utils.c, too.

The problem with that routine is that it is designed to print target
addresses, not host addresses.

-- 
Joel

Re: [RFC] convert a host address to a string

[Kai Tietz]

[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]

2009/1/10 Joel Brobecker <brobecker@adacore.com>:
>> I think it is a sub-optimal solution to have just support for Vista64, but
>> not for XP.
>
> To me, the question is not about whether to support XP64 or not.
> I agree it would be nice to support XP64 as well.  It's about who
> has the time and energy to drive the discussion to find an accepted
> solution.  I decided to drop XP64, because it's not in the list of
> things I'm interested in while I'm sensing that it's going to take
> a bit of effort to reach a consensus. You already made a very nice
> contribution in the coff/pe reader, why not send another patch to
> further improve host_address_to_string for XP64?
>
>> On a second thought, I remembered, that bfd does things right ;) There
>> is the macro sprintf_vma in bfd.h, which handles things pretty well
>> and can be used here in utils.c, too.
>
> The problem with that routine is that it is designed to print target
> addresses, not host addresses.
>
> --
> Joel
>

ok, so I sugget the following patch instead. It is able to generate
addresses for XP64 without the use of any vendor specific printf
formatters, and uses for targets where sizeof(long) == sizeof(void*)
the long variant.

Cheers,
Kai


-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: host_addr_to_string.diff --]
[-- Type: text/x-c; name=host_addr_to_string.diff, Size: 2815 bytes --]

Index: src/gdb/utils.c
===================================================================
--- src.orig/gdb/utils.c
+++ src/gdb/utils.c
@@ -3071,11 +3071,54 @@ host_address_to_string (const void *addr
 {
   char *str = get_cell ();
 
-  /* We could use the %p conversion specifier to sprintf if we had any
-     way of knowing whether this host supports it.  But the following
-     should work on the Alpha and on 32 bit machines.  */
-  sprintf (str, "0x%lx", (unsigned long) addr);
+  /* We do not use the %p conversion specifier, because the resulting
+     image can vary from implementation to implementation.  For instance,
+     some implementations format the pointer value with a leading "0x"
+     whereas others don't (Solaris, for instance).  Also, it is unspecified
+     whether the alphabetical digits are printed using uppercase letters
+     or not (in GDB, we want lowercase).
+
+     So we use the %x type instead.  This, however, introduces
+     a couple of issues:
+
+       1. The %x type expects an integer value, not a pointer.
+          So we first need to cast our pointer to an integer type
+          whose size is identical to the size of our pointer.
+          We use uintptr_t for that.
+
+       2. The %x type alone expects and int, which is not always
+          large enough to hold an address.  Usually, type "long"
+          has the same size as pointers, but certain ABIs define
+          the size of pointers to be larger than the size of long
+          (64bit Windows is one such example).
+
+          So, to be certain to have a type that's large enough
+          to hold an address, we need to use "long long".  But
+          the trick is that not all printf implementations support
+          the "ll" modifier.  On those platforms where the "ll"
+          modifier is not available, we'll assume that type "long"
+          can be used to print an address.
+
+          To make sure that the type we pass to sprintf matches
+          the type we specified in our expression, we perform
+          a second cast to "unsigned long long" if we used "%llx",
+          or "unsigned long" if we used "%lx".  */
+
+#if defined(PRINTF_HAS_LONG_LONG) && BITSIZEOF_SIZE_T == 64 && \
+  SIZEOF_LONG == 4
+  sprintf (str, "0x%llx", (unsigned long long) (uintptr_t) addr);
+#elif BITSIZEOF_SIZE_T == 64 && SIZEOF_LONG == 4
+  unsigned long long val = (unsigned long) (uintptr_t) addr;
+  if ((val & ~0xffffffffull) != 0)
+    sprintf (str, "0x%lx%08lx",
+             (unsigned long) (val >> 32), (unsigned long) val);
+  else
+    sprintf (str, "0x%lx", (unsigned long) (uintptr_t) val);
+#else
+  sprintf (str, "0x%lx", (unsigned long) (uintptr_t) addr);
+#endif
   return str;
+  BITSIZEOF_SIZE_T SIZEOF_LONG
 }
 
 char *

Re: [RFC] convert a host address to a string

[Kai Tietz]

[-- Attachment #1: Type: text/plain, Size: 454 bytes --]

2009/1/10 Kai Tietz <ktietz70@googlemail.com>:
> ok, so I sugget the following patch instead. It is able to generate
> addresses for XP64 without the use of any vendor specific printf
> formatters, and uses for targets where sizeof(long) == sizeof(void*)
> the long variant.

Sorry, I just detected a wrong type cast.

Cheers,
Kai
-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: host_addr_to_string.diff --]
[-- Type: text/x-c; name=host_addr_to_string.diff, Size: 2820 bytes --]

Index: src/gdb/utils.c
===================================================================
--- src.orig/gdb/utils.c
+++ src/gdb/utils.c
@@ -3071,11 +3071,54 @@ host_address_to_string (const void *addr
 {
   char *str = get_cell ();
 
-  /* We could use the %p conversion specifier to sprintf if we had any
-     way of knowing whether this host supports it.  But the following
-     should work on the Alpha and on 32 bit machines.  */
-  sprintf (str, "0x%lx", (unsigned long) addr);
+  /* We do not use the %p conversion specifier, because the resulting
+     image can vary from implementation to implementation.  For instance,
+     some implementations format the pointer value with a leading "0x"
+     whereas others don't (Solaris, for instance).  Also, it is unspecified
+     whether the alphabetical digits are printed using uppercase letters
+     or not (in GDB, we want lowercase).
+
+     So we use the %x type instead.  This, however, introduces
+     a couple of issues:
+
+       1. The %x type expects an integer value, not a pointer.
+          So we first need to cast our pointer to an integer type
+          whose size is identical to the size of our pointer.
+          We use uintptr_t for that.
+
+       2. The %x type alone expects and int, which is not always
+          large enough to hold an address.  Usually, type "long"
+          has the same size as pointers, but certain ABIs define
+          the size of pointers to be larger than the size of long
+          (64bit Windows is one such example).
+
+          So, to be certain to have a type that's large enough
+          to hold an address, we need to use "long long".  But
+          the trick is that not all printf implementations support
+          the "ll" modifier.  On those platforms where the "ll"
+          modifier is not available, we'll assume that type "long"
+          can be used to print an address.
+
+          To make sure that the type we pass to sprintf matches
+          the type we specified in our expression, we perform
+          a second cast to "unsigned long long" if we used "%llx",
+          or "unsigned long" if we used "%lx".  */
+
+#if defined(PRINTF_HAS_LONG_LONG) && BITSIZEOF_SIZE_T == 64 && \
+  SIZEOF_LONG == 4
+  sprintf (str, "0x%llx", (unsigned long long) (uintptr_t) addr);
+#elif BITSIZEOF_SIZE_T == 64 && SIZEOF_LONG == 4
+  unsigned long long val = (unsigned long long) (uintptr_t) addr;
+  if ((val & ~0xffffffffull) != 0)
+    sprintf (str, "0x%lx%08lx",
+             (unsigned long) (val >> 32), (unsigned long) val);
+  else
+    sprintf (str, "0x%lx", (unsigned long) (uintptr_t) val);
+#else
+  sprintf (str, "0x%lx", (unsigned long) (uintptr_t) addr);
+#endif
   return str;
+  BITSIZEOF_SIZE_T SIZEOF_LONG
 }
 
 char *

Re: [RFC] convert a host address to a string

[Mark Kettenis]

> Date: Sat, 10 Jan 2009 14:30:29 +0100
> From: "Kai Tietz" <ktietz70@googlemail.com>
>
> ok, so I sugget the following patch instead. It is able to generate
> addresses for XP64 without the use of any vendor specific printf
> formatters, and uses for targets where sizeof(long) == sizeof(void*)
> the long variant.

Kai please don't send base64 encoded attachments.

> Content-Type: text/x-c; name=host_addr_to_string.diff
> Content-Transfer-Encoding: base64
> X-Attachment-Id: f_fpsbcke80
> Content-Disposition: attachment; filename=host_addr_to_string.diff
> 
> SW5kZXg6IHNyYy9nZGIvdXRpbHMuYwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
> PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBzcmMub3JpZy9nZGIvdXRpbHMu

Re: [RFC] convert a host address to a string

[Mark Kettenis]

> Date: Sat, 10 Jan 2009 14:30:29 +0100
> From: "Kai Tietz" <ktietz70@googlemail.com>
> 
> ok, so I sugget the following patch instead. It is able to generate
> addresses for XP64 without the use of any vendor specific printf
> formatters, and uses for targets where sizeof(long) == sizeof(void*)
> the long variant.

+#if defined(PRINTF_HAS_LONG_LONG) && BITSIZEOF_SIZE_T == 64 && \
+  SIZEOF_LONG == 4
+  sprintf (str, "0x%llx", (unsigned long long) (uintptr_t) addr);
+#elif BITSIZEOF_SIZE_T == 64 && SIZEOF_LONG == 4
+  unsigned long long val = (unsigned long) (uintptr_t) addr;
+  if ((val & ~0xffffffffull) != 0)
+    sprintf (str, "0x%lx%08lx",
+             (unsigned long) (val >> 32), (unsigned long) val);
+  else
+    sprintf (str, "0x%lx", (unsigned long) (uintptr_t) val);
+#else
+  sprintf (str, "0x%lx", (unsigned long) (uintptr_t) addr);
+#endif
   return str;
+  BITSIZEOF_SIZE_T SIZEOF_LONG

This is madness.  If you go this route please do a simple

const char *
host_address_to_string (const void *addr)
{
  return phex_nz ((uintptr_t)addr, sizeof(addr));
}

Re: [RFC] convert a host address to a string

[Kai Tietz]

2009/1/10 Mark Kettenis <mark.kettenis@xs4all.nl>:
>> Date: Sat, 10 Jan 2009 14:30:29 +0100
>> From: "Kai Tietz" <ktietz70@googlemail.com>
>>
>> ok, so I sugget the following patch instead. It is able to generate
>> addresses for XP64 without the use of any vendor specific printf
>> formatters, and uses for targets where sizeof(long) == sizeof(void*)
>> the long variant.
>
> +#if defined(PRINTF_HAS_LONG_LONG) && BITSIZEOF_SIZE_T == 64 && \
> +  SIZEOF_LONG == 4
> +  sprintf (str, "0x%llx", (unsigned long long) (uintptr_t) addr);
> +#elif BITSIZEOF_SIZE_T == 64 && SIZEOF_LONG == 4
> +  unsigned long long val = (unsigned long) (uintptr_t) addr;
> +  if ((val & ~0xffffffffull) != 0)
> +    sprintf (str, "0x%lx%08lx",
> +             (unsigned long) (val >> 32), (unsigned long) val);
> +  else
> +    sprintf (str, "0x%lx", (unsigned long) (uintptr_t) val);
> +#else
> +  sprintf (str, "0x%lx", (unsigned long) (uintptr_t) addr);
> +#endif
>   return str;
> +  BITSIZEOF_SIZE_T SIZEOF_LONG
>
> This is madness.  If you go this route please do a simple
>
> const char *
> host_address_to_string (const void *addr)
> {
>  return phex_nz ((uintptr_t)addr, sizeof(addr));
> }
>

hmm, well, but a "0x" has to be as prefix here, isn't it?

Kai
-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

Re: [RFC] convert a host address to a string

[Mark Kettenis]

> Date: Sat, 10 Jan 2009 15:14:39 +0100
> From: "Kai Tietz" <ktietz70@googlemail.com>
> 
> 2009/1/10 Mark Kettenis <mark.kettenis@xs4all.nl>:
> >> Date: Sat, 10 Jan 2009 14:30:29 +0100
> >> From: "Kai Tietz" <ktietz70@googlemail.com>
> >>
> >> ok, so I sugget the following patch instead. It is able to generate
> >> addresses for XP64 without the use of any vendor specific printf
> >> formatters, and uses for targets where sizeof(long) == sizeof(void*)
> >> the long variant.
> >
> > +#if defined(PRINTF_HAS_LONG_LONG) && BITSIZEOF_SIZE_T == 64 && \
> > +  SIZEOF_LONG == 4
> > +  sprintf (str, "0x%llx", (unsigned long long) (uintptr_t) addr);
> > +#elif BITSIZEOF_SIZE_T == 64 && SIZEOF_LONG == 4
> > +  unsigned long long val = (unsigned long) (uintptr_t) addr;
> > +  if ((val & ~0xffffffffull) != 0)
> > +    sprintf (str, "0x%lx%08lx",
> > +             (unsigned long) (val >> 32), (unsigned long) val);
> > +  else
> > +    sprintf (str, "0x%lx", (unsigned long) (uintptr_t) val);
> > +#else
> > +  sprintf (str, "0x%lx", (unsigned long) (uintptr_t) addr);
> > +#endif
> >   return str;
> > +  BITSIZEOF_SIZE_T SIZEOF_LONG
> >
> > This is madness.  If you go this route please do a simple
> >
> > const char *
> > host_address_to_string (const void *addr)
> > {
> >  return phex_nz ((uintptr_t)addr, sizeof(addr));
> > }
> >
> 
> hmm, well, but a "0x" has to be as prefix here, isn't it?

You're right.  So something like:

{
  char *result = get_cell ();
  xsnprintf (result, CELLSIZE, "0x%s", phex_nz((uintptr_t)addr, sizeof(addr)));
  return result;
}

perhaps?

Re: [RFC] convert a host address to a string

[Kai Tietz]

2009/1/10 Mark Kettenis <mark.kettenis@xs4all.nl>:
>> Date: Sat, 10 Jan 2009 15:14:39 +0100
>> From: "Kai Tietz" <ktietz70@googlemail.com>
>>
>> 2009/1/10 Mark Kettenis <mark.kettenis@xs4all.nl>:
>> >> Date: Sat, 10 Jan 2009 14:30:29 +0100
>> >> From: "Kai Tietz" <ktietz70@googlemail.com>
>> >>
>> >> ok, so I sugget the following patch instead. It is able to generate
>> >> addresses for XP64 without the use of any vendor specific printf
>> >> formatters, and uses for targets where sizeof(long) == sizeof(void*)
>> >> the long variant.
>> >
>> > +#if defined(PRINTF_HAS_LONG_LONG) && BITSIZEOF_SIZE_T == 64 && \
>> > +  SIZEOF_LONG == 4
>> > +  sprintf (str, "0x%llx", (unsigned long long) (uintptr_t) addr);
>> > +#elif BITSIZEOF_SIZE_T == 64 && SIZEOF_LONG == 4
>> > +  unsigned long long val = (unsigned long) (uintptr_t) addr;
>> > +  if ((val & ~0xffffffffull) != 0)
>> > +    sprintf (str, "0x%lx%08lx",
>> > +             (unsigned long) (val >> 32), (unsigned long) val);
>> > +  else
>> > +    sprintf (str, "0x%lx", (unsigned long) (uintptr_t) val);
>> > +#else
>> > +  sprintf (str, "0x%lx", (unsigned long) (uintptr_t) addr);
>> > +#endif
>> >   return str;
>> > +  BITSIZEOF_SIZE_T SIZEOF_LONG
>> >
>> > This is madness.  If you go this route please do a simple
>> >
>> > const char *
>> > host_address_to_string (const void *addr)
>> > {
>> >  return phex_nz ((uintptr_t)addr, sizeof(addr));
>> > }
>> >
>>
>> hmm, well, but a "0x" has to be as prefix here, isn't it?
>
> You're right.  So something like:
>
> {
>  char *result = get_cell ();
>  xsnprintf (result, CELLSIZE, "0x%s", phex_nz((uintptr_t)addr, sizeof(addr)));
>  return result;
> }
>
> perhaps?
>


Looks fine to me.

Thanks,
Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

Re: [RFC] convert a host address to a string

[Joel Brobecker]

Mark,

> You're right.  So something like:
> 
> {
>   char *result = get_cell ();
>   xsnprintf (result, CELLSIZE, "0x%s", phex_nz((uintptr_t)addr, sizeof(addr)));
>   return result;
> }
> 
> perhaps?

Thanks for the suggestion.
Let me know if you'd like me to test your patch and then commit it
for you...

-- 
Joel

Re: [RFC] convert a host address to a string

[Mark Kettenis]

> Date: Sun, 11 Jan 2009 17:30:42 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> 
> Mark,
> 
> > You're right.  So something like:
> > 
> > {
> >   char *result = get_cell ();
> >   xsnprintf (result, CELLSIZE, "0x%s", phex_nz((uintptr_t)addr, sizeof(addr)));
> >   return result;
> > }
> > 
> > perhaps?
> 
> Thanks for the suggestion.
> Let me know if you'd like me to test your patch and then commit it
> for you...

Please do.  I'm glad I can say that I'm not running Windows on any
machine I own, so I have no way to test.

Re: [RFC] convert a host address to a string

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 371 bytes --]

Here is the patch that I ended checkin in...

2009-01-13  Mark Kettenis  <kettenis@gnu.org>

        * utils.c (host_address_to_string): Reimplement in a way that
        avoids the cast of the address to long.

Tested on x86-linux and x86-windows.  I also re-built the debugger
on Vista64, and used "set debug arch" to verify that it can print
host addresses.

-- 
Joel

[-- Attachment #2: utils.c.diff --]
[-- Type: text/plain, Size: 705 bytes --]

Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.203
diff -u -p -r1.203 utils.c
--- utils.c	7 Jan 2009 12:11:07 -0000	1.203
+++ utils.c	13 Jan 2009 12:04:25 -0000
@@ -3071,10 +3071,7 @@ host_address_to_string (const void *addr
 {
   char *str = get_cell ();
 
-  /* We could use the %p conversion specifier to sprintf if we had any
-     way of knowing whether this host supports it.  But the following
-     should work on the Alpha and on 32 bit machines.  */
-  sprintf (str, "0x%lx", (unsigned long) addr);
+  xsnprintf (str, CELLSIZE, "0x%s", phex_nz ((uintptr_t) addr, sizeof (addr)));
   return str;
 }