[PR,tree-optimization/84224] do not ICE on malformed allocas
diff mbox series

Message ID c52eaba2-b80e-a741-6b52-4fdd2319da81@redhat.com
State New
Headers show
Series
  • [PR,tree-optimization/84224] do not ICE on malformed allocas
Related show

Commit Message

Aldy Hernandez Feb. 6, 2018, 9:38 a.m. UTC
The -Walloca pass can receive a malformed alloca, courtesy of someone 
providing a faulty prototype.  This was causing an ICE because we 
assumed alloca calls had at least one argument, which the testcase does not:

+void *alloca ();
+__typeof__(alloca ()) a () { return alloca (); }

I don't believe it should be the responsibility of the 
-Walloca-larger-than=* pass to warn against such things, so I propose we 
just ignore this.

I also think we should handle this testcase, regardless of the target 
having an alloca builtin, since the testcase includes its own 
prototype.  Thus, the missing "{dg-require-effect-target alloca}".

OK?

Comments

Jeff Law Feb. 8, 2018, 4:38 a.m. UTC | #1
On 02/06/2018 02:38 AM, Aldy Hernandez wrote:
> The -Walloca pass can receive a malformed alloca, courtesy of someone
> providing a faulty prototype.  This was causing an ICE because we
> assumed alloca calls had at least one argument, which the testcase does
> not:
> 
> +void *alloca ();
> +__typeof__(alloca ()) a () { return alloca (); }
> 
> I don't believe it should be the responsibility of the
> -Walloca-larger-than=* pass to warn against such things, so I propose we
> just ignore this.
> 
> I also think we should handle this testcase, regardless of the target
> having an alloca builtin, since the testcase includes its own
> prototype.  Thus, the missing "{dg-require-effect-target alloca}".
Wouldn't it make more sense to put the argument check into
gimple_alloca_call_p that way all callers are insulated?

After all if the alloca doesn't have an argument it probably doesn't
have the semantics of alloca that we expect -- like returning nonzero.

And if we look at vr-values.c:gimple_stmt_nonzero_p we see that it calls
gimple_alloca_call_p and if it returns true, then we assume the call
always returns a nonzero value.   Fixing gimple_alloca_call_p would take
care of both problems at once.

OK with that change.

jeff
Aldy Hernandez Feb. 8, 2018, 11:15 a.m. UTC | #2
On 02/07/2018 11:38 PM, Jeff Law wrote:
> On 02/06/2018 02:38 AM, Aldy Hernandez wrote:
>> The -Walloca pass can receive a malformed alloca, courtesy of someone
>> providing a faulty prototype.  This was causing an ICE because we
>> assumed alloca calls had at least one argument, which the testcase does
>> not:
>>
>> +void *alloca ();
>> +__typeof__(alloca ()) a () { return alloca (); }
>>
>> I don't believe it should be the responsibility of the
>> -Walloca-larger-than=* pass to warn against such things, so I propose we
>> just ignore this.
>>
>> I also think we should handle this testcase, regardless of the target
>> having an alloca builtin, since the testcase includes its own
>> prototype.  Thus, the missing "{dg-require-effect-target alloca}".
> Wouldn't it make more sense to put the argument check into
> gimple_alloca_call_p that way all callers are insulated?
> 
> After all if the alloca doesn't have an argument it probably doesn't
> have the semantics of alloca that we expect -- like returning nonzero.
> 
> And if we look at vr-values.c:gimple_stmt_nonzero_p we see that it calls
> gimple_alloca_call_p and if it returns true, then we assume the call
> always returns a nonzero value.   Fixing gimple_alloca_call_p would take
> care of both problems at once.
> 
> OK with that change.

Attached patch is final version.  I retested on x86-64 Linux.

Committing to trunk and closing PR.

Thanks.
gcc/

	PR tree-optimization/84224
	* gimple-ssa-warn-alloca.c (pass_walloca::execute): Remove assert.
	* calls.c (gimple_alloca_call_p): Only return TRUE when we have
	non-zero arguments.

diff --git a/gcc/calls.c b/gcc/calls.c
index 54fea158631..19c95b8455b 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -730,7 +730,7 @@ gimple_alloca_call_p (const gimple *stmt)
     switch (DECL_FUNCTION_CODE (fndecl))
       {
       CASE_BUILT_IN_ALLOCA:
-        return true;
+	return gimple_call_num_args (stmt) > 0;
       default:
 	break;
       }
diff --git a/gcc/gimple-ssa-warn-alloca.c b/gcc/gimple-ssa-warn-alloca.c
index 941810a997e..327c806ae11 100644
--- a/gcc/gimple-ssa-warn-alloca.c
+++ b/gcc/gimple-ssa-warn-alloca.c
@@ -445,7 +445,6 @@ pass_walloca::execute (function *fun)
 
 	  if (!gimple_alloca_call_p (stmt))
 	    continue;
-	  gcc_assert (gimple_call_num_args (stmt) >= 1);
 
 	  const bool is_vla
 	    = gimple_call_alloca_for_var_p (as_a <gcall *> (stmt));
diff --git a/gcc/testsuite/gcc.dg/Walloca-16.c b/gcc/testsuite/gcc.dg/Walloca-16.c
new file mode 100644
index 00000000000..3ee96a9570a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Walloca-16.c
@@ -0,0 +1,6 @@
+/* PR tree-optimization/84224 */
+/* { dg-do compile } */
+/* { dg-options "-O0 -Walloca" } */
+
+void *alloca ();
+__typeof__(alloca ()) a () { return alloca (); }

Patch
diff mbox series

gcc/

	PR tree-optimization/84224
	* gimple-ssa-warn-alloca.c (execute): Do not ICE on malformed
	allocas.

diff --git a/gcc/gimple-ssa-warn-alloca.c b/gcc/gimple-ssa-warn-alloca.c
index 941810a997e..7457a16d21f 100644
--- a/gcc/gimple-ssa-warn-alloca.c
+++ b/gcc/gimple-ssa-warn-alloca.c
@@ -443,9 +443,10 @@  pass_walloca::execute (function *fun)
 	  gimple *stmt = gsi_stmt (si);
 	  location_t loc = gimple_location (stmt);
 
-	  if (!gimple_alloca_call_p (stmt))
+	  if (!gimple_alloca_call_p (stmt)
+	      /* A faulty prototype can yield a malformed alloca() call.  */
+	      || gimple_call_num_args (stmt) < 1)
 	    continue;
-	  gcc_assert (gimple_call_num_args (stmt) >= 1);
 
 	  const bool is_vla
 	    = gimple_call_alloca_for_var_p (as_a <gcall *> (stmt));
diff --git a/gcc/testsuite/gcc.dg/Walloca-16.c b/gcc/testsuite/gcc.dg/Walloca-16.c
new file mode 100644
index 00000000000..3ee96a9570a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Walloca-16.c
@@ -0,0 +1,6 @@ 
+/* PR tree-optimization/84224 */
+/* { dg-do compile } */
+/* { dg-options "-O0 -Walloca" } */
+
+void *alloca ();
+__typeof__(alloca ()) a () { return alloca (); }