Re: [PATCH] add -Wstringop-overflow to LTO options (PR 84212)

2018-02-08 Thread Richard Biener
On Thu, Feb 8, 2018 at 4:08 AM, Martin Sebor  wrote:
> I went ahead and changed all the options on the list below
> to include LTO and tested the attached patch by configuring
> with --with-build-config=bootstrap-lto --disable-werror and
> making profiledbootstrap.  Attached, besides the patch, is
> also the breakdown of warnings.  The interesting column is
> the one labeled Unique.  It gives the number of distinct
> instances of each warning.
>
> This was with all languages but Go and Brig.  Those two fail
> what seem like unrelated reasons.  The Brig error is some
> unsatisfied reference (I lost the log).  Go fails with
> a bunch of errors looking like this, one for each errno
> constant:
>
> /opt/notnfs/msebor/src/gcc/84212/libgo/go/syscall/env_unix.go:96:10: error:
> reference to undefined name ‘EINVAL’
>
> On 02/07/2018 12:30 PM, Martin Sebor wrote:
>>
>> In PR 84212 the reporter asks why -Wno-stringop-overflow has
>> no effect during LTO linking.  It turns out that the reason
>> is the same as in bug 78768: the specification in the c.opt
>> file is missing LTO among the languages.
>>
>> The attached patch adds LTO to it and to -Wstringop-truncation.
>>
>> Bootstrapped and regtested on x86_64-linux.
>>
>> There are other middle-end options in the c.opt file that do
>> not mention LTO that arguably should (*).  I didn't change
>> those in this patch, in part because I don't have test cases
>> showing where it matters, and in part because I don't think
>> that having to remember to include LTO in these options (and,
>> ideally, also include a test in the test suite for each) is
>> a good approach.
>>
>> It seems that including LTO implicitly for all options would
>> obviate this manual step and eliminate the risk of missing
>> them.  Is there a reason not to do that?
>>
>> If implicitly including LTO for every option is not feasible,
>> then it might be worthwhile to write a small script to verify
>> that every middle-end option does mention LTO, and have make
>> run the script as part of the self-test step.

Well, in principle middle-end options belong in common.opt
and should be marked 'Common'.

But obviously the ones listed do not apply to, say, Fortran
but with adding LTO mixed language TUs can get late
warnings from LTO.  But they also get late warnings for
the Fortran parts then.

So related is some other PR which mentions that we do not
save warning options in the LTO IL and thus keep their
setting properly attached to function state.  I guess we should
revisit this at least for LTO supported ones.

Patch is ok.

>> Is there support for either of these changes?  If not, are
>> there any other ideas for how to avoid these kind of bugs?
>>
>> Martin
>>
>> [*] Here are a few examples.  I'm fine with adding LTO to
>> any or all of these as well as any others that I may have
>> missed for GCC 8 if that sounds like a good idea.
>>
>>   -Walloc-size-larger-than
>>   -Warray-bounds
>>   -Wformat-truncation=
>>   -Wmaybe-uninitialized
>>   -Wnonnull
>>   -Wrestrict
>>   -Wstrict-overflow
>>   -Wsuggest-attribute
>>   -Wuninitialized
>
>


Re: [PATCH] add -Wstringop-overflow to LTO options (PR 84212)

2018-02-07 Thread Martin Sebor

I went ahead and changed all the options on the list below
to include LTO and tested the attached patch by configuring
with --with-build-config=bootstrap-lto --disable-werror and
making profiledbootstrap.  Attached, besides the patch, is
also the breakdown of warnings.  The interesting column is
the one labeled Unique.  It gives the number of distinct
instances of each warning.

This was with all languages but Go and Brig.  Those two fail
what seem like unrelated reasons.  The Brig error is some
unsatisfied reference (I lost the log).  Go fails with
a bunch of errors looking like this, one for each errno
constant:

/opt/notnfs/msebor/src/gcc/84212/libgo/go/syscall/env_unix.go:96:10: 
error: reference to undefined name ‘EINVAL’


On 02/07/2018 12:30 PM, Martin Sebor wrote:

In PR 84212 the reporter asks why -Wno-stringop-overflow has
no effect during LTO linking.  It turns out that the reason
is the same as in bug 78768: the specification in the c.opt
file is missing LTO among the languages.

The attached patch adds LTO to it and to -Wstringop-truncation.

Bootstrapped and regtested on x86_64-linux.

There are other middle-end options in the c.opt file that do
not mention LTO that arguably should (*).  I didn't change
those in this patch, in part because I don't have test cases
showing where it matters, and in part because I don't think
that having to remember to include LTO in these options (and,
ideally, also include a test in the test suite for each) is
a good approach.

It seems that including LTO implicitly for all options would
obviate this manual step and eliminate the risk of missing
them.  Is there a reason not to do that?

If implicitly including LTO for every option is not feasible,
then it might be worthwhile to write a small script to verify
that every middle-end option does mention LTO, and have make
run the script as part of the self-test step.

Is there support for either of these changes?  If not, are
there any other ideas for how to avoid these kind of bugs?

Martin

[*] Here are a few examples.  I'm fine with adding LTO to
any or all of these as well as any others that I may have
missed for GCC 8 if that sounds like a good idea.

  -Walloc-size-larger-than
  -Warray-bounds
  -Wformat-truncation=
  -Wmaybe-uninitialized
  -Wnonnull
  -Wrestrict
  -Wstrict-overflow
  -Wsuggest-attribute
  -Wuninitialized


PR lto/84212 -  -Wno-* does not disable warnings from -flto link stage

gcc/c-family/ChangeLog:

	PR lto/84212
	* c.opt (-Wstringop-overflow, -Warray-bounds): Add LTO.
	(-Walloc-size-larger-than, -Wformat-truncation=): Same.
	(-Wmaybe-uninitialized, -Wnonnull, -Wrestrict): Same.
	(-Wstrict-overflow, -Wsuggest-attribute): Same.
	(-Wuninitialized): Same.

gcc/testsuite/ChangeLog:

	PR lto/84212
	* gcc.dg/lto/pr84212_0.c: New test file.
	* gcc.dg/lto/pr84212_1.c: Same.

Index: gcc/c-family/c.opt
===
--- gcc/c-family/c.opt	(revision 257453)
+++ gcc/c-family/c.opt	(working copy)
@@ -304,7 +304,7 @@ C ObjC C++ ObjC++ Var(warn_alloca) Warning
 Warn on any use of alloca.
 
 Walloc-size-larger-than=
-C ObjC C++ ObjC++ Var(warn_alloc_size_limit) Warning Joined LangEnabledBy(C ObjC C++ ObjC++,Wall)
+C ObjC C++ ObjC++ LTO Var(warn_alloc_size_limit) Warning Joined LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall)
 -Walloc-size-larger-than= Warn for calls to allocation functions that
 attempt to allocate objects larger than the specified number of bytes.
 
@@ -319,11 +319,11 @@ alloca, and on bounded uses of alloca whose bound
  bytes.
 
 Warray-bounds
-LangEnabledBy(C ObjC C++ ObjC++,Wall)
+LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall)
 ; in common.opt
 
 Warray-bounds=
-LangEnabledBy(C ObjC C++ ObjC++,Wall,1,0)
+LangEnabledBy(C ObjC C++ ObjC++ LTO,Wall,1,0)
 ; in common.opt
 
 Wassign-intercept
@@ -575,12 +575,12 @@ C ObjC C++ ObjC++ Joined RejectNegative UInteger V
 Warn about printf/scanf/strftime/strfmon format string anomalies.
 
 Wformat-overflow=
-C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_overflow) Warning LangEnabledBy(C ObjC C++ ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
+C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_overflow) Warning LangEnabledBy(C ObjC C++ ObjC++ LTO,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
 Warn about function calls with format strings that write past the end
 of the destination region.
 
 Wformat-truncation=
-C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
+C ObjC C++ ObjC++ LTO Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ ObjC++ LTO,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
 Warn about calls to snprintf and similar functions that truncate output.
 
 Wif-not-aligned
@@ -739,17 +739,17 @@ C ObjC C++ ObjC++ Var(warn_sizeof_array_argument)
 Warn when sizeof is applied on a parameter