fix Issue 13537 - Unions may break immutability

src/aggregate.d

@@ -337,7 +337,7 @@ extern (C++) abstract class AggregateDeclaration : ScopeDsymbol
     }
 
     /***************************************
-     * Calculate field[i].overlapped, and check that all of explicit
+     * Calculate field[i].overlapped and overlapUnsafe, and check that all of explicit
      * field initializers have unique memory space on instance.
      * Returns:
      *      true if any errors happen.
@@ -379,6 +379,11 @@ extern (C++) abstract class AggregateDeclaration : ScopeDsymbol
                 vd.overlapped = true;
                 v2.overlapped = true;
 
+                if (!MODimplicitConv(vd.type.mod, v2.type.mod))
+                    v2.overlapUnsafe = true;
+                if (!MODimplicitConv(v2.type.mod, vd.type.mod))
+                    vd.overlapUnsafe = true;
+
                 if (!vx)
                     continue;
                 if (v2._init && v2._init.isVoidInitializer())

src/declaration.d

@@ -977,6 +977,7 @@ extern (C++) class VarDeclaration : Declaration
 
     int canassign;                  // it can be assigned to
     bool overlapped;                // if it is a field and has overlapping
+    bool overlapUnsafe;             // if it is an overlapping field and the overlaps are unsafe
     ubyte isdataseg;                // private data for isDataseg 0 unset, 1 true, 2 false
     Dsymbol aliassym;               // if redone as alias to another symbol
     VarDeclaration lastVar;         // Linked list of variables for goto-skips-init detection

src/expression.d

@@ -82,74 +82,70 @@ void emplaceExp(T : UnionExp)(T* p, Expression e)
 }
 
 /*************************************************************
- * Check if e is a DotVarExp representing an overlapped pointer.
- * Print error if overlapped pointer and in @safe.
+ * Check for unsafe access in @safe code:
+ * 1. read overlapped pointers
+ * 2. write misaligned pointers
+ * 3. write overlapped storage classes
+ * Print error if unsafe.
  * Params:
  *      sc = scope
  *      e = expression to check
- *      msg = error message string
+ *      readonly = if access is read-only
+ *      printmsg = print error message if true
  * Returns:
  *      true if error
  */
 
-bool checkOverlappedPointer(Scope* sc, Expression e, string msg)
+private bool checkUnsafeAccess(Scope* sc, Expression e, bool readonly, bool printmsg)
 {
     if (e.op != TOKdotvar)
         return false;
     DotVarExp dve = cast(DotVarExp)e;
     if (VarDeclaration v = dve.var.isVarDeclaration())
     {
-        if (v.overlapped && !sc.intypeof && sc.func && v.type.hasPointers())
+        if (sc.intypeof || !sc.func || !sc.func.isSafeBypassingInference())
+            return false;
+
+        auto ad = v.toParent2().isAggregateDeclaration();
+        if (!ad)
+            return false;
+
+        if (v.overlapped && v.type.hasPointers() && sc.func.setUnsafe())
         {
-            if (auto ad = v.toParent2().isAggregateDeclaration())
-            {
-                if (sc.func.setUnsafe())
-                {
-                    e.error(msg.ptr, ad.toChars(), v.toChars());
-                    return true;
-                }
-            }
+            if (printmsg)
+                e.error("field %s.%s cannot access pointers in @safe code that overlap other fields",
+                    ad.toChars(), v.toChars());
+            return true;
         }
-    }
-    return false;
-}
 
-/*************************************************************
- * Check if e is a DotVarExp representing a misaligned pointer.
- * Print error if misaligned pointer and in @safe.
- * Params:
- *      sc = scope
- *      e = expression to check
- *      msg = error message string
- * Returns:
- *      true if error
- */
+        if (readonly || !e.type.isMutable())
+            return false;
 
-bool checkMisalignedPointer(Scope* sc, Expression e, string msg)
-{
-    if (e.op != TOKdotvar)
-        return false;
-    DotVarExp dve = cast(DotVarExp)e;
-    if (VarDeclaration v = dve.var.isVarDeclaration())
-    {
-        if (v.isField() && !sc.intypeof && sc.func &&
-            (v.type.hasPointers() && v.type.toBasetype().ty != Tstruct))
+        if (v.type.hasPointers() && v.type.toBasetype().ty != Tstruct)
         {
-            if (auto ad = v.toParent2().isAggregateDeclaration())
+            if ((ad.type.alignment() < Target.ptrsize ||
+                 (v.offset & (Target.ptrsize - 1))) &&
+                sc.func.setUnsafe())
             {
-                if ((ad.type.alignment() < Target.ptrsize ||
-                     (v.offset & (Target.ptrsize - 1))) &&
-                    sc.func.setUnsafe())
-                {
-                    e.error(msg.ptr, ad.toChars(), v.toChars());
-                    return true;
-                }
+                if (printmsg)
+                    e.error("field %s.%s cannot modify misaligned pointers in @safe code",
+                        ad.toChars(), v.toChars());
+                return true;
             }
         }
+
+        if (v.overlapUnsafe && sc.func.setUnsafe())
+        {
+             if (printmsg)
+                 e.error("field %s.%s cannot modify fields in @safe code that overlap fields with other storage classes",
+                    ad.toChars(), v.toChars());
+             return true;
+        }
     }
     return false;
 }
 
+
 /*************************************************************
  * Given var, we need to get the
  * right 'this' pointer if var is in an outer class, but our
@@ -537,7 +533,7 @@ extern (C++) Expression resolvePropertiesX(Scope* sc, Expression e1, Expression
         else if (e1.op == TOKdotvar)
         {
             // Check for reading overlapped pointer field in @safe code.
-            if (checkOverlappedPointer(sc, e1, "field %s.%s cannot be accessed in @safe code because it overlaps with a pointer"))
+            if (checkUnsafeAccess(sc, e1, true, true))
                 return new ErrorExp();
         }
         else if (e1.op == TOKdot)
@@ -549,7 +545,7 @@ extern (C++) Expression resolvePropertiesX(Scope* sc, Expression e1, Expression
         {
             CallExp ce = cast(CallExp)e1;
             // Check for reading overlapped pointer field in @safe code.
-            if (checkOverlappedPointer(sc, ce.e1, "field %s.%s cannot be accessed in @safe code because it overlaps with a pointer"))
+            if (checkUnsafeAccess(sc, ce.e1, true, true))
                 return new ErrorExp();
         }
     }
@@ -1725,9 +1721,8 @@ extern (C++) bool functionParameters(Loc loc, Scope* sc, TypeFunction tf, Type t
             {
                 arg = arg.toLvalue(sc, arg);
 
-                // Look for mutable misaligned pointer in @safe mode
-                if (arg.type.isMutable())
-                    err |= checkMisalignedPointer(sc, arg, "'ref' of misaligned pointer in field %s.%s is not @safe");
+                // Look for mutable misaligned pointer, etc., in @safe mode
+                err |= checkUnsafeAccess(sc, arg, false, true);
             }
             else if (p.storageClass & STCout)
             {
@@ -1739,8 +1734,8 @@ extern (C++) bool functionParameters(Loc loc, Scope* sc, TypeFunction tf, Type t
                 }
                 else
                 {
-                    // Look for misaligned pointer in @safe mode
-                    err |= checkMisalignedPointer(sc, arg, "'out' of misaligned pointer in field %s.%s is not @safe");
+                    // Look for misaligned pointer, etc., in @safe mode
+                    err |= checkUnsafeAccess(sc, arg, false, true);
                     err |= checkDefCtor(arg.loc, t); // t must be default constructible
                 }
                 arg = arg.toLvalue(sc, arg);
@@ -9100,11 +9095,14 @@ extern (C++) final class DotVarExp : UnaExp
 
     override int checkModifiable(Scope* sc, int flag)
     {
-        //printf("DotVarExp::checkModifiable %s %s\n", toChars(), type->toChars());
+        //printf("DotVarExp::checkModifiable %s %s\n", toChars(), type.toChars());
+        if (checkUnsafeAccess(sc, this, false, !flag))
+            return 2;
+
         if (e1.op == TOKthis)
             return var.checkModify(loc, sc, type, e1, flag);
 
-        //printf("\te1 = %s\n", e1->toChars());
+        //printf("\te1 = %s\n", e1.toChars());
         return e1.checkModifiable(sc, flag);
     }
 
@@ -9130,10 +9128,6 @@ extern (C++) final class DotVarExp : UnaExp
             printf("var->type = %s\n", var.type.toChars());
         }
 
-        // Look for misaligned pointer in @safe mode
-        if (checkMisalignedPointer(sc, this, "writing to misaligned pointer in field %s.%s is not @safe"))
-            return new ErrorExp();
-
         return Expression.modifiableLvalue(sc, e);
     }
 
@@ -10626,7 +10620,7 @@ extern (C++) final class AddrExp : UnaExp
             }
 
             // Look for misaligned pointer in @safe mode
-            if (checkMisalignedPointer(sc, dve, "taking address of misaligned pointer in field %s.%s is not @safe"))
+            if (checkUnsafeAccess(sc, dve, !type.isMutable(), true))
                 return new ErrorExp();
         }
         else if (e1.op == TOKvar)

test/fail_compilation/test13536.d

@@ -1,9 +1,13 @@
 /*
+PERMUTE_ARGS:
 TEST_OUTPUT:
 ---
-fail_compilation/test13536.d(20): Error: field U.safeDg cannot be accessed in @safe code because it overlaps with a pointer
+fail_compilation/test13536.d(23): Error: field U.sysDg cannot access pointers in @safe code that overlap other fields
+fail_compilation/test13536.d(24): Error: field U.safeDg cannot access pointers in @safe code that overlap other fields
 ---
 */
+
+
 // https://issues.dlang.org/show_bug.cgi?id=13536
 
 struct S {

test/fail_compilation/test13537.d

@@ -0,0 +1,61 @@
+/*
+PERMUTE_ARGS:
+TEST_OUTPUT:
+---
+fail_compilation/test13537.d(32): Error: field U.y cannot modify fields in @safe code that overlap fields with other storage classes
+fail_compilation/test13537.d(33): Error: field U.y cannot modify fields in @safe code that overlap fields with other storage classes
+fail_compilation/test13537.d(34): Error: field U.z cannot access pointers in @safe code that overlap other fields
+fail_compilation/test13537.d(35): Error: field U.y cannot modify fields in @safe code that overlap fields with other storage classes
+---
+*/
+
+// https://issues.dlang.org/show_bug.cgi?id=13537
+
+union U
+{
+    immutable int x;
+    int y;
+    int* z;
+}
+
+union V
+{
+    immutable int x;
+    const int y;
+}
+
+void fun() @safe
+{
+    U u;
+
+    // errors
+    u.y = 1;
+    int* p = &u.y;
+    int** q = &u.z;
+    abc(u.y);
+
+    // read access is allowed
+    int a = u.x;
+    a = u.y;
+    def(u.y);
+
+    // Overlapping const/immutable is allowed
+    auto v = V(1);
+    assert(v.y == 1);
+}
+
+void gun() @system
+{
+    U u;
+
+    // allowed because system code
+    u.y = 1;
+    int* p = &u.y;
+    int** q = &u.z;
+    abc(u.y);
+}
+
+@safe:
+void abc(ref int x) { }
+void def(const ref int x) { }
+

test/runnable/testsafe.d

@@ -155,9 +155,9 @@ void safeunions()   // improved for issue 11510
     // Writing field is always allowed, even if it is overlapped.
     su1.a = 7, su1.b = 8, su1.c = null;
     su2.a = 7, su2.b = 8, su2.c = 9;
-    uu1.a = 7,            uu1.c = null;
+    uu1.a = 7,            //uu1.c = null;
     uu2.a = 7; uu2.b = 8, //uu2.c = null;
-    uu3.a = 7;            uu3.c = null;
+    uu3.a = 7;            //uu3.c = null;
     uu4.a = 7; uu4.b = 8, //uu4.c = null;
     uu5.x.a = 7; uu5.x.b = 8, uu5.x.c = 9;
     uud.a.a.a.a = null, uud.a.a.a.b = null;