From 2ac51bdf63b0e17d1b9974f30303fb24e3cbc83d Mon Sep 17 00:00:00 2001 From: Iain Buclaw Date: Wed, 22 Jul 2020 09:50:38 +0200 Subject: [PATCH] d: Fix struct literals that have non-deterministic hash values (PR96153) Adds code generation for generating a temporary for, and pre-filling struct and array literals with zeroes before assigning, so that alignment holes don't cause objects to produce a non-deterministic hash value. A new field has been added to the expression visitor to track whether the result is being generated for another literal, so that memset() is only called once on the top-level literal expression, and not for nesting struct or arrays. gcc/d/ChangeLog: PR d/96153 * d-tree.h (build_expr): Add literalp argument. * expr.cc (ExprVisitor): Add literalp_ field. (ExprVisitor::ExprVisitor): Initialize literalp_. (ExprVisitor::visit (AssignExp *)): Call memset() on blits where RHS is a struct literal. Elide assignment if initializer is all zeroes. (ExprVisitor::visit (CastExp *)): Forward literalp_ to generation of subexpression. (ExprVisitor::visit (AddrExp *)): Likewise. (ExprVisitor::visit (ArrayLiteralExp *)): Use memset() to pre-fill object with zeroes. Set literalp in subexpressions. (ExprVisitor::visit (StructLiteralExp *)): Likewise. (ExprVisitor::visit (TupleExp *)): Set literalp in subexpressions. (ExprVisitor::visit (VectorExp *)): Likewise. (ExprVisitor::visit (VectorArrayExp *)): Likewise. (build_expr): Forward literal_p to ExprVisitor. gcc/testsuite/ChangeLog: PR d/96153 * gdc.dg/pr96153.d: New test. --- gcc/d/d-tree.h | 2 +- gcc/d/expr.cc | 104 ++++++++++++++++++++++----------- gcc/testsuite/gdc.dg/pr96153.d | 31 ++++++++++ 3 files changed, 101 insertions(+), 36 deletions(-) create mode 100644 gcc/testsuite/gdc.dg/pr96153.d diff --git a/gcc/d/d-tree.h b/gcc/d/d-tree.h index 2be80dd1867..072de7e6543 100644 --- a/gcc/d/d-tree.h +++ b/gcc/d/d-tree.h @@ -633,7 +633,7 @@ extern void d_comdat_linkage (tree); extern void d_linkonce_linkage (tree); /* In expr.cc. */ -extern tree build_expr (Expression *, bool = false); +extern tree build_expr (Expression *, bool = false, bool = false); extern tree build_expr_dtor (Expression *); extern tree build_return_dtor (Expression *, Type *, TypeFunction *); diff --git a/gcc/d/expr.cc b/gcc/d/expr.cc index ac3d4aaa171..85407ac7eb0 100644 --- a/gcc/d/expr.cc +++ b/gcc/d/expr.cc @@ -223,12 +223,14 @@ class ExprVisitor : public Visitor tree result_; bool constp_; + bool literalp_; public: - ExprVisitor (bool constp) + ExprVisitor (bool constp, bool literalp) { this->result_ = NULL_TREE; this->constp_ = constp; + this->literalp_ = literalp; } tree result (void) @@ -1072,7 +1074,7 @@ public: if (tb1->ty == Tstruct) { tree t1 = build_expr (e->e1); - tree t2 = convert_for_assignment (build_expr (e->e2), + tree t2 = convert_for_assignment (build_expr (e->e2, false, true), e->e2->type, e->e1->type); StructDeclaration *sd = tb1->isTypeStruct ()->sym; @@ -1101,11 +1103,22 @@ public: tree init = NULL_TREE; /* Fill any alignment holes in the struct using memset. */ - if (e->op == TOKconstruct && !identity_compare_p (sd)) - init = build_memset_call (t1); + if ((e->op == TOKconstruct + || (e->e2->op == TOKstructliteral && e->op == TOKblit)) + && (sd->isUnionDeclaration () || !identity_compare_p (sd))) + { + t1 = stabilize_reference (t1); + init = build_memset_call (t1); + } - tree result = build_assign (modifycode, t1, t2); - this->result_ = compound_expr (init, result); + /* Elide generating assignment if init is all zeroes. */ + if (init != NULL_TREE && initializer_zerop (t2)) + this->result_ = compound_expr (init, t1); + else + { + tree result = build_assign (modifycode, t1, t2); + this->result_ = compound_expr (init, result); + } } return; @@ -1135,6 +1148,7 @@ public: to call postblits, this assignment should call dtors on old assigned elements. */ if ((!postblit && !destructor) + || (e->op == TOKconstruct && e->e2->op == TOKarrayliteral) || (e->op == TOKconstruct && !lvalue && postblit) || (e->op == TOKblit || e->e1->type->size () == 0)) { @@ -1452,7 +1466,7 @@ public: { Type *ebtype = e->e1->type->toBasetype (); Type *tbtype = e->to->toBasetype (); - tree result = build_expr (e->e1, this->constp_); + tree result = build_expr (e->e1, this->constp_, this->literalp_); /* Just evaluate e1 if it has any side effects. */ if (tbtype->ty == Tvoid) @@ -1702,7 +1716,7 @@ public: exp = sle->sym; } else - exp = build_expr (e->e1, this->constp_); + exp = build_expr (e->e1, this->constp_, this->literalp_); TREE_CONSTANT (exp) = 0; this->result_ = d_convert (type, build_address (exp)); @@ -2663,12 +2677,12 @@ public: tree result = NULL_TREE; if (e->e0) - result = build_expr (e->e0); + result = build_expr (e->e0, this->constp_, true); for (size_t i = 0; i < e->exps->length; ++i) { Expression *exp = (*e->exps)[i]; - result = compound_expr (result, build_expr (exp)); + result = compound_expr (result, build_expr (exp, this->constp_, true)); } if (result == NULL_TREE) @@ -2717,7 +2731,7 @@ public: for (size_t i = 0; i < e->elements->length; i++) { Expression *expr = e->getElement (i); - tree value = build_expr (expr, this->constp_); + tree value = build_expr (expr, this->constp_, true); /* Only append nonzero values, the backend will zero out the rest of the constructor as we don't set CONSTRUCTOR_NO_CLEARING. */ @@ -2765,6 +2779,22 @@ public: if (constant_p && initializer_constant_valid_p (ctor, TREE_TYPE (ctor))) TREE_STATIC (ctor) = 1; + /* Use memset to fill any alignment holes in the array. */ + if (!this->constp_ && !this->literalp_) + { + TypeStruct *ts = etype->baseElemOf ()->isTypeStruct (); + + if (ts != NULL && (!identity_compare_p (ts->sym) + || ts->sym->isUnionDeclaration ())) + { + tree var = build_local_temp (TREE_TYPE (ctor)); + tree init = build_memset_call (var); + /* Evaluate memset() first, then any saved elements. */ + saved_elems = compound_expr (init, saved_elems); + ctor = compound_expr (modify_expr (var, ctor), var); + } + } + this->result_ = compound_expr (saved_elems, d_convert (type, ctor)); } else @@ -2885,7 +2915,8 @@ public: if (ftype->ty == Tsarray && !same_type_p (type, ftype)) { /* Initialize a static array with a single element. */ - tree elem = build_expr (exp, this->constp_); + tree elem = build_expr (exp, this->constp_, true); + saved_elems = compound_expr (saved_elems, stabilize_expr (&elem)); elem = d_save_expr (elem); if (initializer_zerop (elem)) @@ -2895,14 +2926,12 @@ public: } else { - value = convert_expr (build_expr (exp, this->constp_), + value = convert_expr (build_expr (exp, this->constp_, true), exp->type, field->type); } /* Split construction of values out of the constructor. */ - tree init = stabilize_expr (&value); - if (init != NULL_TREE) - saved_elems = compound_expr (saved_elems, init); + saved_elems = compound_expr (saved_elems, stabilize_expr (&value)); CONSTRUCTOR_APPEND_ELT (ve, get_symbol_decl (field), value); } @@ -2932,24 +2961,27 @@ public: return; } + /* Construct the struct literal for run-time. */ if (e->sym != NULL) { + /* Store the result in a symbol to initialize the literal. */ tree var = build_deref (e->sym); ctor = compound_expr (modify_expr (var, ctor), var); - this->result_ = compound_expr (saved_elems, ctor); } - else if (e->sd->isUnionDeclaration ()) + else if (!this->literalp_) { - /* For unions, use memset to fill holes in the object. */ - tree var = build_local_temp (TREE_TYPE (ctor)); - tree init = build_memset_call (var); - - init = compound_expr (init, saved_elems); - init = compound_expr (init, modify_expr (var, ctor)); - this->result_ = compound_expr (init, var); + /* Use memset to fill any alignment holes in the object. */ + if (!identity_compare_p (e->sd) || e->sd->isUnionDeclaration ()) + { + tree var = build_local_temp (TREE_TYPE (ctor)); + tree init = build_memset_call (var); + /* Evaluate memset() first, then any saved element constructors. */ + saved_elems = compound_expr (init, saved_elems); + ctor = compound_expr (modify_expr (var, ctor), var); + } } - else - this->result_ = compound_expr (saved_elems, ctor); + + this->result_ = compound_expr (saved_elems, ctor); } /* Build a null literal. */ @@ -2964,7 +2996,6 @@ public: void visit (VectorExp *e) { tree type = build_ctype (e->type); - tree etype = TREE_TYPE (type); /* First handle array literal expressions. */ if (e->e1->op == TOKarrayliteral) @@ -2977,7 +3008,8 @@ public: for (size_t i = 0; i < ale->elements->length; i++) { Expression *expr = ale->getElement (i); - tree value = d_convert (etype, build_expr (expr, this->constp_)); + tree value = d_convert (TREE_TYPE (type), + build_expr (expr, this->constp_, true)); if (!CONSTANT_CLASS_P (value)) constant_p = false; @@ -2993,8 +3025,9 @@ public: else { /* Build constructor from single value. */ - tree val = d_convert (etype, build_expr (e->e1, this->constp_)); - this->result_ = build_vector_from_val (type, val); + tree value = d_convert (TREE_TYPE (type), + build_expr (e->e1, this->constp_, true)); + this->result_ = build_vector_from_val (type, value); } } @@ -3002,7 +3035,7 @@ public: void visit (VectorArrayExp *e) { - this->result_ = convert_expr (build_expr (e->e1, this->constp_), + this->result_ = convert_expr (build_expr (e->e1, this->constp_, true), e->e1->type, e->type); } @@ -3057,12 +3090,13 @@ public: /* Main entry point for ExprVisitor interface to generate code for the Expression AST class E. If CONST_P is true, then E is a - constant expression. */ + constant expression. If LITERAL_P is true, then E is a value used + in the initialization of another literal. */ tree -build_expr (Expression *e, bool const_p) +build_expr (Expression *e, bool const_p, bool literal_p) { - ExprVisitor v = ExprVisitor (const_p); + ExprVisitor v = ExprVisitor (const_p, literal_p); location_t saved_location = input_location; input_location = make_location_t (e->loc); diff --git a/gcc/testsuite/gdc.dg/pr96153.d b/gcc/testsuite/gdc.dg/pr96153.d new file mode 100644 index 00000000000..c0e3ae024f5 --- /dev/null +++ b/gcc/testsuite/gdc.dg/pr96153.d @@ -0,0 +1,31 @@ +// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96153 +// { dg-additional-options "-fmain -funittest" } +// { dg-do run { target hw } } +// { dg-skip-if "needs gcc/config.d" { ! d_runtime } } +struct Checked(T, Hook) +{ + private T payload; + Hook hook; + + size_t toHash() const nothrow @safe + { + return hashOf(payload) ^ hashOf(hook); + } +} + +Checked!(T, Hook) checked(Hook, T)(const T value) +{ + return Checked!(T, Hook)(value); +} + +@system unittest +{ + static struct Hook3 + { + ulong var1 = ulong.max; + uint var2 = uint.max; + } + + assert(checked!Hook3(12).toHash() != checked!Hook3(13).toHash()); + assert(checked!Hook3(13).toHash() == checked!Hook3(13).toHash()); +} -- 2.30.2