[clang] [clang][bytecode] Check dtor instance pointers for active-ness (PR #128732)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 25 08:17:22 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Timm Baeder (tbaederr)
<details>
<summary>Changes</summary>
And diagnose if we're trying to destroy an inactive member of a union.
---
Full diff: https://github.com/llvm/llvm-project/pull/128732.diff
5 Files Affected:
- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+9-2)
- (modified) clang/lib/AST/ByteCode/Interp.cpp (+69-62)
- (modified) clang/lib/AST/ByteCode/Interp.h (+8)
- (modified) clang/lib/AST/ByteCode/Opcodes.td (+2)
- (modified) clang/test/AST/ByteCode/unions.cpp (+35)
``````````diff
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 74f5d6ebd9ca6..869d7e7fcd625 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -4733,8 +4733,12 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
}
// Explicit calls to trivial destructors
if (const auto *DD = dyn_cast_if_present<CXXDestructorDecl>(FuncDecl);
- DD && DD->isTrivial())
- return true;
+ DD && DD->isTrivial()) {
+ const auto *MemberCall = cast<CXXMemberCallExpr>(E);
+ if (!this->visit(MemberCall->getImplicitObjectArgument()))
+ return false;
+ return this->emitCheckDestruction(E) && this->emitPopPtr(E);
+ }
QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
std::optional<PrimType> T = classify(ReturnType);
@@ -5753,6 +5757,9 @@ bool Compiler<Emitter>::compileDestructor(const CXXDestructorDecl *Dtor) {
if (!this->emitThis(Dtor))
return false;
+ if (!this->emitCheckDestruction(Dtor))
+ return false;
+
assert(R);
if (!R->isUnion()) {
// First, destroy all fields.
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 5e0d2e91fb1b2..c0f9ebe4319b8 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -129,68 +129,6 @@ static void diagnoseNonConstVariable(InterpState &S, CodePtr OpPC,
S.Note(VD->getLocation(), diag::note_declared_at);
}
-static bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
- AccessKinds AK) {
- if (Ptr.isActive())
- return true;
-
- assert(Ptr.inUnion());
- assert(Ptr.isField() && Ptr.getField());
-
- Pointer U = Ptr.getBase();
- Pointer C = Ptr;
- while (!U.isRoot() && !U.isActive()) {
- // A little arbitrary, but this is what the current interpreter does.
- // See the AnonymousUnion test in test/AST/ByteCode/unions.cpp.
- // GCC's output is more similar to what we would get without
- // this condition.
- if (U.getRecord() && U.getRecord()->isAnonymousUnion())
- break;
-
- C = U;
- U = U.getBase();
- }
- assert(C.isField());
-
- // Consider:
- // union U {
- // struct {
- // int x;
- // int y;
- // } a;
- // }
- //
- // When activating x, we will also activate a. If we now try to read
- // from y, we will get to CheckActive, because y is not active. In that
- // case, our U will be a (not a union). We return here and let later code
- // handle this.
- if (!U.getFieldDesc()->isUnion())
- return true;
-
- // Get the inactive field descriptor.
- assert(!C.isActive());
- const FieldDecl *InactiveField = C.getField();
- assert(InactiveField);
-
- // Find the active field of the union.
- const Record *R = U.getRecord();
- assert(R && R->isUnion() && "Not a union");
-
- const FieldDecl *ActiveField = nullptr;
- for (const Record::Field &F : R->fields()) {
- const Pointer &Field = U.atField(F.Offset);
- if (Field.isActive()) {
- ActiveField = Field.getField();
- break;
- }
- }
-
- const SourceInfo &Loc = S.Current->getSource(OpPC);
- S.FFDiag(Loc, diag::note_constexpr_access_inactive_union_member)
- << AK << InactiveField << !ActiveField << ActiveField;
- return false;
-}
-
static bool CheckTemporary(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
AccessKinds AK) {
if (auto ID = Ptr.getDeclID()) {
@@ -290,6 +228,68 @@ void cleanupAfterFunctionCall(InterpState &S, CodePtr OpPC,
TYPE_SWITCH(Ty, S.Stk.discard<T>());
}
+bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
+ AccessKinds AK) {
+ if (Ptr.isActive())
+ return true;
+
+ assert(Ptr.inUnion());
+ assert(Ptr.isField() && Ptr.getField());
+
+ Pointer U = Ptr.getBase();
+ Pointer C = Ptr;
+ while (!U.isRoot() && !U.isActive()) {
+ // A little arbitrary, but this is what the current interpreter does.
+ // See the AnonymousUnion test in test/AST/ByteCode/unions.cpp.
+ // GCC's output is more similar to what we would get without
+ // this condition.
+ if (U.getRecord() && U.getRecord()->isAnonymousUnion())
+ break;
+
+ C = U;
+ U = U.getBase();
+ }
+ assert(C.isField());
+
+ // Consider:
+ // union U {
+ // struct {
+ // int x;
+ // int y;
+ // } a;
+ // }
+ //
+ // When activating x, we will also activate a. If we now try to read
+ // from y, we will get to CheckActive, because y is not active. In that
+ // case, our U will be a (not a union). We return here and let later code
+ // handle this.
+ if (!U.getFieldDesc()->isUnion())
+ return true;
+
+ // Get the inactive field descriptor.
+ assert(!C.isActive());
+ const FieldDecl *InactiveField = C.getField();
+ assert(InactiveField);
+
+ // Find the active field of the union.
+ const Record *R = U.getRecord();
+ assert(R && R->isUnion() && "Not a union");
+
+ const FieldDecl *ActiveField = nullptr;
+ for (const Record::Field &F : R->fields()) {
+ const Pointer &Field = U.atField(F.Offset);
+ if (Field.isActive()) {
+ ActiveField = Field.getField();
+ break;
+ }
+ }
+
+ const SourceInfo &Loc = S.Current->getSource(OpPC);
+ S.FFDiag(Loc, diag::note_constexpr_access_inactive_union_member)
+ << AK << InactiveField << !ActiveField << ActiveField;
+ return false;
+}
+
bool CheckExtern(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
if (!Ptr.isExtern())
return true;
@@ -1265,6 +1265,11 @@ static bool checkConstructor(InterpState &S, CodePtr OpPC, const Function *Func,
return false;
}
+static bool checkDestructor(InterpState &S, CodePtr OpPC, const Function *Func,
+ const Pointer &ThisPtr) {
+ return CheckActive(S, OpPC, ThisPtr, AK_Destroy);
+}
+
bool CallVar(InterpState &S, CodePtr OpPC, const Function *Func,
uint32_t VarArgSize) {
if (Func->hasThisPointer()) {
@@ -1347,6 +1352,8 @@ bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
if (Func->isConstructor() && !checkConstructor(S, OpPC, Func, ThisPtr))
return false;
+ if (Func->isDestructor() && !checkDestructor(S, OpPC, Func, ThisPtr))
+ return false;
}
if (!CheckCallable(S, OpPC, Func))
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index db35208a02941..ac8db43ca9fc1 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -137,6 +137,9 @@ bool CheckNewDeleteForms(InterpState &S, CodePtr OpPC,
bool CheckDeleteSource(InterpState &S, CodePtr OpPC, const Expr *Source,
const Pointer &Ptr);
+bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
+ AccessKinds AK);
+
/// Sets the given integral value to the pointer, which is of
/// a std::{weak,partial,strong}_ordering type.
bool SetThreeWayComparisonField(InterpState &S, CodePtr OpPC,
@@ -3063,6 +3066,11 @@ bool GetTypeid(InterpState &S, CodePtr OpPC, const Type *TypePtr,
bool GetTypeidPtr(InterpState &S, CodePtr OpPC, const Type *TypeInfoType);
bool DiagTypeid(InterpState &S, CodePtr OpPC);
+inline bool CheckDestruction(InterpState &S, CodePtr OpPC) {
+ const auto &Ptr = S.Stk.peek<Pointer>();
+ return CheckActive(S, OpPC, Ptr, AK_Destroy);
+}
+
//===----------------------------------------------------------------------===//
// Read opcode arguments
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td
index 41e4bae65c195..2c362d8de96f8 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -857,3 +857,5 @@ def BitCast : Opcode;
def GetTypeid : Opcode { let Args = [ArgTypePtr, ArgTypePtr]; }
def GetTypeidPtr : Opcode { let Args = [ArgTypePtr]; }
def DiagTypeid : Opcode;
+
+def CheckDestruction : Opcode;
diff --git a/clang/test/AST/ByteCode/unions.cpp b/clang/test/AST/ByteCode/unions.cpp
index 2064cae11e970..07b90ddd9a9d6 100644
--- a/clang/test/AST/ByteCode/unions.cpp
+++ b/clang/test/AST/ByteCode/unions.cpp
@@ -504,4 +504,39 @@ namespace AnonymousUnion {
static_assert(return_init_all().a.p == 7); // both-error {{}} \
// both-note {{read of member 'p' of union with no active member}}
}
+
+namespace InactiveDestroy {
+ struct A {
+ constexpr ~A() {}
+ };
+ union U {
+ A a;
+ constexpr ~U() {
+ }
+ };
+
+ constexpr bool foo() { // both-error {{never produces a constant expression}}
+ U u;
+ u.a.~A(); // both-note 2{{destruction of member 'a' of union with no active member}}
+ return true;
+ }
+ static_assert(foo()); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to}}
+}
+
+namespace InactiveTrivialDestroy {
+ struct A {};
+ union U {
+ A a;
+ };
+
+ constexpr bool foo() { // both-error {{never produces a constant expression}}
+ U u;
+ u.a.~A(); // both-note 2{{destruction of member 'a' of union with no active member}}
+ return true;
+ }
+ static_assert(foo()); // both-error {{not an integral constant expression}} \
+ // both-note {{in call to}}
+}
+
#endif
``````````
</details>
https://github.com/llvm/llvm-project/pull/128732
More information about the cfe-commits
mailing list