Fix exception_test.cc and thread.cc and revert 3ddac99d4dc6a036fac59d8f0bdc664ef619fb04.

The test was wrong, and jni_compiler_test failed too because we'd missed one
place where we need to mangle the PC.

Change-Id: Ib67ca081e17b5ee8b8c64696082858b212b157f1

2 files changed, 24 insertions, 18 deletions

diff --git a/src/exception_test.cc b/src/exception_test.cc
index 466211915e..fff638fcde 100644
--- a/src/exception_test.cc
+++ b/src/exception_test.cc
@@ -105,12 +105,13 @@ TEST_F(ExceptionTest, StackTraceElement) {
   // Create two fake stack frames with mapping data created in SetUp. We map offset 3 in the code
   // to dex pc 3, however, we set the return pc to 5 as the stack walker always subtracts two
   // from a return pc.
+  const uintptr_t pc_offset = 3 + 2;
 
   // Create/push fake 16byte stack frame for method g
   fake_stack[top_of_stack++] = reinterpret_cast<uintptr_t>(method_g_);
   fake_stack[top_of_stack++] = 0;
   fake_stack[top_of_stack++] = 0;
-  fake_stack[top_of_stack++] = reinterpret_cast<uintptr_t>(method_f_->GetCode()) + 5;  // return pc
+  fake_stack[top_of_stack++] = reinterpret_cast<uintptr_t>(method_f_->GetCode()) + pc_offset;  // return pc
 
   // Create/push fake 16byte stack frame for method f
   fake_stack[top_of_stack++] = reinterpret_cast<uintptr_t>(method_f_);
@@ -123,7 +124,7 @@ TEST_F(ExceptionTest, StackTraceElement) {
 
   // Set up thread to appear as if we called out of method_g_ at pc 3
   Thread* thread = Thread::Current();
-  thread->SetTopOfStack(fake_stack, reinterpret_cast<uintptr_t>(method_g_->GetCode()) + 3);
+  thread->SetTopOfStack(fake_stack, reinterpret_cast<uintptr_t>(method_g_->GetCode()) + pc_offset);  // return pc
 
   JNIEnv* env = thread->GetJniEnv();
   jobject internal = thread->CreateInternalStackTrace(env);
diff --git a/src/thread.cc b/src/thread.cc
index c50bf0a7ac..5ffd451aaf 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -958,10 +958,25 @@ class BuildInternalStackTraceVisitor : public Thread::StackVisitor {
   jobject local_ref_;
 };
 
+// TODO: remove this.
+uintptr_t ManglePc(uintptr_t pc) {
+  // Move the PC back 2 bytes as a call will frequently terminate the
+  // decoding of a particular instruction and we want to make sure we
+  // get the Dex PC of the instruction with the call and not the
+  // instruction following.
+  if (pc > 0) { pc -= 2; }
+  return pc;
+}
+
+// TODO: remove this.
+uintptr_t DemanglePc(uintptr_t pc) {
+  // Revert mangling for the case where we need the PC to return to the upcall
+  return pc + 2;
+}
 
 void Thread::WalkStack(StackVisitor* visitor) const {
   Frame frame = GetTopOfStack();
-  uintptr_t pc = top_of_managed_stack_pc_;
+  uintptr_t pc = ManglePc(top_of_managed_stack_pc_);
   // TODO: enable this CHECK after native_to_managed_record_ is initialized during startup.
   // CHECK(native_to_managed_record_ != NULL);
   NativeToManagedRecord* record = native_to_managed_record_;
@@ -970,37 +985,27 @@ void Thread::WalkStack(StackVisitor* visitor) const {
     for ( ; frame.GetMethod() != 0; frame.Next()) {
       // DCHECK(frame.GetMethod()->IsWithinCode(pc));  // TODO: restore IsWithinCode
       visitor->VisitFrame(frame, pc);
-      pc = frame.GetReturnPC();
-      // Move the PC back 2 bytes as a call will frequently terminate the
-      // decoding of a particular instruction and we want to make sure we
-      // get the Dex PC of the instruction with the call and not the
-      // instruction following.
-      if (pc > 0) { pc -= 2; }
+      pc = ManglePc(frame.GetReturnPC());
     }
     if (record == NULL) {
       break;
     }
     // last_tos should return Frame instead of sp?
     frame.SetSP(reinterpret_cast<Method**>(record->last_top_of_managed_stack_));
-    pc = record->last_top_of_managed_stack_pc_;
+    pc = ManglePc(record->last_top_of_managed_stack_pc_);
     record = record->link_;
   }
 }
 
 void Thread::WalkStackUntilUpCall(StackVisitor* visitor, bool include_upcall) const {
   Frame frame = GetTopOfStack();
-  uintptr_t pc = top_of_managed_stack_pc_;
+  uintptr_t pc = ManglePc(top_of_managed_stack_pc_);
 
   if (frame.GetSP() != 0) {
     for ( ; frame.GetMethod() != 0; frame.Next()) {
       // DCHECK(frame.GetMethod()->IsWithinCode(pc));  // TODO: restore IsWithinCode
       visitor->VisitFrame(frame, pc);
-      pc = frame.GetReturnPC();
-      // Move the PC back 2 bytes as a call will frequently terminate the
-      // decoding of a particular instruction and we want to make sure we
-      // get the Dex PC of the instruction with the call and not the
-      // instruction following.
-      if (pc > 0) { pc -= 2; }
+      pc = ManglePc(frame.GetReturnPC());
     }
     if (include_upcall) {
       visitor->VisitFrame(frame, pc);
@@ -1138,7 +1143,7 @@ class CatchBlockStackVisitor : public Thread::StackVisitor {
       if (method == NULL) {
         // This is the upcall, we remember the frame and last_pc so that we may
         // long jump to them
-        handler_pc_ = pc + 2;  // We want to return after the call instruction, wind forward 2 again
+        handler_pc_ = DemanglePc(pc);
         handler_frame_ = fr;
         return;
       }