Timm =?utf-8?q?Bäder?= <tbae...@redhat.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/128...@github.com>
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/128732 >From 87209d26acf78f2566a7d1e9fd6dee7f455b4e57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Tue, 25 Feb 2025 17:14:36 +0100 Subject: [PATCH 1/2] [clang][bytecode] Check dtor instance pointers for active-ness And diagnose if we're trying to destroy an inactive member of a union. --- clang/lib/AST/ByteCode/Compiler.cpp | 11 ++- clang/lib/AST/ByteCode/Interp.cpp | 131 +++++++++++++++------------- clang/lib/AST/ByteCode/Interp.h | 8 ++ clang/lib/AST/ByteCode/Opcodes.td | 2 + clang/test/AST/ByteCode/unions.cpp | 35 ++++++++ 5 files changed, 123 insertions(+), 64 deletions(-) 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 >From 6f229c2bfc468418b0eac3daa39ef0ac2ca78e59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Wed, 26 Feb 2025 08:18:35 +0100 Subject: [PATCH 2/2] Add another test case --- clang/test/AST/ByteCode/unions.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/clang/test/AST/ByteCode/unions.cpp b/clang/test/AST/ByteCode/unions.cpp index 07b90ddd9a9d6..2cdac40d54bbb 100644 --- a/clang/test/AST/ByteCode/unions.cpp +++ b/clang/test/AST/ByteCode/unions.cpp @@ -539,4 +539,17 @@ namespace InactiveTrivialDestroy { // 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
