[clang] [clang][bytecode] Make union activation more granular (PR #148835)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 15 05:18:58 PDT 2025
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
More information about the cfe-commits
mailing list