d: Fix OutOfMemoryError thrown when appending to an array with a side effect

When appending a character to an array, the result of that concat
assignment was not the new value of the array, similarly, when appending
an array to another array, side effects were evaluated in reverse to the
expected order of evaluation.

As of this change, the address of the left-hand side expression is
saved and re-used as the result.  Its evaluation is now also forced to
occur before the concat operation itself is called.

gcc/d/ChangeLog:

	PR d/97889
	* expr.cc (ExprVisitor::visit (CatAssignExp *)): Enforce LTR order of
	evaluation on left and right hand side expressions.

gcc/testsuite/ChangeLog:

	PR d/97889
	* gdc.dg/torture/pr97889.d: New test.

diff --git a/gcc/d/expr.cc b/gcc/d/expr.cc
index ef2bf5f2e3689f508181263180a48e3ba5a16c7b..2a1818ab4e5dac151658c28e912a09345c3b2e79 100644 (file)
--- a/gcc/d/expr.cc
+++ b/gcc/d/expr.cc
@@ -838,62 +838,81 @@ public:
     Type *tb2 = e->e2->type->toBasetype ();
     Type *etype = tb1->nextOf ()->toBasetype ();
 
+    /* Save the address of `e1', so it can be evaluated first.
+       As all D run-time library functions for concat assignments update `e1'
+       in-place and then return its value, the saved address can also be used as
+       the result of this expression as well.  */
+    tree lhs = build_expr (e->e1);
+    tree lexpr = stabilize_expr (&lhs);
+    tree ptr = d_save_expr (build_address (lhs));
+    tree result = NULL_TREE;
+
     if (tb1->ty == Tarray && tb2->ty == Tdchar
        && (etype->ty == Tchar || etype->ty == Twchar))
       {
-       /* Append a dchar to a char[] or wchar[]  */
+       /* Append a dchar to a char[] or wchar[]:
+          The assignment is handled by the D run-time library, so only
+          need to call `_d_arrayappend[cw]d(&e1, e2)'  */
        libcall_fn libcall = (etype->ty == Tchar)
          ? LIBCALL_ARRAYAPPENDCD : LIBCALL_ARRAYAPPENDWD;
 
-       this->result_ = build_libcall (libcall, e->type, 2,
-                                      build_address (build_expr (e->e1)),
-                                      build_expr (e->e2));
+       result = build_libcall (libcall, e->type, 2,
+                               ptr, build_expr (e->e2));
       }
     else
       {
        gcc_assert (tb1->ty == Tarray || tb2->ty == Tsarray);
 
-       tree tinfo = build_typeinfo (e->loc, e->type);
-       tree ptr = build_address (build_expr (e->e1));
-
        if ((tb2->ty == Tarray || tb2->ty == Tsarray)
            && same_type_p (etype, tb2->nextOf ()->toBasetype ()))
          {
-           /* Append an array.  */
-           this->result_ = build_libcall (LIBCALL_ARRAYAPPENDT, e->type, 3,
-                                          tinfo, ptr, d_array_convert (e->e2));
-
+           /* Append an array to another array:
+              The assignment is handled by the D run-time library, so only
+              need to call `_d_arrayappendT(ti, &e1, e2)'  */
+           result = build_libcall (LIBCALL_ARRAYAPPENDT, e->type, 3,
+                                   build_typeinfo (e->loc, e->type),
+                                   ptr, d_array_convert (e->e2));
          }
        else if (same_type_p (etype, tb2))
          {
-           /* Append an element.  */
-           tree result = build_libcall (LIBCALL_ARRAYAPPENDCTX, e->type, 3,
-                                        tinfo, ptr, size_one_node);
-           result = d_save_expr (result);
+           /* Append an element to an array:
+              The assignment is generated inline, so need to handle temporaries
+              here, and ensure that they are evaluated in the correct order.
+
+              The generated code should end up being equivalent to:
+                   _d_arrayappendcTX(ti, &e1, 1)[e1.length - 1] = e2
+            */
+           tree callexp = build_libcall (LIBCALL_ARRAYAPPENDCTX, e->type, 3,
+                                         build_typeinfo (e->loc, e->type),
+                                         ptr, size_one_node);
+           callexp = d_save_expr (callexp);
 
            /* Assign e2 to last element.  */
-           tree offexp = d_array_length (result);
+           tree offexp = d_array_length (callexp);
            offexp = build2 (MINUS_EXPR, TREE_TYPE (offexp),
                             offexp, size_one_node);
 
-           tree ptrexp = d_array_ptr (result);
+           tree ptrexp = d_array_ptr (callexp);
            ptrexp = void_okay_p (ptrexp);
            ptrexp = build_array_index (ptrexp, offexp);
 
            /* Evaluate expression before appending.  */
-           tree t2 = build_expr (e->e2);
-           tree expr = stabilize_expr (&t2);
+           tree rhs = build_expr (e->e2);
+           tree rexpr = stabilize_expr (&rhs);
 
-           if (TREE_CODE (t2) == CALL_EXPR)
-             t2 = force_target_expr (t2);
+           if (TREE_CODE (rhs) == CALL_EXPR)
+             rhs = force_target_expr (rhs);
 
-           result = modify_expr (build_deref (ptrexp), t2);
-
-           this->result_ = compound_expr (expr, result);
+           result = modify_expr (build_deref (ptrexp), rhs);
+           result = compound_expr (rexpr, result);
          }
        else
          gcc_unreachable ();
       }
+
+    /* Construct in order: ptr = &e1, _d_arrayappend(ptr, e2), *ptr;  */
+    result = compound_expr (compound_expr (lexpr, ptr), result);
+    this->result_ = compound_expr (result, build_deref (ptr));
   }
 
   /* Build an assignment expression.  The right operand is implicitly

diff --git a/gcc/testsuite/gdc.dg/torture/pr97889.d b/gcc/testsuite/gdc.dg/torture/pr97889.d
new file mode 100644 (file)
index 0000000..9135c8f
--- /dev/null
+++ b/gcc/testsuite/gdc.dg/torture/pr97889.d
@@ -0,0 +1,29 @@
+// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97889
+// { dg-additional-options "-fmain -funittest" }
+// { dg-do run }
+// { dg-skip-if "needs gcc/config.d" { ! d_runtime } }
+
+auto cat11ret3(T)(ref T s)
+{
+    s ~= 11;
+    return [3];
+}
+
+unittest
+{
+    static auto test1(int[] val) { val ~= cat11ret3(val); return val; }
+    assert(test1([1]) == [1, 11, 3]);
+    static assert(test1([1]) == [1, 11, 3]);
+
+    static auto test2(int[] val) { val = val ~ cat11ret3(val); return val; }
+    // FIXME: assert(test2([1]) == [1, 3]);
+    static assert(test2([1]) == [1, 3]);
+
+    static auto test3(int[] val) { (val ~= 7) ~= cat11ret3(val); return val; }
+    assert(test3([2]) == [2, 7, 11, 3]);
+    static assert(test3([2]) == [2, 7, 11, 3]);
+
+    static auto test4(int[] val) { (val ~= cat11ret3(val)) ~= 7; return val; }
+    assert(test4([2]) == [2, 11, 3, 7]);
+    static assert(test4([2]) == [2, 11, 3, 7]);
+}