Update implementation of method unwind callbacks

Earlier method unwind callbacks for quick / native methods was based on
the instrumentation stack entries. JITed code no longer have entries in
instrumentation stack and native code also will soon not use
instrumentation stack. So update the implementation to record the
methods on the stack as we walk them for finding the catch block and use
them to call the method unwind callbacks when necessary.

Bug: 206029744
Test: art/test.py
Change-Id: I9a5f6e544cadc78e1e93d8b2f4e5ec58b04d6ea3
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 58a98c8..0454e21 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -107,69 +107,6 @@
   Instrumentation* const instrumentation_;
 };
 
-InstrumentationStackPopper::InstrumentationStackPopper(Thread* self)
-      : self_(self),
-        instrumentation_(Runtime::Current()->GetInstrumentation()),
-        pop_until_(0u) {}
-
-InstrumentationStackPopper::~InstrumentationStackPopper() {
-  std::map<uintptr_t, instrumentation::InstrumentationStackFrame>* stack =
-      self_->GetInstrumentationStack();
-  for (auto i = stack->begin(); i != stack->end() && i->first <= pop_until_;) {
-    i = stack->erase(i);
-  }
-}
-
-bool InstrumentationStackPopper::PopFramesTo(uintptr_t stack_pointer,
-                                             MutableHandle<mirror::Throwable>& exception) {
-  std::map<uintptr_t, instrumentation::InstrumentationStackFrame>* stack =
-      self_->GetInstrumentationStack();
-  DCHECK(!self_->IsExceptionPending());
-  if (!instrumentation_->HasMethodUnwindListeners()) {
-    pop_until_ = stack_pointer;
-    return true;
-  }
-  if (kVerboseInstrumentation) {
-    LOG(INFO) << "Popping frames for exception " << exception->Dump();
-  }
-  // The instrumentation events expect the exception to be set.
-  self_->SetException(exception.Get());
-  bool new_exception_thrown = false;
-  auto i = stack->upper_bound(pop_until_);
-
-  // Now pop all frames until reaching stack_pointer, or a new exception is
-  // thrown. Note that `stack_pointer` doesn't need to be a return PC address
-  // (in fact the exception handling code passes the start of the frame where
-  // the catch handler is).
-  for (; i != stack->end() && i->first <= stack_pointer; i++) {
-    const InstrumentationStackFrame& frame = i->second;
-    ArtMethod* method = frame.method_;
-    // Notify listeners of method unwind.
-    // TODO: improve the dex_pc information here.
-    uint32_t dex_pc = dex::kDexNoIndex;
-    if (kVerboseInstrumentation) {
-      LOG(INFO) << "Popping for unwind " << method->PrettyMethod();
-    }
-    if (!method->IsRuntimeMethod() && !frame.interpreter_entry_) {
-      instrumentation_->MethodUnwindEvent(self_, method, dex_pc);
-      new_exception_thrown = self_->GetException() != exception.Get();
-      if (new_exception_thrown) {
-        pop_until_ = i->first;
-        break;
-      }
-    }
-  }
-  if (!new_exception_thrown) {
-    pop_until_ = stack_pointer;
-  }
-  exception.Assign(self_->GetException());
-  self_->ClearException();
-  if (kVerboseInstrumentation && new_exception_thrown) {
-    LOG(INFO) << "Did partial pop of frames due to new exception";
-  }
-  return !new_exception_thrown;
-}
-
 Instrumentation::Instrumentation()
     : current_force_deopt_id_(0),
       instrumentation_stubs_installed_(false),
@@ -191,6 +128,51 @@
       alloc_entrypoints_instrumented_(false) {
 }
 
+bool Instrumentation::ProcessMethodUnwindCallbacks(Thread* self,
+                                                   std::queue<ArtMethod*>& methods,
+                                                   MutableHandle<mirror::Throwable>& exception) {
+  DCHECK(!self->IsExceptionPending());
+  if (!HasMethodUnwindListeners()) {
+    return true;
+  }
+  if (kVerboseInstrumentation) {
+    LOG(INFO) << "Popping frames for exception " << exception->Dump();
+  }
+  // The instrumentation events expect the exception to be set.
+  self->SetException(exception.Get());
+  bool new_exception_thrown = false;
+
+  // Process callbacks for all methods that would be unwound until a new exception is thrown.
+  while(!methods.empty()) {
+    ArtMethod* method = methods.front();
+    methods.pop();
+    if (kVerboseInstrumentation) {
+      LOG(INFO) << "Popping for unwind " << method->PrettyMethod();
+    }
+
+    if (method->IsRuntimeMethod()) {
+      continue;
+    }
+
+    // Notify listeners of method unwind.
+    // TODO: improve the dex_pc information here.
+    uint32_t dex_pc = dex::kDexNoIndex;
+    MethodUnwindEvent(self, method, dex_pc);
+    new_exception_thrown = self->GetException() != exception.Get();
+    if (new_exception_thrown) {
+      break;
+    }
+  }
+
+  exception.Assign(self->GetException());
+  self->ClearException();
+  if (kVerboseInstrumentation && new_exception_thrown) {
+    LOG(INFO) << "Did partial pop of frames due to new exception";
+  }
+  return !new_exception_thrown;
+}
+
+
 void Instrumentation::InstallStubsForClass(ObjPtr<mirror::Class> klass) {
   if (!klass->IsResolved()) {
     // We need the class to be resolved to install/uninstall stubs. Otherwise its methods
@@ -1778,19 +1760,17 @@
   }
 }
 
-uintptr_t Instrumentation::PopFramesForDeoptimization(Thread* self, uintptr_t pop_until) const {
+uintptr_t Instrumentation::PopInstrumentationStackUntil(Thread* self, uintptr_t pop_until) const {
   std::map<uintptr_t, instrumentation::InstrumentationStackFrame>* stack =
       self->GetInstrumentationStack();
   // Pop all instrumentation frames below `pop_until`.
   uintptr_t return_pc = 0u;
   for (auto i = stack->begin(); i != stack->end() && i->first <= pop_until;) {
-    auto e = i;
-    ++i;
     if (kVerboseInstrumentation) {
-      LOG(INFO) << "Popping for deoptimization " << e->second.method_->PrettyMethod();
+      LOG(INFO) << "Popping for deoptimization " << i->second.method_->PrettyMethod();
     }
-    return_pc = e->second.return_pc_;
-    stack->erase(e);
+    return_pc = i->second.return_pc_;
+    i = stack->erase(i);
   }
   return return_pc;
 }
diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
index f760606..48401f2 100644
--- a/runtime/instrumentation.h
+++ b/runtime/instrumentation.h
@@ -23,6 +23,7 @@
 #include <list>
 #include <memory>
 #include <optional>
+#include <queue>
 #include <unordered_set>
 
 #include "arch/instruction_set.h"
@@ -528,9 +529,9 @@
                                              uint64_t* fpr_result)
       REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!GetDeoptimizedMethodsLock());
 
-  // Pops nframes instrumentation frames from the current thread. Returns the return pc for the last
-  // instrumentation frame that's popped.
-  uintptr_t PopFramesForDeoptimization(Thread* self, uintptr_t stack_pointer) const
+  // Pops instrumentation frames until the specified stack_pointer from the current thread. Returns
+  // the return pc for the last instrumentation frame that's popped.
+  uintptr_t PopInstrumentationStackUntil(Thread* self, uintptr_t stack_pointer) const
       REQUIRES_SHARED(Locks::mutator_lock_);
 
   // Call back for configure stubs.
@@ -571,6 +572,11 @@
     return alloc_entrypoints_instrumented_;
   }
 
+  bool ProcessMethodUnwindCallbacks(Thread* self,
+                                    std::queue<ArtMethod*>& methods,
+                                    MutableHandle<mirror::Throwable>& exception)
+      REQUIRES_SHARED(Locks::mutator_lock_);
+
   InstrumentationLevel GetCurrentInstrumentationLevel() const;
 
  private:
diff --git a/runtime/quick_exception_handler.cc b/runtime/quick_exception_handler.cc
index 2a6929a..f8bfb5a 100644
--- a/runtime/quick_exception_handler.cc
+++ b/runtime/quick_exception_handler.cc
@@ -16,6 +16,7 @@
 
 #include "quick_exception_handler.h"
 #include <ios>
+#include <queue>
 
 #include "arch/context.h"
 #include "art_method-inl.h"
@@ -95,7 +96,19 @@
       DCHECK(method->IsCalleeSaveMethod());
       return true;
     }
-    return HandleTryItems(method);
+    bool continue_stack_walk = HandleTryItems(method);
+    // Collect methods for which MethodUnwind callback needs to be invoked. MethodUnwind callback
+    // can potentially throw, so we want to call these after we find the catch block.
+    // We stop the stack walk when we find the catch block. If we are ending the stack walk we don't
+    // have to unwind this method so don't record it.
+    if (continue_stack_walk) {
+      unwound_methods_.push(method);
+    }
+    return continue_stack_walk;
+  }
+
+  std::queue<ArtMethod*>& GetUnwoundMethods() {
+    return unwound_methods_;
   }
 
  private:
@@ -139,6 +152,9 @@
   QuickExceptionHandler* const exception_handler_;
   // The number of frames to skip searching for catches in.
   uint32_t skip_frames_;
+  // The list of methods we would skip to reach the catch block. We record these to call
+  // MethodUnwind callbacks.
+  std::queue<ArtMethod*> unwound_methods_;
 
   DISALLOW_COPY_AND_ASSIGN(CatchBlockStackVisitor);
 };
@@ -147,7 +163,7 @@
 // Note that this might change the exception being thrown.
 void QuickExceptionHandler::FindCatch(ObjPtr<mirror::Throwable> exception) {
   DCHECK(!is_deoptimization_);
-  instrumentation::InstrumentationStackPopper popper(self_);
+  instrumentation::Instrumentation* instr = Runtime::Current()->GetInstrumentation();
   // The number of total frames we have so far popped.
   uint32_t already_popped = 0;
   bool popped_to_top = true;
@@ -195,9 +211,13 @@
         handler_method_header_->IsOptimized()) {
       SetCatchEnvironmentForOptimizedHandler(&visitor);
     }
-    popped_to_top =
-        popper.PopFramesTo(reinterpret_cast<uintptr_t>(handler_quick_frame_), exception_ref);
+    popped_to_top = instr->ProcessMethodUnwindCallbacks(self_,
+                                                        visitor.GetUnwoundMethods(),
+                                                        exception_ref);
   } while (!popped_to_top);
+
+  // Pop off frames on instrumentation stack to keep it in sync with what is on the stack.
+  instr->PopInstrumentationStackUntil(self_, reinterpret_cast<uintptr_t>(handler_quick_frame_));
   if (!clear_exception_) {
     // Put exception back in root set with clear throw location.
     self_->SetException(exception_ref.Get());
@@ -642,7 +662,7 @@
   uintptr_t return_pc = 0;
   if (method_tracing_active_) {
     instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
-    return_pc = instrumentation->PopFramesForDeoptimization(
+    return_pc = instrumentation->PopInstrumentationStackUntil(
         self_, reinterpret_cast<uintptr_t>(handler_quick_frame_));
   }
   return return_pc;