From 23045f8b062e20672f5170fc66532de7a5d9a1d6 Mon Sep 17 00:00:00 2001 From: Iain Buclaw Date: Sun, 22 Nov 2020 14:29:54 +0100 Subject: [PATCH] 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. --- gcc/d/expr.cc | 67 +++++++++++++++++--------- gcc/testsuite/gdc.dg/torture/pr97889.d | 29 +++++++++++ 2 files changed, 72 insertions(+), 24 deletions(-) create mode 100644 gcc/testsuite/gdc.dg/torture/pr97889.d diff --git a/gcc/d/expr.cc b/gcc/d/expr.cc index ef2bf5f2e36..2a1818ab4e5 100644 --- 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 index 00000000000..9135c8fa5cf --- /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]); +} -- 2.30.2