Revert "[clang][bytecode] Stack-allocate bottom function frame"#125325
Merged
Revert "[clang][bytecode] Stack-allocate bottom function frame"#125325
Conversation
…)" This reverts commit f354981.
Member
|
@llvm/pr-subscribers-clang Author: Jorge Gorbe Moya (slackito) ChangesReverts llvm/llvm-project#125253 It introduced an msan failure. Caught by a buildbot here: https://lab.llvm.org/buildbot/#/builders/164/builds/6922/steps/17/logs/stdio Full diff: https://github.com/llvm/llvm-project/pull/125325.diff 6 Files Affected:
diff --git a/clang/lib/AST/ByteCode/EvalEmitter.cpp b/clang/lib/AST/ByteCode/EvalEmitter.cpp
index e857e239560575..8134bbf270363e 100644
--- a/clang/lib/AST/ByteCode/EvalEmitter.cpp
+++ b/clang/lib/AST/ByteCode/EvalEmitter.cpp
@@ -17,9 +17,10 @@ using namespace clang::interp;
EvalEmitter::EvalEmitter(Context &Ctx, Program &P, State &Parent,
InterpStack &Stk)
- : Ctx(Ctx), P(P), S(Parent, P, Stk, Ctx, this), EvalResult(&Ctx),
- BottomFrame(S) {
- S.Current = &BottomFrame;
+ : 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(), 0);
}
EvalEmitter::~EvalEmitter() {
diff --git a/clang/lib/AST/ByteCode/EvalEmitter.h b/clang/lib/AST/ByteCode/EvalEmitter.h
index 784553b6c056cf..2cac2ba2ef2212 100644
--- a/clang/lib/AST/ByteCode/EvalEmitter.h
+++ b/clang/lib/AST/ByteCode/EvalEmitter.h
@@ -122,8 +122,6 @@ class EvalEmitter : public SourceMapper {
/// Active block which should be executed.
LabelTy ActiveLabel = 0;
- InterpFrame BottomFrame;
-
protected:
#define GET_EVAL_PROTO
#include "Opcodes.inc"
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index 91a82a25944fb5..063970afec9e35 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -325,11 +325,11 @@ bool Ret(InterpState &S, CodePtr &PC) {
if (InterpFrame *Caller = S.Current->Caller) {
PC = S.Current->getRetPC();
- InterpFrame::free(S.Current);
+ delete S.Current;
S.Current = Caller;
S.Stk.push<T>(Ret);
} else {
- InterpFrame::free(S.Current);
+ delete S.Current;
S.Current = nullptr;
// The topmost frame should come from an EvalEmitter,
// which has its own implementation of the Ret<> instruction.
diff --git a/clang/lib/AST/ByteCode/InterpFrame.cpp b/clang/lib/AST/ByteCode/InterpFrame.cpp
index dff9b211ad198c..48a3db055c6c9b 100644
--- a/clang/lib/AST/ByteCode/InterpFrame.cpp
+++ b/clang/lib/AST/ByteCode/InterpFrame.cpp
@@ -23,10 +23,6 @@
using namespace clang;
using namespace clang::interp;
-InterpFrame::InterpFrame(InterpState &S)
- : Caller(nullptr), S(S), Depth(0), Func(nullptr), RetPC(CodePtr()),
- ArgSize(0), Args(nullptr), FrameOffset(0), IsBottom(true) {}
-
InterpFrame::InterpFrame(InterpState &S, const Function *Func,
InterpFrame *Caller, CodePtr RetPC, unsigned ArgSize)
: Caller(Caller), S(S), Depth(Caller ? Caller->Depth + 1 : 0), Func(Func),
diff --git a/clang/lib/AST/ByteCode/InterpFrame.h b/clang/lib/AST/ByteCode/InterpFrame.h
index 8cabb9cd06fac0..7cfc3ac68b4f3e 100644
--- a/clang/lib/AST/ByteCode/InterpFrame.h
+++ b/clang/lib/AST/ByteCode/InterpFrame.h
@@ -28,9 +28,6 @@ class InterpFrame final : public Frame {
/// The frame of the previous function.
InterpFrame *Caller;
- /// Bottom Frame.
- InterpFrame(InterpState &S);
-
/// Creates a new frame for a method call.
InterpFrame(InterpState &S, const Function *Func, InterpFrame *Caller,
CodePtr RetPC, unsigned ArgSize);
@@ -45,11 +42,6 @@ class InterpFrame final : public Frame {
/// Destroys the frame, killing all live pointers to stack slots.
~InterpFrame();
- static void free(InterpFrame *F) {
- if (!F->isBottomFrame())
- delete F;
- }
-
/// Invokes the destructors for a scope.
void destroy(unsigned Idx);
void initScope(unsigned Idx);
@@ -127,8 +119,6 @@ class InterpFrame final : public Frame {
bool isStdFunction() const;
- bool isBottomFrame() const { return IsBottom; }
-
void dump() const { dump(llvm::errs(), 0); }
void dump(llvm::raw_ostream &OS, unsigned Indent = 0) const;
@@ -177,7 +167,6 @@ class InterpFrame final : public Frame {
const size_t FrameOffset;
/// Mapping from arg offsets to their argument blocks.
llvm::DenseMap<unsigned, std::unique_ptr<char[]>> Params;
- bool IsBottom = false;
};
} // namespace interp
diff --git a/clang/lib/AST/ByteCode/InterpState.cpp b/clang/lib/AST/ByteCode/InterpState.cpp
index 8aa44e48842e96..287c3bd3bca3a5 100644
--- a/clang/lib/AST/ByteCode/InterpState.cpp
+++ b/clang/lib/AST/ByteCode/InterpState.cpp
@@ -27,7 +27,7 @@ bool InterpState::inConstantContext() const {
}
InterpState::~InterpState() {
- while (Current && !Current->isBottomFrame()) {
+ while (Current) {
InterpFrame *Next = Current->Caller;
delete Current;
Current = Next;
|
tbaederr
added a commit
to tbaederr/llvm-project
that referenced
this pull request
Feb 1, 2025
…m#125325) Move the BottomFrame to InterpState instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #125253
It introduced an msan failure. Caught by a buildbot here: https://lab.llvm.org/buildbot/#/builders/164/builds/6922/steps/17/logs/stdio