[clang] 3825a7c - [clang][Interp] Fix diagnosing uninitialized nested union fields (#102824)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 12 08:11:27 PDT 2024
Author: Timm Baeder
Date: 2024-08-12T17:11:23+02:00
New Revision: 3825a7c542f362ace2e943f4fc3ec8538750db3c
URL: https://github.com/llvm/llvm-project/commit/3825a7c542f362ace2e943f4fc3ec8538750db3c
DIFF: https://github.com/llvm/llvm-project/commit/3825a7c542f362ace2e943f4fc3ec8538750db3c.diff
LOG: [clang][Interp] Fix diagnosing uninitialized nested union fields (#102824)
We were calling initialize() unconditionally when copying the union.
Added:
Modified:
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/InterpBuiltin.cpp
clang/test/AST/Interp/unions.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 13390007fde33c..4a50b4487b6654 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/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);
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..35b4a520baa269 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,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
More information about the cfe-commits
mailing list