[clang][Interp] Fix diagnosing uninitialized nested union fields (#102824)

We were calling initialize() unconditionally when copying the union.

Author: tbaederr

clang/lib/AST/Interp/Interp.cpp

@@ -126,13 +126,17 @@ static bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
     return true;
 
   assert(Ptr.inUnion());
+  assert(Ptr.isField() && Ptr.getField());
 
   Pointer U = Ptr.getBase();
   Pointer C = Ptr;
   while (!U.isRoot() && U.inUnion() && !U.isActive()) {
-    C = U;
+    if (U.getField())
+      C = U;
     U = U.getBase();
   }
+  assert(C.isField());
+
   // Get the inactive field descriptor.
   const FieldDecl *InactiveField = C.getField();
   assert(InactiveField);

clang/lib/AST/Interp/InterpBuiltin.cpp

@@ -1635,7 +1635,58 @@ bool SetThreeWayComparisonField(InterpState &S, CodePtr OpPC,
   return true;
 }
 
-bool DoMemcpy(InterpState &S, CodePtr OpPC, const Pointer &Src, Pointer &Dest) {
+static bool copyComposite(InterpState &S, CodePtr OpPC, const Pointer &Src,
+                          Pointer &Dest, bool Activate);
+static bool copyRecord(InterpState &S, CodePtr OpPC, const Pointer &Src,
+                       Pointer &Dest, bool Activate = false) {
+  [[maybe_unused]] const Descriptor *SrcDesc = Src.getFieldDesc();
+  const Descriptor *DestDesc = Dest.getFieldDesc();
+
+  auto copyField = [&](const Record::Field &F, bool Activate) -> bool {
+    Pointer DestField = Dest.atField(F.Offset);
+    if (std::optional<PrimType> FT = S.Ctx.classify(F.Decl->getType())) {
+      TYPE_SWITCH(*FT, {
+        DestField.deref<T>() = Src.atField(F.Offset).deref<T>();
+        if (Src.atField(F.Offset).isInitialized())
+          DestField.initialize();
+        if (Activate)
+          DestField.activate();
+      });
+      return true;
+    }
+    // Composite field.
+    return copyComposite(S, OpPC, Src.atField(F.Offset), DestField, Activate);
+  };
+
+  assert(SrcDesc->isRecord());
+  assert(SrcDesc->ElemRecord == DestDesc->ElemRecord);
+  const Record *R = DestDesc->ElemRecord;
+  for (const Record::Field &F : R->fields()) {
+    if (R->isUnion()) {
+      // For unions, only copy the active field.
+      const Pointer &SrcField = Src.atField(F.Offset);
+      if (SrcField.isActive()) {
+        if (!copyField(F, /*Activate=*/true))
+          return false;
+      }
+    } else {
+      if (!copyField(F, Activate))
+        return false;
+    }
+  }
+
+  for (const Record::Base &B : R->bases()) {
+    Pointer DestBase = Dest.atField(B.Offset);
+    if (!copyRecord(S, OpPC, Src.atField(B.Offset), DestBase, Activate))
+      return false;
+  }
+
+  Dest.initialize();
+  return true;
+}
+
+static bool copyComposite(InterpState &S, CodePtr OpPC, const Pointer &Src,
+                          Pointer &Dest, bool Activate = false) {
   assert(Src.isLive() && Dest.isLive());
 
   [[maybe_unused]] const Descriptor *SrcDesc = Src.getFieldDesc();
@@ -1657,44 +1708,14 @@ bool DoMemcpy(InterpState &S, CodePtr OpPC, const Pointer &Src, Pointer &Dest) {
     return true;
   }
 
-  if (DestDesc->isRecord()) {
-    auto copyField = [&](const Record::Field &F, bool Activate) -> bool {
-      Pointer DestField = Dest.atField(F.Offset);
-      if (std::optional<PrimType> FT = S.Ctx.classify(F.Decl->getType())) {
-        TYPE_SWITCH(*FT, {
-          DestField.deref<T>() = Src.atField(F.Offset).deref<T>();
-          DestField.initialize();
-          if (Activate)
-            DestField.activate();
-        });
-        return true;
-      }
-      return Invalid(S, OpPC);
-    };
-
-    assert(SrcDesc->isRecord());
-    assert(SrcDesc->ElemRecord == DestDesc->ElemRecord);
-    const Record *R = DestDesc->ElemRecord;
-    for (const Record::Field &F : R->fields()) {
-      if (R->isUnion()) {
-        // For unions, only copy the active field.
-        const Pointer &SrcField = Src.atField(F.Offset);
-        if (SrcField.isActive()) {
-          if (!copyField(F, /*Activate=*/true))
-            return false;
-        }
-      } else {
-        if (!copyField(F, /*Activate=*/false))
-          return false;
-      }
-    }
-    return true;
-  }
-
-  // FIXME: Composite types.
-
+  if (DestDesc->isRecord())
+    return copyRecord(S, OpPC, Src, Dest, Activate);
   return Invalid(S, OpPC);
 }
 
+bool DoMemcpy(InterpState &S, CodePtr OpPC, const Pointer &Src, Pointer &Dest) {
+  return copyComposite(S, OpPC, Src, Dest);
+}
+
 } // namespace interp
 } // namespace clang

clang/test/AST/Interp/unions.cpp

@@ -361,7 +361,7 @@ namespace CopyCtor {
 
 namespace UnionInBase {
   struct Base {
-    int y;
+    int y; // both-note {{subobject declared here}}
   };
   struct A : Base {
     int x;
@@ -380,5 +380,29 @@ namespace UnionInBase {
   }
   static_assert(read_wrong_member_indirect() == 1); // both-error {{not an integral constant expression}} \
                                                     // both-note {{in call to}}
+  constexpr int read_uninitialized() {
+    B b = {.b = 1};
+    int *p = &b.a.y;
+    b.a.x = 1;
+    return *p; // both-note {{read of uninitialized object}}
+  }
+  static_assert(read_uninitialized() == 0); // both-error {{constant}} \
+                                            // both-note {{in call}}
+  constexpr int write_uninitialized() {
+    B b = {.b = 1};
+    int *p = &b.a.y;
+    b.a.x = 1;
+    *p = 1;
+    return *p;
+  }
+
+  constexpr B return_uninit() {
+    B b = {.b = 1};
+    b.a.x = 2;
+    return b;
+  }
+  constexpr B uninit = return_uninit(); // both-error {{constant expression}} \
+                                        // both-note {{subobject 'y' is not initialized}}
+  static_assert(return_uninit().a.x == 2);
 }
 #endif