[clang] ca1bde0 - [clang][bytecode] Check dtor instance pointers for active-ness (#128732)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 17 11:01:38 PDT 2025
Author: Timm Baeder
Date: 2025-03-17T19:01:35+01:00
New Revision: ca1bde0b91a6129e7bacee0fa67e4331b06dd683
URL: https://github.com/llvm/llvm-project/commit/ca1bde0b91a6129e7bacee0fa67e4331b06dd683
DIFF: https://github.com/llvm/llvm-project/commit/ca1bde0b91a6129e7bacee0fa67e4331b06dd683.diff
LOG: [clang][bytecode] Check dtor instance pointers for active-ness (#128732)
And diagnose if we're trying to destroy an inactive member of a union.
Added:
Modified:
clang/lib/AST/ByteCode/Compiler.cpp
clang/lib/AST/ByteCode/Interp.cpp
clang/lib/AST/ByteCode/Interp.h
clang/lib/AST/ByteCode/Opcodes.td
clang/test/AST/ByteCode/unions.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 3524ab5f86de8..e76d93728e0db 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -4787,8 +4787,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);
@@ -5829,6 +5833,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 ffd2b31147d20..9b30a3d418ee3 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -203,68 +203,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()) {
@@ -376,7 +314,68 @@ bool CheckBCPResult(InterpState &S, const Pointer &Ptr) {
if (const Expr *Base = Ptr.getDeclDesc()->asExpr())
return isa<StringLiteral>(Base);
+ return false;
+}
+
+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;
}
@@ -1358,6 +1357,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);
+}
+
static void compileFunction(InterpState &S, const Function *Func) {
Compiler<ByteCodeEmitter>(S.getContext(), S.P)
.compileFunc(Func->getDecl(), const_cast<Function *>(Func));
@@ -1443,13 +1447,15 @@ bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
} else {
if (!CheckInvoke(S, OpPC, ThisPtr))
return cleanup();
- if (!Func->isConstructor() &&
+ if (!Func->isConstructor() && !Func->isDestructor() &&
!CheckActive(S, OpPC, ThisPtr, AK_MemberCall))
return false;
}
if (Func->isConstructor() && !checkConstructor(S, OpPC, Func, ThisPtr))
return false;
+ if (Func->isDestructor() && !checkDestructor(S, OpPC, Func, ThisPtr))
+ return false;
}
if (!Func->isFullyCompiled())
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index f2ddeac99cd7e..26882e1bc9c2d 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,
@@ -3105,6 +3108,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 98f9818cb5ffb..c29793ec886e5 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -865,3 +865,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 72b9b18609720..70524fd36bcc2 100644
--- a/clang/test/AST/ByteCode/unions.cpp
+++ b/clang/test/AST/ByteCode/unions.cpp
@@ -522,4 +522,52 @@ namespace MemberCalls {
static_assert(foo()); // both-error {{not an integral constant expression}} \
// both-note {{in call to}}
}
+
+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}}
+}
+
+namespace ActiveDestroy {
+ struct A {};
+ union U {
+ A a;
+ };
+ constexpr bool foo2() {
+ U u{};
+ u.a.~A();
+ return true;
+ }
+ static_assert(foo2());
+}
+
#endif
More information about the cfe-commits
mailing list