[PATCH] -Walloca-larger-than with constant sizes at -O0 (PR 100425)

[PATCH] -Walloca-larger-than with constant sizes at -O0 (PR 100425)

[Martin Sebor]

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

Even when explicitly enabled, -Walloca-larger-than doesn't run
unless optimization is enabled as well.  This prevents diagnosing
alloca calls with constant arguments in excess of the limit that
could otherwise be flagged even at -O0, making the warning less
consistent and less useful than is possible.

The attached patch enables -Walloca-larger-than for calls with
constant arguments in excess of the limit even at -O0 (variable
arguments are only handled with optimization, when VRP runs).

Martin

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

PR middle-end/100425 - missing -Walloca-larger-than with -O0

gcc/ChangeLog:

	PR middle-end/100425
	* gimple-ssa-warn-alloca.c (pass_walloca::firast_time_p): Rename...
	(pass_walloca::xlimit_certain_p): ...to this.
	(pass_walloca::gate): Execute for any kind of handled warning.
	(pass_walloca::execute): Avoid issuing "maybe" and "unbounded"
	warnings when xlimit_certain_p is set.

gcc/testsuite/ChangeLog:

	PR middle-end/100425
	* gcc.dg/Walloca-larger-than-4.c: New test.

diff --git a/gcc/gimple-ssa-warn-alloca.c b/gcc/gimple-ssa-warn-alloca.c
index 42c0ba1d87b..e9a24d4d1d0 100644
--- a/gcc/gimple-ssa-warn-alloca.c
+++ b/gcc/gimple-ssa-warn-alloca.c
@@ -56,31 +56,30 @@ class pass_walloca : public gimple_opt_pass
 {
 public:
   pass_walloca (gcc::context *ctxt)
-    : gimple_opt_pass(pass_data_walloca, ctxt), first_time_p (false)
+    : gimple_opt_pass(pass_data_walloca, ctxt), xlimit_certain_p (false)
   {}
   opt_pass *clone () { return new pass_walloca (m_ctxt); }
   void set_pass_param (unsigned int n, bool param)
     {
       gcc_assert (n == 0);
-      first_time_p = param;
+      // Set to true to enable only warnings for alloca calls that
+      // are certainly in excess of the limit.  This includes calls
+      // with constant arguments but excludes those in ranges (that
+      // can only be determined by range analysis) as well as
+      // the "may be too large" kind.
+      xlimit_certain_p = param;
     }
   virtual bool gate (function *);
   virtual unsigned int execute (function *);
 
  private:
   // Set to TRUE the first time we run this pass on a function.
-  bool first_time_p;
+  bool xlimit_certain_p;
 };
 
 bool
 pass_walloca::gate (function *fun ATTRIBUTE_UNUSED)
 {
-  // The first time this pass is called, it is called before
-  // optimizations have been run and range information is unavailable,
-  // so we can only perform strict alloca checking.
-  if (first_time_p)
-    return warn_alloca != 0;
-
   // Warning is disabled when its size limit is greater than PTRDIFF_MAX
   // for the target maximum, which makes the limit negative since when
   // represented in signed HOST_WIDE_INT.
@@ -317,6 +316,9 @@ pass_walloca::execute (function *fun)
 	      break;
 	    case ALLOCA_BOUND_MAYBE_LARGE:
 	      {
+		if (xlimit_certain_p)
+		  break;
+
 		auto_diagnostic_group d;
 		if (warning_at (loc, wcode,
 				(is_vla
@@ -354,6 +356,9 @@ pass_walloca::execute (function *fun)
 	      }
 	      break;
 	    case ALLOCA_UNBOUNDED:
+	      if (xlimit_certain_p)
+		break;
+
 	      warning_at (loc, wcode,
 			  (is_vla
 			   ? G_("%Gunbounded use of variable-length array")
diff --git a/gcc/testsuite/gcc.dg/Walloca-larger-than-4.c b/gcc/testsuite/gcc.dg/Walloca-larger-than-4.c
new file mode 100644
index 00000000000..f2ccc93d92b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Walloca-larger-than-4.c
@@ -0,0 +1,18 @@
+/* PR middle-end/100425 - missing -Walloca-larger-than with -O0
+   { dg-do compile }
+   { dg-options "-O0 -Wall -Walloca-larger-than=128" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+void* alloca (size_t);
+
+void sink (void*);
+
+void warn_alloca_too_large (void)
+{
+  sink (alloca (1));
+  sink (alloca (128));
+  sink (alloca (129));        // { dg-warning "\\\[-Walloca-larger-than" }
+  sink (alloca (128 + 2));    // { dg-warning "\\\[-Walloca-larger-than" }
+  sink (alloca (1024));       // { dg-warning "\\\[-Walloca-larger-than" }
+}

Re: [PATCH] -Walloca-larger-than with constant sizes at -O0 (PR 100425)

[Richard Biener]

On Wed, May 5, 2021 at 4:20 AM Martin Sebor via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Even when explicitly enabled, -Walloca-larger-than doesn't run
> unless optimization is enabled as well.  This prevents diagnosing
> alloca calls with constant arguments in excess of the limit that
> could otherwise be flagged even at -O0, making the warning less
> consistent and less useful than is possible.
>
> The attached patch enables -Walloca-larger-than for calls with
> constant arguments in excess of the limit even at -O0 (variable
> arguments are only handled with optimization, when VRP runs).

Hmm, but then the pass runs even without -Walloca or -Walloca-larger-than
or -Wvla[-larger-than].  It performs an IL walk we should avoid in those
cases.

So the patch is OK but can you please come up with a gate that disables
the pass when all of the warnings it handles won't fire anyway?

Thanks,
Richard.

> Martin

Re: [PATCH] -Walloca-larger-than with constant sizes at -O0 (PR 100425)

[Martin Sebor]

On 5/5/21 1:32 AM, Richard Biener wrote:
> On Wed, May 5, 2021 at 4:20 AM Martin Sebor via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> Even when explicitly enabled, -Walloca-larger-than doesn't run
>> unless optimization is enabled as well.  This prevents diagnosing
>> alloca calls with constant arguments in excess of the limit that
>> could otherwise be flagged even at -O0, making the warning less
>> consistent and less useful than is possible.
>>
>> The attached patch enables -Walloca-larger-than for calls with
>> constant arguments in excess of the limit even at -O0 (variable
>> arguments are only handled with optimization, when VRP runs).
> 
> Hmm, but then the pass runs even without -Walloca or -Walloca-larger-than
> or -Wvla[-larger-than].  It performs an IL walk we should avoid in those
> cases.
> 
> So the patch is OK but can you please come up with a gate that disables
> the pass when all of the warnings it handles won't fire anyway?

-W{alloca,vla}-larger-than=PTRDIFF_MAX are enabled by default so
the pass needs to do walk.

FWIW, it would make sense to me to consolidate all the checking of
calls for arguments with excessive sizes/values into the same pass
and single walk (with code still in separate source files).  As it
is, some are done in their own passes (like alloca and sprintf),
and others during expansion (-Wstringop-overflow), and others in
calls.c (-Walloc-size-larger-than).  That leads to repetitive code
and inconsistent approaches and inconsistent false positives and
negatives (because some are done at -O0 but others require
optimization).

Martin

> 
> Thanks,
> Richard.
> 
>> Martin

Re: [PATCH] -Walloca-larger-than with constant sizes at -O0 (PR 100425)

[Richard Biener]

On Wed, May 5, 2021 at 7:29 PM Martin Sebor <msebor@gmail.com> wrote:
>
> On 5/5/21 1:32 AM, Richard Biener wrote:
> > On Wed, May 5, 2021 at 4:20 AM Martin Sebor via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> Even when explicitly enabled, -Walloca-larger-than doesn't run
> >> unless optimization is enabled as well.  This prevents diagnosing
> >> alloca calls with constant arguments in excess of the limit that
> >> could otherwise be flagged even at -O0, making the warning less
> >> consistent and less useful than is possible.
> >>
> >> The attached patch enables -Walloca-larger-than for calls with
> >> constant arguments in excess of the limit even at -O0 (variable
> >> arguments are only handled with optimization, when VRP runs).
> >
> > Hmm, but then the pass runs even without -Walloca or -Walloca-larger-than
> > or -Wvla[-larger-than].  It performs an IL walk we should avoid in those
> > cases.
> >
> > So the patch is OK but can you please come up with a gate that disables
> > the pass when all of the warnings it handles won't fire anyway?
>
> -W{alloca,vla}-larger-than=PTRDIFF_MAX are enabled by default so
> the pass needs to do walk.
>
> FWIW, it would make sense to me to consolidate all the checking of
> calls for arguments with excessive sizes/values into the same pass
> and single walk (with code still in separate source files).  As it
> is, some are done in their own passes (like alloca and sprintf),
> and others during expansion (-Wstringop-overflow), and others in
> calls.c (-Walloc-size-larger-than).  That leads to repetitive code
> and inconsistent approaches and inconsistent false positives and
> negatives (because some are done at -O0 but others require
> optimization).

True - that would be a nice cleanup (and speedup as well).

Richard.

> Martin
>
> >
> > Thanks,
> > Richard.
> >
> >> Martin
>