Refactor StackVisitor to take a Thread*.

This allows assertion checking on the thread, principally that we never try to
walk the stack of an unsuspended thread.
Fix bug in the OwnedMonitorVisitor where GetVReg could be called on a
StackVisitor with no context.

Change-Id: I06539b624b253b6fb7385e7be11a4bced1d417b2

13 files changed, 114 insertions, 154 deletions

diff --git a/src/debugger.cc b/src/debugger.cc
index cdc2178341..3e93511ce2 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -685,10 +685,9 @@ JDWP::JdwpError Dbg::GetOwnedMonitors(JDWP::ObjectId thread_id,
   }
 
   struct OwnedMonitorVisitor : public StackVisitor {
-    OwnedMonitorVisitor(const ManagedStack* stack,
-                        const std::deque<InstrumentationStackFrame>* instrumentation_stack)
+    OwnedMonitorVisitor(Thread* thread, Context* context)
         SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-        : StackVisitor(stack, instrumentation_stack, NULL), current_stack_depth(0) {}
+        : StackVisitor(thread, context), current_stack_depth(0) {}
 
     // TODO: Enable annotalysis. We know lock is held in constructor, but abstraction confuses
     // annotalysis.
@@ -700,8 +699,8 @@ JDWP::JdwpError Dbg::GetOwnedMonitors(JDWP::ObjectId thread_id,
       return true;
     }
 
-    static void AppendOwnedMonitors(Object* owned_monitor, void* context) {
-      OwnedMonitorVisitor* visitor = reinterpret_cast<OwnedMonitorVisitor*>(context);
+    static void AppendOwnedMonitors(Object* owned_monitor, void* arg) {
+      OwnedMonitorVisitor* visitor = reinterpret_cast<OwnedMonitorVisitor*>(arg);
       visitor->monitors.push_back(owned_monitor);
       visitor->stack_depths.push_back(visitor->current_stack_depth);
     }
@@ -710,7 +709,8 @@ JDWP::JdwpError Dbg::GetOwnedMonitors(JDWP::ObjectId thread_id,
     std::vector<Object*> monitors;
     std::vector<uint32_t> stack_depths;
   };
-  OwnedMonitorVisitor visitor(thread->GetManagedStack(), thread->GetInstrumentationStack());
+  UniquePtr<Context> context(Context::Create());
+  OwnedMonitorVisitor visitor(thread, context.get());
   visitor.WalkStack();
 
   for (size_t i = 0; i < visitor.monitors.size(); ++i) {
@@ -1768,9 +1768,8 @@ void Dbg::GetChildThreadGroups(JDWP::ObjectId thread_group_id, std::vector<JDWP:
 static int GetStackDepth(Thread* thread)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
   struct CountStackDepthVisitor : public StackVisitor {
-    CountStackDepthVisitor(const ManagedStack* stack,
-                           const std::deque<InstrumentationStackFrame>* instrumentation_stack)
-        : StackVisitor(stack, instrumentation_stack, NULL), depth(0) {}
+    CountStackDepthVisitor(Thread* thread)
+        : StackVisitor(thread, NULL), depth(0) {}
 
     bool VisitFrame() {
       if (!GetMethod()->IsRuntimeMethod()) {
@@ -1781,11 +1780,7 @@ static int GetStackDepth(Thread* thread)
     size_t depth;
   };
 
-  if (kIsDebugBuild) {
-    MutexLock mu(Thread::Current(), *Locks::thread_suspend_count_lock_);
-    CHECK(thread == Thread::Current() || thread->IsSuspended());
-  }
-  CountStackDepthVisitor visitor(thread->GetManagedStack(), thread->GetInstrumentationStack());
+  CountStackDepthVisitor visitor(thread);
   visitor.WalkStack();
   return visitor.depth;
 }
@@ -1809,11 +1804,9 @@ JDWP::JdwpError Dbg::GetThreadFrames(JDWP::ObjectId thread_id, size_t start_fram
                                      size_t frame_count, JDWP::ExpandBuf* buf) {
   class GetFrameVisitor : public StackVisitor {
    public:
-    GetFrameVisitor(const ManagedStack* stack,
-                    const std::deque<InstrumentationStackFrame>* instrumentation_stack,
-                    size_t start_frame, size_t frame_count, JDWP::ExpandBuf* buf)
+    GetFrameVisitor(Thread* thread, size_t start_frame, size_t frame_count, JDWP::ExpandBuf* buf)
         SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-        : StackVisitor(stack, instrumentation_stack, NULL), depth_(0),
+        : StackVisitor(thread, NULL), depth_(0),
           start_frame_(start_frame), frame_count_(frame_count), buf_(buf) {
       expandBufAdd4BE(buf_, frame_count_);
     }
@@ -1856,8 +1849,7 @@ JDWP::JdwpError Dbg::GetThreadFrames(JDWP::ObjectId thread_id, size_t start_fram
   if (!IsSuspendedForDebugger(soa, thread)) {
     return JDWP::ERR_THREAD_NOT_SUSPENDED;
   }
-  GetFrameVisitor visitor(thread->GetManagedStack(), thread->GetInstrumentationStack(),
-                          start_frame, frame_count, buf);
+  GetFrameVisitor visitor(thread, start_frame, frame_count, buf);
   visitor.WalkStack();
   return JDWP::ERR_NONE;
 }
@@ -1923,11 +1915,9 @@ void Dbg::SuspendSelf() {
 }
 
 struct GetThisVisitor : public StackVisitor {
-  GetThisVisitor(const ManagedStack* stack,
-                 const std::deque<InstrumentationStackFrame>* instrumentation_stack,
-                 Context* context, JDWP::FrameId frame_id)
+  GetThisVisitor(Thread* thread, Context* context, JDWP::FrameId frame_id)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-      : StackVisitor(stack, instrumentation_stack, context), this_object(NULL), frame_id(frame_id) {}
+      : StackVisitor(thread, context), this_object(NULL), frame_id(frame_id) {}
 
   // TODO: Enable annotalysis. We know lock is held in constructor, but abstraction confuses
   // annotalysis.
@@ -1957,7 +1947,7 @@ static Object* GetThis(Thread* self, AbstractMethod* m, size_t frame_id)
   }
 
   UniquePtr<Context> context(Context::Create());
-  GetThisVisitor visitor(self->GetManagedStack(), self->GetInstrumentationStack(), context.get(), frame_id);
+  GetThisVisitor visitor(self, context.get(), frame_id);
   visitor.WalkStack();
   return visitor.this_object;
 }
@@ -1977,7 +1967,7 @@ JDWP::JdwpError Dbg::GetThisObject(JDWP::ObjectId thread_id, JDWP::FrameId frame
     }
   }
   UniquePtr<Context> context(Context::Create());
-  GetThisVisitor visitor(thread->GetManagedStack(), thread->GetInstrumentationStack(), context.get(), frame_id);
+  GetThisVisitor visitor(thread, context.get(), frame_id);
   visitor.WalkStack();
   *result = gRegistry->Add(visitor.this_object);
   return JDWP::ERR_NONE;
@@ -1986,12 +1976,10 @@ JDWP::JdwpError Dbg::GetThisObject(JDWP::ObjectId thread_id, JDWP::FrameId frame
 void Dbg::GetLocalValue(JDWP::ObjectId thread_id, JDWP::FrameId frame_id, int slot, JDWP::JdwpTag tag,
                         uint8_t* buf, size_t width) {
   struct GetLocalVisitor : public StackVisitor {
-    GetLocalVisitor(const ManagedStack* stack,
-                    const std::deque<InstrumentationStackFrame>* instrumentation_stack,
-                    Context* context, JDWP::FrameId frame_id, int slot, JDWP::JdwpTag tag,
-                    uint8_t* buf, size_t width)
+    GetLocalVisitor(Thread* thread, Context* context, JDWP::FrameId frame_id, int slot,
+                    JDWP::JdwpTag tag, uint8_t* buf, size_t width)
         SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-        : StackVisitor(stack, instrumentation_stack, context), frame_id_(frame_id), slot_(slot), tag_(tag),
+        : StackVisitor(thread, context), frame_id_(frame_id), slot_(slot), tag_(tag),
           buf_(buf), width_(width) {}
 
     // TODO: Enable annotalysis. We know lock is held in constructor, but abstraction confuses
@@ -2119,19 +2107,18 @@ void Dbg::GetLocalValue(JDWP::ObjectId thread_id, JDWP::FrameId frame_id, int sl
     return;
   }
   UniquePtr<Context> context(Context::Create());
-  GetLocalVisitor visitor(thread->GetManagedStack(), thread->GetInstrumentationStack(), context.get(),
-                          frame_id, slot, tag, buf, width);
+  GetLocalVisitor visitor(thread, context.get(), frame_id, slot, tag, buf, width);
   visitor.WalkStack();
 }
 
 void Dbg::SetLocalValue(JDWP::ObjectId thread_id, JDWP::FrameId frame_id, int slot, JDWP::JdwpTag tag,
                         uint64_t value, size_t width) {
   struct SetLocalVisitor : public StackVisitor {
-    SetLocalVisitor(const ManagedStack* stack, const std::deque<InstrumentationStackFrame>* instrumentation_stack, Context* context,
+    SetLocalVisitor(Thread* thread, Context* context,
                     JDWP::FrameId frame_id, int slot, JDWP::JdwpTag tag, uint64_t value,
                     size_t width)
         SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-        : StackVisitor(stack, instrumentation_stack, context),
+        : StackVisitor(thread, context),
           frame_id_(frame_id), slot_(slot), tag_(tag), value_(value), width_(width) {}
 
     // TODO: Enable annotalysis. We know lock is held in constructor, but abstraction confuses
@@ -2207,8 +2194,7 @@ void Dbg::SetLocalValue(JDWP::ObjectId thread_id, JDWP::FrameId frame_id, int sl
     return;
   }
   UniquePtr<Context> context(Context::Create());
-  SetLocalVisitor visitor(thread->GetManagedStack(), thread->GetInstrumentationStack(), context.get(),
-                          frame_id, slot, tag, value, width);
+  SetLocalVisitor visitor(thread, context.get(), frame_id, slot, tag, value, width);
   visitor.WalkStack();
 }
 
@@ -2247,7 +2233,7 @@ void Dbg::PostException(Thread* thread,
 
   // We need 'this' for InstanceOnly filters.
   UniquePtr<Context> context(Context::Create());
-  GetThisVisitor visitor(thread->GetManagedStack(), thread->GetInstrumentationStack(), context.get(), throw_frame_id);
+  GetThisVisitor visitor(thread, context.get(), throw_frame_id);
   visitor.WalkStack();
   JDWP::ObjectId this_id = gRegistry->Add(visitor.this_object);
 
@@ -2430,11 +2416,10 @@ JDWP::JdwpError Dbg::ConfigureStep(JDWP::ObjectId thread_id, JDWP::JdwpStepSize
   //
 
   struct SingleStepStackVisitor : public StackVisitor {
-    SingleStepStackVisitor(const ManagedStack* stack,
-                           const std::deque<InstrumentationStackFrame>* instrumentation_stack)
+    SingleStepStackVisitor(Thread* thread)
         EXCLUSIVE_LOCKS_REQUIRED(Locks::breakpoint_lock_)
         SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-        : StackVisitor(stack, instrumentation_stack, NULL) {
+        : StackVisitor(thread, NULL) {
       gSingleStepControl.method = NULL;
       gSingleStepControl.stack_depth = 0;
     }
@@ -2459,7 +2444,7 @@ JDWP::JdwpError Dbg::ConfigureStep(JDWP::ObjectId thread_id, JDWP::JdwpStepSize
       return true;
     }
   };
-  SingleStepStackVisitor visitor(thread->GetManagedStack(), thread->GetInstrumentationStack());
+  SingleStepStackVisitor visitor(thread);
   visitor.WalkStack();
 
   //
@@ -3421,11 +3406,9 @@ void Dbg::SetAllocTrackingEnabled(bool enabled) {
 }
 
 struct AllocRecordStackVisitor : public StackVisitor {
-  AllocRecordStackVisitor(const ManagedStack* stack,
-                          const std::deque<InstrumentationStackFrame>* instrumentation_stack,
-                          AllocRecord* record)
+  AllocRecordStackVisitor(Thread* thread, AllocRecord* record)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-      : StackVisitor(stack, instrumentation_stack, NULL), record(record), depth(0) {}
+      : StackVisitor(thread, NULL), record(record), depth(0) {}
 
   // TODO: Enable annotalysis. We know lock is held in constructor, but abstraction confuses
   // annotalysis.
@@ -3475,7 +3458,7 @@ void Dbg::RecordAllocation(Class* type, size_t byte_count) {
   record->thin_lock_id = self->GetThinLockId();
 
   // Fill in the stack trace.
-  AllocRecordStackVisitor visitor(self->GetManagedStack(), self->GetInstrumentationStack(), record);
+  AllocRecordStackVisitor visitor(self, record);
   visitor.WalkStack();
 
   if (gAllocRecordCount < kNumAllocRecords) {
diff --git a/src/instrumentation.cc b/src/instrumentation.cc
index bcf7c5d140..065758d16e 100644
--- a/src/instrumentation.cc
+++ b/src/instrumentation.cc
@@ -76,8 +76,8 @@ void InstrumentationInstallStack(Thread* self, void* arg)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
   struct InstallStackVisitor : public StackVisitor {
     InstallStackVisitor(Thread* self, uintptr_t instrumentation_exit_pc)
-        : StackVisitor(self->GetManagedStack(), self->GetInstrumentationStack(), NULL),
-          self_(self), instrumentation_exit_pc_(instrumentation_exit_pc) {}
+        : StackVisitor(self, NULL),  self_(self),
+          instrumentation_exit_pc_(instrumentation_exit_pc) {}
 
     virtual bool VisitFrame() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
       if (GetCurrentQuickFrame() == NULL) {
@@ -118,8 +118,8 @@ static void InstrumentationRestoreStack(Thread* self, void*)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
   struct RestoreStackVisitor : public StackVisitor {
     RestoreStackVisitor(Thread* self, uintptr_t instrumentation_exit_pc)
-        : StackVisitor(self->GetManagedStack(), self->GetInstrumentationStack(), NULL),
-          self_(self), instrumentation_exit_pc_(instrumentation_exit_pc) {}
+        : StackVisitor(self, NULL), self_(self),
+          instrumentation_exit_pc_(instrumentation_exit_pc) {}
 
     virtual bool VisitFrame() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
       if (self_->IsInstrumentationStackEmpty()) {
diff --git a/src/interpreter/interpreter.cc b/src/interpreter/interpreter.cc
index d816830beb..820348e081 100644
--- a/src/interpreter/interpreter.cc
+++ b/src/interpreter/interpreter.cc
@@ -137,7 +137,7 @@ static void UnstartedRuntimeJni(Thread* self, AbstractMethod* method,
   if (name == "java.lang.ClassLoader dalvik.system.VMStack.getCallingClassLoader()") {
     result->SetL(NULL);
   } else if (name == "java.lang.Class dalvik.system.VMStack.getStackClass2()") {
-    NthCallerVisitor visitor(self->GetManagedStack(), NULL, 3);
+    NthCallerVisitor visitor(self, 3);
     visitor.WalkStack();
     result->SetL(visitor.caller->GetDeclaringClass());
   } else if (name == "double java.lang.Math.log(double)") {
diff --git a/src/native/dalvik_system_VMStack.cc b/src/native/dalvik_system_VMStack.cc
index cd28b5df83..494f38d38a 100644
--- a/src/native/dalvik_system_VMStack.cc
+++ b/src/native/dalvik_system_VMStack.cc
@@ -65,7 +65,7 @@ static jint VMStack_fillStackTraceElements(JNIEnv* env, jclass, jobject javaThre
 // Returns the defining class loader of the caller's caller.
 static jobject VMStack_getCallingClassLoader(JNIEnv* env, jclass) {
   ScopedObjectAccess soa(env);
-  NthCallerVisitor visitor(soa.Self()->GetManagedStack(), soa.Self()->GetInstrumentationStack(), 2);
+  NthCallerVisitor visitor(soa.Self(), 2);
   visitor.WalkStack();
   return soa.AddLocalReference<jobject>(visitor.caller->GetDeclaringClass()->GetClassLoader());
 }
@@ -73,11 +73,8 @@ static jobject VMStack_getCallingClassLoader(JNIEnv* env, jclass) {
 static jobject VMStack_getClosestUserClassLoader(JNIEnv* env, jclass, jobject javaBootstrap,
                                                  jobject javaSystem) {
   struct ClosestUserClassLoaderVisitor : public StackVisitor {
-    ClosestUserClassLoaderVisitor(const ManagedStack* stack,
-                                  const std::deque<InstrumentationStackFrame>* instrumentation_stack,
-                                  Object* bootstrap, Object* system)
-      : StackVisitor(stack, instrumentation_stack, NULL),
-        bootstrap(bootstrap), system(system), class_loader(NULL) {}
+    ClosestUserClassLoaderVisitor(Thread* thread, Object* bootstrap, Object* system)
+      : StackVisitor(thread, NULL), bootstrap(bootstrap), system(system), class_loader(NULL) {}
 
     bool VisitFrame() {
       DCHECK(class_loader == NULL);
@@ -97,8 +94,7 @@ static jobject VMStack_getClosestUserClassLoader(JNIEnv* env, jclass, jobject ja
   ScopedObjectAccess soa(env);
   Object* bootstrap = soa.Decode<Object*>(javaBootstrap);
   Object* system = soa.Decode<Object*>(javaSystem);
-  ClosestUserClassLoaderVisitor visitor(soa.Self()->GetManagedStack(), soa.Self()->GetInstrumentationStack(),
-                                        bootstrap, system);
+  ClosestUserClassLoaderVisitor visitor(soa.Self(), bootstrap, system);
   visitor.WalkStack();
   return soa.AddLocalReference<jobject>(visitor.class_loader);
 }
@@ -106,7 +102,7 @@ static jobject VMStack_getClosestUserClassLoader(JNIEnv* env, jclass, jobject ja
 // Returns the class of the caller's caller's caller.
 static jclass VMStack_getStackClass2(JNIEnv* env, jclass) {
   ScopedObjectAccess soa(env);
-  NthCallerVisitor visitor(soa.Self()->GetManagedStack(), soa.Self()->GetInstrumentationStack(), 3);
+  NthCallerVisitor visitor(soa.Self(), 3);
   visitor.WalkStack();
   return soa.AddLocalReference<jclass>(visitor.caller->GetDeclaringClass());
 }
diff --git a/src/nth_caller_visitor.h b/src/nth_caller_visitor.h
index 15e62e29e9..efed16316d 100644
--- a/src/nth_caller_visitor.h
+++ b/src/nth_caller_visitor.h
@@ -24,8 +24,8 @@ namespace art {
 
 // Walks up the stack 'n' callers, when used with Thread::WalkStack.
 struct NthCallerVisitor : public StackVisitor {
-  NthCallerVisitor(const ManagedStack* stack, const std::deque<InstrumentationStackFrame>* instrumentation_stack, size_t n)
-      : StackVisitor(stack, instrumentation_stack, NULL), n(n), count(0), caller(NULL) {}
+  NthCallerVisitor(Thread* thread, size_t n)
+      : StackVisitor(thread, NULL), n(n), count(0), caller(NULL) {}
 
   bool VisitFrame() {
     DCHECK(caller == NULL);
diff --git a/src/oat/runtime/support_deoptimize.cc b/src/oat/runtime/support_deoptimize.cc
index 13fd3ae4ee..75e671febd 100644
--- a/src/oat/runtime/support_deoptimize.cc
+++ b/src/oat/runtime/support_deoptimize.cc
@@ -32,11 +32,8 @@ extern "C" uint64_t artDeoptimize(JValue ret_val, Thread* self, AbstractMethod**
   CHECK(old_cause == NULL);
   class DeoptimizationVisitor : public StackVisitor {
    public:
-    DeoptimizationVisitor(const ManagedStack* stack,
-                          const std::deque<InstrumentationStackFrame>* instrumentation_stack,
-                          Context* context)
-        : StackVisitor(stack, instrumentation_stack, context), shadow_frame_(NULL),
-          runtime_frames_(0) { }
+    DeoptimizationVisitor(Thread* thread, Context* context)
+        : StackVisitor(thread, context), shadow_frame_(NULL), runtime_frames_(0) { }
 
     virtual bool VisitFrame() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
       AbstractMethod* m = GetMethod();
@@ -76,7 +73,7 @@ extern "C" uint64_t artDeoptimize(JValue ret_val, Thread* self, AbstractMethod**
     }
     ShadowFrame* shadow_frame_;
     uint32_t runtime_frames_;
-  } visitor(self->GetManagedStack(), self->GetInstrumentationStack(), self->GetLongJumpContext());
+  } visitor(self, self->GetLongJumpContext());
   visitor.WalkStack(false);
   if (visitor.shadow_frame_ != NULL) {
     self->SetDeoptimizationShadowFrame(visitor.shadow_frame_, ret_val);
diff --git a/src/oat/runtime/support_instrumentation.cc b/src/oat/runtime/support_instrumentation.cc
index 8a2ac0a458..73e43717a3 100644
--- a/src/oat/runtime/support_instrumentation.cc
+++ b/src/oat/runtime/support_instrumentation.cc
@@ -29,8 +29,7 @@ extern "C" const void* artInstrumentationMethodEntryFromCode(AbstractMethod* met
   self->VerifyStack();
   Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
   // +1 as frame id's start at 1, +1 as we haven't yet built this method's frame.
-  size_t frame_id = StackVisitor::ComputeNumFrames(self->GetManagedStack(),
-                                                   self->GetInstrumentationStack()) + 2;
+  size_t frame_id = StackVisitor::ComputeNumFrames(self) + 2;
   InstrumentationStackFrame instrumentation_frame(method, lr, frame_id);
   self->PushInstrumentationStackFrame(instrumentation_frame);
 
@@ -47,13 +46,12 @@ extern "C" uint64_t artInstrumentationMethodExitFromCode(Thread* self, AbstractM
   self->SetTopOfStack(sp, 0);
   self->VerifyStack();
   // +1 as frame id's start at 1, +1 as we want the called frame not the frame being returned into.
-  size_t frame_id = StackVisitor::ComputeNumFrames(self->GetManagedStack(),
-                                                   self->GetInstrumentationStack()) + 2;
+  size_t frame_id = StackVisitor::ComputeNumFrames(self) + 2;
   InstrumentationStackFrame instrumentation_frame;
   instrumentation_frame = self->PopInstrumentationStackFrame();
   if (frame_id != instrumentation_frame.frame_id_) {
     LOG(ERROR) << "Expected frame_id=" << frame_id << " but found " << instrumentation_frame.frame_id_;
-    StackVisitor::DescribeStack(self->GetManagedStack(), self->GetInstrumentationStack());
+    StackVisitor::DescribeStack(self);
   }
   Runtime* runtime = Runtime::Current();
   if (runtime->IsMethodTracingActive()) {
diff --git a/src/stack.cc b/src/stack.cc
index 228f96d31d..2e1f4ae311 100644
--- a/src/stack.cc
+++ b/src/stack.cc
@@ -52,6 +52,13 @@ bool ManagedStack::ShadowFramesContain(Object** shadow_frame_entry) const {
   return false;
 }
 
+StackVisitor::StackVisitor(Thread* thread, Context* context)
+    : thread_(thread), cur_shadow_frame_(NULL),
+      cur_quick_frame_(NULL), cur_quick_frame_pc_(0), num_frames_(0), cur_depth_(0),
+      context_(context) {
+  DCHECK(thread == Thread::Current() || thread->IsSuspended());
+}
+
 uint32_t StackVisitor::GetDexPc() const {
   if (cur_shadow_frame_ != NULL) {
     return cur_shadow_frame_->GetDexPC();
@@ -142,12 +149,10 @@ void StackVisitor::SetReturnPc(uintptr_t new_ret_pc) {
   *reinterpret_cast<uintptr_t*>(pc_addr) = new_ret_pc;
 }
 
-size_t StackVisitor::ComputeNumFrames(const ManagedStack* stack,
-                                      const std::deque<InstrumentationStackFrame>* instr_stack) {
+size_t StackVisitor::ComputeNumFrames(Thread* thread) {
   struct NumFramesVisitor : public StackVisitor {
-    explicit NumFramesVisitor(const ManagedStack* stack,
-                              const std::deque<InstrumentationStackFrame>* instrumentation_stack)
-        : StackVisitor(stack, instrumentation_stack, NULL), frames(0) {}
+    explicit NumFramesVisitor(Thread* thread)
+        : StackVisitor(thread, NULL), frames(0) {}
 
     virtual bool VisitFrame() {
       frames++;
@@ -156,25 +161,22 @@ size_t StackVisitor::ComputeNumFrames(const ManagedStack* stack,
 
     size_t frames;
   };
-  UNUSED(instr_stack);  // We don't use the instrumentation stack as we don't require dex pcs...
-  NumFramesVisitor visitor(stack, NULL);
+  NumFramesVisitor visitor(thread);
   visitor.WalkStack(true);
   return visitor.frames;
 }
 
-void StackVisitor::DescribeStack(const ManagedStack* stack,
-                                 const std::deque<InstrumentationStackFrame>* instr_stack) {
+void StackVisitor::DescribeStack(Thread* thread) {
   struct DescribeStackVisitor : public StackVisitor {
-    explicit DescribeStackVisitor(const ManagedStack* stack,
-                              const std::deque<InstrumentationStackFrame>* instrumentation_stack)
-        : StackVisitor(stack, instrumentation_stack, NULL) {}
+    explicit DescribeStackVisitor(Thread* thread)
+        : StackVisitor(thread, NULL) {}
 
     virtual bool VisitFrame() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
       LOG(INFO) << "Frame Id=" << GetFrameId() << " " << DescribeLocation();
       return true;
     }
   };
-  DescribeStackVisitor visitor(stack, instr_stack);
+  DescribeStackVisitor visitor(thread);
   visitor.WalkStack(true);
 }
 
@@ -192,6 +194,10 @@ std::string StackVisitor::DescribeLocation() const {
   return result;
 }
 
+InstrumentationStackFrame StackVisitor::GetInstrumentationStackFrame(uint32_t depth) const {
+  return thread_->GetInstrumentationStack()->at(depth);
+}
+
 void StackVisitor::SanityCheckFrame() const {
 #ifndef NDEBUG
   AbstractMethod* method = GetMethod();
@@ -210,9 +216,12 @@ void StackVisitor::SanityCheckFrame() const {
 }
 
 void StackVisitor::WalkStack(bool include_transitions) {
-  bool method_tracing_active = instrumentation_stack_ != NULL;
+  DCHECK(thread_ == Thread::Current() || thread_->IsSuspended());
+  const std::deque<InstrumentationStackFrame>* instrumentation_stack =
+      thread_->GetInstrumentationStack();
+  bool method_tracing_active = instrumentation_stack != NULL;
   uint32_t instrumentation_stack_depth = 0;
-  for (const ManagedStack* current_fragment = stack_start_; current_fragment != NULL;
+  for (const ManagedStack* current_fragment = thread_->GetManagedStack(); current_fragment != NULL;
        current_fragment = current_fragment->GetLink()) {
     cur_shadow_frame_ = current_fragment->GetTopShadowFrame();
     cur_quick_frame_ = current_fragment->GetTopQuickFrame();
diff --git a/src/stack.h b/src/stack.h
index c3b837ff4c..8d0efe9e33 100644
--- a/src/stack.h
+++ b/src/stack.h
@@ -331,13 +331,7 @@ class PACKED(4) ManagedStack {
 
 class StackVisitor {
  protected:
-  StackVisitor(const ManagedStack* stack,
-               const std::deque<InstrumentationStackFrame>* instrumentation_stack,
-               Context* context)
-      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-      : stack_start_(stack), instrumentation_stack_(instrumentation_stack), cur_shadow_frame_(NULL),
-        cur_quick_frame_(NULL), cur_quick_frame_pc_(0), num_frames_(0), cur_depth_(0),
-        context_(context) {}
+  StackVisitor(Thread* thread, Context* context) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
  public:
   virtual ~StackVisitor() {}
@@ -389,7 +383,7 @@ class StackVisitor {
 
   size_t GetNumFrames() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
     if (num_frames_ == 0) {
-      num_frames_ = ComputeNumFrames(stack_start_, instrumentation_stack_);
+      num_frames_ = ComputeNumFrames(thread_);
     }
     return num_frames_;
   }
@@ -493,24 +487,17 @@ class StackVisitor {
 
   std::string DescribeLocation() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
-  static size_t ComputeNumFrames(const ManagedStack* stack,
-                                 const std::deque<InstrumentationStackFrame>* instr_stack)
-      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+  static size_t ComputeNumFrames(Thread* thread) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
-  static void DescribeStack(const ManagedStack* stack,
-                            const std::deque<InstrumentationStackFrame>* instr_stack)
-      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+  static void DescribeStack(Thread* thread) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
  private:
 
-  InstrumentationStackFrame GetInstrumentationStackFrame(uint32_t depth) const {
-    return instrumentation_stack_->at(depth);
-  }
+  InstrumentationStackFrame GetInstrumentationStackFrame(uint32_t depth) const;
 
   void SanityCheckFrame() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
-  const ManagedStack* const stack_start_;
-  const std::deque<InstrumentationStackFrame>* const instrumentation_stack_;
+  Thread* const thread_;
   ShadowFrame* cur_shadow_frame_;
   AbstractMethod** cur_quick_frame_;
   uintptr_t cur_quick_frame_pc_;
diff --git a/src/thread.cc b/src/thread.cc
index d27710b6bf..46cba06dc3 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -835,10 +835,9 @@ void Thread::DumpState(std::ostream& os) const {
 }
 
 struct StackDumpVisitor : public StackVisitor {
-  StackDumpVisitor(std::ostream& os, const Thread* thread, Context* context, bool can_allocate)
+  StackDumpVisitor(std::ostream& os, Thread* thread, Context* context, bool can_allocate)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-      : StackVisitor(thread->GetManagedStack(), thread->GetInstrumentationStack(), context),
-        os(os), thread(thread), can_allocate(can_allocate),
+      : StackVisitor(thread, context), os(os), thread(thread), can_allocate(can_allocate),
         last_method(NULL), last_line_number(0), repetition_count(0), frame_count(0) {
   }
 
@@ -917,7 +916,7 @@ void Thread::DumpStack(std::ostream& os) const {
     DumpNativeStack(os, GetTid(), "  native: ", false);
   }
   UniquePtr<Context> context(Context::Create());
-  StackDumpVisitor dumper(os, this, context.get(), !throwing_OutOfMemoryError_);
+  StackDumpVisitor dumper(os, const_cast<Thread*>(this), context.get(), !throwing_OutOfMemoryError_);
   dumper.WalkStack();
 }
 
@@ -1264,10 +1263,9 @@ void Thread::NotifyLocked(Thread* self) {
 
 class CountStackDepthVisitor : public StackVisitor {
  public:
-  CountStackDepthVisitor(const ManagedStack* stack,
-                         const std::deque<InstrumentationStackFrame>* instrumentation_stack)
+  CountStackDepthVisitor(Thread* thread)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-      : StackVisitor(stack, instrumentation_stack, NULL),
+      : StackVisitor(thread, NULL),
         depth_(0), skip_depth_(0), skipping_(true) {}
 
   bool VisitFrame() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
@@ -1305,10 +1303,8 @@ class CountStackDepthVisitor : public StackVisitor {
 
 class BuildInternalStackTraceVisitor : public StackVisitor {
  public:
-  explicit BuildInternalStackTraceVisitor(Thread* self, const ManagedStack* stack,
-                                          const std::deque<InstrumentationStackFrame>* instrumentation_stack,
-                                          int skip_depth)
-      : StackVisitor(stack, instrumentation_stack, NULL), self_(self),
+  explicit BuildInternalStackTraceVisitor(Thread* self, Thread* thread, int skip_depth)
+      : StackVisitor(thread, NULL), self_(self),
         skip_depth_(skip_depth), count_(0), dex_pc_trace_(NULL), method_trace_(NULL) {}
 
   bool Init(int depth)
@@ -1379,19 +1375,25 @@ class BuildInternalStackTraceVisitor : public StackVisitor {
 
 jobject Thread::CreateInternalStackTrace(const ScopedObjectAccessUnchecked& soa) const {
   // Compute depth of stack
-  CountStackDepthVisitor count_visitor(GetManagedStack(), GetInstrumentationStack());
+  CountStackDepthVisitor count_visitor(const_cast<Thread*>(this));
   count_visitor.WalkStack();
   int32_t depth = count_visitor.GetDepth();
   int32_t skip_depth = count_visitor.GetSkipDepth();
 
   // Build internal stack trace.
-  BuildInternalStackTraceVisitor build_trace_visitor(soa.Self(), GetManagedStack(),
-                                                     GetInstrumentationStack(), skip_depth);
+  BuildInternalStackTraceVisitor build_trace_visitor(soa.Self(), const_cast<Thread*>(this),
+                                                     skip_depth);
   if (!build_trace_visitor.Init(depth)) {
     return NULL;  // Allocation failed.
   }
   build_trace_visitor.WalkStack();
-  return soa.AddLocalReference<jobjectArray>(build_trace_visitor.GetInternalStackTrace());
+  ObjectArray<Object>* trace = build_trace_visitor.GetInternalStackTrace();
+  if (kIsDebugBuild) {
+    for (int32_t i = 0; i < trace->GetLength(); ++i) {
+      CHECK(trace->Get(i) != NULL);
+    }
+  }
+  return soa.AddLocalReference<jobjectArray>(trace);
 }
 
 jobjectArray Thread::InternalStackTraceToStackTraceElementArray(JNIEnv* env, jobject internal,
@@ -1691,7 +1693,7 @@ class CatchBlockStackVisitor : public StackVisitor {
  public:
   CatchBlockStackVisitor(Thread* self, Throwable* exception)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-      : StackVisitor(self->GetManagedStack(), self->GetInstrumentationStack(), self->GetLongJumpContext()),
+      : StackVisitor(self, self->GetLongJumpContext()),
         self_(self), exception_(exception), to_find_(exception->GetClass()), throw_method_(NULL),
         throw_frame_id_(0), throw_dex_pc_(0), handler_quick_frame_(NULL),
         handler_quick_frame_pc_(0), handler_dex_pc_(0), native_method_count_(0),
@@ -1826,10 +1828,9 @@ Context* Thread::GetLongJumpContext() {
 
 AbstractMethod* Thread::GetCurrentMethod(uint32_t* dex_pc, size_t* frame_id) const {
   struct CurrentMethodVisitor : public StackVisitor {
-    CurrentMethodVisitor(const ManagedStack* stack,
-                         const std::deque<InstrumentationStackFrame>* instrumentation_stack)
+    CurrentMethodVisitor(Thread* thread)
         SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-        : StackVisitor(stack, instrumentation_stack, NULL), method_(NULL), dex_pc_(0), frame_id_(0) {}
+        : StackVisitor(thread, NULL), method_(NULL), dex_pc_(0), frame_id_(0) {}
 
     virtual bool VisitFrame() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
       AbstractMethod* m = GetMethod();
@@ -1847,7 +1848,7 @@ AbstractMethod* Thread::GetCurrentMethod(uint32_t* dex_pc, size_t* frame_id) con
     size_t frame_id_;
   };
 
-  CurrentMethodVisitor visitor(GetManagedStack(), GetInstrumentationStack());
+  CurrentMethodVisitor visitor(const_cast<Thread*>(this));
   visitor.WalkStack(false);
   if (dex_pc != NULL) {
     *dex_pc = visitor.dex_pc_;
@@ -1869,11 +1870,9 @@ bool Thread::HoldsLock(Object* object) {
 template <typename RootVisitor>
 class ReferenceMapVisitor : public StackVisitor {
  public:
-  ReferenceMapVisitor(const ManagedStack* stack,
-                      const std::deque<InstrumentationStackFrame>* instrumentation_stack,
-                      Context* context, const RootVisitor& visitor)
+  ReferenceMapVisitor(Thread* thread, Context* context, const RootVisitor& visitor)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-      : StackVisitor(stack, instrumentation_stack, context), visitor_(visitor) {}
+      : StackVisitor(thread, context), visitor_(visitor) {}
 
   bool VisitFrame() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
     if (false) {
@@ -2039,8 +2038,7 @@ void Thread::VerifyRoots(Heap::VerifyRootVisitor* visitor, void* arg) {
   // Visit roots on this thread's stack
   Context* context = GetLongJumpContext();
   VerifyCallbackVisitor visitorToCallback(visitor, arg);
-  ReferenceMapVisitor<VerifyCallbackVisitor> mapper(GetManagedStack(), GetInstrumentationStack(),
-                                                    context, visitorToCallback);
+  ReferenceMapVisitor<VerifyCallbackVisitor> mapper(this, context, visitorToCallback);
   mapper.WalkStack();
   ReleaseLongJumpContext(context);
 }
@@ -2063,8 +2061,7 @@ void Thread::VisitRoots(Heap::RootVisitor* visitor, void* arg) {
   // Visit roots on this thread's stack
   Context* context = GetLongJumpContext();
   RootCallbackVisitor visitorToCallback(visitor, arg);
-  ReferenceMapVisitor<RootCallbackVisitor> mapper(GetManagedStack(), GetInstrumentationStack(),
-                                                  context, visitorToCallback);
+  ReferenceMapVisitor<RootCallbackVisitor> mapper(this, context, visitorToCallback);
   mapper.WalkStack();
   ReleaseLongJumpContext(context);
 }
diff --git a/src/thread.h b/src/thread.h
index 76aea759c9..13e1cab058 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -26,9 +26,7 @@
 #include <string>
 
 #include "base/macros.h"
-#include "closure.h"
 #include "globals.h"
-#include "instrumentation.h"
 #include "jvalue.h"
 #include "oat/runtime/oat_support_entrypoints.h"
 #include "locks.h"
@@ -46,6 +44,7 @@ class BaseMutex;
 class Class;
 class ClassLinker;
 class ClassLoader;
+class Closure;
 class Context;
 struct DebugInvokeReq;
 class DexFile;
@@ -57,7 +56,6 @@ class Runtime;
 class ScopedObjectAccess;
 class ScopedObjectAccessUnchecked;
 class ShadowFrame;
-class StackIndirectReferenceTable;
 class StackTraceElement;
 class StaticStorageBase;
 class Thread;
diff --git a/test/ReferenceMap/stack_walk_refmap_jni.cc b/test/ReferenceMap/stack_walk_refmap_jni.cc
index 8b1e6cf938..60182e2fae 100644
--- a/test/ReferenceMap/stack_walk_refmap_jni.cc
+++ b/test/ReferenceMap/stack_walk_refmap_jni.cc
@@ -42,10 +42,9 @@ namespace art {
   } while (false)
 
 struct ReferenceMap2Visitor : public StackVisitor {
-  explicit ReferenceMap2Visitor(const ManagedStack* stack,
-                                const std::deque<InstrumentationStackFrame>* instrumentation_stack)
+  explicit ReferenceMap2Visitor(Thread* thread)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-      : StackVisitor(stack, instrumentation_stack, NULL) {
+      : StackVisitor(thread, NULL) {
   }
 
   bool VisitFrame() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
@@ -274,9 +273,8 @@ struct ReferenceMap2Visitor : public StackVisitor {
 
 extern "C" JNIEXPORT jint JNICALL Java_ReferenceMap_refmap(JNIEnv*, jobject, jint count) {
   // Visitor
-  ScopedObjectAccess ts(Thread::Current());
-  ReferenceMap2Visitor mapper(Thread::Current()->GetManagedStack(),
-                              Thread::Current()->GetInstrumentationStack());
+  ScopedObjectAccess soa(Thread::Current());
+  ReferenceMap2Visitor mapper(soa.Self());
   mapper.WalkStack();
 
   return count + 1;
diff --git a/test/StackWalk/stack_walk_jni.cc b/test/StackWalk/stack_walk_jni.cc
index 8db36e91c0..dccd69fe0d 100644
--- a/test/StackWalk/stack_walk_jni.cc
+++ b/test/StackWalk/stack_walk_jni.cc
@@ -40,10 +40,9 @@ namespace art {
 static int gJava_StackWalk_refmap_calls = 0;
 
 struct TestReferenceMapVisitor : public StackVisitor {
-  explicit TestReferenceMapVisitor(const ManagedStack* stack,
-                                   const std::deque<InstrumentationStackFrame>* instrumentation_stack)
+  explicit TestReferenceMapVisitor(Thread* thread)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
-      : StackVisitor(stack, instrumentation_stack, NULL) {
+      : StackVisitor(thread, NULL) {
   }
 
   bool VisitFrame() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
@@ -102,25 +101,23 @@ struct TestReferenceMapVisitor : public StackVisitor {
 };
 
 extern "C" JNIEXPORT jint JNICALL Java_StackWalk_refmap(JNIEnv*, jobject, jint count) {
-  ScopedObjectAccess ts(Thread::Current());
+  ScopedObjectAccess soa(Thread::Current());
   CHECK_EQ(count, 0);
   gJava_StackWalk_refmap_calls++;
 
   // Visitor
-  TestReferenceMapVisitor mapper(Thread::Current()->GetManagedStack(),
-                                 Thread::Current()->GetInstrumentationStack());
+  TestReferenceMapVisitor mapper(soa.Self());
   mapper.WalkStack();
 
   return count + 1;
 }
 
 extern "C" JNIEXPORT jint JNICALL Java_StackWalk2_refmap2(JNIEnv*, jobject, jint count) {
-  ScopedObjectAccess ts(Thread::Current());
+  ScopedObjectAccess soa(Thread::Current());
   gJava_StackWalk_refmap_calls++;
 
   // Visitor
-  TestReferenceMapVisitor mapper(Thread::Current()->GetManagedStack(),
-                                 Thread::Current()->GetInstrumentationStack());
+  TestReferenceMapVisitor mapper(soa.Self());
   mapper.WalkStack();
 
   return count + 1;