llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> Only activate things if the syntactical structure suggests so. This adds a bunch of new opcodes to control whether to activate in stores, etc. Fixes #<!-- -->134789 --- Patch is 33.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/148835.diff 8 Files Affected: - (modified) clang/lib/AST/ByteCode/Compiler.cpp (+137-49) - (modified) clang/lib/AST/ByteCode/Compiler.h (+3-1) - (modified) clang/lib/AST/ByteCode/Interp.cpp (+5-2) - (modified) clang/lib/AST/ByteCode/Interp.h (+159-36) - (modified) clang/lib/AST/ByteCode/Opcodes.td (+17-11) - (modified) clang/lib/AST/ByteCode/Pointer.cpp (+10-1) - (modified) clang/test/AST/ByteCode/cxx23.cpp (+19) - (modified) clang/test/AST/ByteCode/unions.cpp (+138-3) ``````````diff diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index afa3b7ea7de7e..bdbfb294ff28e 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -25,6 +25,28 @@ using APSInt = llvm::APSInt; namespace clang { namespace interp { +static bool refersToUnion(const Expr *E) { + QualType Ty = E->getType(); + if (Ty->isPointerType() && Ty->getPointeeType()->isUnionType()) + return true; + if (Ty->isUnionType()) + return true; + + if (const auto *ME = dyn_cast<MemberExpr>(E)) + return refersToUnion(ME->getBase()->IgnoreParenImpCasts()); + + if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(E)) + return refersToUnion(ASE->getBase()->IgnoreImplicit()); + + if (const auto *ICE = dyn_cast<ImplicitCastExpr>(E); + ICE && (ICE->getCastKind() == CK_NoOp || + ICE->getCastKind() == CK_DerivedToBase || + ICE->getCastKind() == CK_UncheckedDerivedToBase)) + return refersToUnion(ICE->getSubExpr()); + + return false; +} + static std::optional<bool> getBoolValue(const Expr *E) { if (const auto *CE = dyn_cast_if_present<ConstantExpr>(E); CE && CE->hasAPValueResult() && @@ -880,22 +902,11 @@ bool Compiler<Emitter>::VisitBinaryOperator(const BinaryOperator *BO) { return this->VisitPointerArithBinOp(BO); } - // Assignments require us to evalute the RHS first. - if (BO->getOpcode() == BO_Assign) { + if (BO->getOpcode() == BO_Assign) + return this->visitAssignment(LHS, RHS, BO); - if (!visit(RHS) || !visit(LHS)) - return false; - - // We don't support assignments in C. - if (!Ctx.getLangOpts().CPlusPlus && !this->emitInvalid(BO)) - return false; - - if (!this->emitFlip(*LT, *RT, BO)) - return false; - } else { - if (!visit(LHS) || !visit(RHS)) - return false; - } + if (!visit(LHS) || !visit(RHS)) + return false; // For languages such as C, cast the result of one // of our comparision opcodes to T (which is usually int). @@ -946,22 +957,6 @@ bool Compiler<Emitter>::VisitBinaryOperator(const BinaryOperator *BO) { if (BO->getType()->isFloatingType()) return Discard(this->emitDivf(getFPOptions(BO), BO)); return Discard(this->emitDiv(*T, BO)); - case BO_Assign: - if (DiscardResult) - return LHS->refersToBitField() ? this->emitStoreBitFieldPop(*T, BO) - : this->emitStorePop(*T, BO); - if (LHS->refersToBitField()) { - if (!this->emitStoreBitField(*T, BO)) - return false; - } else { - if (!this->emitStore(*T, BO)) - return false; - } - // Assignments aren't necessarily lvalues in C. - // Load from them in that case. - if (!BO->isLValue()) - return this->emitLoadPop(*T, BO); - return true; case BO_And: return Discard(this->emitBitAnd(*T, BO)); case BO_Or: @@ -1790,19 +1785,26 @@ bool Compiler<Emitter>::visitInitList(ArrayRef<const Expr *> Inits, return this->delegate(Inits[0]); auto initPrimitiveField = [=](const Record::Field *FieldToInit, - const Expr *Init, PrimType T) -> bool { + const Expr *Init, PrimType T, + bool Activate = false) -> bool { InitStackScope<Emitter> ISS(this, isa<CXXDefaultInitExpr>(Init)); InitLinkScope<Emitter> ILS(this, InitLink::Field(FieldToInit->Offset)); if (!this->visit(Init)) return false; - if (FieldToInit->isBitField()) + bool BitField = FieldToInit->isBitField(); + if (BitField && Activate) + return this->emitInitBitFieldActivate(T, FieldToInit, E); + if (BitField) return this->emitInitBitField(T, FieldToInit, E); + if (Activate) + return this->emitInitFieldActivate(T, FieldToInit->Offset, E); return this->emitInitField(T, FieldToInit->Offset, E); }; auto initCompositeField = [=](const Record::Field *FieldToInit, - const Expr *Init) -> bool { + const Expr *Init, + bool Activate = false) -> bool { InitStackScope<Emitter> ISS(this, isa<CXXDefaultInitExpr>(Init)); InitLinkScope<Emitter> ILS(this, InitLink::Field(FieldToInit->Offset)); @@ -1810,6 +1812,10 @@ bool Compiler<Emitter>::visitInitList(ArrayRef<const Expr *> Inits, // on the stack and recurse into visitInitializer(). if (!this->emitGetPtrField(FieldToInit->Offset, Init)) return false; + + if (Activate && !this->emitActivate(E)) + return false; + if (!this->visitInitializer(Init)) return false; return this->emitPopPtr(E); @@ -1829,10 +1835,10 @@ bool Compiler<Emitter>::visitInitList(ArrayRef<const Expr *> Inits, const Record::Field *FieldToInit = R->getField(FToInit); if (std::optional<PrimType> T = classify(Init)) { - if (!initPrimitiveField(FieldToInit, Init, *T)) + if (!initPrimitiveField(FieldToInit, Init, *T, /*Activate=*/true)) return false; } else { - if (!initCompositeField(FieldToInit, Init)) + if (!initCompositeField(FieldToInit, Init, /*Activate=*/true)) return false; } } @@ -2023,7 +2029,8 @@ bool Compiler<Emitter>::visitArrayElemInit(unsigned ElemIndex, const Expr *Init, template <class Emitter> bool Compiler<Emitter>::visitCallArgs(ArrayRef<const Expr *> Args, - const FunctionDecl *FuncDecl) { + const FunctionDecl *FuncDecl, + bool Activate) { assert(VarScope->getKind() == ScopeKind::Call); llvm::BitVector NonNullArgs = collectNonNullArgs(FuncDecl, Args); @@ -2046,6 +2053,11 @@ bool Compiler<Emitter>::visitCallArgs(ArrayRef<const Expr *> Args, return false; } + if (ArgIndex == 1 && Activate) { + if (!this->emitActivate(Arg)) + return false; + } + if (FuncDecl && NonNullArgs[ArgIndex]) { PrimType ArgT = classify(Arg).value_or(PT_Ptr); if (ArgT == PT_Ptr) { @@ -4227,10 +4239,13 @@ bool Compiler<Emitter>::visitZeroRecordInitializer(const Record *R, PrimType T = classifyPrim(D->getType()); if (!this->visitZeroInitializer(T, QT, E)) return false; + if (R->isUnion()) { + if (!this->emitInitFieldActivate(T, Field.Offset, E)) + return false; + break; + } if (!this->emitInitField(T, Field.Offset, E)) return false; - if (R->isUnion()) - break; continue; } @@ -4256,13 +4271,15 @@ bool Compiler<Emitter>::visitZeroRecordInitializer(const Record *R, } else return false; - if (!this->emitFinishInitPop(E)) - return false; - // C++11 [dcl.init]p5: If T is a (possibly cv-qualified) union type, the // object's first non-static named data member is zero-initialized - if (R->isUnion()) + if (R->isUnion()) { + if (!this->emitFinishInitActivatePop(E)) + return false; break; + } + if (!this->emitFinishInitPop(E)) + return false; } for (const Record::Base &B : R->bases()) { @@ -4325,6 +4342,61 @@ bool Compiler<Emitter>::visitZeroArrayInitializer(QualType T, const Expr *E) { return false; } +template <class Emitter> +bool Compiler<Emitter>::visitAssignment(const Expr *LHS, const Expr *RHS, + const Expr *E) { + std::optional<PrimType> T = classify(E->getType()); + + if (!T) + return false; + + if (!this->visit(RHS)) + return false; + if (!this->visit(LHS)) + return false; + + // We don't support assignments in C. + if (!Ctx.getLangOpts().CPlusPlus && !this->emitInvalid(E)) + return false; + + PrimType RHT = classifyPrim(RHS); + bool Activates = refersToUnion(LHS); + bool BitField = LHS->refersToBitField(); + + if (!this->emitFlip(PT_Ptr, RHT, E)) + return false; + + if (DiscardResult) { + if (BitField && Activates) + return this->emitStoreBitFieldActivatePop(RHT, E); + if (BitField) + return this->emitStoreBitFieldPop(RHT, E); + if (Activates) + return this->emitStoreActivatePop(RHT, E); + // Otherwise, regular non-activating store. + return this->emitStorePop(RHT, E); + } + + auto maybeLoad = [&](bool Result) -> bool { + if (!Result) + return false; + // Assignments aren't necessarily lvalues in C. + // Load from them in that case. + if (!E->isLValue()) + return this->emitLoadPop(RHT, E); + return true; + }; + + if (BitField && Activates) + return maybeLoad(this->emitStoreBitFieldActivate(RHT, E)); + if (BitField) + return maybeLoad(this->emitStoreBitField(RHT, E)); + if (Activates) + return maybeLoad(this->emitStoreActivate(RHT, E)); + // Otherwise, regular non-activating store. + return maybeLoad(this->emitStore(RHT, E)); +} + template <class Emitter> template <typename T> bool Compiler<Emitter>::emitConst(T Value, PrimType Ty, const Expr *E) { @@ -5067,7 +5139,7 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) { return false; } - if (!this->visitCallArgs(Args, FuncDecl)) + if (!this->visitCallArgs(Args, FuncDecl, IsAssignmentOperatorCall)) return false; // Undo the argument reversal we did earlier. @@ -5851,7 +5923,8 @@ bool Compiler<Emitter>::compileConstructor(const CXXConstructorDecl *Ctor) { assert(!ReturnType); auto emitFieldInitializer = [&](const Record::Field *F, unsigned FieldOffset, - const Expr *InitExpr) -> bool { + const Expr *InitExpr, + bool Activate = false) -> bool { // We don't know what to do with these, so just return false. if (InitExpr->getType().isNull()) return false; @@ -5860,8 +5933,13 @@ bool Compiler<Emitter>::compileConstructor(const CXXConstructorDecl *Ctor) { if (!this->visit(InitExpr)) return false; - if (F->isBitField()) + bool BitField = F->isBitField(); + if (BitField && Activate) + return this->emitInitThisBitFieldActivate(*T, F, FieldOffset, InitExpr); + if (BitField) return this->emitInitThisBitField(*T, F, FieldOffset, InitExpr); + if (Activate) + return this->emitInitThisFieldActivate(*T, FieldOffset, InitExpr); return this->emitInitThisField(*T, FieldOffset, InitExpr); } // Non-primitive case. Get a pointer to the field-to-initialize @@ -5870,6 +5948,9 @@ bool Compiler<Emitter>::compileConstructor(const CXXConstructorDecl *Ctor) { if (!this->emitGetPtrThisField(FieldOffset, InitExpr)) return false; + if (Activate && !this->emitActivate(InitExpr)) + return false; + if (!this->visitInitializer(InitExpr)) return false; @@ -5880,8 +5961,9 @@ bool Compiler<Emitter>::compileConstructor(const CXXConstructorDecl *Ctor) { const Record *R = this->getRecord(RD); if (!R) return false; + bool IsUnion = R->isUnion(); - if (R->isUnion() && Ctor->isCopyOrMoveConstructor()) { + if (IsUnion && Ctor->isCopyOrMoveConstructor()) { if (R->getNumFields() == 0) return this->emitRetVoid(Ctor); // union copy and move ctors are special. @@ -5908,7 +5990,7 @@ bool Compiler<Emitter>::compileConstructor(const CXXConstructorDecl *Ctor) { if (const FieldDecl *Member = Init->getMember()) { const Record::Field *F = R->getField(Member); - if (!emitFieldInitializer(F, F->Offset, InitExpr)) + if (!emitFieldInitializer(F, F->Offset, InitExpr, IsUnion)) return false; } else if (const Type *Base = Init->getBaseClass()) { const auto *BaseDecl = Base->getAsCXXRecordDecl(); @@ -5928,11 +6010,15 @@ bool Compiler<Emitter>::compileConstructor(const CXXConstructorDecl *Ctor) { return false; } + if (IsUnion && !this->emitActivate(InitExpr)) + return false; + if (!this->visitInitializer(InitExpr)) return false; if (!this->emitFinishInitPop(InitExpr)) return false; } else if (const IndirectFieldDecl *IFD = Init->getIndirectMember()) { + assert(IFD->getChainingSize() >= 2); unsigned NestedFieldOffset = 0; @@ -5944,12 +6030,14 @@ bool Compiler<Emitter>::compileConstructor(const CXXConstructorDecl *Ctor) { NestedField = FieldRecord->getField(FD); assert(NestedField); + IsUnion = IsUnion || FieldRecord->isUnion(); NestedFieldOffset += NestedField->Offset; } assert(NestedField); - if (!emitFieldInitializer(NestedField, NestedFieldOffset, InitExpr)) + if (!emitFieldInitializer(NestedField, NestedFieldOffset, InitExpr, + IsUnion)) return false; // Mark all chain links as initialized. diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h index 05ba7460b343b..debee6726853b 100644 --- a/clang/lib/AST/ByteCode/Compiler.h +++ b/clang/lib/AST/ByteCode/Compiler.h @@ -307,7 +307,8 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, const Expr *E); bool visitArrayElemInit(unsigned ElemIndex, const Expr *Init, std::optional<PrimType> InitT); - bool visitCallArgs(ArrayRef<const Expr *> Args, const FunctionDecl *FuncDecl); + bool visitCallArgs(ArrayRef<const Expr *> Args, const FunctionDecl *FuncDecl, + bool Activate); /// Creates a local primitive value. unsigned allocateLocalPrimitive(DeclTy &&Decl, PrimType Ty, bool IsConst, @@ -342,6 +343,7 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, bool visitZeroInitializer(PrimType T, QualType QT, const Expr *E); bool visitZeroRecordInitializer(const Record *R, const Expr *E); bool visitZeroArrayInitializer(QualType T, const Expr *E); + bool visitAssignment(const Expr *LHS, const Expr *RHS, const Expr *E); /// Emits an APSInt constant. bool emitConst(const llvm::APSInt &Value, PrimType Ty, const Expr *E); diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp index 457de2bed37d6..98fb8c8fcded5 100644 --- a/clang/lib/AST/ByteCode/Interp.cpp +++ b/clang/lib/AST/ByteCode/Interp.cpp @@ -326,7 +326,6 @@ 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; @@ -805,6 +804,8 @@ bool CheckStore(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { return false; if (!CheckRange(S, OpPC, Ptr, AK_Assign)) return false; + if (!CheckActive(S, OpPC, Ptr, AK_Assign)) + return false; if (!CheckGlobal(S, OpPC, Ptr)) return false; if (!CheckConst(S, OpPC, Ptr)) @@ -1500,7 +1501,6 @@ bool Call(InterpState &S, CodePtr OpPC, const Function *Func, if (!CheckInvoke(S, OpPC, ThisPtr)) return cleanup(); if (!Func->isConstructor() && !Func->isDestructor() && - !Func->isCopyOrMoveOperator() && !CheckActive(S, OpPC, ThisPtr, AK_MemberCall)) return false; } @@ -1773,6 +1773,9 @@ bool CheckNewTypeMismatch(InterpState &S, CodePtr OpPC, const Expr *E, std::optional<uint64_t> ArraySize) { const Pointer &Ptr = S.Stk.peek<Pointer>(); + if (Ptr.inUnion() && Ptr.getBase().getRecord()->isUnion()) + Ptr.activate(); + // Similar to CheckStore(), but with the additional CheckTemporary() call and // the AccessKinds are different. if (!CheckTemporary(S, OpPC, Ptr, AK_Construct)) diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 9d17f96c97c85..6be68e4a978b5 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -1592,6 +1592,21 @@ bool InitThisField(InterpState &S, CodePtr OpPC, uint32_t I) { if (!CheckThis(S, OpPC, This)) return false; const Pointer &Field = This.atField(I); + assert(Field.canBeInitialized()); + Field.deref<T>() = S.Stk.pop<T>(); + Field.initialize(); + return true; +} + +template <PrimType Name, class T = typename PrimConv<Name>::T> +bool InitThisFieldActivate(InterpState &S, CodePtr OpPC, uint32_t I) { + if (S.checkingPotentialConstantExpression() && S.Current->getDepth() == 0) + return false; + const Pointer &This = S.Current->getThis(); + if (!CheckThis(S, OpPC, This)) + return false; + const Pointer &Field = This.atField(I); + assert(Field.canBeInitialized()); Field.deref<T>() = S.Stk.pop<T>(); Field.activate(); Field.initialize(); @@ -1610,9 +1625,28 @@ bool InitThisBitField(InterpState &S, CodePtr OpPC, const Record::Field *F, if (!CheckThis(S, OpPC, This)) return false; const Pointer &Field = This.atField(FieldOffset); + assert(Field.canBeInitialized()); + const auto &Value = S.Stk.pop<T>(); + Field.deref<T>() = Value.truncate(F->Decl->getBitWidthValue()); + Field.initialize(); + return true; +} + +template <PrimType Name, class T = typename PrimConv<Name>::T> +bool InitThisBitFieldActivate(InterpState &S, CodePtr OpPC, + const Record::Field *F, uint32_t FieldOffset) { + assert(F->isBitField()); + if (S.checkingPotentialConstantExpression() && S.Current->getDepth() == 0) + return false; + const Pointer &This = S.Current->getThis(); + if (!CheckThis(S, OpPC, This)) + return false; + const Pointer &Field = This.atField(FieldOffset); + assert(Field.canBeInitialized()); const auto &Value = S.Stk.pop<T>(); Field.deref<T>() = Value.truncate(F->Decl->getBitWidthValue()); Field.initialize(); + Field.activate(); return true; } @@ -1621,6 +1655,18 @@ bool InitThisBitField(InterpState &S, CodePtr OpPC, const Record::Field *F, /// 3) Pushes the value to field I of the pointer on the stack template <PrimType Name, class T = typename PrimConv<Name>::T> bool InitField(InterpState &S, CodePtr OpPC, uint32_t I) { + const T &Value = S.Stk.pop<T>(); + const Pointer &Ptr = S.Stk.peek<Pointer>(); + if (!CheckRange(S, OpPC, Ptr, CSK_Field)) + return false; + const Pointer &Field = Ptr.atField(I); + Field.deref<T>() = Value; + Field.initialize(); + return true; +} + +template <PrimType Name, class T = typename PrimConv<Name>::T> +bool InitFieldActivate(InterpState &S, CodePtr OpPC, uint32_t I) { const T &Value = S.Stk.pop<T>(); const Pointer &Ptr = S.Stk.peek<Pointer>(); if (!CheckRange(S, OpPC, Ptr, CSK_Field)) @@ -1638,6 +1684,32 @@ bool InitBitField(InterpState &S, CodePtr OpPC, const Record::Field *F) { const T &Value = S.Stk.pop<T>(); const Pointer &Field = S.Stk.peek<Pointer>().atField(F->Offset); + if constexpr (needsAlloc<T>()) { + T Result = S.allocAP<T>(Value.bitWidth()); + if (T::isSigned()) + Result.copy(Value.toAPSInt() + .trunc(F->Decl->getBitWidthValue()) + .sextOrTrunc(Value.bitWidth())); + else + Result.copy(Value.toAPSInt() + .trunc(F->Decl->getBitWidthValue()) + .zextOrTrunc(Value.bitWidth())); + + Field.deref<T>() = Result; + } else { + Field.deref<T>() = Value.truncate(F->Decl->getBitWidthValue()); + } + Field.initialize(); + return true; +} + +template <PrimType Name, class T = typename PrimConv<Name>::T> +bool InitBitFieldActivate(InterpState &S, CodePtr OpPC, + const Record::Field *F) { + assert(F->isBitField()); + const T &Value = S.Stk.pop<T>(); + const Pointer &Field = S.Stk.peek<Pointer>().atField(F->Offset); + if constexpr (needsAlloc<T>()) { T Result = S.allocAP<T>(Value.bitWidth()); if (T::isSigned()) @@ -1695,32 +1767,6 @@ inline bool GetPtrThisField(InterpState &S, CodePtr OpPC, uint32_t Off) { return true; } -inline bool GetPtrActiveField(InterpState &S, CodePtr OpPC, uint32_t Off) { - const Pointer &Ptr = S.Stk.pop<Pointer>(); - if (!CheckNull(S, OpPC, Ptr, CSK_Field)) - return false; - if (!CheckRange(S, OpPC, Ptr, CSK_Field)) - return false; - Pointer Field = Ptr.atField(Off); - Ptr.deactivate(); - Field.activate(); - S.Stk.push<Pointer>(std::move(Field)); - return true; -} - -inline bool GetPtrActiveThisField(InterpState &S, CodePtr OpPC, uint32_t Of... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/148835 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
