Revert "Don't use instrumentation stubs for native methods in debuggable"

This reverts commit 90f12677f80169dc3ef919c2067349f94b943e7f.

Reason for revert: Failures on device
https://ci.chromium.org/ui/p/art/builders/ci/angler-armv7-ndebug/3058/overview
https://ci.chromium.org/ui/p/art/builders/ci/angler-armv8-ndebug/3049/overview

Change-Id: I43f943f9180b8c76db02a2a5c228a209a2f18a82

16 files changed, 36 insertions, 115 deletions

diff --git a/runtime/arch/arm/jni_entrypoints_arm.S b/runtime/arch/arm/jni_entrypoints_arm.S
index d91882c95d..7270d2002a 100644
--- a/runtime/arch/arm/jni_entrypoints_arm.S
+++ b/runtime/arch/arm/jni_entrypoints_arm.S
@@ -327,11 +327,6 @@ JNI_SAVE_MANAGED_ARGS_TRAMPOLINE art_jni_read_barrier, artJniReadBarrier
 JNI_SAVE_MANAGED_ARGS_TRAMPOLINE art_jni_method_start, artJniMethodStart, rSELF
 
     /*
-     * Trampoline to `artJniMethodEntryHook()` that preserves all managed arguments.
-     */
-JNI_SAVE_MANAGED_ARGS_TRAMPOLINE art_jni_method_entry_hook, artJniMethodEntryHook, rSELF
-
-    /*
      * Trampoline to `artJniMonitoredMethodStart()` that preserves all managed arguments.
      */
 JNI_SAVE_MANAGED_ARGS_TRAMPOLINE art_jni_monitored_method_start, artJniMonitoredMethodStart, rSELF
diff --git a/runtime/arch/arm64/jni_entrypoints_arm64.S b/runtime/arch/arm64/jni_entrypoints_arm64.S
index 9612a7b54f..b3ea40d931 100644
--- a/runtime/arch/arm64/jni_entrypoints_arm64.S
+++ b/runtime/arch/arm64/jni_entrypoints_arm64.S
@@ -366,11 +366,6 @@ JNI_SAVE_MANAGED_ARGS_TRAMPOLINE art_jni_read_barrier, artJniReadBarrier
 JNI_SAVE_MANAGED_ARGS_TRAMPOLINE art_jni_method_start, artJniMethodStart, xSELF
 
     /*
-     * Trampoline to `artJniMethodEntryHook` that preserves all managed arguments.
-     */
-JNI_SAVE_MANAGED_ARGS_TRAMPOLINE art_jni_method_entry_hook, artJniMethodEntryHook, xSELF
-
-    /*
      * Trampoline to `artJniMonitoredMethodStart()` that preserves all managed arguments.
      */
 JNI_SAVE_MANAGED_ARGS_TRAMPOLINE art_jni_monitored_method_start, artJniMonitoredMethodStart, xSELF
diff --git a/runtime/arch/x86/jni_entrypoints_x86.S b/runtime/arch/x86/jni_entrypoints_x86.S
index c7cf856e60..4b43814d66 100644
--- a/runtime/arch/x86/jni_entrypoints_x86.S
+++ b/runtime/arch/x86/jni_entrypoints_x86.S
@@ -286,12 +286,6 @@ JNI_SAVE_MANAGED_ARGS_TRAMPOLINE art_jni_read_barrier, artJniReadBarrier, eax
 JNI_SAVE_MANAGED_ARGS_TRAMPOLINE art_jni_method_start, artJniMethodStart, fs:THREAD_SELF_OFFSET
 
     /*
-     * Trampoline to `artJniMethodEntryHook` that preserves all managed arguments.
-     */
-JNI_SAVE_MANAGED_ARGS_TRAMPOLINE \
-    art_jni_method_entry_hook, artJniMethodEntryHook, fs:THREAD_SELF_OFFSET
-
-    /*
      * Trampoline to `artJniMonitoredMethodStart()` that preserves all managed arguments.
      */
 JNI_SAVE_MANAGED_ARGS_TRAMPOLINE \
diff --git a/runtime/arch/x86_64/jni_entrypoints_x86_64.S b/runtime/arch/x86_64/jni_entrypoints_x86_64.S
index 55f01b78fa..d2f1fe1dbd 100644
--- a/runtime/arch/x86_64/jni_entrypoints_x86_64.S
+++ b/runtime/arch/x86_64/jni_entrypoints_x86_64.S
@@ -400,12 +400,6 @@ JNI_SAVE_MANAGED_ARGS_TRAMPOLINE art_jni_read_barrier, artJniReadBarrier, none
 JNI_SAVE_MANAGED_ARGS_TRAMPOLINE art_jni_method_start, artJniMethodStart, gs:THREAD_SELF_OFFSET
 
     /*
-     * Trampoline to `artJniMethodEntryHook` that preserves all managed arguments.
-     */
-JNI_SAVE_MANAGED_ARGS_TRAMPOLINE \
-    art_jni_method_entry_hook, artJniMethodEntryHook, gs:THREAD_SELF_OFFSET
-
-    /*
      * Trampoline to `artJniMonitoredMethodStart()` that preserves all managed arguments.
      */
 JNI_SAVE_MANAGED_ARGS_TRAMPOLINE \
diff --git a/runtime/entrypoints/quick/quick_default_externs.h b/runtime/entrypoints/quick/quick_default_externs.h
index cb3caac9ab..f8856d82b9 100644
--- a/runtime/entrypoints/quick/quick_default_externs.h
+++ b/runtime/entrypoints/quick/quick_default_externs.h
@@ -122,7 +122,6 @@ extern "C" void art_jni_method_start();
 extern "C" void art_jni_monitored_method_start();
 extern "C" void art_jni_method_end();
 extern "C" void art_jni_monitored_method_end();
-extern "C" void art_jni_method_entry_hook();
 
 // JNI lock/unlock entrypoints. Note: Custom calling convention.
 extern "C" void art_jni_lock_object(art::mirror::Object*);
diff --git a/runtime/entrypoints/quick/quick_default_init_entrypoints.h b/runtime/entrypoints/quick/quick_default_init_entrypoints.h
index ea077889ee..939feeebcc 100644
--- a/runtime/entrypoints/quick/quick_default_init_entrypoints.h
+++ b/runtime/entrypoints/quick/quick_default_init_entrypoints.h
@@ -79,7 +79,6 @@ static void DefaultInitEntryPoints(JniEntryPoints* jpoints,
   qpoints->SetQuickGenericJniTrampoline(art_quick_generic_jni_trampoline);
   qpoints->SetJniDecodeReferenceResult(JniDecodeReferenceResult);
   qpoints->SetJniReadBarrier(art_jni_read_barrier);
-  qpoints->SetJniMethodEntryHook(art_jni_method_entry_hook);
 
   // Locks
   if (UNLIKELY(VLOG_IS_ON(systrace_lock_logging))) {
diff --git a/runtime/entrypoints/quick/quick_entrypoints.h b/runtime/entrypoints/quick/quick_entrypoints.h
index 0e73c63828..7af1a0b14e 100644
--- a/runtime/entrypoints/quick/quick_entrypoints.h
+++ b/runtime/entrypoints/quick/quick_entrypoints.h
@@ -67,7 +67,6 @@ extern "C" void artJniUnlockObject(mirror::Object* locked, Thread* self)
 // JNI entrypoints when monitoring entry/exit.
 extern "C" void artJniMonitoredMethodStart(Thread* self) UNLOCK_FUNCTION(Locks::mutator_lock_);
 extern "C" void artJniMonitoredMethodEnd(Thread* self) SHARED_LOCK_FUNCTION(Locks::mutator_lock_);
-extern "C" void artJniMethodEntryHook(Thread* self);
 
 // StringAppend pattern entrypoint.
 extern "C" mirror::String* artStringBuilderAppend(uint32_t format,
diff --git a/runtime/entrypoints/quick/quick_entrypoints_list.h b/runtime/entrypoints/quick/quick_entrypoints_list.h
index 4534bba8ef..dffaa4bb25 100644
--- a/runtime/entrypoints/quick/quick_entrypoints_list.h
+++ b/runtime/entrypoints/quick/quick_entrypoints_list.h
@@ -78,7 +78,6 @@
   V(JniLockObject, void, mirror::Object*) \
   V(JniUnlockObject, void, mirror::Object*) \
   V(QuickGenericJniTrampoline, void, ArtMethod*) \
-  V(JniMethodEntryHook, void) \
 \
   V(LockObject, void, mirror::Object*) \
   V(UnlockObject, void, mirror::Object*) \
diff --git a/runtime/entrypoints/quick/quick_jni_entrypoints.cc b/runtime/entrypoints/quick/quick_jni_entrypoints.cc
index fb5dd7c5b5..ab13bd95b1 100644
--- a/runtime/entrypoints/quick/quick_jni_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_jni_entrypoints.cc
@@ -38,11 +38,6 @@
 
 namespace art {
 
-extern "C" int artMethodExitHook(Thread* self,
-                                 ArtMethod* method,
-                                 uint64_t* gpr_result,
-                                 uint64_t* fpr_result);
-
 static_assert(sizeof(IRTSegmentState) == sizeof(uint32_t), "IRTSegmentState size unexpected");
 static_assert(std::is_trivial<IRTSegmentState>::value, "IRTSegmentState not trivial");
 
@@ -179,11 +174,11 @@ extern uint64_t GenericJniMethodEnd(Thread* self,
     artJniUnlockObject(lock.Ptr(), self);
   }
   char return_shorty_char = called->GetShorty()[0];
-  uint64_t ret;
   if (return_shorty_char == 'L') {
-    ret = reinterpret_cast<uint64_t>(
+    uint64_t ret = reinterpret_cast<uint64_t>(
         UNLIKELY(self->IsExceptionPending()) ? nullptr : JniDecodeReferenceResult(result.l, self));
     PopLocalReferences(saved_local_ref_cookie, self);
+    return ret;
   } else {
     if (LIKELY(!critical_native)) {
       PopLocalReferences(saved_local_ref_cookie, self);
@@ -193,54 +188,32 @@ extern uint64_t GenericJniMethodEnd(Thread* self,
         if (kRuntimeISA == InstructionSet::kX86) {
           // Convert back the result to float.
           double d = bit_cast<double, uint64_t>(result_f);
-          ret = bit_cast<uint32_t, float>(static_cast<float>(d));
+          return bit_cast<uint32_t, float>(static_cast<float>(d));
         } else {
-          ret = result_f;
+          return result_f;
         }
       }
-      break;
       case 'D':
-        ret = result_f;
-        break;
+        return result_f;
       case 'Z':
-        ret = result.z;
-        break;
+        return result.z;
       case 'B':
-        ret = result.b;
-        break;
+        return result.b;
       case 'C':
-        ret = result.c;
-        break;
+        return result.c;
       case 'S':
-        ret = result.s;
-        break;
+        return result.s;
       case 'I':
-        ret = result.i;
-        break;
+        return result.i;
       case 'J':
-        ret = result.j;
-        break;
+        return result.j;
       case 'V':
-        ret = 0;
-        break;
+        return 0;
       default:
         LOG(FATAL) << "Unexpected return shorty character " << return_shorty_char;
         UNREACHABLE();
     }
   }
-
-  instrumentation::Instrumentation* instr = Runtime::Current()->GetInstrumentation();
-  // @CriticalNative methods don't do a suspend check so there is no need to check for a
-  // deoptimization here but we need method exit hooks for processing method exit callbacks.
-  // Don't call method exit hooks when there is a pending exception. Method exit hooks are expected
-  // to be called only on regular exits. When there is an exception method unwind events are called.
-  // Exceptions are handled later in art_quick_generic_jni_trampoline so just return here.
-  if (UNLIKELY(instr->AreExitStubsInstalled() &&
-               !self->IsExceptionPending() &&
-               Runtime::Current()->IsJavaDebuggable())) {
-    artMethodExitHook(self, called, &ret, &ret);
-  }
-  return ret;
 }
 
 extern "C" void artJniMonitoredMethodStart(Thread* self) {
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index ba43dbd3c4..1c934607fe 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -2110,14 +2110,6 @@ extern "C" const void* artQuickGenericJniTrampoline(Thread* self,
     }
   }
 
-  instrumentation::Instrumentation* instr = Runtime::Current()->GetInstrumentation();
-  if (UNLIKELY(instr->AreExitStubsInstalled() && Runtime::Current()->IsJavaDebuggable())) {
-    instr->MethodEnterEvent(self, called);
-    if (self->IsExceptionPending()) {
-      return nullptr;
-    }
-  }
-
   // Skip calling `artJniMethodStart()` for @CriticalNative and @FastNative.
   if (LIKELY(normal_native)) {
     // Start JNI.
@@ -2659,13 +2651,6 @@ extern "C" uint64_t artInvokeCustom(uint32_t call_site_idx, Thread* self, ArtMet
   return result.GetJ();
 }
 
-extern "C" void artJniMethodEntryHook(Thread* self)
-    REQUIRES_SHARED(Locks::mutator_lock_) {
-  instrumentation::Instrumentation* instr = Runtime::Current()->GetInstrumentation();
-  ArtMethod* method = *self->GetManagedStack()->GetTopQuickFrame();
-  instr->MethodEnterEvent(self, method);
-}
-
 extern "C" void artMethodEntryHook(ArtMethod* method, Thread* self, ArtMethod** sp ATTRIBUTE_UNUSED)
     REQUIRES_SHARED(Locks::mutator_lock_) {
   instrumentation::Instrumentation* instr = Runtime::Current()->GetInstrumentation();
@@ -2711,9 +2696,8 @@ extern "C" void artMethodExitHook(Thread* self,
     deoptimize = instr->ShouldDeoptimizeCaller(self, visitor);
 
     // If we need a deoptimization MethodExitEvent will be called by the interpreter when it
-    // re-executes the return instruction. For native methods we have to process method exit
-    // events here since deoptimization just removes the native frame.
-    if (!deoptimize || method->IsNative()) {
+    // re-executes the return instruction.
+    if (!deoptimize) {
       instr->MethodExitEvent(self,
                              method,
                              /* frame= */ {},
@@ -2736,11 +2720,7 @@ extern "C" void artMethodExitHook(Thread* self,
 
   if (deoptimize) {
     DeoptimizationMethodType deopt_method_type = instr->GetDeoptimizationMethodType(method);
-    self->PushDeoptimizationContext(return_value,
-                                    is_ref,
-                                    self->GetException(),
-                                    false,
-                                    deopt_method_type);
+    self->PushDeoptimizationContext(return_value, is_ref, nullptr, false, deopt_method_type);
     artDeoptimize(self);
     UNREACHABLE();
   }
diff --git a/runtime/entrypoints_order_test.cc b/runtime/entrypoints_order_test.cc
index 2cd58dbf1b..240ecbd216 100644
--- a/runtime/entrypoints_order_test.cc
+++ b/runtime/entrypoints_order_test.cc
@@ -225,9 +225,7 @@ class EntrypointsOrderTest : public CommonRuntimeTest {
                          pJniUnlockObject, sizeof(void*));
     EXPECT_OFFSET_DIFFNP(QuickEntryPoints, pJniUnlockObject,
                          pQuickGenericJniTrampoline, sizeof(void*));
-    EXPECT_OFFSET_DIFFNP(QuickEntryPoints, pQuickGenericJniTrampoline,
-                         pJniMethodEntryHook, sizeof(void*));
-    EXPECT_OFFSET_DIFFNP(QuickEntryPoints, pJniMethodEntryHook, pLockObject, sizeof(void*));
+    EXPECT_OFFSET_DIFFNP(QuickEntryPoints, pQuickGenericJniTrampoline, pLockObject, sizeof(void*));
     EXPECT_OFFSET_DIFFNP(QuickEntryPoints, pLockObject, pUnlockObject, sizeof(void*));
     EXPECT_OFFSET_DIFFNP(QuickEntryPoints, pUnlockObject, pCmpgDouble, sizeof(void*));
     EXPECT_OFFSET_DIFFNP(QuickEntryPoints, pCmpgDouble, pCmpgFloat, sizeof(void*));
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 5f8b6daf2f..3b17bb5ee4 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -230,19 +230,20 @@ static bool CodeNeedsEntryExitStub(const void* code, ArtMethod* method)
     return false;
   }
 
+  // When jiting code for debuggable apps we generate the code to call method
+  // entry / exit hooks when required. Hence it is not required to update
+  // to instrumentation entry point for JITed code in debuggable mode.
   if (!Runtime::Current()->IsJavaDebuggable()) {
     return true;
   }
 
-  // Native methods don't need method entry / exit hooks in debuggable runtimes.
-  // GenericJni trampoline and JITed JNI stubs handle entry / exit hooks
+  // Native functions can have JITed entry points but we don't include support
+  // for calling entry / exit hooks directly from the JITed code for native
+  // functions. So we still have to install entry exit stubs for such cases.
   if (method->IsNative()) {
-    return false;
+    return true;
   }
 
-  // When jiting code for debuggable apps we generate the code to call method
-  // entry / exit hooks when required. Hence it is not required to update
-  // to instrumentation entry point for JITed code in debuggable mode.
   jit::Jit* jit = Runtime::Current()->GetJit();
   if (jit != nullptr && jit->GetCodeCache()->ContainsPc(code)) {
     return false;
@@ -507,11 +508,6 @@ void InstrumentationInstallStack(Thread* thread, void* arg, bool deopt_all_frame
           LOG(INFO) << "Ignoring already instrumented " << frame.Dump();
         }
       } else {
-        if (m->IsNative() && Runtime::Current()->IsJavaDebuggable()) {
-          // Native methods in debuggable runtimes don't use instrumentation stubs.
-          return true;
-        }
-
         // If it is a JITed frame then just set the deopt bit if required
         // otherwise continue
         const OatQuickMethodHeader* method_header = GetCurrentOatQuickMethodHeader();
diff --git a/runtime/oat.h b/runtime/oat.h
index 341e70bbe9..14b389d670 100644
--- a/runtime/oat.h
+++ b/runtime/oat.h
@@ -32,8 +32,8 @@ class InstructionSetFeatures;
 class PACKED(4) OatHeader {
  public:
   static constexpr std::array<uint8_t, 4> kOatMagic { { 'o', 'a', 't', '\n' } };
-  // Last oat version changed reason: Don't use instrumentation stubs for native methods.
-  static constexpr std::array<uint8_t, 4> kOatVersion { { '2', '2', '7', '\0' } };
+  // Last oat version changed reason: Update deoptimization from runtime methods.
+  static constexpr std::array<uint8_t, 4> kOatVersion { { '2', '2', '6', '\0' } };
 
   static constexpr const char* kDex2OatCmdLineKey = "dex2oat-cmdline";
   static constexpr const char* kDebuggableKey = "debuggable";
diff --git a/runtime/quick_exception_handler.cc b/runtime/quick_exception_handler.cc
index 40a1c16905..8adc3b3e49 100644
--- a/runtime/quick_exception_handler.cc
+++ b/runtime/quick_exception_handler.cc
@@ -399,10 +399,9 @@ class DeoptimizeStackVisitor final : public StackVisitor {
       return true;
     } else if (method->IsNative()) {
       // If we return from JNI with a pending exception and want to deoptimize, we need to skip
-      // the native method. The top method is a runtime method, the native method comes next.
-      // We also deoptimize due to method instrumentation reasons from method entry / exit
-      // callbacks. In these cases native method is at the top of stack.
-      CHECK((GetFrameDepth() == 1U) || (GetFrameDepth() == 0U));
+      // the native method.
+      // The top method is a runtime method, the native method comes next.
+      CHECK_EQ(GetFrameDepth(), 1U);
       callee_method_ = method;
       return true;
     } else if (!single_frame_deopt_ &&
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 07822f3c2f..1e58216b6c 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -3195,12 +3195,14 @@ void Runtime::DeoptimizeBootImage() {
   // If we've already started and we are setting this runtime to debuggable,
   // we patch entry points of methods in boot image to interpreter bridge, as
   // boot image code may be AOT compiled as not debuggable.
-  UpdateEntryPointsClassVisitor visitor(GetInstrumentation());
-  GetClassLinker()->VisitClasses(&visitor);
-  jit::Jit* jit = GetJit();
-  if (jit != nullptr) {
-    // Code previously compiled may not be compiled debuggable.
-    jit->GetCodeCache()->TransitionToDebuggable();
+  if (!GetInstrumentation()->IsForcedInterpretOnly()) {
+    UpdateEntryPointsClassVisitor visitor(GetInstrumentation());
+    GetClassLinker()->VisitClasses(&visitor);
+    jit::Jit* jit = GetJit();
+    if (jit != nullptr) {
+      // Code previously compiled may not be compiled debuggable.
+      jit->GetCodeCache()->TransitionToDebuggable();
+    }
   }
 }
 
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 94c539f592..0c536090b1 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -3592,7 +3592,6 @@ void Thread::DumpThreadOffset(std::ostream& os, uint32_t offset) {
   QUICK_ENTRY_POINT_INFO(pAputObject)
   QUICK_ENTRY_POINT_INFO(pJniMethodStart)
   QUICK_ENTRY_POINT_INFO(pJniMethodEnd)
-  QUICK_ENTRY_POINT_INFO(pJniMethodEntryHook)
   QUICK_ENTRY_POINT_INFO(pJniDecodeReferenceResult)
   QUICK_ENTRY_POINT_INFO(pJniLockObject)
   QUICK_ENTRY_POINT_INFO(pJniUnlockObject)