commit 6641ea12b98dda9ec45d29f20e43f85698b88a02
author jeffhao <jeffhao@google.com> Wed Jan 02 18:13:42 2013 -0800
committer jeffhao <jeffhao@google.com> Thu Jan 03 14:40:36 2013 -0800
tree 1ffeecc0d83c3ec6158bcdbaf8851e716f24f2d7
parent 1d6df8edb5b177bd24d692e1fa854272af394d0a

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

src/oat/runtime/support_instrumentation.cc

diff --git a/src/oat/runtime/support_instrumentation.cc b/src/oat/runtime/support_instrumentation.cc
index 339c11a..8a2ac0a 100644
--- a/src/oat/runtime/support_instrumentation.cc
+++ b/src/oat/runtime/support_instrumentation.cc
@@ -25,15 +25,12 @@
 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" 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();

src/stack.cc

diff --git a/src/stack.cc b/src/stack.cc
index e962dec..228f96d 100644
--- a/src/stack.cc
+++ b/src/stack.cc
@@ -221,7 +221,7 @@
       // 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 @@
         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();