Fix stack walking when top frame has null method pointer.

Changed the do-while loop to just a while loop in the stack walk. This
fixes the case where the top frame has a null method pointer, which can
happen during instrumentation.

Change-Id: If7d67cd315d31bac4c1bbe31d6e385612b182935

2 files changed, 6 insertions, 18 deletions

diff --git a/src/oat/runtime/support_instrumentation.cc b/src/oat/runtime/support_instrumentation.cc
index 339c11aa2b..8a2ac0a458 100644
--- a/src/oat/runtime/support_instrumentation.cc
+++ b/src/oat/runtime/support_instrumentation.cc
@@ -25,15 +25,12 @@ namespace art {
 extern "C" const void* artInstrumentationMethodEntryFromCode(AbstractMethod* method, Thread* self,
                                                              AbstractMethod** sp, uintptr_t lr)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
-  if (*sp != NULL) {
-    self->SetTopOfStack(sp, lr);
-  }
+  self->SetTopOfStack(sp, lr);
   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.
-  const size_t kFrameIdAdjust = 2;
   size_t frame_id = StackVisitor::ComputeNumFrames(self->GetManagedStack(),
-                                                   self->GetInstrumentationStack()) + kFrameIdAdjust;
+                                                   self->GetInstrumentationStack()) + 2;
   InstrumentationStackFrame instrumentation_frame(method, lr, frame_id);
   self->PushInstrumentationStackFrame(instrumentation_frame);
 
@@ -47,26 +44,17 @@ extern "C" const void* artInstrumentationMethodEntryFromCode(AbstractMethod* met
 
 extern "C" uint64_t artInstrumentationMethodExitFromCode(Thread* self, AbstractMethod** sp)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
-  if (*sp != NULL) {
-    self->SetTopOfStack(sp, 0);
-  }
+  self->SetTopOfStack(sp, 0);
   self->VerifyStack();
-
-  /*
-  // TODO: ComputeNumFrames currently fails here, so it's disabled.
   // +1 as frame id's start at 1, +1 as we want the called frame not the frame being returned into.
-  const size_t kFrameIdAdjust = 2;
   size_t frame_id = StackVisitor::ComputeNumFrames(self->GetManagedStack(),
-                                                   self->GetInstrumentationStack()) + kFrameIdAdjust;
-  */
+                                                   self->GetInstrumentationStack()) + 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());
   }
-  */
   Runtime* runtime = Runtime::Current();
   if (runtime->IsMethodTracingActive()) {
     Trace* trace = runtime->GetInstrumentation()->GetTrace();
diff --git a/src/stack.cc b/src/stack.cc
index e962dec8b6..228f96d31d 100644
--- a/src/stack.cc
+++ b/src/stack.cc
@@ -221,7 +221,7 @@ void StackVisitor::WalkStack(bool include_transitions) {
       // Can't be both a shadow and a quick fragment.
       DCHECK(current_fragment->GetTopShadowFrame() == NULL);
       AbstractMethod* method = *cur_quick_frame_;
-      do {
+      while (method != NULL) {
         SanityCheckFrame();
         bool should_continue = VisitFrame();
         if (UNLIKELY(!should_continue)) {
@@ -253,7 +253,7 @@ void StackVisitor::WalkStack(bool include_transitions) {
         cur_quick_frame_ = reinterpret_cast<AbstractMethod**>(next_frame);
         cur_depth_++;
         method = *cur_quick_frame_;
-      } while (method != NULL);
+      }
     } else if (cur_shadow_frame_ != NULL) {
       do {
         SanityCheckFrame();