| Message ID | 20160309110550.GS10006@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Mar 09, 2016 at 12:05:51PM +0100, Marek Polacek wrote: > This PR points out that nested functions returning VM types don't work as > expected (yeah, go figure). We got an ICE on the testcase because we were > trying to allocate variable-sized temporary instead of using __builtin_alloca > or its kin. Jakub suggested to follow what the C++ front end does here. It > seems to be the case that it creates a TARGET_EXPR if the call doesn't have > a LHS. That seems to work out well. The run-time testcase sanity-checks that > we do something reasonable. > > Not a regression, but on the other hand the patch doesn't change anything for > 99.9% programs out there. Wonder if you still can get an ICE if you add __attribute__((noreturn)) to such nested function. Quick grep shows that there are some suspicious spots and others are fine: cgraphunit.c- /* Build call to the function being thunked. */ cgraphunit.c- if (!VOID_TYPE_P (restype) cgraphunit.c: && (!alias_is_noreturn || TREE_ADDRESSABLE (restype))) cgraphunit.c- { ^^^ needs checking gimplify.c: if (!gimple_call_noreturn_p (call_stmt) gimplify.c- || TREE_ADDRESSABLE (TREE_TYPE (*to_p))) gimplify.c- gimple_call_set_lhs (call_stmt, *to_p); ^^^ likewise tree-cfg.c- if (lhs tree-cfg.c- && gimple_call_ctrl_altering_p (stmt) tree-cfg.c: && gimple_call_noreturn_p (stmt) tree-cfg.c- && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST tree-cfg.c- && !TREE_ADDRESSABLE (TREE_TYPE (lhs))) tree-cfg.c- { tree-cfg.c: error ("LHS in noreturn call"); tree-cfg.c- return true; tree-cfg.c- } ^^^ looks fine tree-cfgcleanup.c- /* If there is an LHS, remove it, but only if its type has fixed size. tree-cfgcleanup.c- The LHS will need to be recreated during RTL expansion and creating tree-cfgcleanup.c- temporaries of variable-sized types is not supported. Also don't tree-cfgcleanup.c- do this with TREE_ADDRESSABLE types, as assign_temp will abort. */ tree-cfgcleanup.c- tree lhs = gimple_call_lhs (stmt); tree-cfgcleanup.c- if (lhs && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST tree-cfgcleanup.c- && !TREE_ADDRESSABLE (TREE_TYPE (lhs))) tree-cfgcleanup.c- { tree-cfgcleanup.c- gimple_call_set_lhs (stmt, NULL_TREE); ^^^ likewise Jakub
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c index 6aa0f03..de9d465 100644 --- gcc/c/c-typeck.c +++ gcc/c/c-typeck.c @@ -3068,6 +3068,16 @@ build_function_call_vec (location_t loc, vec<location_t> arg_loc, result = build_call_array_loc (loc, TREE_TYPE (fntype), function, nargs, argarray); + /* In this improbable scenario, a nested function returns a VM type. + Create a TARGET_EXPR so that the call always has a LHS, much as + what the C++ FE does for functions returning non-PODs. */ + if (variably_modified_type_p (TREE_TYPE (fntype), NULL_TREE)) + { + tree tmp = create_tmp_var_raw (TREE_TYPE (fntype)); + result = build4 (TARGET_EXPR, TREE_TYPE (fntype), tmp, result, + NULL_TREE, NULL_TREE); + } + if (VOID_TYPE_P (TREE_TYPE (result))) { if (TYPE_QUALS (TREE_TYPE (result)) != TYPE_UNQUALIFIED) diff --git gcc/testsuite/gcc.dg/nested-func-9.c gcc/testsuite/gcc.dg/nested-func-9.c index e69de29..b703f3a 100644 --- gcc/testsuite/gcc.dg/nested-func-9.c +++ gcc/testsuite/gcc.dg/nested-func-9.c @@ -0,0 +1,45 @@ +/* PR c/70093 */ +/* { dg-do run } */ +/* { dg-options "" } */ + +void +foo (int n) +{ + struct S { int a[n]; }; + + struct S + fn (void) + { + struct S s; + s.a[0] = 42; + return s; + } + + auto struct S + fn2 (void) + { + return fn (); + } + + struct S x; + x = fn (); + + if (x.a[0] != 42) + __builtin_abort (); + + if (fn ().a[0] != 42) + __builtin_abort (); + + __typeof__ (fn ()) *p = &x; + if (p->a[0] != 42) + __builtin_abort (); + + if (fn2 ().a[0] != 42) + __builtin_abort (); +} + +int +main (void) +{ + foo (1); +}