[RFA/windows] cast of address to DWORD warning (handle_unload_dll)

[RFA/windows] cast of address to DWORD warning (handle_unload_dll)

[Joel Brobecker]

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

Hi Chris,

This is one last case of a conversion of an address to an integer
type whose size doesn't match on Windows64. I missed this one previously
because the sources used by AdaCore don't produce this error message[1].

Other than that, I verified that a clean checkout of the sources
allow us to build GDB on Vista64 with no warning. What's left to do
is to add the last necessary bits to allow this port to be built
as a target only, and then announce the port in the NEWS file...

2009-11-14  Joel Brobecker  <brobecker@adacore.com>

        * windows-nat.c (handle_unload_dll): Use host_address to string
        in order to print the base address of the DLL that was unloaded.

This one is hard to test, because it's supposed to never happen.
So I tested it by, ahem, visual inspection (I learnt that expression
at my first job, where we were producing safety critical software).

Thanks,
-- 
Joel

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

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 724c18f..43b3d3d 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -761,7 +761,8 @@ handle_unload_dll (void *dummy)
 	return 1;
       }
 
-  error (_("Error: dll starting at 0x%lx not found."), (DWORD) lpBaseOfDll);
+  error (_("Error: dll starting at %s not found."),
+	 host_address_to_string (lpBaseOfDll));
 
   return 0;
 }

Re: [RFA/windows] cast of address to DWORD warning (handle_unload_dll)

[Christopher Faylor]

On Wed, Jan 14, 2009 at 07:42:02AM +0400, Joel Brobecker wrote:
>Hi Chris,
>
>This is one last case of a conversion of an address to an integer
>type whose size doesn't match on Windows64. I missed this one previously
>because the sources used by AdaCore don't produce this error message[1].
>
>Other than that, I verified that a clean checkout of the sources
>allow us to build GDB on Vista64 with no warning. What's left to do
>is to add the last necessary bits to allow this port to be built
>as a target only, and then announce the port in the NEWS file...
>
>2009-11-14  Joel Brobecker  <brobecker@adacore.com>
>
>        * windows-nat.c (handle_unload_dll): Use host_address to string
>        in order to print the base address of the DLL that was unloaded.
>
>This one is hard to test, because it's supposed to never happen.
>So I tested it by, ahem, visual inspection (I learnt that expression
>at my first job, where we were producing safety critical software).

I guess this is ok.  I just changed other similar occurrences to %p.
Maybe all of those should also be changed for hobgoblinish consistency
or this one should be %p too.

cgf

Re: [RFA/windows] cast of address to DWORD warning (handle_unload_dll)

[Joel Brobecker]

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

> >2009-11-14  Joel Brobecker  <brobecker@adacore.com>
> >
> >        * windows-nat.c (handle_unload_dll): Use host_address to string
> >        in order to print the base address of the DLL that was unloaded.
> >
> >This one is hard to test, because it's supposed to never happen.
> >So I tested it by, ahem, visual inspection (I learnt that expression
> >at my first job, where we were producing safety critical software).
> 
> I guess this is ok.  I just changed other similar occurrences to %p.
> Maybe all of those should also be changed for hobgoblinish consistency
> or this one should be %p too.

I don't mind changing the patch to using %p at all. The reason I avoided
%p is because I was still in this frame of mind that %p depends on
the implementation - to have consistent output, we have to be careful
to not use %p in the core code.  However, I just realized that it's not
necessarily a problem to do so in nat files.

I did double-check what %p does on Windows, though, and apparently it
does not output the leading 0x (but that's only a detail), but also it
prints the hexa digits in capital letters. This may be from not
being used to seeing addresses printed with capital letters, but
I find it harder to read. On the other hand, I have seen this style
being used in stack dumps printed by Windows, so this may be the
usual style on this OS.

For now, I think it's important to be consistent, so here is a new
patch that uses %p (no leading 0x).

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

        * windows-nat.c (handle_unload_dll): Use %p to print the DLL
        base address instead of casting it to DWORD.

-- 
Joel

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

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 724c18f..7e43f87 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -761,7 +761,7 @@ handle_unload_dll (void *dummy)
 	return 1;
       }
 
-  error (_("Error: dll starting at 0x%lx not found."), (DWORD) lpBaseOfDll);
+  error (_("Error: dll starting at %p not found."), lpBaseOfDll);
 
   return 0;
 }

Re: [RFA/windows] cast of address to DWORD warning (handle_unload_dll)

[Christopher Faylor]

On Wed, Jan 14, 2009 at 08:46:15AM +0400, Joel Brobecker wrote:
>> >2009-11-14  Joel Brobecker  <brobecker@adacore.com>
>> >
>> >        * windows-nat.c (handle_unload_dll): Use host_address to string
>> >        in order to print the base address of the DLL that was unloaded.
>> >
>> >This one is hard to test, because it's supposed to never happen.
>> >So I tested it by, ahem, visual inspection (I learnt that expression
>> >at my first job, where we were producing safety critical software).
>> 
>> I guess this is ok.  I just changed other similar occurrences to %p.
>> Maybe all of those should also be changed for hobgoblinish consistency
>> or this one should be %p too.
>
>I don't mind changing the patch to using %p at all. The reason I avoided
>%p is because I was still in this frame of mind that %p depends on
>the implementation - to have consistent output, we have to be careful
>to not use %p in the core code.  However, I just realized that it's not
>necessarily a problem to do so in nat files.
>
>I did double-check what %p does on Windows, though, and apparently it
>does not output the leading 0x (but that's only a detail), but also it
>prints the hexa digits in capital letters. This may be from not
>being used to seeing addresses printed with capital letters, but
>I find it harder to read. On the other hand, I have seen this style
>being used in stack dumps printed by Windows, so this may be the
>usual style on this OS.

Ugh.  I didn't know Windows behaved that way.  I use Cygwin's
implementation internally but I actually don't know what Cygwin does
with %p for the rest of the world and my PC is turned off right now.
Maybe it does the same thing.

I guess that's an argument to change this at some point but for now,
it is, of course, ok to check this in.

cgf

Re: [RFA/windows] cast of address to DWORD warning (handle_unload_dll)

[Joel Brobecker]

> Ugh.  I didn't know Windows behaved that way.  I use Cygwin's
> implementation internally but I actually don't know what Cygwin does
> with %p for the rest of the world and my PC is turned off right now.
> Maybe it does the same thing.

I didn't realize that cygwin had its own implementation of printf.
I tried on x86 XP, and cygwin does print %p as 0xdeadbeef.  The same
program, compiled with our MinGW compiler generates DEADBEEF.

> I guess that's an argument to change this at some point but for now,
> it is, of course, ok to check this in.

Thanks. Checking in the patch now...

-- 
Joel

Re: [RFA/windows] cast of address to DWORD warning (handle_unload_dll)

[Christopher Faylor]

On Wed, Jan 14, 2009 at 09:23:32AM +0400, Joel Brobecker wrote:
>> Ugh.  I didn't know Windows behaved that way.  I use Cygwin's
>> implementation internally but I actually don't know what Cygwin does
>> with %p for the rest of the world and my PC is turned off right now.
>> Maybe it does the same thing.
>
>I didn't realize that cygwin had its own implementation of printf.
>I tried on x86 XP, and cygwin does print %p as 0xdeadbeef.  The same
>program, compiled with our MinGW compiler generates DEADBEEF.

Cygwin has its own implementation of every POSIX function.  There isn't
any overlap between Windows versions of functions like printf and the
Cygwin version.  In many cases Cygwin just uses newlib, though.

cgf