Re: Recent change broke ppc64le and i686 gas builds

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

Re: Recent change broke ppc64le and i686 gas builds

Alan Modra-3
On Tue, Aug 21, 2018 at 08:41:30AM -0600, Jeff Law wrote:

>
> Tester started throwing errors due to this change:
>
> 706704c88344314646e4edcc0840cb18a9cb4c82 is the first bad commit
> commit 706704c88344314646e4edcc0840cb18a9cb4c82
> Author: Alan Modra <[hidden email]>
> Date:   Tue Aug 21 11:54:29 2018 +0930
>
>     Pack reloc_howto_struct
>
>     This patch uses bitfields in reloc_howto_struct, reducing its size
>     from 80 to 40 bytes on 64-bit hosts and from 52 to 32 bytes on 32-bit
>     hosts (with a 32-bit bfd_vma).  I've also added a new "negate" field
>     rather than making the encoded "size" field do double duty as both
>     a size and a flag.
>
> [ ... ]
>
> Compiling with gcc-4.8.5 on gcc112.fsffrance.org I get the following error:
>
> gcc -DHAVE_CONFIG_H -I. -I../../../binutils/gas  -I.
> -I../../../binutils/gas -I../bfd -I../../../binutils/gas/config
> -I../../../binutils/gas/../include -I../../../binutils/gas/..
> -I../../../binutils/gas/../bfd
> -DLOCALEDIR="\"/home/law/jenkins/workspace/powerpc64le-linux-gnu/installed/share/locale\""
>  -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Werror
> -Wwrite-strings -I../../../binutils/gas/../zlib -g -O2 -MT dw2gencfi.o
> -MD -MP -MF $depbase.Tpo -c -o dw2gencfi.o
> ../../../binutils/gas/dw2gencfi.c &&\
> mv -f $depbase.Tpo $depbase.Po
> In file included from ../../../binutils/gas/dw2gencfi.c:22:0:
> ../../../binutils/gas/dw2gencfi.c: In function ‘emit_expr_encoded’:
> ../../../binutils/gas/dw2gencfi.c:200:24: error: comparison between
> signed and unsigned integer expressions [-Werror=sign-compare]
>        gas_assert (size == howto->bitsize / 8);
>                         ^
> ../../../binutils/gas/as.h:88:33: note: in definition of macro ‘gas_assert’
>  #define gas_assert(P) ((void) ((P) ? 0 : (abort (), 0)))
>                                  ^
> ../../../binutils/gas/dw2gencfi.c: In function ‘output_fde’:
> ../../../binutils/gas/dw2gencfi.c:1982:26: error: comparison between
> signed and unsigned integer expressions [-Werror=sign-compare]
>     gas_assert (addr_size == howto->bitsize / 8);
>                           ^
> ../../../binutils/gas/as.h:88:33: note: in definition of macro ‘gas_assert’
>  #define gas_assert(P) ((void) ((P) ? 0 : (abort (), 0)))

I fixed one of these sorts of warnings in bfd/coff-rs6000.c but for
some reason my compilers (7.3.0 and 8.1.1) didn't complain here.  Oh
well, presumably the warning has been quieted for simpler cases of
unsigned int bit-field being promoted to int as per the C standard.
The one in coff-rs6000.c involved "howto->bitsize + howto->rightshift
== some_unsigned_value".

I've committed the following.

        * dw2gencfi.c (emit_expr_encoded, output_fde): Warning fixes.

diff --git a/gas/dw2gencfi.c b/gas/dw2gencfi.c
index b15172e8df..3b61070802 100644
--- a/gas/dw2gencfi.c
+++ b/gas/dw2gencfi.c
@@ -197,7 +197,7 @@ emit_expr_encoded (expressionS *exp, int encoding, bfd_boolean emit_encoding)
     {
       reloc_howto_type *howto = bfd_reloc_type_lookup (stdoutput, code);
       char *p = frag_more (size);
-      gas_assert (size == howto->bitsize / 8);
+      gas_assert (size == (unsigned) howto->bitsize / 8);
       md_number_to_chars (p, 0, size);
       fix_new (frag_now, p - frag_now->fr_literal, size, exp->X_add_symbol,
        exp->X_add_number, howto->pc_relative, code);
@@ -1979,7 +1979,7 @@ output_fde (struct fde_entry *fde, struct cie_entry *cie,
  {
   reloc_howto_type *howto = bfd_reloc_type_lookup (stdoutput, code);
   char *p = frag_more (addr_size);
-  gas_assert (addr_size == howto->bitsize / 8);
+  gas_assert (addr_size == (unsigned) howto->bitsize / 8);
   md_number_to_chars (p, 0, addr_size);
   fix_new (frag_now, p - frag_now->fr_literal, addr_size,
    fde->start_address, 0, howto->pc_relative, code);

--
Alan Modra
Australia Development Lab, IBM
Reply | Threaded
Open this post in threaded view
|

Re: Recent change broke ppc64le and i686 gas builds

Jeff Law
On 08/22/2018 12:14 AM, Alan Modra wrote:

> On Tue, Aug 21, 2018 at 08:41:30AM -0600, Jeff Law wrote:
>>
>> Tester started throwing errors due to this change:
>>
>> 706704c88344314646e4edcc0840cb18a9cb4c82 is the first bad commit
>> commit 706704c88344314646e4edcc0840cb18a9cb4c82
>> Author: Alan Modra <[hidden email]>
>> Date:   Tue Aug 21 11:54:29 2018 +0930
>>
>>     Pack reloc_howto_struct
>>
>>     This patch uses bitfields in reloc_howto_struct, reducing its size
>>     from 80 to 40 bytes on 64-bit hosts and from 52 to 32 bytes on 32-bit
>>     hosts (with a 32-bit bfd_vma).  I've also added a new "negate" field
>>     rather than making the encoded "size" field do double duty as both
>>     a size and a flag.
>>
>> [ ... ]
>>
>> Compiling with gcc-4.8.5 on gcc112.fsffrance.org I get the following error:
>>
>> gcc -DHAVE_CONFIG_H -I. -I../../../binutils/gas  -I.
>> -I../../../binutils/gas -I../bfd -I../../../binutils/gas/config
>> -I../../../binutils/gas/../include -I../../../binutils/gas/..
>> -I../../../binutils/gas/../bfd
>> -DLOCALEDIR="\"/home/law/jenkins/workspace/powerpc64le-linux-gnu/installed/share/locale\""
>>  -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Werror
>> -Wwrite-strings -I../../../binutils/gas/../zlib -g -O2 -MT dw2gencfi.o
>> -MD -MP -MF $depbase.Tpo -c -o dw2gencfi.o
>> ../../../binutils/gas/dw2gencfi.c &&\
>> mv -f $depbase.Tpo $depbase.Po
>> In file included from ../../../binutils/gas/dw2gencfi.c:22:0:
>> ../../../binutils/gas/dw2gencfi.c: In function ‘emit_expr_encoded’:
>> ../../../binutils/gas/dw2gencfi.c:200:24: error: comparison between
>> signed and unsigned integer expressions [-Werror=sign-compare]
>>        gas_assert (size == howto->bitsize / 8);
>>                         ^
>> ../../../binutils/gas/as.h:88:33: note: in definition of macro ‘gas_assert’
>>  #define gas_assert(P) ((void) ((P) ? 0 : (abort (), 0)))
>>                                  ^
>> ../../../binutils/gas/dw2gencfi.c: In function ‘output_fde’:
>> ../../../binutils/gas/dw2gencfi.c:1982:26: error: comparison between
>> signed and unsigned integer expressions [-Werror=sign-compare]
>>     gas_assert (addr_size == howto->bitsize / 8);
>>                           ^
>> ../../../binutils/gas/as.h:88:33: note: in definition of macro ‘gas_assert’
>>  #define gas_assert(P) ((void) ((P) ? 0 : (abort (), 0)))
>
> I fixed one of these sorts of warnings in bfd/coff-rs6000.c but for
> some reason my compilers (7.3.0 and 8.1.1) didn't complain here.  Oh
> well, presumably the warning has been quieted for simpler cases of
> unsigned int bit-field being promoted to int as per the C standard.
> The one in coff-rs6000.c involved "howto->bitsize + howto->rightshift
> == some_unsigned_value".
>
> I've committed the following.
>
> * dw2gencfi.c (emit_expr_encoded, output_fde): Warning fixes.
Yup.  That took care of things...

Thanks,
jeff