diff mbox

adding missing LTO to some warning options (PR 78606)

Message ID 06c24069-90af-5778-52a2-c9167eb109ab@gmail.com
State New
Headers show

Commit Message

Martin Sebor Jan. 10, 2017, 10:16 p.m. UTC 
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

Comments

Richard Biener Jan. 11, 2017, 8:21 a.m. UTC | #1 
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)
Andreas Schwab Jan. 13, 2017, 6:49 p.m. UTC | #2 
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."
Kyrill Tkachov Jan. 17, 2017, 12:04 p.m. UTC | #3 
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
Martin Sebor Jan. 17, 2017, 4:39 p.m. UTC | #4 
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
Tom de Vries April 30, 2017, 8:02 p.m. UTC | #5 
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
Martin Sebor May 1, 2017, 6:05 p.m. UTC | #6 
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
Tom de Vries May 2, 2017, 10:17 a.m. UTC | #7 
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,
- TomRequire 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)
diff mbox

Patch

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;
+}