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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
