[clang] [clang][Interp] Do r-to-l conversion immediately when returning (PR #80662)
Timm Baeder via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 14 08:34:00 PST 2024
Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>,
Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/80662 at github.com>
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/80662
>From c10713b9a6494aa052541c4ac3a296d83a890867 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Wed, 14 Feb 2024 15:29:18 +0100
Subject: [PATCH 1/3] [clang][Interp][NFC] Make a local variable const
---
clang/lib/AST/Interp/Source.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/AST/Interp/Source.cpp b/clang/lib/AST/Interp/Source.cpp
index 4e032c92d26df1..45cd0ad4fd4273 100644
--- a/clang/lib/AST/Interp/Source.cpp
+++ b/clang/lib/AST/Interp/Source.cpp
@@ -33,7 +33,7 @@ SourceRange SourceInfo::getRange() const {
}
const Expr *SourceInfo::asExpr() const {
- if (auto *S = Source.dyn_cast<const Stmt *>())
+ if (const auto *S = Source.dyn_cast<const Stmt *>())
return dyn_cast<Expr>(S);
return nullptr;
}
>From beccd36e34a067a7193b32c27656dca1a38116d7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Wed, 14 Feb 2024 15:29:47 +0100
Subject: [PATCH 2/3] [clang][Interp] Fix variadic member functions
For variadic member functions, the way we calculated the instance
pointer and RVO pointer offsts on the stack was incorrect, due
to Func->getArgSize() not returning the full size of all the
passed arguments. When calling variadic functions, we need
to pass the size of the passed (variadic) arguments to the Call*
ops, so they can use that information to properly check the
instance pointer, etc.
This patch adds a bit of code duplication in Interp.h, which I
will get rid of in later cleanup NFC patches.
---
clang/lib/AST/Interp/ByteCodeExprGen.cpp | 45 +++++++++++---
clang/lib/AST/Interp/ByteCodeStmtGen.cpp | 2 +-
clang/lib/AST/Interp/Context.cpp | 3 +-
clang/lib/AST/Interp/EvalEmitter.cpp | 2 +-
clang/lib/AST/Interp/Function.h | 10 ++++
clang/lib/AST/Interp/Interp.cpp | 25 +++++---
clang/lib/AST/Interp/Interp.h | 76 ++++++++++++++++++++----
clang/lib/AST/Interp/InterpFrame.cpp | 11 ++--
clang/lib/AST/Interp/InterpFrame.h | 5 +-
clang/lib/AST/Interp/Opcodes.td | 11 +++-
clang/test/AST/Interp/functions.cpp | 75 +++++++++++++++++++++++
11 files changed, 226 insertions(+), 39 deletions(-)
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 91b9985eefbd30..988765972a36e6 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1829,8 +1829,19 @@ bool ByteCodeExprGen<Emitter>::VisitCXXConstructExpr(
return false;
}
- if (!this->emitCall(Func, E))
- return false;
+ if (Func->isVariadic()) {
+ uint32_t VarArgSize = 0;
+ unsigned NumParams = Func->getNumWrittenParams();
+ for (unsigned I = NumParams, N = E->getNumArgs(); I != N; ++I) {
+ VarArgSize +=
+ align(primSize(classify(E->getArg(I)->getType()).value_or(PT_Ptr)));
+ }
+ if (!this->emitCallVar(Func, VarArgSize, E))
+ return false;
+ } else {
+ if (!this->emitCall(Func, 0, E))
+ return false;
+ }
// Immediately call the destructor if we have to.
if (DiscardResult) {
@@ -1863,7 +1874,7 @@ bool ByteCodeExprGen<Emitter>::VisitCXXConstructExpr(
return false;
}
- if (!this->emitCall(Func, E))
+ if (!this->emitCall(Func, 0, E))
return false;
}
return true;
@@ -2049,7 +2060,7 @@ bool ByteCodeExprGen<Emitter>::VisitCXXInheritedCtorInitExpr(
Offset += align(primSize(PT));
}
- return this->emitCall(F, E);
+ return this->emitCall(F, 0, E);
}
template <class Emitter>
@@ -2846,20 +2857,38 @@ bool ByteCodeExprGen<Emitter>::VisitCallExpr(const CallExpr *E) {
// and if the function has RVO, we already have the pointer on the stack to
// write the result into.
if (IsVirtual && !HasQualifier) {
- if (!this->emitCallVirt(Func, E))
+ uint32_t VarArgSize = 0;
+ unsigned NumParams = Func->getNumWrittenParams();
+ for (unsigned I = NumParams, N = E->getNumArgs(); I != N; ++I)
+ VarArgSize += align(primSize(classify(E->getArg(I)).value_or(PT_Ptr)));
+
+ if (!this->emitCallVirt(Func, VarArgSize, E))
+ return false;
+ } else if (Func->isVariadic()) {
+ uint32_t VarArgSize = 0;
+ unsigned NumParams = Func->getNumWrittenParams();
+ for (unsigned I = NumParams, N = E->getNumArgs(); I != N; ++I)
+ VarArgSize += align(primSize(classify(E->getArg(I)).value_or(PT_Ptr)));
+ if (!this->emitCallVar(Func, VarArgSize, E))
return false;
} else {
- if (!this->emitCall(Func, E))
+ if (!this->emitCall(Func, 0, E))
return false;
}
} else {
// Indirect call. Visit the callee, which will leave a FunctionPointer on
// the stack. Cleanup of the returned value if necessary will be done after
// the function call completed.
+
+ // Sum the size of all args from the call expr.
+ uint32_t ArgSize = 0;
+ for (unsigned I = 0, N = E->getNumArgs(); I != N; ++I)
+ ArgSize += align(primSize(classify(E->getArg(I)).value_or(PT_Ptr)));
+
if (!this->visit(E->getCallee()))
return false;
- if (!this->emitCallPtr(E))
+ if (!this->emitCallPtr(ArgSize, E))
return false;
}
@@ -3386,7 +3415,7 @@ bool ByteCodeExprGen<Emitter>::emitRecordDestruction(const Descriptor *Desc) {
assert(DtorFunc->getNumParams() == 1);
if (!this->emitDupPtr(SourceInfo{}))
return false;
- if (!this->emitCall(DtorFunc, SourceInfo{}))
+ if (!this->emitCall(DtorFunc, 0, SourceInfo{}))
return false;
}
}
diff --git a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
index bedcc78dc23555..7e2043f8de90b0 100644
--- a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -126,7 +126,7 @@ bool ByteCodeStmtGen<Emitter>::emitLambdaStaticInvokerBody(
return false;
}
- if (!this->emitCall(Func, LambdaCallOp))
+ if (!this->emitCall(Func, 0, LambdaCallOp))
return false;
this->emitCleanup();
diff --git a/clang/lib/AST/Interp/Context.cpp b/clang/lib/AST/Interp/Context.cpp
index 5f5a6622f10f3d..7396db22943663 100644
--- a/clang/lib/AST/Interp/Context.cpp
+++ b/clang/lib/AST/Interp/Context.cpp
@@ -209,7 +209,8 @@ bool Context::Run(State &Parent, const Function *Func, APValue &Result) {
{
InterpState State(Parent, *P, Stk, *this);
- State.Current = new InterpFrame(State, Func, /*Caller=*/nullptr, {});
+ State.Current = new InterpFrame(State, Func, /*Caller=*/nullptr, CodePtr(),
+ Func->getArgSize());
if (Interpret(State, Result)) {
assert(Stk.empty());
return true;
diff --git a/clang/lib/AST/Interp/EvalEmitter.cpp b/clang/lib/AST/Interp/EvalEmitter.cpp
index 945b78d7a609d7..c1e4ce3ebb0729 100644
--- a/clang/lib/AST/Interp/EvalEmitter.cpp
+++ b/clang/lib/AST/Interp/EvalEmitter.cpp
@@ -22,7 +22,7 @@ EvalEmitter::EvalEmitter(Context &Ctx, Program &P, State &Parent,
: Ctx(Ctx), P(P), S(Parent, P, Stk, Ctx, this), EvalResult(&Ctx) {
// Create a dummy frame for the interpreter which does not have locals.
S.Current =
- new InterpFrame(S, /*Func=*/nullptr, /*Caller=*/nullptr, CodePtr());
+ new InterpFrame(S, /*Func=*/nullptr, /*Caller=*/nullptr, CodePtr(), 0);
}
EvalEmitter::~EvalEmitter() {
diff --git a/clang/lib/AST/Interp/Function.h b/clang/lib/AST/Interp/Function.h
index 7c3e0f63024908..6500e0126c226f 100644
--- a/clang/lib/AST/Interp/Function.h
+++ b/clang/lib/AST/Interp/Function.h
@@ -183,6 +183,16 @@ class Function final {
unsigned getNumParams() const { return ParamTypes.size(); }
+ /// Returns the number of parameter this function takes when it's called,
+ /// i.e excluding the instance pointer and the RVO pointer.
+ unsigned getNumWrittenParams() const {
+ assert(getNumParams() >= (hasThisPointer() + hasRVO()));
+ return getNumParams() - hasThisPointer() - hasRVO();
+ }
+ unsigned getWrittenArgSize() const {
+ return ArgSize - (align(primSize(PT_Ptr)) * (hasThisPointer() + hasRVO()));
+ }
+
unsigned getParamOffset(unsigned ParamIndex) const {
return ParamOffsets[ParamIndex];
}
diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 683151f7caf528..2338f88569db8b 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -169,16 +169,27 @@ void cleanupAfterFunctionCall(InterpState &S, CodePtr OpPC) {
// 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.
- const auto *CE =
- cast<CallExpr>(S.Current->Caller->getExpr(S.Current->getRetPC()));
- unsigned FixedParams = CurFunc->getNumParams();
- int32_t ArgsToPop = CE->getNumArgs() - FixedParams;
- assert(ArgsToPop >= 0);
- for (int32_t I = ArgsToPop - 1; I >= 0; --I) {
- const Expr *A = CE->getArg(FixedParams + I);
+ unsigned NumVarArgs;
+ const Expr *const *Args = nullptr;
+ unsigned NumArgs = 0;
+ const Expr *CallSite = S.Current->Caller->getExpr(S.Current->getRetPC());
+ if (const auto *CE = dyn_cast<CallExpr>(CallSite)) {
+ Args = CE->getArgs();
+ NumArgs = CE->getNumArgs();
+ } else if (const auto *CE = dyn_cast<CXXConstructExpr>(CallSite)) {
+ Args = CE->getArgs();
+ NumArgs = CE->getNumArgs();
+ } else
+ assert(false && "Can't get arguments from that expression type");
+
+ assert(NumArgs >= CurFunc->getNumWrittenParams());
+ NumVarArgs = NumArgs - CurFunc->getNumWrittenParams();
+ for (unsigned I = 0; I != NumVarArgs; ++I) {
+ const Expr *A = Args[NumArgs - 1 - I];
popArg(S, A);
}
}
+
// And in any case, remove the fixed parameters (the non-variadic ones)
// at the end.
S.Current->popArgs();
diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index e2fda18e3f44d4..77c724f08e8eef 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1915,10 +1915,60 @@ inline bool ArrayDecay(InterpState &S, CodePtr OpPC) {
return false;
}
-inline bool Call(InterpState &S, CodePtr OpPC, const Function *Func) {
+inline bool CallVar(InterpState &S, CodePtr OpPC, const Function *Func,
+ uint32_t VarArgSize) {
if (Func->hasThisPointer()) {
- size_t ThisOffset =
- Func->getArgSize() - (Func->hasRVO() ? primSize(PT_Ptr) : 0);
+ size_t ArgSize = Func->getArgSize() + VarArgSize;
+ size_t ThisOffset = ArgSize - (Func->hasRVO() ? primSize(PT_Ptr) : 0);
+ const Pointer &ThisPtr = S.Stk.peek<Pointer>(ThisOffset);
+
+ // If the current function is a lambda static invoker and
+ // the function we're about to call is a lambda call operator,
+ // skip the CheckInvoke, since the ThisPtr is a null pointer
+ // anyway.
+ if (!(S.Current->getFunction() &&
+ S.Current->getFunction()->isLambdaStaticInvoker() &&
+ Func->isLambdaCallOperator())) {
+ if (!CheckInvoke(S, OpPC, ThisPtr))
+ return false;
+ }
+
+ if (S.checkingPotentialConstantExpression())
+ return false;
+ }
+
+ if (!CheckCallable(S, OpPC, Func))
+ return false;
+
+ if (!CheckCallDepth(S, OpPC))
+ return false;
+
+ auto NewFrame = std::make_unique<InterpFrame>(S, Func, OpPC, VarArgSize);
+ InterpFrame *FrameBefore = S.Current;
+ S.Current = NewFrame.get();
+
+ APValue CallResult;
+ // Note that we cannot assert(CallResult.hasValue()) here since
+ // Ret() above only sets the APValue if the curent frame doesn't
+ // have a caller set.
+ if (Interpret(S, CallResult)) {
+ NewFrame.release(); // Frame was delete'd already.
+ assert(S.Current == FrameBefore);
+ return true;
+ }
+
+ // Interpreting the function failed somehow. Reset to
+ // previous state.
+ S.Current = FrameBefore;
+ return false;
+
+ return false;
+}
+inline bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
+ uint32_t VarArgSize) {
+ if (Func->hasThisPointer()) {
+ size_t ArgSize = Func->getArgSize() + VarArgSize;
+ size_t ThisOffset = ArgSize - (Func->hasRVO() ? primSize(PT_Ptr) : 0);
const Pointer &ThisPtr = S.Stk.peek<Pointer>(ThisOffset);
@@ -1943,7 +1993,7 @@ inline bool Call(InterpState &S, CodePtr OpPC, const Function *Func) {
if (!CheckCallDepth(S, OpPC))
return false;
- auto NewFrame = std::make_unique<InterpFrame>(S, Func, OpPC);
+ auto NewFrame = std::make_unique<InterpFrame>(S, Func, OpPC, VarArgSize);
InterpFrame *FrameBefore = S.Current;
S.Current = NewFrame.get();
@@ -1963,11 +2013,12 @@ inline bool Call(InterpState &S, CodePtr OpPC, const Function *Func) {
return false;
}
-inline bool CallVirt(InterpState &S, CodePtr OpPC, const Function *Func) {
+inline bool CallVirt(InterpState &S, CodePtr OpPC, const Function *Func,
+ uint32_t VarArgSize) {
assert(Func->hasThisPointer());
assert(Func->isVirtual());
- size_t ThisOffset =
- Func->getArgSize() - (Func->hasRVO() ? primSize(PT_Ptr) : 0);
+ size_t ArgSize = Func->getArgSize() + VarArgSize;
+ size_t ThisOffset = ArgSize - (Func->hasRVO() ? primSize(PT_Ptr) : 0);
Pointer &ThisPtr = S.Stk.peek<Pointer>(ThisOffset);
const CXXRecordDecl *DynamicDecl =
@@ -1998,7 +2049,7 @@ inline bool CallVirt(InterpState &S, CodePtr OpPC, const Function *Func) {
}
}
- return Call(S, OpPC, Func);
+ return Call(S, OpPC, Func, VarArgSize);
}
inline bool CallBI(InterpState &S, CodePtr &PC, const Function *Func,
@@ -2016,17 +2067,20 @@ inline bool CallBI(InterpState &S, CodePtr &PC, const Function *Func,
return false;
}
-inline bool CallPtr(InterpState &S, CodePtr OpPC) {
+inline bool CallPtr(InterpState &S, CodePtr OpPC, uint32_t ArgSize) {
const FunctionPointer &FuncPtr = S.Stk.pop<FunctionPointer>();
const Function *F = FuncPtr.getFunction();
if (!F || !F->isConstexpr())
return false;
+ assert(ArgSize >= F->getWrittenArgSize());
+ uint32_t VarArgSize = ArgSize - F->getWrittenArgSize();
+
if (F->isVirtual())
- return CallVirt(S, OpPC, F);
+ return CallVirt(S, OpPC, F, VarArgSize);
- return Call(S, OpPC, F);
+ return Call(S, OpPC, F, VarArgSize);
}
inline bool GetFnPtr(InterpState &S, CodePtr OpPC, const Function *Func) {
diff --git a/clang/lib/AST/Interp/InterpFrame.cpp b/clang/lib/AST/Interp/InterpFrame.cpp
index bf2cca733b66bb..f69ff06b5e81b5 100644
--- a/clang/lib/AST/Interp/InterpFrame.cpp
+++ b/clang/lib/AST/Interp/InterpFrame.cpp
@@ -22,10 +22,10 @@ using namespace clang;
using namespace clang::interp;
InterpFrame::InterpFrame(InterpState &S, const Function *Func,
- InterpFrame *Caller, CodePtr RetPC)
+ InterpFrame *Caller, CodePtr RetPC, unsigned ArgSize)
: Caller(Caller), S(S), Depth(Caller ? Caller->Depth + 1 : 0), Func(Func),
- RetPC(RetPC), ArgSize(Func ? Func->getArgSize() : 0),
- Args(static_cast<char *>(S.Stk.top())), FrameOffset(S.Stk.size()) {
+ RetPC(RetPC), ArgSize(ArgSize), Args(static_cast<char *>(S.Stk.top())),
+ FrameOffset(S.Stk.size()) {
if (!Func)
return;
@@ -43,8 +43,9 @@ InterpFrame::InterpFrame(InterpState &S, const Function *Func,
}
}
-InterpFrame::InterpFrame(InterpState &S, const Function *Func, CodePtr RetPC)
- : InterpFrame(S, Func, S.Current, RetPC) {
+InterpFrame::InterpFrame(InterpState &S, const Function *Func, CodePtr RetPC,
+ unsigned VarArgSize)
+ : InterpFrame(S, Func, S.Current, RetPC, Func->getArgSize() + VarArgSize) {
// As per our calling convention, the this pointer is
// part of the ArgSize.
// If the function has RVO, the RVO pointer is first.
diff --git a/clang/lib/AST/Interp/InterpFrame.h b/clang/lib/AST/Interp/InterpFrame.h
index cba4f9560bf56a..322d5dcfa698ae 100644
--- a/clang/lib/AST/Interp/InterpFrame.h
+++ b/clang/lib/AST/Interp/InterpFrame.h
@@ -32,13 +32,14 @@ class InterpFrame final : public Frame {
/// Creates a new frame for a method call.
InterpFrame(InterpState &S, const Function *Func, InterpFrame *Caller,
- CodePtr RetPC);
+ CodePtr RetPC, unsigned ArgSize);
/// Creates a new frame with the values that make sense.
/// I.e., the caller is the current frame of S,
/// the This() pointer is the current Pointer on the top of S's stack,
/// and the RVO pointer is before that.
- InterpFrame(InterpState &S, const Function *Func, CodePtr RetPC);
+ InterpFrame(InterpState &S, const Function *Func, CodePtr RetPC,
+ unsigned VarArgSize = 0);
/// Destroys the frame, killing all live pointers to stack slots.
~InterpFrame();
diff --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td
index 7f5bd7e5b44bca..f1b08944a8812e 100644
--- a/clang/lib/AST/Interp/Opcodes.td
+++ b/clang/lib/AST/Interp/Opcodes.td
@@ -191,12 +191,12 @@ def NoRet : Opcode {}
def Call : Opcode {
- let Args = [ArgFunction];
+ let Args = [ArgFunction, ArgUint32];
let Types = [];
}
def CallVirt : Opcode {
- let Args = [ArgFunction];
+ let Args = [ArgFunction, ArgUint32];
let Types = [];
}
@@ -206,7 +206,12 @@ def CallBI : Opcode {
}
def CallPtr : Opcode {
- let Args = [];
+ let Args = [ArgUint32];
+ let Types = [];
+}
+
+def CallVar : Opcode {
+ let Args = [ArgFunction, ArgUint32];
let Types = [];
}
diff --git a/clang/test/AST/Interp/functions.cpp b/clang/test/AST/Interp/functions.cpp
index 6e995ce704e394..7b8278cf13aa88 100644
--- a/clang/test/AST/Interp/functions.cpp
+++ b/clang/test/AST/Interp/functions.cpp
@@ -381,6 +381,81 @@ namespace Variadic {
constexpr int (*VFP)(...) = variadic_function2;
static_assert(VFP() == 12, "");
+
+ /// Member functions
+ struct Foo {
+ int a = 0;
+ constexpr void bla(...) {}
+ constexpr S bla2(...) {
+ return S{12, true};
+ }
+ constexpr Foo(...) : a(1337) {}
+ constexpr Foo(void *c, bool b, void*p, ...) : a('a' + b) {}
+ constexpr Foo(int a, const S* s, ...) : a(a) {}
+ };
+
+ constexpr int foo2() {
+ Foo f(1, nullptr);
+ auto s = f.bla2(1, 2, S{1, false});
+ return s.a + s.b;
+ }
+ static_assert(foo2() == 13, "");
+
+ constexpr Foo _f = 123;
+ static_assert(_f.a == 1337, "");
+
+ constexpr Foo __f(nullptr, false, nullptr, nullptr, 'a', Foo());
+ static_assert(__f.a == 'a', "");
+
+
+#if __cplusplus >= 202002L
+namespace VariadicVirtual {
+ class A {
+ public:
+ constexpr virtual void foo(int &a, ...) {
+ a = 1;
+ }
+ };
+
+ class B : public A {
+ public:
+ constexpr void foo(int &a, ...) override {
+ a = 2;
+ }
+ };
+
+ constexpr int foo() {
+ B b;
+ int a;
+ b.foo(a, 1,2,nullptr);
+ return a;
+ }
+ static_assert(foo() == 2, "");
+} // VariadicVirtual
+
+namespace VariadicQualified {
+ class A {
+ public:
+ constexpr virtual int foo(...) const {
+ return 5;
+ }
+ };
+ class B : public A {};
+ class C : public B {
+ public:
+ constexpr int foo(...) const override {
+ return B::foo(1,2,3); // B doesn't have a foo(), so this should call A::foo().
+ }
+ constexpr int foo2() const {
+ return this->A::foo(1,2,3,this);
+ }
+ };
+ constexpr C c;
+ static_assert(c.foo() == 5);
+ static_assert(c.foo2() == 5);
+} // VariadicQualified
+#endif
+
}
namespace Packs {
>From 06bc1b0bdacccd32752ffb8928ad85c1497dd5bc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Mon, 5 Feb 2024 10:29:45 +0100
Subject: [PATCH 3/3] [clang][Interp] Do r-to-l conversion immediately when
returning
First, we need to register local constant variables in C, so we get
the same diagnostic behavior as the current interpeter.
Second, when returning an LValue (as a Pointer), which we eventually
convert to an RValue, we need to do the conversion immediately when
saving the Pointer in the EvaluationResult. Otherwise, we will
possibly deallocate the data before doing the conversion (which will
look at the Block*).
---
clang/lib/AST/Interp/ByteCodeExprGen.cpp | 3 +--
clang/lib/AST/Interp/Context.cpp | 16 ++++------------
clang/lib/AST/Interp/EvalEmitter.cpp | 20 ++++++++++++++++++--
clang/lib/AST/Interp/EvalEmitter.h | 5 ++++-
clang/lib/AST/Interp/EvaluationResult.h | 2 +-
clang/test/AST/Interp/c.c | 13 +++++++++++++
clang/test/Sema/warn-char-subscripts.c | 1 +
7 files changed, 42 insertions(+), 18 deletions(-)
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 988765972a36e6..31e7f02dd4305c 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -3243,8 +3243,7 @@ bool ByteCodeExprGen<Emitter>::VisitDeclRefExpr(const DeclRefExpr *E) {
// This happens in C.
if (!Ctx.getLangOpts().CPlusPlus) {
if (const auto *VD = dyn_cast<VarDecl>(D);
- VD && VD->hasGlobalStorage() && VD->getAnyInitializer() &&
- VD->getType().isConstQualified()) {
+ VD && VD->getAnyInitializer() && VD->getType().isConstQualified()) {
if (!this->visitVarDecl(VD))
return false;
// Retry.
diff --git a/clang/lib/AST/Interp/Context.cpp b/clang/lib/AST/Interp/Context.cpp
index 7396db22943663..6068b1a5680c83 100644
--- a/clang/lib/AST/Interp/Context.cpp
+++ b/clang/lib/AST/Interp/Context.cpp
@@ -44,7 +44,7 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
assert(Stk.empty());
ByteCodeExprGen<EvalEmitter> C(*this, *P, Parent, Stk, Result);
- auto Res = C.interpretExpr(E);
+ auto Res = C.interpretExpr(E, /*ConvertResultToRValue=*/E->isGLValue());
if (Res.isInvalid()) {
Stk.clear();
@@ -58,16 +58,7 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
Stk.clear();
#endif
- // Implicit lvalue-to-rvalue conversion.
- if (E->isGLValue()) {
- std::optional<APValue> RValueResult = Res.toRValue();
- if (!RValueResult) {
- return false;
- }
- Result = *RValueResult;
- } else {
- Result = Res.toAPValue();
- }
+ Result = Res.toAPValue();
return true;
}
@@ -120,7 +111,8 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
!Res.checkFullyInitialized(C.getState()))
return false;
- // lvalue-to-rvalue conversion.
+ // lvalue-to-rvalue conversion. We do this manually here so we can
+ // examine the result above before converting and returning it.
std::optional<APValue> RValueResult = Res.toRValue();
if (!RValueResult)
return false;
diff --git a/clang/lib/AST/Interp/EvalEmitter.cpp b/clang/lib/AST/Interp/EvalEmitter.cpp
index c1e4ce3ebb0729..f14023a23af9b3 100644
--- a/clang/lib/AST/Interp/EvalEmitter.cpp
+++ b/clang/lib/AST/Interp/EvalEmitter.cpp
@@ -33,7 +33,9 @@ EvalEmitter::~EvalEmitter() {
}
}
-EvaluationResult EvalEmitter::interpretExpr(const Expr *E) {
+EvaluationResult EvalEmitter::interpretExpr(const Expr *E,
+ bool ConvertResultToRValue) {
+ this->ConvertResultToRValue = ConvertResultToRValue;
EvalResult.setSource(E);
if (!this->visitExpr(E) && EvalResult.empty())
@@ -119,12 +121,26 @@ template <PrimType OpType> bool EvalEmitter::emitRet(const SourceInfo &Info) {
template <> bool EvalEmitter::emitRet<PT_Ptr>(const SourceInfo &Info) {
if (!isActive())
return true;
- EvalResult.setPointer(S.Stk.pop<Pointer>());
+
+ const Pointer &Ptr = S.Stk.pop<Pointer>();
+ // Implicitly convert lvalue to rvalue, if requested.
+ if (ConvertResultToRValue) {
+ if (std::optional<APValue> V = Ptr.toRValue(Ctx)) {
+ EvalResult.setValue(*V);
+ } else {
+ return false;
+ }
+ } else {
+ EvalResult.setPointer(Ptr);
+ }
+
return true;
}
template <> bool EvalEmitter::emitRet<PT_FnPtr>(const SourceInfo &Info) {
if (!isActive())
return true;
+ // Function pointers cannot be converted to rvalues.
+ assert(!ConvertResultToRValue);
EvalResult.setFunctionPointer(S.Stk.pop<FunctionPointer>());
return true;
}
diff --git a/clang/lib/AST/Interp/EvalEmitter.h b/clang/lib/AST/Interp/EvalEmitter.h
index deb2ebc4e61fa0..8159e489f168e3 100644
--- a/clang/lib/AST/Interp/EvalEmitter.h
+++ b/clang/lib/AST/Interp/EvalEmitter.h
@@ -34,7 +34,8 @@ class EvalEmitter : public SourceMapper {
using AddrTy = uintptr_t;
using Local = Scope::Local;
- EvaluationResult interpretExpr(const Expr *E);
+ EvaluationResult interpretExpr(const Expr *E,
+ bool ConvertResultToRValue = false);
EvaluationResult interpretDecl(const VarDecl *VD);
InterpState &getState() { return S; }
@@ -86,6 +87,8 @@ class EvalEmitter : public SourceMapper {
InterpState S;
/// Location to write the result to.
EvaluationResult EvalResult;
+ /// Whether the result should be converted to an RValue.
+ bool ConvertResultToRValue = false;
/// Temporaries which require storage.
llvm::DenseMap<unsigned, std::unique_ptr<char[]>> Locals;
diff --git a/clang/lib/AST/Interp/EvaluationResult.h b/clang/lib/AST/Interp/EvaluationResult.h
index 2b9fc16f1a0abc..52a6c011e39e1b 100644
--- a/clang/lib/AST/Interp/EvaluationResult.h
+++ b/clang/lib/AST/Interp/EvaluationResult.h
@@ -56,8 +56,8 @@ class EvaluationResult final {
void setSource(DeclTy D) { Source = D; }
void setValue(const APValue &V) {
+ // V could still be an LValue.
assert(empty());
- assert(!V.isLValue());
Value = std::move(V);
Kind = RValue;
}
diff --git a/clang/test/AST/Interp/c.c b/clang/test/AST/Interp/c.c
index 85c195d33a96d7..31cd2729f0bc72 100644
--- a/clang/test/AST/Interp/c.c
+++ b/clang/test/AST/Interp/c.c
@@ -125,3 +125,16 @@ struct XY { int before; struct XX xx, *xp; float* after; } xy[] = {
0, // all-warning {{initializer overrides prior initialization of this subobject}}
&xy[2].xx.a, &xy[2].xx, &global_float
};
+
+void t14(void) {
+ int array[256] = { 0 }; // expected-note {{array 'array' declared here}} \
+ // pedantic-expected-note {{array 'array' declared here}} \
+ // ref-note {{array 'array' declared here}} \
+ // pedantic-ref-note {{array 'array' declared here}}
+ const char b = -1;
+ int val = array[b]; // expected-warning {{array index -1 is before the beginning of the array}} \
+ // pedantic-expected-warning {{array index -1 is before the beginning of the array}} \
+ // ref-warning {{array index -1 is before the beginning of the array}} \
+ // pedantic-ref-warning {{array index -1 is before the beginning of the array}}
+
+}
diff --git a/clang/test/Sema/warn-char-subscripts.c b/clang/test/Sema/warn-char-subscripts.c
index 0a012f68feae07..c2f7a3731d72c8 100644
--- a/clang/test/Sema/warn-char-subscripts.c
+++ b/clang/test/Sema/warn-char-subscripts.c
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -Wchar-subscripts -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wchar-subscripts -fsyntax-only -verify %s -fexperimental-new-constant-interpreter
void t1(void) {
int array[1] = { 0 };
More information about the cfe-commits
mailing list