diff mbox

Fix GLIBC build error with recent GCC6

Message ID 002b01d0c309$84ed71a0$8ec854e0$@com
State New
Headers show

Commit Message

Wilco July 20, 2015, 4:31 p.m. UTC
iconvdata/iso-2022-cn-ext.c fails to build with modern GCC, so add a warning disable. I created PR
66946 to see whether this can be fixed in GCC.


ChangeLog:
2015-07-20  Wilco Dijkstra  <wdijkstr@arm.com>

        * iconvdata/iso-2022-cn-ext.c: Add warning disable.


---
 iconvdata/iso-2022-cn-ext.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Carlos O'Donell July 20, 2015, 5:40 p.m. UTC | #1
On 07/20/2015 12:31 PM, Wilco Dijkstra wrote:
> iconvdata/iso-2022-cn-ext.c fails to build with modern GCC, so add a warning disable. I created PR
> 66946 to see whether this can be fixed in GCC.
> 
> 
> ChangeLog:
> 2015-07-20  Wilco Dijkstra  <wdijkstr@arm.com>
> 
>         * iconvdata/iso-2022-cn-ext.c: Add warning disable.
> 
> 
> ---
>  iconvdata/iso-2022-cn-ext.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/iconvdata/iso-2022-cn-ext.c b/iconvdata/iso-2022-cn-ext.c
> index 2c9846d..75590a0 100644
> --- a/iconvdata/iso-2022-cn-ext.c
> +++ b/iconvdata/iso-2022-cn-ext.c
> @@ -656,6 +656,7 @@ enum
>  				while (0)
>  #define UPDATE_PARAMS		*setp = (set | ann) << 3
>  #define LOOP_NEED_FLAGS
> +# pragma GCC diagnostic ignored "-Wuninitialized"
>  #include <iconv/loop.c>

Can you localize it more? Add a comment and reference the BZ?

e.g.

      /* Building with -O3 GCC emits a `array subscript is above array
         bounds' warning.  GCC BZ #64739 has been opened for this.  */
      DIAG_PUSH_NEEDS_COMMENT;
      DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Warray-bounds");
      while (inptr < inend)
        bytebuf[inlen++] = *inptr++;
      DIAG_POP_NEEDS_COMMENT;


Cheers,
Carlos.
Jeff Law July 20, 2015, 5:46 p.m. UTC | #2
On 07/20/2015 11:40 AM, Carlos O'Donell wrote:
> On 07/20/2015 12:31 PM, Wilco Dijkstra wrote:
>> iconvdata/iso-2022-cn-ext.c fails to build with modern GCC, so add a warning disable. I created PR
>> 66946 to see whether this can be fixed in GCC.
>>
>>
>> ChangeLog:
>> 2015-07-20  Wilco Dijkstra  <wdijkstr@arm.com>
>>
>>          * iconvdata/iso-2022-cn-ext.c: Add warning disable.
>>
>>
>> ---
>>   iconvdata/iso-2022-cn-ext.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/iconvdata/iso-2022-cn-ext.c b/iconvdata/iso-2022-cn-ext.c
>> index 2c9846d..75590a0 100644
>> --- a/iconvdata/iso-2022-cn-ext.c
>> +++ b/iconvdata/iso-2022-cn-ext.c
>> @@ -656,6 +656,7 @@ enum
>>   				while (0)
>>   #define UPDATE_PARAMS		*setp = (set | ann) << 3
>>   #define LOOP_NEED_FLAGS
>> +# pragma GCC diagnostic ignored "-Wuninitialized"
>>   #include <iconv/loop.c>
>
> Can you localize it more? Add a comment and reference the BZ?
>
> e.g.
>
>        /* Building with -O3 GCC emits a `array subscript is above array
>           bounds' warning.  GCC BZ #64739 has been opened for this.  */
>        DIAG_PUSH_NEEDS_COMMENT;
>        DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Warray-bounds");
>        while (inptr < inend)
>          bytebuf[inlen++] = *inptr++;
>        DIAG_POP_NEEDS_COMMENT;
Extra credit if you can file a bug report upstream for GCC.

This kind of false positive often leads to discovery of cases that GCC 
is not optimizing as well as it should.  Killing the false positive and 
generating better code in the process is obviously a good thing.

The preprocessed source & options ought to be enough.  We can reduce it 
to a minimized form on the GCC side of things.


jeff
Florian Weimer July 20, 2015, 5:51 p.m. UTC | #3
On 07/20/2015 07:46 PM, Jeff Law wrote:

> Extra credit if you can file a bug report upstream for GCC.

<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66946>

> This kind of false positive often leads to discovery of cases that GCC
> is not optimizing as well as it should.  Killing the false positive and
> generating better code in the process is obviously a good thing.

What's particularly worrying is that the warning says “is”, not ”may
be”.  Doesn't this indicate the GCC miscompiles some paths?
Jeff Law July 20, 2015, 5:53 p.m. UTC | #4
On 07/20/2015 11:51 AM, Florian Weimer wrote:
> On 07/20/2015 07:46 PM, Jeff Law wrote:
>
>> Extra credit if you can file a bug report upstream for GCC.
>
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66946>
>
>> This kind of false positive often leads to discovery of cases that GCC
>> is not optimizing as well as it should.  Killing the false positive and
>> generating better code in the process is obviously a good thing.
>
> What's particularly worrying is that the warning says “is”, not ”may
> be”.  Doesn't this indicate the GCC miscompiles some paths?
Not really.  I'd have to sit down with the minimized testcase & a CFG to 
know for sure.

jeff
diff mbox

Patch

diff --git a/iconvdata/iso-2022-cn-ext.c b/iconvdata/iso-2022-cn-ext.c
index 2c9846d..75590a0 100644
--- a/iconvdata/iso-2022-cn-ext.c
+++ b/iconvdata/iso-2022-cn-ext.c
@@ -656,6 +656,7 @@  enum
 				while (0)
 #define UPDATE_PARAMS		*setp = (set | ann) << 3
 #define LOOP_NEED_FLAGS
+# pragma GCC diagnostic ignored "-Wuninitialized"
 #include <iconv/loop.c>