[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>
diff --git a/runtime/vm/exceptions.cc b/runtime/vm/exceptions.cc
index 45d836a..06f5a47 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->isolate()->interpreter();
+ Interpreter* interpreter = thread->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 deac92b..3185b93 100644
--- a/runtime/vm/interpreter.cc
+++ b/runtime/vm/interpreter.cc
@@ -568,18 +568,14 @@
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 = Isolate::Current()->interpreter();
+ Interpreter* interpreter = Thread::Current()->interpreter();
if (interpreter == NULL) {
interpreter = new Interpreter();
- Isolate::Current()->set_interpreter(interpreter);
+ Thread::Current()->set_interpreter(interpreter);
}
return interpreter;
}
diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc
index cefcfc2..114918a 100644
--- a/runtime/vm/isolate.cc
+++ b/runtime/vm/isolate.cc
@@ -909,9 +909,6 @@
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_"))),
@@ -993,9 +990,6 @@
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
@@ -2003,12 +1997,6 @@
}
#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 9db354c..dd92e29 100644
--- a/runtime/vm/isolate.h
+++ b/runtime/vm/isolate.h
@@ -420,11 +420,6 @@
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; }
@@ -973,9 +968,6 @@
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 e58fb70..ad00ea5 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 = Isolate::Current()->interpreter();
+ Interpreter* interpreter = Thread::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 796eae1..a8936df 100644
--- a/runtime/vm/stack_frame.cc
+++ b/runtime/vm/stack_frame.cc
@@ -436,6 +436,7 @@
}
void StackFrameIterator::SetupNextExitFrameData() {
+ ASSERT(entry_.fp() != 0);
uword exit_address =
entry_.fp() + ((entry_.is_interpreted() ? kKBCExitLinkSlotFromEntryFp
: kExitLinkSlotFromEntryFp) *
@@ -583,8 +584,7 @@
// 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);
- Isolate* isolate = thread_->isolate();
- Interpreter* interpreter = isolate != NULL ? isolate->interpreter() : NULL;
+ Interpreter* interpreter = thread_->interpreter();
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 1f267cc..0f42ef5 100644
--- a/runtime/vm/thread.cc
+++ b/runtime/vm/thread.cc
@@ -33,6 +33,10 @@
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.
@@ -95,6 +99,9 @@
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),
@@ -668,6 +675,12 @@
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 4e01da6..5f4c710 100644
--- a/runtime/vm/thread.h
+++ b/runtime/vm/thread.h
@@ -34,6 +34,7 @@
class Heap;
class HierarchyInfo;
class Instance;
+class Interpreter;
class Isolate;
class Library;
class LongJumpScope;
@@ -401,6 +402,11 @@
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() {
@@ -877,6 +883,10 @@
TypeUsageInfo* type_usage_info_;
RawGrowableObjectArray* pending_functions_;
+#if !defined(DART_PRECOMPILED_RUNTIME)
+ Interpreter* interpreter_;
+#endif
+
// JumpToExceptionHandler state:
RawObject* active_exception_;
RawObject* active_stacktrace_;
