d: Fix struct literals that have non-deterministic hash values (PR96153)
authorIain Buclaw <ibuclaw@gdcproject.org>
Wed, 22 Jul 2020 07:50:38 +0000 (09:50 +0200)
committerIain Buclaw <ibuclaw@gdcproject.org>
Tue, 4 Aug 2020 15:02:56 +0000 (17:02 +0200)
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
gcc/d/expr.cc
gcc/testsuite/gdc.dg/pr96153.d [new file with mode: 0644]

index 2be80dd18673e9ce3c98992a2fa48c61c45d0c64..072de7e65439c20458eccc40071c4528998300a1 100644 (file)
@@ -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 *);
 
index ac3d4aaa1717c72b53217d7ec6c6425bd85599f3..85407ac7eb05523722e746e1930142a78a5f7934 100644 (file)
@@ -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 (file)
index 0000000..c0e3ae0
--- /dev/null
@@ -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());
+}