Message ID | c52eaba2-b80e-a741-6b52-4fdd2319da81@redhat.com |
---|---|
State | New |
Headers | show |
Series | [PR,tree-optimization/84224] do not ICE on malformed allocas | expand |
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
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 (); }
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 (); }