[PATCH] adding missing LTO to some warning options (PR 78606)

[PATCH] adding missing LTO to some warning options (PR 78606)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 754 bytes --]

The -Walloca-larger-than, -Wformat-length, and -Wformat-truncation
options do not mention LTO among the supported languages and so are
disabled when -flto is used, causing false negatives.

The attached patch adds the missing LTO to the three options.  This
makes -Walloca-larger-than work with LTO but not the other two
options, implying that something else is preventing the gimple-ssa-
sprintf pass from running when -flto is enabled.  I haven't had
the cycles to look into what that might be yet.  Since the root
causes are independent I'd like to commit this patch first and
deal with the  -Wformat-{length,truncation} problem separately,
under a new bug (or give someone with a better understanding of
LTO the opportunity to do it).

Thanks
Martin

[-- Attachment #2: gcc-78768.diff --]
[-- Type: text/x-patch, Size: 2423 bytes --]

PR c/78768 -  -Walloca-larger-than and -Wformat-length warnings disabled by -flto

gcc/c-family/ChangeLog:

	PR c/78768
	* c.opt (-Walloca-larger-than, -Wformat-length, -Wformat-truncation):
	Also enable for LTO.

gcc/testsuite/ChangeLog:

	PR c/78768
	* gcc.dg/pr78768.c: New test.

Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 244294)
+++ gcc/c-family/c.opt	(working copy)
@@ -313,7 +313,7 @@ C ObjC C++ ObjC++ Var(warn_alloc_zero) Warning
 -Walloc-zero Warn for calls to allocation functions that specify zero bytes.
 
 Walloca-larger-than=
-C ObjC C++ ObjC++ Var(warn_alloca_limit) Warning Joined RejectNegative UInteger
+C ObjC C++ LTO ObjC++ Var(warn_alloca_limit) Warning Joined RejectNegative UInteger
 -Walloca-larger-than=<number> Warn on unbounded uses of
 alloca, and on bounded uses of alloca whose bound can be larger than
 <number> bytes.
@@ -521,7 +521,7 @@ C ObjC C++ ObjC++ Var(warn_format_extra_args) Warn
 Warn if passing too many arguments to a function for its format string.
 
 Wformat-length
-C ObjC C++ ObjC++ Warning Alias(Wformat-length=, 1, 0)
+C ObjC C++ LTO ObjC++ Warning Alias(Wformat-length=, 1, 0)
 Warn about function calls with format strings that write past the end
 of the destination region.  Same as -Wformat-length=1.
 
@@ -538,7 +538,7 @@ C ObjC C++ ObjC++ Var(warn_format_signedness) Warn
 Warn about sign differences with format functions.
 
 Wformat-truncation
-C ObjC C++ ObjC++ Warning Alias(Wformat-truncation=, 1, 0)
+C ObjC C++ LTO ObjC++ Warning Alias(Wformat-truncation=, 1, 0)
 Warn about calls to snprintf and similar functions that truncate output.
 Same as -Wformat-truncation=1.
 
Index: gcc/testsuite/gcc.dg/pr78768.c
===================================================================
--- gcc/testsuite/gcc.dg/pr78768.c	(revision 0)
+++ gcc/testsuite/gcc.dg/pr78768.c	(working copy)
@@ -0,0 +1,13 @@
+/* PR c/78768 - -Walloca-larger-than and -Wformat-length warnings disabled
+   by -flto
+  { dg-do run }
+  { dg-options "-O2 -Walloca-larger-than=10 -Wformat -Wformat-length -flto" } */
+
+int main (void)
+{
+  char *d = (char *)__builtin_alloca (12);  /* { dg-warning "argument to .alloca. is too large" } */
+
+  __builtin_sprintf (d, "%32s", "x");   /* { dg-warning "directive writing 32 bytes into a region of size 12" "-Wformat-length" { xfail *-*-* } } */
+
+  return 0;
+}

Re: [PATCH] adding missing LTO to some warning options (PR 78606)

[Richard Biener]

On Tue, 10 Jan 2017, Martin Sebor wrote:

> The -Walloca-larger-than, -Wformat-length, and -Wformat-truncation
> options do not mention LTO among the supported languages and so are
> disabled when -flto is used, causing false negatives.
> 
> The attached patch adds the missing LTO to the three options.  This
> makes -Walloca-larger-than work with LTO but not the other two
> options, implying that something else is preventing the gimple-ssa-
> sprintf pass from running when -flto is enabled.  I haven't had
> the cycles to look into what that might be yet.  Since the root
> causes are independent I'd like to commit this patch first and
> deal with the  -Wformat-{length,truncation} problem separately,
> under a new bug (or give someone with a better understanding of
> LTO the opportunity to do it).

Ok.

Richarx.

> Thanks
> Martin
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH] adding missing LTO to some warning options (PR 78606)

[Andreas Schwab]

On Jan 10 2017, Martin Sebor <msebor@gmail.com> wrote:

> Index: gcc/testsuite/gcc.dg/pr78768.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/pr78768.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/pr78768.c	(working copy)
> @@ -0,0 +1,13 @@
> +/* PR c/78768 - -Walloca-larger-than and -Wformat-length warnings disabled
> +   by -flto
> +  { dg-do run }
> +  { dg-options "-O2 -Walloca-larger-than=10 -Wformat -Wformat-length -flto" } */
> +
> +int main (void)
> +{
> +  char *d = (char *)__builtin_alloca (12);  /* { dg-warning "argument to .alloca. is too large" } */
> +
> +  __builtin_sprintf (d, "%32s", "x");   /* { dg-warning "directive writing 32 bytes into a region of size 12" "-Wformat-length" { xfail *-*-* } } */
> +
> +  return 0;
> +}

Why is that a run test?  It cannot be usefully executed.

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: [PATCH] adding missing LTO to some warning options (PR 78606)

[Kyrill Tkachov]

Hi Martin,

On 10/01/17 22:16, Martin Sebor wrote:
> The -Walloca-larger-than, -Wformat-length, and -Wformat-truncation
> options do not mention LTO among the supported languages and so are
> disabled when -flto is used, causing false negatives.
>
> The attached patch adds the missing LTO to the three options. This
> makes -Walloca-larger-than work with LTO but not the other two
> options, implying that something else is preventing the gimple-ssa-
> sprintf pass from running when -flto is enabled.  I haven't had
> the cycles to look into what that might be yet.  Since the root
> causes are independent I'd like to commit this patch first and
> deal with the  -Wformat-{length,truncation} problem separately,
> under a new bug (or give someone with a better understanding of
> LTO the opportunity to do it).
>

I see the new test FAILing on arm and aarch64 targets.
FAIL: gcc.dg/pr78768.c execution test

Thanks,
Kyrill

> Thanks
> Martin

Re: [PATCH] adding missing LTO to some warning options (PR 78606)

[Martin Sebor]

On 01/17/2017 05:04 AM, Kyrill Tkachov wrote:
> Hi Martin,
>
> On 10/01/17 22:16, Martin Sebor wrote:
>> The -Walloca-larger-than, -Wformat-length, and -Wformat-truncation
>> options do not mention LTO among the supported languages and so are
>> disabled when -flto is used, causing false negatives.
>>
>> The attached patch adds the missing LTO to the three options. This
>> makes -Walloca-larger-than work with LTO but not the other two
>> options, implying that something else is preventing the gimple-ssa-
>> sprintf pass from running when -flto is enabled.  I haven't had
>> the cycles to look into what that might be yet.  Since the root
>> causes are independent I'd like to commit this patch first and
>> deal with the  -Wformat-{length,truncation} problem separately,
>> under a new bug (or give someone with a better understanding of
>> LTO the opportunity to do it).
>>
>
> I see the new test FAILing on arm and aarch64 targets.
> FAIL: gcc.dg/pr78768.c execution test

Thanks.  The test doesn't need to run.  It just needs to link.
I changed it in r244537.

Martin

Re: [PATCH] adding missing LTO to some warning options (PR 78606)

[Tom de Vries]

On 01/10/2017 11:16 PM, Martin Sebor wrote:
> +  __builtin_sprintf (d, "%32s", "x");   /* { dg-warning "directive writing 32 bytes into a region of size 12" "-Wformat-length" { xfail *-*-* } } */

This xpasses for me on an older system:
...
XPASS: gcc.dg/pr78768.c -Wformat-overflow (test for warnings, line 11)
...

The mechanism is as follows:
- the system doesn't have linker plugin support, and sets
   HAVE_LTO_PLUGIN to 0
- consequently, opts.c:finish_options() sets x_flag_fat_lto_objects to 1
   in cc1
- cgraphunit.c:symbol_table::compile() does not return
   here:
   ...
      /* Do nothing else if any IPA pass found errors or if we are just
         streaming LTO.  */
      if (seen_error ()
         || (!in_lto_p && flag_lto && !flag_fat_lto_objects))
       {
         timevar_pop (TV_CGRAPHOPT);
         return;
       }
   ...
   and ends up calling expand_all_functions, which calls
   pass_sprintf_length
- the warning is generated by cc1

Maybe the test needs:
...
/* { dg-require-linker-plugin "" } */
...
?

Thanks,
- Tom

Re: [PATCH] adding missing LTO to some warning options (PR 78606)

[Martin Sebor]

On 04/30/2017 02:02 PM, Tom de Vries wrote:
> On 01/10/2017 11:16 PM, Martin Sebor wrote:
>> +  __builtin_sprintf (d, "%32s", "x");   /* { dg-warning "directive
>> writing 32 bytes into a region of size 12" "-Wformat-length" { xfail
>> *-*-* } } */
>
> This xpasses for me on an older system:
> ...
> XPASS: gcc.dg/pr78768.c -Wformat-overflow (test for warnings, line 11)
> ...
>
> The mechanism is as follows:
> - the system doesn't have linker plugin support, and sets
>   HAVE_LTO_PLUGIN to 0
> - consequently, opts.c:finish_options() sets x_flag_fat_lto_objects to 1
>   in cc1
> - cgraphunit.c:symbol_table::compile() does not return
>   here:
>   ...
>      /* Do nothing else if any IPA pass found errors or if we are just
>         streaming LTO.  */
>      if (seen_error ()
>         || (!in_lto_p && flag_lto && !flag_fat_lto_objects))
>       {
>         timevar_pop (TV_CGRAPHOPT);
>         return;
>       }
>   ...
>   and ends up calling expand_all_functions, which calls
>   pass_sprintf_length
> - the warning is generated by cc1
>
> Maybe the test needs:
> ...
> /* { dg-require-linker-plugin "" } */
> ...
> ?

That seems possible.  IIUC, without linker plugin support the
pass will run when the ordinary object file is created during
the first stage of compilation.  I don't have a system to confirm
it on but if pr78768.c xfails when you add the directive I'd say
go ahead and commit the fix as obvious.

Thanks
Martin

Re: [PATCH] adding missing LTO to some warning options (PR 78606)

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 1542 bytes --]

On 05/01/2017 08:05 PM, Martin Sebor wrote:
> On 04/30/2017 02:02 PM, Tom de Vries wrote:
>> On 01/10/2017 11:16 PM, Martin Sebor wrote:
>>> +  __builtin_sprintf (d, "%32s", "x");   /* { dg-warning "directive
>>> writing 32 bytes into a region of size 12" "-Wformat-length" { xfail
>>> *-*-* } } */
>>
>> This xpasses for me on an older system:
>> ...
>> XPASS: gcc.dg/pr78768.c -Wformat-overflow (test for warnings, line 11)
>> ...
>>
>> The mechanism is as follows:
>> - the system doesn't have linker plugin support, and sets
>>   HAVE_LTO_PLUGIN to 0
>> - consequently, opts.c:finish_options() sets x_flag_fat_lto_objects to 1
>>   in cc1
>> - cgraphunit.c:symbol_table::compile() does not return
>>   here:
>>   ...
>>      /* Do nothing else if any IPA pass found errors or if we are just
>>         streaming LTO.  */
>>      if (seen_error ()
>>         || (!in_lto_p && flag_lto && !flag_fat_lto_objects))
>>       {
>>         timevar_pop (TV_CGRAPHOPT);
>>         return;
>>       }
>>   ...
>>   and ends up calling expand_all_functions, which calls
>>   pass_sprintf_length
>> - the warning is generated by cc1
>>
>> Maybe the test needs:
>> ...
>> /* { dg-require-linker-plugin "" } */
>> ...
>> ?
>
> That seems possible.  IIUC, without linker plugin support the
> pass will run when the ordinary object file is created during
> the first stage of compilation.  I don't have a system to confirm
> it on but if pr78768.c xfails when you add the directive I'd say
> go ahead and commit the fix as obvious.

Done.

Thanks,
- Tom

[-- Attachment #2: 0001-Require-linker-plugin-for-pr78768.c.patch --]
[-- Type: text/x-patch, Size: 854 bytes --]

Require linker plugin for pr78768.c

The test-case has an xfail-ed line.  For linkers without plugin support, that
line happens to xpass.  Require linker with plugin support, such that the line
is no longer xpass-ing, but unsupported.

2017-05-01  Tom de Vries  <tom@codesourcery.com>

	* gcc.dg/pr78768.c: Require linker plugin.

---
 gcc/testsuite/ChangeLog        | 4 ++++
 gcc/testsuite/gcc.dg/pr78768.c | 1 +
 2 files changed, 5 insertions(+)

diff --git a/gcc/testsuite/gcc.dg/pr78768.c b/gcc/testsuite/gcc.dg/pr78768.c
index 68d717a..b6cda47 100644
--- a/gcc/testsuite/gcc.dg/pr78768.c
+++ b/gcc/testsuite/gcc.dg/pr78768.c
@@ -2,6 +2,7 @@
    by -flto
   { dg-do link }
   { dg-require-effective-target lto }
+  { dg-require-linker-plugin "" }
   { dg-options "-O2 -Walloca-larger-than=10 -Wformat -Wformat-overflow -flto" } */
 
 int main (void)