[clang][Interp] Fix diagnosing uninitialized nested union fields

[tbaederr]

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

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

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

---

Full diff: https://github.com/llvm/llvm-project/pull/102824.diff

3 Files Affected:

• (modified) clang/lib/AST/Interp/Interp.cpp (+6-1)
• (modified) clang/lib/AST/Interp/InterpBuiltin.cpp (+58-37)
• (modified) clang/test/AST/Interp/unions.cpp (+24-1)

diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 89ac6938931133..c65d3789333852 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -125,12 +125,17 @@ static bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
   if (Ptr.isActive())
     return true;
 
+  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);
diff --git a/clang/lib/AST/Interp/InterpBuiltin.cpp b/clang/lib/AST/Interp/InterpBuiltin.cpp
index 1841a2a4714d89..c3370e2e5286e0 100644
--- a/clang/lib/AST/Interp/InterpBuiltin.cpp
+++ b/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
diff --git a/clang/test/AST/Interp/unions.cpp b/clang/test/AST/Interp/unions.cpp
index 996d29e143fe2c..a8db0597ec8e36 100644
--- a/clang/test/AST/Interp/unions.cpp
+++ b/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,28 @@ 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}}
 }
 #endif