Your binutils patch today caused a "fatal warning"

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Your binutils patch today caused a "fatal warning"

Hans-Peter Nilsson
My autotester notified me of the following breakage, building
--target=cris-axis-elf, apparently caused by your patch:

...
gcc -DHAVE_CONFIG_H -I. -I/h/hp/binutils/cvs_latest/src/bfd -I. -I. -I/h/hp/binutils/cvs_latest/src/bfd -I/h/hp/binutils/cvs_latest/src/bfd/../include -I/h/hp/binutils/cvs_latest/src/bfd/../intl -I../intl -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Werror -g -O2 -c /h/hp/binutils/cvs_latest/src/bfd/elf.c -o elf.o
cc1: warnings being treated as errors
/h/hp/binutils/cvs_latest/src/bfd/elf.c: In function `bfd_section_from_shdr':
/h/hp/binutils/cvs_latest/src/bfd/elf.c:1952: warning: comparison between signed and unsigned
make[3]: *** [elf.lo] Error 1
make[3]: Leaving directory `/n/asic_slask/hp/autobinutest/bfd'

(The build system is i686 RH 7.3.)

brgds, H-P
Reply | Threaded
Open this post in threaded view
|

Re: Your binutils patch today caused a "fatal warning"

Jakub Jelinek
On Fri, Jun 17, 2005 at 05:28:06PM +0200, Hans-Peter Nilsson wrote:

> My autotester notified me of the following breakage, building
> --target=cris-axis-elf, apparently caused by your patch:
>
> ...
> gcc -DHAVE_CONFIG_H -I. -I/h/hp/binutils/cvs_latest/src/bfd -I. -I. -I/h/hp/binutils/cvs_latest/src/bfd -I/h/hp/binutils/cvs_latest/src/bfd/../include -I/h/hp/binutils/cvs_latest/src/bfd/../intl -I../intl -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Werror -g -O2 -c /h/hp/binutils/cvs_latest/src/bfd/elf.c -o elf.o
> cc1: warnings being treated as errors
> /h/hp/binutils/cvs_latest/src/bfd/elf.c: In function `bfd_section_from_shdr':
> /h/hp/binutils/cvs_latest/src/bfd/elf.c:1952: warning: comparison between signed and unsigned
> make[3]: *** [elf.lo] Error 1
> make[3]: Leaving directory `/n/asic_slask/hp/autobinutest/bfd'
>
> (The build system is i686 RH 7.3.)

Works for me just fine.
        if (hdr->sh_entsize != (hdr->sh_type == SHT_REL
                                ? bed->s->sizeof_rel : bed->s->sizeof_rela))
          return FALSE;

Now, sizeof_rel and sizeof_rela are unsigned char, hdr->sh_entsize
is bfd_size_type, which ought to be unsigned type.

        Jakub
Reply | Threaded
Open this post in threaded view
|

Re: Your binutils patch today caused a "fatal warning"

Jakub Jelinek
On Fri, Jun 17, 2005 at 11:39:14AM -0400, Jakub Jelinek wrote:

> > gcc -DHAVE_CONFIG_H -I. -I/h/hp/binutils/cvs_latest/src/bfd -I. -I. -I/h/hp/binutils/cvs_latest/src/bfd -I/h/hp/binutils/cvs_latest/src/bfd/../include -I/h/hp/binutils/cvs_latest/src/bfd/../intl -I../intl -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Werror -g -O2 -c /h/hp/binutils/cvs_latest/src/bfd/elf.c -o elf.o
> > cc1: warnings being treated as errors
> > /h/hp/binutils/cvs_latest/src/bfd/elf.c: In function `bfd_section_from_shdr':
> > /h/hp/binutils/cvs_latest/src/bfd/elf.c:1952: warning: comparison between signed and unsigned
> > make[3]: *** [elf.lo] Error 1
> > make[3]: Leaving directory `/n/asic_slask/hp/autobinutest/bfd'
> >
> > (The build system is i686 RH 7.3.)
>
> Works for me just fine.
>         if (hdr->sh_entsize != (hdr->sh_type == SHT_REL
>                                 ? bed->s->sizeof_rel : bed->s->sizeof_rela))
>           return FALSE;
>
> Now, sizeof_rel and sizeof_rela are unsigned char, hdr->sh_entsize
> is bfd_size_type, which ought to be unsigned type.

But apparently older GCCs (3.2, 2.9x, not sure about 3.3) are not able
to see through the promotion that the value will be never negative.
GCC 3.4 or 4.0 doesn't warn here.

Comitted this as obvious.

2005-06-17  Jakub Jelinek  <[hidden email]>

        * elf.c (bfd_section_from_shdr): Kill bogus warning.

--- bfd/elf.c.jj 2005-06-17 15:31:09.000000000 +0200
+++ bfd/elf.c 2005-06-17 17:41:12.000000000 +0200
@@ -1948,7 +1948,8 @@ bfd_section_from_shdr (bfd *abfd, unsign
  Elf_Internal_Shdr *hdr2;
  unsigned int num_sec = elf_numsections (abfd);
 
- if (hdr->sh_entsize != (hdr->sh_type == SHT_REL
+ if (hdr->sh_entsize
+    != (bfd_size_type) (hdr->sh_type == SHT_REL
  ? bed->s->sizeof_rel : bed->s->sizeof_rela))
   return FALSE;
 


        Jakub
Reply | Threaded
Open this post in threaded view
|

Re: Your binutils patch today caused a "fatal warning"

Andreas Schwab
In reply to this post by Jakub Jelinek
Jakub Jelinek <[hidden email]> writes:

> Works for me just fine.
>         if (hdr->sh_entsize != (hdr->sh_type == SHT_REL
>                                 ? bed->s->sizeof_rel : bed->s->sizeof_rela))
>           return FALSE;
>
> Now, sizeof_rel and sizeof_rela are unsigned char, hdr->sh_entsize
> is bfd_size_type, which ought to be unsigned type.

unsigned char gets promoted to int, thus the warning is correct.

Andreas.

--
Andreas Schwab, SuSE Labs, [hidden email]
SuSE Linux Products GmbH, Maxfeldstra?e 5, 90409 N?rnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Re: Your binutils patch today caused a "fatal warning"

Jakub Jelinek
On Mon, Jun 20, 2005 at 03:41:51PM +0200, Andreas Schwab wrote:

> Jakub Jelinek <[hidden email]> writes:
>
> > Works for me just fine.
> >         if (hdr->sh_entsize != (hdr->sh_type == SHT_REL
> >                                 ? bed->s->sizeof_rel : bed->s->sizeof_rela))
> >           return FALSE;
> >
> > Now, sizeof_rel and sizeof_rela are unsigned char, hdr->sh_entsize
> > is bfd_size_type, which ought to be unsigned type.
>
> unsigned char gets promoted to int, thus the warning is correct.

I know very well it promotes to int.  Still, GCC knows there was
a smaller unsigned type promoted to that signed int and that it will
be always non-negative, so it can avoid the unnecessary warning.

        Jakub