Compilation warning in simple-object-xcoff.c

Compilation warning in simple-object-xcoff.c

[Eli Zaretskii]

Compiling GDB 8.0.91 with mingw.org's MinGW GCC 6.0.3 produces this
warning in libiberty:

     gcc -c -DHAVE_CONFIG_H -O2 -gdwarf-4 -g3 -D__USE_MINGW_ACCESS  -I. -I./../include   -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic  -D_GNU_SOURCE ./simple-object-xcoff.c -o simple-object-xcoff.o
     ./simple-object-xcoff.c: In function 'simple_object_xcoff_find_sections':
     ./simple-object-xcoff.c:605:25: warning: left shift count >= width of type [-Wshift-count-overflow]
	  x_scnlen = x_scnlen << 32
                         ^~

And indeed x_scnlen is declared as a 32-bit data type off_t.

I'm willing to test patches if needed.

Thanks.

Re: Compilation warning in simple-object-xcoff.c

[DJ Delorie]

I think that warning is valid - the host has a 32-bit limit to file
sizes (off_t) but it's trying to read a 64-bit offset (in that clause).
It's warning you that you won't be able to handle files as large as the
field implies.

Can we hide the warning?  Probably.  Should we?  Debatable, as long as
we want 64-bit xcoff support in 32-bit filesystems.

Otherwise, we'd need to detect off_t overflow somehow, down the slippery
slope of reporting the error to the caller...

Re: Compilation warning in simple-object-xcoff.c

[Eli Zaretskii]

> From: DJ Delorie <dj@redhat.com>
> Cc: gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> Date: Tue, 16 Jan 2018 13:00:48 -0500
> 
> 
> I think that warning is valid - the host has a 32-bit limit to file
> sizes (off_t) but it's trying to read a 64-bit offset (in that clause).
> It's warning you that you won't be able to handle files as large as the
> field implies.

If 32-bit off_t cannot handle this, then perhaps this file (or that
function) should not be compiled for a 32-bit host?

Re: Compilation warning in simple-object-xcoff.c

[DJ Delorie]

Well, it should all work fine as long as the xcoff64 file is less than 4
Gb.

And it's not the host's bit size that counts; there are usually ways to
get 64-bit file operations on 32-bit hosts.

Re: Compilation warning in simple-object-xcoff.c

[Andreas Schwab]

On Jan 16 2018, DJ Delorie <dj@redhat.com> wrote:

> And it's not the host's bit size that counts; there are usually ways to
> get 64-bit file operations on 32-bit hosts.

If ACX_LARGEFILE doesn't succeed in enabling those 64-bit file
operations (thus making off_t a 64-bit type) then you are out of luck
(or AC_SYS_LARGEFILE doesn't support your host yet).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Compilation warning in simple-object-xcoff.c

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  gcc-patches@gcc.gnu.org,  gdb-patches@sourceware.org
> Date: Tue, 16 Jan 2018 23:00:55 +0100
> 
> On Jan 16 2018, DJ Delorie <dj@redhat.com> wrote:
> 
> > And it's not the host's bit size that counts; there are usually ways to
> > get 64-bit file operations on 32-bit hosts.
> 
> If ACX_LARGEFILE doesn't succeed in enabling those 64-bit file
> operations (thus making off_t a 64-bit type) then you are out of luck
> (or AC_SYS_LARGEFILE doesn't support your host yet).

Yes, AC_SYS_LARGEFILE doesn't support MinGW.

DJ, would the following semi-kludgey workaround be acceptable?

--- libiberty/simple-object-xcoff.c~0	2018-01-12 05:31:04.000000000 +0200
+++ libiberty/simple-object-xcoff.c	2018-01-17 12:21:08.496186000 +0200
@@ -596,13 +596,19 @@ simple_object_xcoff_find_sections (simpl
 	      aux = (unsigned char *) auxent;
 	      if (u64)
 		{
+		  /* Use an intermediate 64-bit type to avoid
+		     compilation warning about 32-bit shift below on
+		     hosts with 32-bit off_t which aren't supported by
+		     AC_SYS_LARGEFILE.  */
+		  ulong_type x_scnlen64;
+
 		  if ((auxent->u.xcoff64.x_csect.x_smtyp & 0x7) != XTY_SD
 		      || auxent->u.xcoff64.x_csect.x_smclas != XMC_XO)
 		    continue;
 
-		  x_scnlen = fetch_32 (aux + offsetof (union external_auxent,
-						       u.xcoff64.x_csect.x_scnlen_hi));
-		  x_scnlen = x_scnlen << 32
+		  x_scnlen64 = fetch_32 (aux + offsetof (union external_auxent,
+							 u.xcoff64.x_csect.x_scnlen_hi));
+		  x_scnlen = x_scnlen64 << 32
 			   | fetch_32 (aux + offsetof (union external_auxent,
 						       u.xcoff64.x_csect.x_scnlen_lo));
 		}

Re: Compilation warning in simple-object-xcoff.c

[DJ Delorie]

Eli Zaretskii <eliz@gnu.org> writes:

> DJ, would the following semi-kludgey workaround be acceptable?

It would be no worse than what we have now, if the only purpose is to
avoid a warning.

Ideally, we would check to see if we're discarding non-zero values from
that offset, and not call the callback with known bogus data.  I suppose
the usefulness of that depends on how often you'll encounter 4Gb+ xcoff64
files on mingw32 ?

Re: Compilation warning in simple-object-xcoff.c

[Eli Zaretskii]

> From: DJ Delorie <dj@redhat.com>
> Cc: schwab@linux-m68k.org, gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> Date: Wed, 17 Jan 2018 15:47:49 -0500
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > DJ, would the following semi-kludgey workaround be acceptable?
> 
> It would be no worse than what we have now, if the only purpose is to
> avoid a warning.
> 
> Ideally, we would check to see if we're discarding non-zero values from
> that offset, and not call the callback with known bogus data.  I suppose
> the usefulness of that depends on how often you'll encounter 4Gb+ xcoff64
> files on mingw32 ?

The answer to that question is "never", AFAIU.

Re: Compilation warning in simple-object-xcoff.c

[Eli Zaretskii]

> Date: Thu, 18 Jan 2018 05:25:20 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> CC: schwab@linux-m68k.org, gcc-patches@gcc.gnu.org,	gdb-patches@sourceware.org
> 
> > From: DJ Delorie <dj@redhat.com>
> > Cc: schwab@linux-m68k.org, gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> > Date: Wed, 17 Jan 2018 15:47:49 -0500
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > > DJ, would the following semi-kludgey workaround be acceptable?
> > 
> > It would be no worse than what we have now, if the only purpose is to
> > avoid a warning.
> > 
> > Ideally, we would check to see if we're discarding non-zero values from
> > that offset, and not call the callback with known bogus data.  I suppose
> > the usefulness of that depends on how often you'll encounter 4Gb+ xcoff64
> > files on mingw32 ?
> 
> The answer to that question is "never", AFAIU.

So can the patch I proposed be applied, please?

TIA

Re: Compilation warning in simple-object-xcoff.c

[Ian Lance Taylor via gdb-patches]

On Sat, Jan 20, 2018 at 4:47 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Thu, 18 Jan 2018 05:25:20 +0200
>> From: Eli Zaretskii <eliz@gnu.org>
>> CC: schwab@linux-m68k.org, gcc-patches@gcc.gnu.org,   gdb-patches@sourceware.org
>>
>> > From: DJ Delorie <dj@redhat.com>
>> > Cc: schwab@linux-m68k.org, gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
>> > Date: Wed, 17 Jan 2018 15:47:49 -0500
>> >
>> > Eli Zaretskii <eliz@gnu.org> writes:
>> >
>> > > DJ, would the following semi-kludgey workaround be acceptable?
>> >
>> > It would be no worse than what we have now, if the only purpose is to
>> > avoid a warning.
>> >
>> > Ideally, we would check to see if we're discarding non-zero values from
>> > that offset, and not call the callback with known bogus data.  I suppose
>> > the usefulness of that depends on how often you'll encounter 4Gb+ xcoff64
>> > files on mingw32 ?
>>
>> The answer to that question is "never", AFAIU.
>
> So can the patch I proposed be applied, please?

I committed the patch.

Ian

Re: Compilation warning in simple-object-xcoff.c

[Eli Zaretskii]

> From: Ian Lance Taylor <iant@google.com>
> Date: Sat, 20 Jan 2018 21:01:09 -0800
> Cc: DJ Delorie <dj@redhat.com>, gcc-patches <gcc-patches@gcc.gnu.org>, 
> 	gdb-patches <gdb-patches@sourceware.org>
> 
> On Sat, Jan 20, 2018 at 4:47 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> Date: Thu, 18 Jan 2018 05:25:20 +0200
> >> From: Eli Zaretskii <eliz@gnu.org>
> >> CC: schwab@linux-m68k.org, gcc-patches@gcc.gnu.org,   gdb-patches@sourceware.org
> >>
> >> > From: DJ Delorie <dj@redhat.com>
> >> > Cc: schwab@linux-m68k.org, gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> >> > Date: Wed, 17 Jan 2018 15:47:49 -0500
> >> >
> >> > Eli Zaretskii <eliz@gnu.org> writes:
> >> >
> >> > > DJ, would the following semi-kludgey workaround be acceptable?
> >> >
> >> > It would be no worse than what we have now, if the only purpose is to
> >> > avoid a warning.
> >> >
> >> > Ideally, we would check to see if we're discarding non-zero values from
> >> > that offset, and not call the callback with known bogus data.  I suppose
> >> > the usefulness of that depends on how often you'll encounter 4Gb+ xcoff64
> >> > files on mingw32 ?
> >>
> >> The answer to that question is "never", AFAIU.
> >
> > So can the patch I proposed be applied, please?
> 
> I committed the patch.

So now that this patch is committed to upstream libiberty, is it OK to
push it to GDB's copy of the library?

Re: Compilation warning in simple-object-xcoff.c

[Eli Zaretskii]

> From: Ian Lance Taylor <iant@google.com>
> Date: Sat, 20 Jan 2018 21:01:09 -0800
> Cc: DJ Delorie <dj@redhat.com>, gcc-patches <gcc-patches@gcc.gnu.org>, 
> 	gdb-patches <gdb-patches@sourceware.org>
> 
> On Sat, Jan 20, 2018 at 4:47 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> Date: Thu, 18 Jan 2018 05:25:20 +0200
> >> From: Eli Zaretskii <eliz@gnu.org>
> >> CC: schwab@linux-m68k.org, gcc-patches@gcc.gnu.org,   gdb-patches@sourceware.org
> >>
> >> > From: DJ Delorie <dj@redhat.com>
> >> > Cc: schwab@linux-m68k.org, gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> >> > Date: Wed, 17 Jan 2018 15:47:49 -0500
> >> >
> >> > Eli Zaretskii <eliz@gnu.org> writes:
> >> >
> >> > > DJ, would the following semi-kludgey workaround be acceptable?
> >> >
> >> > It would be no worse than what we have now, if the only purpose is to
> >> > avoid a warning.
> >> >
> >> > Ideally, we would check to see if we're discarding non-zero values from
> >> > that offset, and not call the callback with known bogus data.  I suppose
> >> > the usefulness of that depends on how often you'll encounter 4Gb+ xcoff64
> >> > files on mingw32 ?
> >>
> >> The answer to that question is "never", AFAIU.
> >
> > So can the patch I proposed be applied, please?
> 
> I committed the patch.

Thanks, Ian!

Re: Compilation warning in simple-object-xcoff.c

[Joel Brobecker]

> > I committed the patch.
> 
> So now that this patch is committed to upstream libiberty, is it OK to
> push it to GDB's copy of the library?

Yes. For libiberty, incorporating patches from upstream is considered
an "obvious" change in the sense that it does not require prior
approval. We should actually always keep the two in sync, but as
we know, they easily forget, hence the occasion "resync" patches...

-- 
Joel

Re: Compilation warning in simple-object-xcoff.c

[Eli Zaretskii]

> Date: Mon, 22 Jan 2018 08:27:44 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > > I committed the patch.
> > 
> > So now that this patch is committed to upstream libiberty, is it OK to
> > push it to GDB's copy of the library?
> 
> Yes. For libiberty, incorporating patches from upstream is considered
> an "obvious" change in the sense that it does not require prior
> approval. We should actually always keep the two in sync, but as
> we know, they easily forget, hence the occasion "resync" patches...

Thanks, pushed to master and cherry-picked to gdb-8.1-branch.