On Wed, Mar 09, 2016 at 12:24:42PM +0100, Jakub Jelinek wrote:
> 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:
[...]
Wow, indeed it ICEs with __attribute__((noreturn)). Technically, only the
gimplify.c part is needed to fix the new ICE, but I've also fixed the
cgraphunit.c spot for good measure. New compile test added.
Bootstrapped/regtested on x86_64-linux, ok for trunk or gcc-7?
2016-03-09 Marek Polacek <pola...@redhat.com>
PR c/70093
* c-typeck.c (build_function_call_vec): Create a TARGET_EXPR for
nested functions returning VM types.
* cgraphunit.c (cgraph_node::expand_thunk): Also build call to the
function being thunked if the result type doesn't have fixed size.
* gimplify.c (gimplify_modify_expr): Also set LHS if the result type
doesn't have fixed size.
* gcc.dg/nested-func-10.c: New test.
* gcc.dg/nested-func-9.c: New test.
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/cgraphunit.c gcc/cgraphunit.c
index 8b3fddc..4351ae4 100644
--- gcc/cgraphunit.c
+++ gcc/cgraphunit.c
@@ -1708,7 +1708,9 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool
force_gimple_thunk)
/* Build call to the function being thunked. */
if (!VOID_TYPE_P (restype)
- && (!alias_is_noreturn || TREE_ADDRESSABLE (restype)))
+ && (!alias_is_noreturn
+ || TREE_ADDRESSABLE (restype)
+ || TREE_CODE (TYPE_SIZE_UNIT (restype)) != INTEGER_CST))
{
if (DECL_BY_REFERENCE (resdecl))
{
diff --git gcc/gimplify.c gcc/gimplify.c
index b331e41..692d168 100644
--- gcc/gimplify.c
+++ gcc/gimplify.c
@@ -4838,7 +4838,8 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p,
}
notice_special_calls (call_stmt);
if (!gimple_call_noreturn_p (call_stmt)
- || TREE_ADDRESSABLE (TREE_TYPE (*to_p)))
+ || TREE_ADDRESSABLE (TREE_TYPE (*to_p))
+ || TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (*to_p))) != INTEGER_CST)
gimple_call_set_lhs (call_stmt, *to_p);
assign = call_stmt;
}
diff --git gcc/testsuite/gcc.dg/nested-func-10.c
gcc/testsuite/gcc.dg/nested-func-10.c
index e69de29..1b869ac 100644
--- gcc/testsuite/gcc.dg/nested-func-10.c
+++ gcc/testsuite/gcc.dg/nested-func-10.c
@@ -0,0 +1,45 @@
+/* PR c/70093 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+void
+foo (int n)
+{
+ struct S { int a[n]; };
+
+ struct S __attribute__((noreturn))
+ fn (void)
+ {
+ struct S s;
+ s.a[0] = 42;
+ return s; /* { dg-warning "function declared .noreturn.|.noreturn.
function does return" } */
+ }
+
+ auto struct S __attribute__((noreturn))
+ fn2 (void)
+ {
+ return fn (); /* { dg-warning "function declared .noreturn." } */
+ }
+
+ 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);
+}
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);
+}
Marek
