Method entry / exit listeners don't need to force interpreter

Method entry / exit listeners used to install interpreter stubs because
of some inconsistencies with the way frame pop was working with
instrumentation stubs. We no longer use instrumentation stubs and hence
we can support method entry / exit listeners without forcing
interpreter.

The only additional change is to deopt when an exception is thrown if
method exit listeners are installed and non-standard exits are enabled.
We could potentially fix this too by checking if a deopt is needed after
each method unwind callback but it may not be worth the complexity
added.

Bug: 206029744
Test: art/test.py

Change-Id: I437d2aebc764182088826eadc9174a3b5f26215f

2 files changed, 10 insertions, 6 deletions

diff --git a/openjdkjvmti/events.cc b/openjdkjvmti/events.cc
index 55fe9efa77..8f54003156 100644
--- a/openjdkjvmti/events.cc
+++ b/openjdkjvmti/events.cc
@@ -1130,13 +1130,11 @@ static DeoptRequirement GetDeoptRequirement(ArtJvmtiEvent event, jthread thread)
   switch (event) {
     case ArtJvmtiEvent::kBreakpoint:
     case ArtJvmtiEvent::kException:
-      return DeoptRequirement::kLimited;
-    // TODO MethodEntry is needed due to inconsistencies between the interpreter and the trampoline
-    // in how to handle exceptions.
     case ArtJvmtiEvent::kMethodEntry:
+    case ArtJvmtiEvent::kMethodExit:
+      return DeoptRequirement::kLimited;
     case ArtJvmtiEvent::kExceptionCatch:
       return DeoptRequirement::kFull;
-    case ArtJvmtiEvent::kMethodExit:
     case ArtJvmtiEvent::kFieldModification:
     case ArtJvmtiEvent::kFieldAccess:
     case ArtJvmtiEvent::kSingleStep:
diff --git a/runtime/thread.cc b/runtime/thread.cc
index ff0a020b0c..ab4069c78e 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -3911,8 +3911,14 @@ void Thread::QuickDeliverException(bool skip_method_exit_callbacks) {
   // Note: we do this *after* reporting the exception to instrumentation in case it now requires
   // deoptimization. It may happen if a debugger is attached and requests new events (single-step,
   // breakpoint, ...) when the exception is reported.
-  //
-    if (Dbg::IsForcedInterpreterNeededForException(this) || IsForceInterpreter()) {
+  // Frame pop can be requested on a method unwind callback which requires a deopt. We could
+  // potentially check after each unwind callback to see if a frame pop was requested and deopt if
+  // needed. Since this is a debug only feature and this path is only taken when an exception is
+  // thrown, it is not performance critical and we keep it simple by just deopting if method exit
+  // listeners are installed and frame pop feature is supported.
+  bool needs_deopt =
+      instrumentation->HasMethodExitListeners() && Runtime::Current()->AreNonStandardExitsEnabled();
+  if (Dbg::IsForcedInterpreterNeededForException(this) || IsForceInterpreter() || needs_deopt) {
     NthCallerVisitor visitor(this, 0, false);
     visitor.WalkStack();
     if (visitor.GetCurrentQuickFrame() != nullptr) {