[PR,gdb/24351] Fix wrong format specification in display_selector()

  On 17/03/2019 18:44, Simon Marchi wrote:
> On 2019-03-17 05:48, Владимир Мартьянов wrote:
>> There are a wrong format strings in function display_selector() in
>> file windows-nat.c. This leads to build error using Cygwin on Windows.
>> LDT_ENTRY.HighWord is a DWORD, which is unsigned long int, so the
>> format specification should be for long int, not simply int.
>>
>> gdb/ChangeLog:
>> 2019-03-17  Vladimir Martyanov  <vilgeforce@gmail.com>
>>
>>     PR gdb/24351
>>     * windows-nat.c (display_selector): Format specifications fixed
>>
>> Patch and changelog files are attached
> 
> Thanks, this LGTM.  I altavista'ed and it looks like this is the right 
> thing to do to print DWORDs.  It also builds fine with 
> i686-w64-mingw32-gcc and x86_64-w64-mingw32-gcc on Linux.

Thanks for looking at this.  But did you test this with 
x86_64-pc-cygwin?  It fails to build for me:

> ../../gdb/windows-nat.c: In function ‘int display_selector(HANDLE, DWORD)’:
> ../../gdb/windows-nat.c:1099:65: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘unsigned int’ [-Werror=format=]
>     printf_filtered ("Unknown type 0x%lx",info.HighWord.Bits.Type);
>                                           ~~~~~~~~~~~~~~~~~~~~~~~^
> ../../gdb/windows-nat.c:1106:74: error: format ‘%ld’ expects argument of type ‘long int’, but argument 2 has type ‘unsigned int’ [-Werror=format=]
>        printf_filtered ("Priviledge level = %ld. ", info.HighWord.Bits.Dpl);

I believe this is because, unfortunately, DWORD is not long on 64-bit 
Cygwin, because that is LP64 (See [1]).

I think the only portable way to write this (short of introducing 
inttypes.h PRI_-style macros) is to explicitly cast DWORD type values to 
unsigned long or unsigned int, and use the appropriate format (e.g. see 
[2] for a similar patch I wrote for xserver)

Patch attached.

[1] https://cygwin.com/faq.html#faq.programming.64bitporting
[2] 
https://cgit.freedesktop.org/xorg/xserver/commit/?id=aa83c61f510121da20b56e8f7de700193f7d16b5
From e4aba2d5248cd8d2390e9028aa0131174c1cdb6f Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.turney@dronecode.org.uk>
Date: Thu, 28 Mar 2019 14:02:25 +0000
Subject: [PATCH] Fix format specification in display_selector() (again)

DWORD type is not a long on 64-bit Cygwin, because that it is LP64.
Explicitly cast DWORD values to unsigned long and use an appropriate
format.

gdb/ChangeLog:

2019-03-28  Jon Turney  <jon.turney@dronecode.org.uk>

	* windows-nat.c (display_selector): Fixed format specifications
	for 64-bit Cygwin.
---
 gdb/ChangeLog     | 5 +++++
 gdb/windows-nat.c | 6 ++++--
 2 files changed, 9 insertions(+), 2 deletions(-)
  

чт, 28 мар. 2019 г. в 18:46, Jon Turney <jon.turney@dronecode.org.uk>:

> Thanks for looking at this.  But did you test this with
> x86_64-pc-cygwin?  It fails to build for me:

No, I didn't. I'm new to Cygwin and didn't know about this feature.

I tested you patch with Cygwin-x86 and gdb was built without errors.
  

On 2019-03-28 11:46, Jon Turney wrote:
> On 17/03/2019 18:44, Simon Marchi wrote:
>> On 2019-03-17 05:48, Владимир Мартьянов wrote:
>>> There are a wrong format strings in function display_selector() in
>>> file windows-nat.c. This leads to build error using Cygwin on 
>>> Windows.
>>> LDT_ENTRY.HighWord is a DWORD, which is unsigned long int, so the
>>> format specification should be for long int, not simply int.
>>> 
>>> gdb/ChangeLog:
>>> 2019-03-17  Vladimir Martyanov  <vilgeforce@gmail.com>
>>> 
>>>     PR gdb/24351
>>>     * windows-nat.c (display_selector): Format specifications fixed
>>> 
>>> Patch and changelog files are attached
>> 
>> Thanks, this LGTM.  I altavista'ed and it looks like this is the right 
>> thing to do to print DWORDs.  It also builds fine with 
>> i686-w64-mingw32-gcc and x86_64-w64-mingw32-gcc on Linux.
> 
> Thanks for looking at this.  But did you test this with
> x86_64-pc-cygwin?  It fails to build for me:
> 
>> ../../gdb/windows-nat.c: In function ‘int display_selector(HANDLE, 
>> DWORD)’:
>> ../../gdb/windows-nat.c:1099:65: error: format ‘%lx’ expects argument 
>> of type ‘long unsigned int’, but argument 2 has type ‘unsigned int’ 
>> [-Werror=format=]
>>     printf_filtered ("Unknown type 0x%lx",info.HighWord.Bits.Type);
>>                                           ~~~~~~~~~~~~~~~~~~~~~~~^
>> ../../gdb/windows-nat.c:1106:74: error: format ‘%ld’ expects argument 
>> of type ‘long int’, but argument 2 has type ‘unsigned int’ 
>> [-Werror=format=]
>>        printf_filtered ("Priviledge level = %ld. ", 
>> info.HighWord.Bits.Dpl);
> 
> I believe this is because, unfortunately, DWORD is not long on 64-bit
> Cygwin, because that is LP64 (See [1]).
> 
> I think the only portable way to write this (short of introducing
> inttypes.h PRI_-style macros) is to explicitly cast DWORD type values
> to unsigned long or unsigned int, and use the appropriate format (e.g.
> see [2] for a similar patch I wrote for xserver)
> 
> Patch attached.
> 
> [1] https://cygwin.com/faq.html#faq.programming.64bitporting
> [2]
> https://cgit.freedesktop.org/xorg/xserver/commit/?id=aa83c61f510121da20b56e8f7de700193f7d16b5

Thanks for the patch, I pushed it.

Simon
  

>>>>> "Jon" == Jon Turney <jon.turney@dronecode.org.uk> writes:

Jon> I believe this is because, unfortunately, DWORD is not long on 64-bit
Jon> Cygwin, because that is LP64 (See [1]).

Jon> I think the only portable way to write this (short of introducing
Jon> inttypes.h PRI_-style macros) is to explicitly cast DWORD type values
Jon> to unsigned long or unsigned int, and use the appropriate format
Jon> (e.g. see [2] for a similar patch I wrote for xserver)

Just FYI, a more typical way in gdb is to use "%s" along with something
like pulongest.  What you did is fine, though, and I don't think
anything needs to be changed.

Tom