diff mbox series

Fix call expansion ICE (PR c++/79085)

Message ID 20180315192518.GW8577@tucnak
State New
Headers show
Series Fix call expansion ICE (PR c++/79085) | expand

Commit Message

Jakub Jelinek March 15, 2018, 7:25 p.m. UTC
Hi!

The following testcase ICEs on arm (or other strict alignment targets).
The problem is we have a call that returns a TREE_ADDRESSABLE type,
and the lhs of the call is some memory that isn't known to be sufficiently
aligned.  expand_call in that case allocates a temporary, lets the call
return into that object and copies it; this can't work for TREE_ADDRESSABLE
types, assign_temp ICEs for those, we don't want to create temporaries in
that case.
This patch just avoids the temporary for TREE_ADDRESSABLE types and returns
directly into the provided target memory.
Either the compiler just doesn't know whether the target is properly aligned
or not and it is properly aligned at runtime, then it will work properly,
or it isn't properly aligned at runtime (perhaps compiler can even prove
that) and it will be UB, which is user's fault.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2018-03-15  Jakub Jelinek  <jakub@redhat.com>

	PR c++/79085
	* calls.c (expand_call): For TREE_ADDRESSABLE rettype ignore alignment
	check and use address of target always.

	* g++.dg/opt/pr79085.C: New test.


	Jakub

Comments

Jason Merrill March 15, 2018, 7:59 p.m. UTC | #1
LGTM.

On Thu, Mar 15, 2018 at 3:25 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The following testcase ICEs on arm (or other strict alignment targets).
> The problem is we have a call that returns a TREE_ADDRESSABLE type,
> and the lhs of the call is some memory that isn't known to be sufficiently
> aligned.  expand_call in that case allocates a temporary, lets the call
> return into that object and copies it; this can't work for TREE_ADDRESSABLE
> types, assign_temp ICEs for those, we don't want to create temporaries in
> that case.
> This patch just avoids the temporary for TREE_ADDRESSABLE types and returns
> directly into the provided target memory.
> Either the compiler just doesn't know whether the target is properly aligned
> or not and it is properly aligned at runtime, then it will work properly,
> or it isn't properly aligned at runtime (perhaps compiler can even prove
> that) and it will be UB, which is user's fault.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2018-03-15  Jakub Jelinek  <jakub@redhat.com>
>
>         PR c++/79085
>         * calls.c (expand_call): For TREE_ADDRESSABLE rettype ignore alignment
>         check and use address of target always.
>
>         * g++.dg/opt/pr79085.C: New test.
>
> --- gcc/calls.c.jj      2018-02-22 12:37:02.641387687 +0100
> +++ gcc/calls.c 2018-03-15 15:17:42.596835316 +0100
> @@ -3422,9 +3422,14 @@ expand_call (tree exp, rtx target, int i
>         if (CALL_EXPR_RETURN_SLOT_OPT (exp)
>             && target
>             && MEM_P (target)
> -           && !(MEM_ALIGN (target) < TYPE_ALIGN (rettype)
> -                && targetm.slow_unaligned_access (TYPE_MODE (rettype),
> -                                                  MEM_ALIGN (target))))
> +           /* If rettype is addressable, we may not create a temporary.
> +              If target is properly aligned at runtime and the compiler
> +              just doesn't know about it, it will work fine, otherwise it
> +              will be UB.  */
> +           && (TREE_ADDRESSABLE (rettype)
> +               || !(MEM_ALIGN (target) < TYPE_ALIGN (rettype)
> +                    && targetm.slow_unaligned_access (TYPE_MODE (rettype),
> +                                                      MEM_ALIGN (target)))))
>           structure_value_addr = XEXP (target, 0);
>         else
>           {
> --- gcc/testsuite/g++.dg/opt/pr79085.C.jj       2018-03-15 15:42:05.382927611 +0100
> +++ gcc/testsuite/g++.dg/opt/pr79085.C  2018-03-15 15:32:56.626552182 +0100
> @@ -0,0 +1,24 @@
> +// PR c++/79085
> +// { dg-do compile }
> +// { dg-options "-Os" }
> +// { dg-additional-options "-mstrict-align" { target { aarch64*-*-* powerpc*-*-linux* powerpc*-*-elf* } } }
> +
> +void *operator new (__SIZE_TYPE__, void *p) { return p; }
> +
> +struct S
> +{
> +  S ();
> +  S (const S &);
> +  ~S (void);
> +  int i;
> +};
> +
> +S foo ();
> +
> +static char buf [sizeof (S) + 1];
> +
> +S *
> +bar ()
> +{
> +  return new (buf + 1) S (foo ());
> +}
>
>         Jakub
diff mbox series

Patch

--- gcc/calls.c.jj	2018-02-22 12:37:02.641387687 +0100
+++ gcc/calls.c	2018-03-15 15:17:42.596835316 +0100
@@ -3422,9 +3422,14 @@  expand_call (tree exp, rtx target, int i
 	if (CALL_EXPR_RETURN_SLOT_OPT (exp)
 	    && target
 	    && MEM_P (target)
-	    && !(MEM_ALIGN (target) < TYPE_ALIGN (rettype)
-		 && targetm.slow_unaligned_access (TYPE_MODE (rettype),
-						   MEM_ALIGN (target))))
+	    /* If rettype is addressable, we may not create a temporary.
+	       If target is properly aligned at runtime and the compiler
+	       just doesn't know about it, it will work fine, otherwise it
+	       will be UB.  */
+	    && (TREE_ADDRESSABLE (rettype)
+		|| !(MEM_ALIGN (target) < TYPE_ALIGN (rettype)
+		     && targetm.slow_unaligned_access (TYPE_MODE (rettype),
+						       MEM_ALIGN (target)))))
 	  structure_value_addr = XEXP (target, 0);
 	else
 	  {
--- gcc/testsuite/g++.dg/opt/pr79085.C.jj	2018-03-15 15:42:05.382927611 +0100
+++ gcc/testsuite/g++.dg/opt/pr79085.C	2018-03-15 15:32:56.626552182 +0100
@@ -0,0 +1,24 @@ 
+// PR c++/79085
+// { dg-do compile }
+// { dg-options "-Os" }
+// { dg-additional-options "-mstrict-align" { target { aarch64*-*-* powerpc*-*-linux* powerpc*-*-elf* } } }
+
+void *operator new (__SIZE_TYPE__, void *p) { return p; }
+
+struct S
+{
+  S ();
+  S (const S &);
+  ~S (void);
+  int i;
+};
+
+S foo ();
+
+static char buf [sizeof (S) + 1];
+
+S *
+bar ()
+{
+  return new (buf + 1) S (foo ());
+}