Revert "[vm/interpreter] Fix interpreter frame recognition in stack walking"
This reverts commit a0cc3e2684b7e3b29cb6126a899ac749c163c444.
Reason for revert: broken precompiler bots
Original change's description:
> [vm/interpreter] Fix interpreter frame recognition in stack walking
>
> When GC is visiting pointers, it scans mutator thread even if it is not
> active. It could happen if a thread has (temporarily) exitted isolate.
> In such case, stack frame iteration would not find interpreter and
> would not recognize interpreter frames. An attempt to visit interpreter
> frame as non-interpreter causes crashes/assertions.
>
> The fix is to move interpreter from Isolate to Thread. This way
> interpreter is available for stack walking even if thread doesn't have
> isolate.
>
> Change-Id: Iff12ce7391b19a6f87bd4fac94137cba5623dfb9
> Reviewed-on: https://dart-review.googlesource.com/c/80760
> Reviewed-by: Régis Crelier <regis@google.com>
> Commit-Queue: Alexander Markov <alexmarkov@google.com>
TBR=rmacnak@google.com,alexmarkov@google.com,zra@google.com,asiva@google.com,regis@google.com
Change-Id: Ic4d73536eadb6601ee323918c65f5f2b2b2b2418
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/80781
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
diff --git a/runtime/vm/exceptions.cc b/runtime/vm/exceptions.cc
index 06f5a47..45d836a 100644
--- a/runtime/vm/exceptions.cc
+++ b/runtime/vm/exceptions.cc
@@ -540,7 +540,7 @@
// TODO(regis): We still possibly need to unwind interpreter frames if they
// are callee frames of the C++ frame handling the exception.
if (FLAG_enable_interpreter) {
- Interpreter* interpreter = thread->interpreter();
+ Interpreter* interpreter = thread->isolate()->interpreter();
if ((interpreter != NULL) && interpreter->HasFrame(frame_pointer)) {
interpreter->JumpToFrame(program_counter, stack_pointer, frame_pointer,
thread);
diff --git a/runtime/vm/interpreter.cc b/runtime/vm/interpreter.cc
index 3185b93..deac92b 100644
--- a/runtime/vm/interpreter.cc
+++ b/runtime/vm/interpreter.cc
@@ -568,14 +568,18 @@
Interpreter::~Interpreter() {
delete[] stack_;
+ Isolate* isolate = Isolate::Current();
+ if (isolate != NULL) {
+ isolate->set_interpreter(NULL);
+ }
}
// Get the active Interpreter for the current isolate.
Interpreter* Interpreter::Current() {
- Interpreter* interpreter = Thread::Current()->interpreter();
+ Interpreter* interpreter = Isolate::Current()->interpreter();
if (interpreter == NULL) {
interpreter = new Interpreter();
- Thread::Current()->set_interpreter(interpreter);
+ Isolate::Current()->set_interpreter(interpreter);
}
return interpreter;
}
diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc
index 114918a..cefcfc2 100644
--- a/runtime/vm/isolate.cc
+++ b/runtime/vm/isolate.cc
@@ -909,6 +909,9 @@
library_tag_handler_(NULL),
api_state_(NULL),
random_(),
+#if !defined(DART_PRECOMPILED_RUNTIME)
+ interpreter_(NULL),
+#endif
simulator_(NULL),
mutex_(new Mutex(NOT_IN_PRODUCT("Isolate::mutex_"))),
symbols_mutex_(new Mutex(NOT_IN_PRODUCT("Isolate::symbols_mutex_"))),
@@ -990,6 +993,9 @@
ASSERT(marking_stack_ == NULL);
delete object_store_;
delete api_state_;
+#if !defined(DART_PRECOMPILED_RUNTIME)
+ delete interpreter_;
+#endif
#if defined(USING_SIMULATOR)
delete simulator_;
#endif
@@ -1997,6 +2003,12 @@
}
#endif // !defined(DART_PRECOMPILED_RUNTIME)
+#if !defined(DART_PRECOMPILED_RUNTIME)
+ if (interpreter() != NULL) {
+ interpreter()->VisitObjectPointers(visitor);
+ }
+#endif
+
#if defined(TARGET_ARCH_DBC)
if (simulator() != NULL) {
simulator()->VisitObjectPointers(visitor);
diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h
index dd92e29..9db354c 100644
--- a/runtime/vm/isolate.h
+++ b/runtime/vm/isolate.h
@@ -420,6 +420,11 @@
Random* random() { return &random_; }
+#if !defined(DART_PRECOMPILED_RUNTIME)
+ Interpreter* interpreter() const { return interpreter_; }
+ void set_interpreter(Interpreter* value) { interpreter_ = value; }
+#endif
+
Simulator* simulator() const { return simulator_; }
void set_simulator(Simulator* value) { simulator_ = value; }
@@ -968,6 +973,9 @@
Dart_LibraryTagHandler library_tag_handler_;
ApiState* api_state_;
Random random_;
+#if !defined(DART_PRECOMPILED_RUNTIME)
+ Interpreter* interpreter_;
+#endif
Simulator* simulator_;
Mutex* mutex_; // Protects compiler stats.
Mutex* symbols_mutex_; // Protects concurrent access to the symbol table.
diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc
index ad00ea5..e58fb70 100644
--- a/runtime/vm/runtime_entry.cc
+++ b/runtime/vm/runtime_entry.cc
@@ -2091,7 +2091,7 @@
#if !defined(DART_PRECOMPILED_RUNTIME)
if (FLAG_enable_interpreter) {
// Do not allocate an interpreter, if none is allocated yet.
- Interpreter* interpreter = Thread::Current()->interpreter();
+ Interpreter* interpreter = Isolate::Current()->interpreter();
if (interpreter != NULL) {
interpreter_stack_overflow =
interpreter->get_sp() >= interpreter->stack_limit();
diff --git a/runtime/vm/stack_frame.cc b/runtime/vm/stack_frame.cc
index a8936df..796eae1 100644
--- a/runtime/vm/stack_frame.cc
+++ b/runtime/vm/stack_frame.cc
@@ -436,7 +436,6 @@
}
void StackFrameIterator::SetupNextExitFrameData() {
- ASSERT(entry_.fp() != 0);
uword exit_address =
entry_.fp() + ((entry_.is_interpreted() ? kKBCExitLinkSlotFromEntryFp
: kExitLinkSlotFromEntryFp) *
@@ -584,7 +583,8 @@
// TODO(regis): We should rely on a new thread vm_tag to identify an
// interpreter frame and not need the HasFrame() method.
ASSERT(FLAG_enable_interpreter);
- Interpreter* interpreter = thread_->interpreter();
+ Isolate* isolate = thread_->isolate();
+ Interpreter* interpreter = isolate != NULL ? isolate->interpreter() : NULL;
is_interpreted_ = (interpreter != NULL) && interpreter->HasFrame(exit_marker);
#endif // !defined(DART_PRECOMPILED_RUNTIME)
}
diff --git a/runtime/vm/thread.cc b/runtime/vm/thread.cc
index 0f42ef5..1f267cc 100644
--- a/runtime/vm/thread.cc
+++ b/runtime/vm/thread.cc
@@ -33,10 +33,6 @@
ASSERT(isolate_ == NULL);
ASSERT(store_buffer_block_ == NULL);
ASSERT(marking_stack_block_ == NULL);
-#if !defined(DART_PRECOMPILED_RUNTIME)
- delete interpreter_;
- interpreter_ = nullptr;
-#endif
// There should be no top api scopes at this point.
ASSERT(api_top_scope() == NULL);
// Delete the resusable api scope if there is one.
@@ -99,9 +95,6 @@
hierarchy_info_(NULL),
type_usage_info_(NULL),
pending_functions_(GrowableObjectArray::null()),
-#if !defined(DART_PRECOMPILED_RUNTIME)
- interpreter_(nullptr),
-#endif
active_exception_(Object::null()),
active_stacktrace_(Object::null()),
resume_pc_(0),
@@ -675,12 +668,6 @@
visitor->VisitPointer(reinterpret_cast<RawObject**>(&sticky_error_));
visitor->VisitPointer(reinterpret_cast<RawObject**>(&async_stack_trace_));
-#if !defined(DART_PRECOMPILED_RUNTIME)
- if (interpreter() != NULL) {
- interpreter()->VisitObjectPointers(visitor);
- }
-#endif
-
// Visit the api local scope as it has all the api local handles.
ApiLocalScope* scope = api_top_scope_;
while (scope != NULL) {
diff --git a/runtime/vm/thread.h b/runtime/vm/thread.h
index 5f4c710..4e01da6 100644
--- a/runtime/vm/thread.h
+++ b/runtime/vm/thread.h
@@ -34,7 +34,6 @@
class Heap;
class HierarchyInfo;
class Instance;
-class Interpreter;
class Isolate;
class Library;
class LongJumpScope;
@@ -402,11 +401,6 @@
type_usage_info_ = value;
}
-#if !defined(DART_PRECOMPILED_RUNTIME)
- Interpreter* interpreter() const { return interpreter_; }
- void set_interpreter(Interpreter* value) { interpreter_ = value; }
-#endif
-
int32_t no_callback_scope_depth() const { return no_callback_scope_depth_; }
void IncrementNoCallbackScopeDepth() {
@@ -883,10 +877,6 @@
TypeUsageInfo* type_usage_info_;
RawGrowableObjectArray* pending_functions_;
-#if !defined(DART_PRECOMPILED_RUNTIME)
- Interpreter* interpreter_;
-#endif
-
// JumpToExceptionHandler state:
RawObject* active_exception_;
RawObject* active_stacktrace_;
