https://github.com/tbaederr updated
https://github.com/llvm/llvm-project/pull/107231
>From 6bfbb1b280490a18d817d9ef0be0491925a0a9ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com>
Date: Wed, 4 Sep 2024 14:32:32 +0200
Subject: [PATCH] [clang][bytecode] Allow continuing when discarded MemberExpr
Base fails
We don't need the value in this case, since we're discarding it anyway.
Allow continuing the interpretation but note the side effect.
---
clang/lib/AST/ByteCode/ByteCodeEmitter.h | 2 +-
clang/lib/AST/ByteCode/Compiler.cpp | 39 +++++++++++++++++++-----
clang/lib/AST/ByteCode/Compiler.h | 2 +-
clang/lib/AST/ByteCode/Context.cpp | 7 +++--
clang/lib/AST/ByteCode/Context.h | 3 +-
clang/lib/AST/ByteCode/EvalEmitter.cpp | 5 +--
clang/lib/AST/ByteCode/EvalEmitter.h | 5 +--
clang/lib/AST/ByteCode/Interp.cpp | 33 ++++++++++++--------
clang/lib/AST/ByteCode/Interp.h | 10 ++++--
clang/lib/AST/ByteCode/InterpFrame.cpp | 5 ---
clang/lib/AST/ByteCode/InterpFrame.h | 3 --
clang/lib/AST/ByteCode/InterpState.h | 4 +++
clang/lib/AST/ByteCode/Opcodes.td | 1 +
clang/lib/AST/ByteCode/State.h | 2 ++
clang/lib/AST/ExprConstant.cpp | 6 ++--
clang/test/AST/ByteCode/codegen.cpp | 18 +++++++++++
16 files changed, 101 insertions(+), 44 deletions(-)
diff --git a/clang/lib/AST/ByteCode/ByteCodeEmitter.h
b/clang/lib/AST/ByteCode/ByteCodeEmitter.h
index 7cbbe651699b34..ac728830527a26 100644
--- a/clang/lib/AST/ByteCode/ByteCodeEmitter.h
+++ b/clang/lib/AST/ByteCode/ByteCodeEmitter.h
@@ -46,7 +46,7 @@ class ByteCodeEmitter {
/// Methods implemented by the compiler.
virtual bool visitFunc(const FunctionDecl *E) = 0;
- virtual bool visitExpr(const Expr *E) = 0;
+ virtual bool visitExpr(const Expr *E, bool DestroyToplevelScope) = 0;
virtual bool visitDeclAndReturn(const VarDecl *E, bool ConstantContext) = 0;
/// Emits jumps.
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp
b/clang/lib/AST/ByteCode/Compiler.cpp
index a831f196abdcb5..36a17b88193bfc 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -1773,8 +1773,12 @@ bool Compiler<Emitter>::VisitMemberExpr(const MemberExpr
*E) {
return false;
}
- if (!isa<FieldDecl>(Member))
- return this->discard(Base) && this->visitDeclRef(Member, E);
+ if (!isa<FieldDecl>(Member)) {
+ if (!this->discard(Base) && !this->emitSideEffect(E))
+ return false;
+
+ return this->visitDeclRef(Member, E);
+ }
if (Initializing) {
if (!this->delegate(Base))
@@ -2565,8 +2569,14 @@ bool Compiler<Emitter>::VisitCXXConstructExpr(const
CXXConstructExpr *E) {
if (!this->emitCallVar(Func, VarArgSize, E))
return false;
} else {
- if (!this->emitCall(Func, 0, E))
+ if (!this->emitCall(Func, 0, E)) {
+ // When discarding, we don't need the result anyway, so clean up
+ // the instance dup we did earlier in case surrounding code wants
+ // to keep evaluating.
+ if (DiscardResult)
+ (void)this->emitPopPtr(E);
return false;
+ }
}
if (DiscardResult)
@@ -3249,7 +3259,11 @@ bool Compiler<Emitter>::VisitStmtExpr(const StmtExpr *E)
{
template <class Emitter> bool Compiler<Emitter>::discard(const Expr *E) {
OptionScope<Emitter> Scope(this, /*NewDiscardResult=*/true,
/*NewInitializing=*/false);
+
return this->Visit(E);
+ if (!this->Visit(E))
+ return this->emitSideEffect(E);
+ return true;
}
template <class Emitter> bool Compiler<Emitter>::delegate(const Expr *E) {
@@ -3615,20 +3629,29 @@ const Function *Compiler<Emitter>::getFunction(const
FunctionDecl *FD) {
return Ctx.getOrCreateFunction(FD);
}
-template <class Emitter> bool Compiler<Emitter>::visitExpr(const Expr *E) {
+template <class Emitter>
+bool Compiler<Emitter>::visitExpr(const Expr *E, bool DestroyToplevelScope) {
LocalScope<Emitter> RootScope(this);
+
+ auto maybeDestroyLocals = [&]() -> bool {
+ if (DestroyToplevelScope)
+ return RootScope.destroyLocals();
+ return true;
+ };
+
// Void expressions.
if (E->getType()->isVoidType()) {
if (!visit(E))
return false;
- return this->emitRetVoid(E) && RootScope.destroyLocals();
+ return this->emitRetVoid(E) && maybeDestroyLocals();
}
// Expressions with a primitive return type.
if (std::optional<PrimType> T = classify(E)) {
if (!visit(E))
return false;
- return this->emitRet(*T, E) && RootScope.destroyLocals();
+
+ return this->emitRet(*T, E) && maybeDestroyLocals();
}
// Expressions with a composite return type.
@@ -3646,10 +3669,10 @@ template <class Emitter> bool
Compiler<Emitter>::visitExpr(const Expr *E) {
// We are destroying the locals AFTER the Ret op.
// The Ret op needs to copy the (alive) values, but the
// destructors may still turn the entire expression invalid.
- return this->emitRetValue(E) && RootScope.destroyLocals();
+ return this->emitRetValue(E) && maybeDestroyLocals();
}
- RootScope.destroyLocals();
+ (void)maybeDestroyLocals();
return false;
}
diff --git a/clang/lib/AST/ByteCode/Compiler.h
b/clang/lib/AST/ByteCode/Compiler.h
index b18afacdb2e491..69fd63964becc4 100644
--- a/clang/lib/AST/ByteCode/Compiler.h
+++ b/clang/lib/AST/ByteCode/Compiler.h
@@ -221,7 +221,7 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>,
bool>,
protected:
bool visitStmt(const Stmt *S);
- bool visitExpr(const Expr *E) override;
+ bool visitExpr(const Expr *E, bool DestroyToplevelScope) override;
bool visitFunc(const FunctionDecl *F) override;
bool visitDeclAndReturn(const VarDecl *VD, bool ConstantContext) override;
diff --git a/clang/lib/AST/ByteCode/Context.cpp
b/clang/lib/AST/ByteCode/Context.cpp
index e682d87b703ad7..3fadf858e23d59 100644
--- a/clang/lib/AST/ByteCode/Context.cpp
+++ b/clang/lib/AST/ByteCode/Context.cpp
@@ -69,12 +69,15 @@ bool Context::evaluateAsRValue(State &Parent, const Expr
*E, APValue &Result) {
return true;
}
-bool Context::evaluate(State &Parent, const Expr *E, APValue &Result) {
+bool Context::evaluate(State &Parent, const Expr *E, APValue &Result,
+ ConstantExprKind Kind) {
++EvalID;
bool Recursing = !Stk.empty();
Compiler<EvalEmitter> C(*this, *P, Parent, Stk);
- auto Res = C.interpretExpr(E);
+ auto Res = C.interpretExpr(E, /*ConvertResultToRValue=*/false,
+ /*DestroyToplevelScope=*/Kind ==
+ ConstantExprKind::ClassTemplateArgument);
if (Res.isInvalid()) {
C.cleanup();
Stk.clear();
diff --git a/clang/lib/AST/ByteCode/Context.h b/clang/lib/AST/ByteCode/Context.h
index b8ea4ad6b3b447..e0d4bafdebaf2b 100644
--- a/clang/lib/AST/ByteCode/Context.h
+++ b/clang/lib/AST/ByteCode/Context.h
@@ -52,7 +52,8 @@ class Context final {
bool evaluateAsRValue(State &Parent, const Expr *E, APValue &Result);
/// Like evaluateAsRvalue(), but does no implicit lvalue-to-rvalue
conversion.
- bool evaluate(State &Parent, const Expr *E, APValue &Result);
+ bool evaluate(State &Parent, const Expr *E, APValue &Result,
+ ConstantExprKind Kind);
/// Evaluates a toplevel initializer.
bool evaluateAsInitializer(State &Parent, const VarDecl *VD, APValue
&Result);
diff --git a/clang/lib/AST/ByteCode/EvalEmitter.cpp
b/clang/lib/AST/ByteCode/EvalEmitter.cpp
index 3b9e5f9f9f69cd..7eecee25bb3c1e 100644
--- a/clang/lib/AST/ByteCode/EvalEmitter.cpp
+++ b/clang/lib/AST/ByteCode/EvalEmitter.cpp
@@ -38,13 +38,14 @@ EvalEmitter::~EvalEmitter() {
void EvalEmitter::cleanup() { S.cleanup(); }
EvaluationResult EvalEmitter::interpretExpr(const Expr *E,
- bool ConvertResultToRValue) {
+ bool ConvertResultToRValue,
+ bool DestroyToplevelScope) {
S.setEvalLocation(E->getExprLoc());
this->ConvertResultToRValue = ConvertResultToRValue && !isa<ConstantExpr>(E);
this->CheckFullyInitialized = isa<ConstantExpr>(E);
EvalResult.setSource(E);
- if (!this->visitExpr(E)) {
+ if (!this->visitExpr(E, DestroyToplevelScope)) {
// EvalResult may already have a result set, but something failed
// after that (e.g. evaluating destructors).
EvalResult.setInvalid();
diff --git a/clang/lib/AST/ByteCode/EvalEmitter.h
b/clang/lib/AST/ByteCode/EvalEmitter.h
index 338786d3dea91c..e7c9e80d75d934 100644
--- a/clang/lib/AST/ByteCode/EvalEmitter.h
+++ b/clang/lib/AST/ByteCode/EvalEmitter.h
@@ -35,7 +35,8 @@ class EvalEmitter : public SourceMapper {
using Local = Scope::Local;
EvaluationResult interpretExpr(const Expr *E,
- bool ConvertResultToRValue = false);
+ bool ConvertResultToRValue = false,
+ bool DestroyToplevelScope = false);
EvaluationResult interpretDecl(const VarDecl *VD, bool
CheckFullyInitialized);
/// Clean up all resources.
@@ -54,7 +55,7 @@ class EvalEmitter : public SourceMapper {
LabelTy getLabel();
/// Methods implemented by the compiler.
- virtual bool visitExpr(const Expr *E) = 0;
+ virtual bool visitExpr(const Expr *E, bool DestroyToplevelScope) = 0;
virtual bool visitDeclAndReturn(const VarDecl *VD, bool ConstantContext) = 0;
virtual bool visitFunc(const FunctionDecl *F) = 0;
diff --git a/clang/lib/AST/ByteCode/Interp.cpp
b/clang/lib/AST/ByteCode/Interp.cpp
index 6777ac150abf4f..03da990499984d 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -221,17 +221,17 @@ static void popArg(InterpState &S, const Expr *Arg) {
TYPE_SWITCH(Ty, S.Stk.discard<T>());
}
-void cleanupAfterFunctionCall(InterpState &S, CodePtr OpPC) {
+void cleanupAfterFunctionCall(InterpState &S, CodePtr OpPC,
+ const Function *Func) {
assert(S.Current);
- const Function *CurFunc = S.Current->getFunction();
- assert(CurFunc);
+ assert(Func);
- if (CurFunc->isUnevaluatedBuiltin())
+ if (Func->isUnevaluatedBuiltin())
return;
// Some builtin functions require us to only look at the call site, since
// the classified parameter types do not match.
- if (unsigned BID = CurFunc->getBuiltinID();
+ if (unsigned BID = Func->getBuiltinID();
BID && S.getASTContext().BuiltinInfo.hasCustomTypechecking(BID)) {
const auto *CE =
cast<CallExpr>(S.Current->Caller->getExpr(S.Current->getRetPC()));
@@ -242,7 +242,7 @@ void cleanupAfterFunctionCall(InterpState &S, CodePtr OpPC)
{
return;
}
- if (S.Current->Caller && CurFunc->isVariadic()) {
+ if (S.Current->Caller && Func->isVariadic()) {
// CallExpr we're look for is at the return PC of the current function,
i.e.
// in the caller.
// This code path should be executed very rarely.
@@ -259,8 +259,8 @@ void cleanupAfterFunctionCall(InterpState &S, CodePtr OpPC)
{
} else
assert(false && "Can't get arguments from that expression type");
- assert(NumArgs >= CurFunc->getNumWrittenParams());
- NumVarArgs = NumArgs - (CurFunc->getNumWrittenParams() +
+ assert(NumArgs >= Func->getNumWrittenParams());
+ NumVarArgs = NumArgs - (Func->getNumWrittenParams() +
isa<CXXOperatorCallExpr>(CallSite));
for (unsigned I = 0; I != NumVarArgs; ++I) {
const Expr *A = Args[NumArgs - 1 - I];
@@ -270,7 +270,8 @@ void cleanupAfterFunctionCall(InterpState &S, CodePtr OpPC)
{
// And in any case, remove the fixed parameters (the non-variadic ones)
// at the end.
- S.Current->popArgs();
+ for (PrimType Ty : Func->args_reverse())
+ TYPE_SWITCH(Ty, S.Stk.discard<T>());
}
bool CheckExtern(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
@@ -1036,6 +1037,12 @@ bool CallVar(InterpState &S, CodePtr OpPC, const
Function *Func,
bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
uint32_t VarArgSize) {
+ assert(Func);
+ auto cleanup = [&]() -> bool {
+ cleanupAfterFunctionCall(S, OpPC, Func);
+ return false;
+ };
+
if (Func->hasThisPointer()) {
size_t ArgSize = Func->getArgSize() + VarArgSize;
size_t ThisOffset = ArgSize - (Func->hasRVO() ? primSize(PT_Ptr) : 0);
@@ -1052,22 +1059,22 @@ bool Call(InterpState &S, CodePtr OpPC, const Function
*Func,
assert(ThisPtr.isZero());
} else {
if (!CheckInvoke(S, OpPC, ThisPtr))
- return false;
+ return cleanup();
}
}
if (!CheckCallable(S, OpPC, Func))
- return false;
+ return cleanup();
// FIXME: The isConstructor() check here is not always right. The current
// constant evaluator is somewhat inconsistent in when it allows a function
// call when checking for a constant expression.
if (Func->hasThisPointer() && S.checkingPotentialConstantExpression() &&
!Func->isConstructor())
- return false;
+ return cleanup();
if (!CheckCallDepth(S, OpPC))
- return false;
+ return cleanup();
auto NewFrame = std::make_unique<InterpFrame>(S, Func, OpPC, VarArgSize);
InterpFrame *FrameBefore = S.Current;
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index be900769f25845..31c2667f29362e 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -281,7 +281,8 @@ enum class ArithOp { Add, Sub };
// Returning values
//===----------------------------------------------------------------------===//
-void cleanupAfterFunctionCall(InterpState &S, CodePtr OpPC);
+void cleanupAfterFunctionCall(InterpState &S, CodePtr OpPC,
+ const Function *Func);
template <PrimType Name, class T = typename PrimConv<Name>::T>
bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
@@ -302,7 +303,7 @@ bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
assert(S.Current);
assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
if (!S.checkingPotentialConstantExpression() || S.Current->Caller)
- cleanupAfterFunctionCall(S, PC);
+ cleanupAfterFunctionCall(S, PC, S.Current->getFunction());
if (InterpFrame *Caller = S.Current->Caller) {
PC = S.Current->getRetPC();
@@ -322,7 +323,7 @@ inline bool RetVoid(InterpState &S, CodePtr &PC, APValue
&Result) {
assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
if (!S.checkingPotentialConstantExpression() || S.Current->Caller)
- cleanupAfterFunctionCall(S, PC);
+ cleanupAfterFunctionCall(S, PC, S.Current->getFunction());
if (InterpFrame *Caller = S.Current->Caller) {
PC = S.Current->getRetPC();
@@ -2658,6 +2659,9 @@ inline bool Unsupported(InterpState &S, CodePtr OpPC) {
/// Do nothing and just abort execution.
inline bool Error(InterpState &S, CodePtr OpPC) { return false; }
+inline bool SideEffect(InterpState &S, CodePtr OpPC) {
+ return S.noteSideEffect();
+}
/// Same here, but only for casts.
inline bool InvalidCast(InterpState &S, CodePtr OpPC, CastKind Kind,
diff --git a/clang/lib/AST/ByteCode/InterpFrame.cpp
b/clang/lib/AST/ByteCode/InterpFrame.cpp
index c75386eaeb4c7e..6830a7b37f1da5 100644
--- a/clang/lib/AST/ByteCode/InterpFrame.cpp
+++ b/clang/lib/AST/ByteCode/InterpFrame.cpp
@@ -96,11 +96,6 @@ void InterpFrame::destroy(unsigned Idx) {
}
}
-void InterpFrame::popArgs() {
- for (PrimType Ty : Func->args_reverse())
- TYPE_SWITCH(Ty, S.Stk.discard<T>());
-}
-
template <typename T>
static void print(llvm::raw_ostream &OS, const T &V, ASTContext &ASTCtx,
QualType Ty) {
diff --git a/clang/lib/AST/ByteCode/InterpFrame.h
b/clang/lib/AST/ByteCode/InterpFrame.h
index 1e0d2b1d4424b1..802777a523d9b3 100644
--- a/clang/lib/AST/ByteCode/InterpFrame.h
+++ b/clang/lib/AST/ByteCode/InterpFrame.h
@@ -46,9 +46,6 @@ class InterpFrame final : public Frame {
void destroy(unsigned Idx);
void initScope(unsigned Idx);
- /// Pops the arguments off the stack.
- void popArgs();
-
/// Describes the frame with arguments for diagnostic purposes.
void describe(llvm::raw_ostream &OS) const override;
diff --git a/clang/lib/AST/ByteCode/InterpState.h
b/clang/lib/AST/ByteCode/InterpState.h
index 961ba5f5c28a09..4b7371450cc98e 100644
--- a/clang/lib/AST/ByteCode/InterpState.h
+++ b/clang/lib/AST/ByteCode/InterpState.h
@@ -68,6 +68,9 @@ class InterpState final : public State, public SourceMapper {
bool keepEvaluatingAfterFailure() const override {
return Parent.keepEvaluatingAfterFailure();
}
+ bool keepEvaluatingAfterSideEffect() const override {
+ return Parent.keepEvaluatingAfterSideEffect();
+ }
bool checkingPotentialConstantExpression() const override {
return Parent.checkingPotentialConstantExpression();
}
@@ -83,6 +86,7 @@ class InterpState final : public State, public SourceMapper {
Parent.setFoldFailureDiagnostic(Flag);
}
bool hasPriorDiagnostic() override { return Parent.hasPriorDiagnostic(); }
+ bool noteSideEffect() override { return Parent.noteSideEffect(); }
/// Reports overflow and return true if evaluation should continue.
bool reportOverflow(const Expr *E, const llvm::APSInt &Value);
diff --git a/clang/lib/AST/ByteCode/Opcodes.td
b/clang/lib/AST/ByteCode/Opcodes.td
index 67350abe5401e1..5d7a6e94f6e228 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -732,6 +732,7 @@ def Flip : Opcode {
def Invalid : Opcode {}
def Unsupported : Opcode {}
def Error : Opcode {}
+def SideEffect : Opcode {}
def InvalidCast : Opcode {
let Args = [ArgCastKind, ArgBool];
}
diff --git a/clang/lib/AST/ByteCode/State.h b/clang/lib/AST/ByteCode/State.h
index 2cffce4bc2ae40..846a6b77d16b37 100644
--- a/clang/lib/AST/ByteCode/State.h
+++ b/clang/lib/AST/ByteCode/State.h
@@ -61,6 +61,7 @@ class State {
virtual bool checkingPotentialConstantExpression() const = 0;
virtual bool noteUndefinedBehavior() = 0;
virtual bool keepEvaluatingAfterFailure() const = 0;
+ virtual bool keepEvaluatingAfterSideEffect() const = 0;
virtual Frame *getCurrentFrame() = 0;
virtual const Frame *getBottomFrame() const = 0;
virtual bool hasActiveDiagnostic() = 0;
@@ -70,6 +71,7 @@ class State {
virtual ASTContext &getASTContext() const = 0;
virtual bool hasPriorDiagnostic() = 0;
virtual unsigned getCallStackDepth() = 0;
+ virtual bool noteSideEffect() = 0;
public:
State() = default;
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 3dc13c14c00343..9904feb5bf32b5 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -1222,7 +1222,7 @@ namespace {
public:
/// Should we continue evaluation after encountering a side-effect that we
/// couldn't model?
- bool keepEvaluatingAfterSideEffect() {
+ bool keepEvaluatingAfterSideEffect() const override {
switch (EvalMode) {
case EM_IgnoreSideEffects:
return true;
@@ -1240,7 +1240,7 @@ namespace {
/// Note that we have had a side-effect, and determine whether we should
/// keep evaluating.
- bool noteSideEffect() {
+ bool noteSideEffect() override {
EvalStatus.HasSideEffects = true;
return keepEvaluatingAfterSideEffect();
}
@@ -16269,7 +16269,7 @@ bool Expr::EvaluateAsConstantExpr(EvalResult &Result,
const ASTContext &Ctx,
Info.InConstantContext = true;
if (Info.EnableNewConstInterp) {
- if (!Info.Ctx.getInterpContext().evaluate(Info, this, Result.Val))
+ if (!Info.Ctx.getInterpContext().evaluate(Info, this, Result.Val, Kind))
return false;
return CheckConstantExpression(Info, getExprLoc(),
getStorageType(Ctx, this), Result.Val,
Kind);
diff --git a/clang/test/AST/ByteCode/codegen.cpp
b/clang/test/AST/ByteCode/codegen.cpp
index 9fac28a52d3150..12d8b5a5c548e1 100644
--- a/clang/test/AST/ByteCode/codegen.cpp
+++ b/clang/test/AST/ByteCode/codegen.cpp
@@ -73,3 +73,21 @@ namespace Null {
// CHECK: call {{.*}} @_ZN4Null4nullEv(
int S::*q = null();
}
+
+struct A {
+ A();
+ ~A();
+ enum E { Foo };
+};
+
+A *g();
+
+void f(A *a) {
+ A::E e1 = a->Foo;
+
+ // CHECK: call noundef ptr @_Z1gv()
+ A::E e2 = g()->Foo;
+ // CHECK: call void @_ZN1AC1Ev(
+ // CHECK: call void @_ZN1AD1Ev(
+ A::E e3 = A().Foo;
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
