[PATCH][gdb] Fix s390x -m31 build

[PATCH][gdb] Fix s390x -m31 build

[Tom de Vries]

Hi,

When building gdb on s390x with -m31, we run into this Wformat
warning (which Werror turns into an error):
...
gdb/dwarf2read.c: In function \
  'void create_addrmap_from_aranges(dwarf2_per_objfile*, \
                                    dwarf2_section_info*)':
gdb/dwarf2read.c:3277:22: error: format '%zu' expects argument of type \
  'size_t', but argument 3 has type 'int' [-Werror=format=]
    warning (_("Section .debug_aranges in %s entry at offset %zu "
...

The Wformat warning is triggered in this statement:
...
          warning (_("Section .debug_aranges in %s entry at offset %zu "
                     "length %s exceeds section length %s, "
                     "ignoring .debug_aranges."),
                   objfile_name (objfile), entry_addr - section->buffer,
                   plongest (bytes_read + entry_length),
                   pulongest (section->size));
...
where 'entry_addr - section->buffer' is of type ptrdiff_t and '%zu' prints an
unsigned with the same size as size_t/ssize_t.

On s390x with -m31, we have:
- size_t   : unsigned long int (32-bit)
- ptrdiff_t: int               (32-bit)

Wformat warns against this because even though long int and int have the same
size, the types are not compatible.

[ The Wformat warning is to similar to what we would get for x86_64 -m32
(where long and int are also the same size) and:
...
int i;
printf ("%ld", i);
... ]

Fix this by changing the length modifier from 'z' to the ptrdiff_t length
modifier 't', and change the conversion specifier from 'u' to the more
appropriate 'd'.

Build and reg-tested on x86_64.
Build on s390x-linux with -m31.

OK for trunk?

Thanks,
- Tom

[gdb] Fix s390x -m31 build

gdb/ChangeLog:

2019-06-20  Tom de Vries  <tdevries@suse.de>

	* dwarf2read.c (create_addrmap_from_aranges)
	(read_debug_names_from_section): Print ptrdiff_t using '%td' instead
	of '%zu'.

---
 gdb/dwarf2read.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 9cf513b582..0fcd0defce 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3246,7 +3246,7 @@ create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
       const uint8_t offset_size = dwarf5_is_dwarf64 ? 8 : 4;
       if (addr + entry_length > section->buffer + section->size)
 	{
-	  warning (_("Section .debug_aranges in %s entry at offset %zu "
+	  warning (_("Section .debug_aranges in %s entry at offset %td "
 	             "length %s exceeds section length %s, "
 		     "ignoring .debug_aranges."),
 		   objfile_name (objfile), entry_addr - section->buffer,
@@ -3260,7 +3260,7 @@ create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
       addr += 2;
       if (version != 2)
 	{
-	  warning (_("Section .debug_aranges in %s entry at offset %zu "
+	  warning (_("Section .debug_aranges in %s entry at offset %tu "
 		     "has unsupported version %d, ignoring .debug_aranges."),
 		   objfile_name (objfile), entry_addr - section->buffer,
 		   version);
@@ -3274,7 +3274,7 @@ create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
 	= debug_info_offset_to_per_cu.find (sect_offset (debug_info_offset));
       if (per_cu_it == debug_info_offset_to_per_cu.cend ())
 	{
-	  warning (_("Section .debug_aranges in %s entry at offset %zu "
+	  warning (_("Section .debug_aranges in %s entry at offset %tu "
 		     "debug_info_offset %s does not exists, "
 		     "ignoring .debug_aranges."),
 		   objfile_name (objfile), entry_addr - section->buffer,
@@ -3286,7 +3286,7 @@ create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
       const uint8_t address_size = *addr++;
       if (address_size < 1 || address_size > 8)
 	{
-	  warning (_("Section .debug_aranges in %s entry at offset %zu "
+	  warning (_("Section .debug_aranges in %s entry at offset %tu "
 		     "address_size %u is invalid, ignoring .debug_aranges."),
 		   objfile_name (objfile), entry_addr - section->buffer,
 		   address_size);
@@ -3296,7 +3296,7 @@ create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
       const uint8_t segment_selector_size = *addr++;
       if (segment_selector_size != 0)
 	{
-	  warning (_("Section .debug_aranges in %s entry at offset %zu "
+	  warning (_("Section .debug_aranges in %s entry at offset %tu "
 		     "segment_selector_size %u is not supported, "
 		     "ignoring .debug_aranges."),
 		   objfile_name (objfile), entry_addr - section->buffer,
@@ -3312,7 +3312,7 @@ create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
            padding > 0; padding--)
 	if (*addr++ != 0)
 	  {
-	    warning (_("Section .debug_aranges in %s entry at offset %zu "
+	    warning (_("Section .debug_aranges in %s entry at offset %tu "
 		       "padding is not zero, ignoring .debug_aranges."),
 		     objfile_name (objfile), entry_addr - section->buffer);
 	    return;
@@ -3322,7 +3322,7 @@ create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
 	{
 	  if (addr + 2 * address_size > entry_end)
 	    {
-	      warning (_("Section .debug_aranges in %s entry at offset %zu "
+	      warning (_("Section .debug_aranges in %s entry at offset %tu "
 			 "address list is not properly terminated, "
 			 "ignoring .debug_aranges."),
 		       objfile_name (objfile), entry_addr - section->buffer);
@@ -5537,7 +5537,7 @@ read_debug_names_from_section (struct objfile *objfile,
   if (addr != abbrev_table_start + abbrev_table_size)
     {
       warning (_("Section .debug_names in %s has abbreviation_table "
-                 "of size %zu vs. written as %u, ignoring .debug_names."),
+		 "of size %tu vs. written as %u, ignoring .debug_names."),
 	       filename, addr - abbrev_table_start, abbrev_table_size);
       return false;
     }

Re: [PATCH][gdb] Fix s390x -m31 build

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Fix this by changing the length modifier from 'z' to the ptrdiff_t length
Tom> modifier 't', and change the conversion specifier from 'u' to the more
Tom> appropriate 'd'.

gdb probably shouldn't be using 'z' anyway.  IIRC it causes problems on
Windows.

How portable is %td?  I do not know.  Maybe check the gnulib manual and
see what they have to say about it?  Anyway, this is my main concern
with this patch.

The standard gdb approach here is to use plongest / pulongest in
conjunction with %s.  That avoids any problems.

thanks,
Tom

Re: [PATCH][gdb] Fix s390x -m31 build

[Tom de Vries]

On 21-06-19 18:05, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> Fix this by changing the length modifier from 'z' to the ptrdiff_t length
> Tom> modifier 't', and change the conversion specifier from 'u' to the more
> Tom> appropriate 'd'.
> 
> gdb probably shouldn't be using 'z' anyway.  IIRC it causes problems on
> Windows.
> 
> How portable is %td?  I do not know.  Maybe check the gnulib manual and
> see what they have to say about it?  Anyway, this is my main concern
> with this patch.
> 

I found here ( https://www.gnu.org/software/gnulib/manual/gnulib.html ):
...
Portability problems fixed by Gnulib module fprintf-posix:

    This function does not support size specifiers as in C99 (hh, ll, j,
t, z) on some platforms: AIX 5.1, HP-UX 11.23, IRIX 6.5, OSF/1 5.1,
Solaris 9, Cygwin 1.5.24, mingw, MSVC 14, BeOS.
...

> The standard gdb approach here is to use plongest / pulongest in
> conjunction with %s.  That avoids any problems.

I'll update the patch to use that method, thanks for pointing that out.

Thanks,
- Tom

Re: [PATCH][gdb] Fix s390x -m31 build

[Tom de Vries]

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

On 21-06-19 19:07, Tom de Vries wrote:
> On 21-06-19 18:05, Tom Tromey wrote:
>>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>>
>> Tom> Fix this by changing the length modifier from 'z' to the ptrdiff_t length
>> Tom> modifier 't', and change the conversion specifier from 'u' to the more
>> Tom> appropriate 'd'.
>>
>> gdb probably shouldn't be using 'z' anyway.  IIRC it causes problems on
>> Windows.
>>
>> How portable is %td?  I do not know.  Maybe check the gnulib manual and
>> see what they have to say about it?  Anyway, this is my main concern
>> with this patch.
>>
> 
> I found here ( https://www.gnu.org/software/gnulib/manual/gnulib.html ):
> ...
> Portability problems fixed by Gnulib module fprintf-posix:
> 
>     This function does not support size specifiers as in C99 (hh, ll, j,
> t, z) on some platforms: AIX 5.1, HP-UX 11.23, IRIX 6.5, OSF/1 5.1,
> Solaris 9, Cygwin 1.5.24, mingw, MSVC 14, BeOS.
> ...
> 
>> The standard gdb approach here is to use plongest / pulongest in
>> conjunction with %s.  That avoids any problems.
> 
> I'll update the patch to use that method, thanks for pointing that out.

Committed as attached.

Thanks,
- Tom




[-- Attachment #2: 0001-gdb-Fix-s390x-m31-build.patch --]
[-- Type: text/x-patch, Size: 6638 bytes --]

[gdb] Fix s390x -m31 build

When building gdb on s390x with -m31, we run into this Wformat
warning (which Werror turns into an error):
...
gdb/dwarf2read.c: In function \
  'void create_addrmap_from_aranges(dwarf2_per_objfile*, \
                                    dwarf2_section_info*)':
gdb/dwarf2read.c:3277:22: error: format '%zu' expects argument of type \
  'size_t', but argument 3 has type 'int' [-Werror=format=]
    warning (_("Section .debug_aranges in %s entry at offset %zu "
...

The Wformat warning is triggered in this statement:
...
          warning (_("Section .debug_aranges in %s entry at offset %zu "
                     "length %s exceeds section length %s, "
                     "ignoring .debug_aranges."),
                   objfile_name (objfile), entry_addr - section->buffer,
                   plongest (bytes_read + entry_length),
                   pulongest (section->size));
...
where 'entry_addr - section->buffer' is of type ptrdiff_t and '%zu' prints an
unsigned with the same size as size_t/ssize_t.

On s390x with -m31, we have:
- size_t   : unsigned long int (32-bit)
- ptrdiff_t: int               (32-bit)

Wformat warns against this because even though long int and int have the same
size, the types are not compatible.

[ The Wformat warning is to similar to what we would get for x86_64 -m32
(where long and int are also the same size) and:
...
int i;
printf ("%ld", i);
... ]

Fix this by using '%s' and plongest instead of '%zu' to print ptrdiff_t.

Build and reg-tested on x86_64.

gdb/ChangeLog:

2019-06-20  Tom de Vries  <tdevries@suse.de>

	* dwarf2read.c (create_addrmap_from_aranges)
	(read_debug_names_from_section): Print ptrdiff_t using '%s' and plongest
	instead of '%zu'.

---
 gdb/dwarf2read.c | 42 ++++++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index fccbd05e2d..85f2b1dfc4 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3229,10 +3229,11 @@ create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
       const uint8_t offset_size = dwarf5_is_dwarf64 ? 8 : 4;
       if (addr + entry_length > section->buffer + section->size)
 	{
-	  warning (_("Section .debug_aranges in %s entry at offset %zu "
+	  warning (_("Section .debug_aranges in %s entry at offset %s "
 	             "length %s exceeds section length %s, "
 		     "ignoring .debug_aranges."),
-		   objfile_name (objfile), entry_addr - section->buffer,
+		   objfile_name (objfile),
+		   plongest (entry_addr - section->buffer),
 		   plongest (bytes_read + entry_length),
 		   pulongest (section->size));
 	  return;
@@ -3243,10 +3244,10 @@ create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
       addr += 2;
       if (version != 2)
 	{
-	  warning (_("Section .debug_aranges in %s entry at offset %zu "
+	  warning (_("Section .debug_aranges in %s entry at offset %s "
 		     "has unsupported version %d, ignoring .debug_aranges."),
-		   objfile_name (objfile), entry_addr - section->buffer,
-		   version);
+		   objfile_name (objfile),
+		   plongest (entry_addr - section->buffer), version);
 	  return;
 	}
 
@@ -3257,10 +3258,11 @@ create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
 	= debug_info_offset_to_per_cu.find (sect_offset (debug_info_offset));
       if (per_cu_it == debug_info_offset_to_per_cu.cend ())
 	{
-	  warning (_("Section .debug_aranges in %s entry at offset %zu "
+	  warning (_("Section .debug_aranges in %s entry at offset %s "
 		     "debug_info_offset %s does not exists, "
 		     "ignoring .debug_aranges."),
-		   objfile_name (objfile), entry_addr - section->buffer,
+		   objfile_name (objfile),
+		   plongest (entry_addr - section->buffer),
 		   pulongest (debug_info_offset));
 	  return;
 	}
@@ -3269,20 +3271,21 @@ create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
       const uint8_t address_size = *addr++;
       if (address_size < 1 || address_size > 8)
 	{
-	  warning (_("Section .debug_aranges in %s entry at offset %zu "
+	  warning (_("Section .debug_aranges in %s entry at offset %s "
 		     "address_size %u is invalid, ignoring .debug_aranges."),
-		   objfile_name (objfile), entry_addr - section->buffer,
-		   address_size);
+		   objfile_name (objfile),
+		   plongest (entry_addr - section->buffer), address_size);
 	  return;
 	}
 
       const uint8_t segment_selector_size = *addr++;
       if (segment_selector_size != 0)
 	{
-	  warning (_("Section .debug_aranges in %s entry at offset %zu "
+	  warning (_("Section .debug_aranges in %s entry at offset %s "
 		     "segment_selector_size %u is not supported, "
 		     "ignoring .debug_aranges."),
-		   objfile_name (objfile), entry_addr - section->buffer,
+		   objfile_name (objfile),
+		   plongest (entry_addr - section->buffer),
 		   segment_selector_size);
 	  return;
 	}
@@ -3295,9 +3298,10 @@ create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
            padding > 0; padding--)
 	if (*addr++ != 0)
 	  {
-	    warning (_("Section .debug_aranges in %s entry at offset %zu "
+	    warning (_("Section .debug_aranges in %s entry at offset %s "
 		       "padding is not zero, ignoring .debug_aranges."),
-		     objfile_name (objfile), entry_addr - section->buffer);
+		     objfile_name (objfile),
+		     plongest (entry_addr - section->buffer));
 	    return;
 	  }
 
@@ -3305,10 +3309,11 @@ create_addrmap_from_aranges (struct dwarf2_per_objfile *dwarf2_per_objfile,
 	{
 	  if (addr + 2 * address_size > entry_end)
 	    {
-	      warning (_("Section .debug_aranges in %s entry at offset %zu "
+	      warning (_("Section .debug_aranges in %s entry at offset %s "
 			 "address list is not properly terminated, "
 			 "ignoring .debug_aranges."),
-		       objfile_name (objfile), entry_addr - section->buffer);
+		       objfile_name (objfile),
+		       plongest (entry_addr - section->buffer));
 	      return;
 	    }
 	  ULONGEST start = extract_unsigned_integer (addr, address_size,
@@ -5517,8 +5522,9 @@ read_debug_names_from_section (struct objfile *objfile,
   if (addr != abbrev_table_start + abbrev_table_size)
     {
       warning (_("Section .debug_names in %s has abbreviation_table "
-                 "of size %zu vs. written as %u, ignoring .debug_names."),
-	       filename, addr - abbrev_table_start, abbrev_table_size);
+		 "of size %s vs. written as %u, ignoring .debug_names."),
+	       filename, plongest (addr - abbrev_table_start),
+	       abbrev_table_size);
       return false;
     }
   map.entry_pool = addr;