Revert "Reland "Update instrumentation support for non-java debuggabl…

…e runtimes"" This reverts commit 322ef18. Reason for revert: Failures on art-tracing: https://android-build.googleplex.com/builds/submitted/9062353/art-tracing/latest/view/logs/build_error.log Change-Id: I59f39e2833b63be15e7507a53ddb5ff6a30d55f3
FortuneOS-CAF · Sep 14, 2022 · 778800e · 778800e
1 parent f853790
commit 778800e
Show file tree

Hide file tree

Showing 37 changed files with 88 additions and 1,850 deletions.
diff --git a/compiler/jit/jit_compiler.cc b/compiler/jit/jit_compiler.cc
@@ -41,10 +41,6 @@ JitCompiler* JitCompiler::Create() {
   return new JitCompiler();
 }
 
-void JitCompiler::SetDebuggableCompilerOption(bool value) {
-  compiler_options_->SetDebuggable(value);
-}
-
 void JitCompiler::ParseCompilerOptions() {
   // Special case max code units for inlining, whose default is "unset" (implictly
   // meaning no limit). Do this before parsing the actual passed options.

diff --git a/compiler/jit/jit_compiler.h b/compiler/jit/jit_compiler.h
@@ -50,8 +50,6 @@ class JitCompiler : public JitCompilerInterface {
 
   bool IsBaselineCompiler() const override;
 
-  void SetDebuggableCompilerOption(bool val) override;
-
   bool GenerateDebugInfo() override;
 
   void ParseCompilerOptions() override;

diff --git a/openjdkjvmti/OpenjdkJvmTi.cc b/openjdkjvmti/OpenjdkJvmTi.cc
@@ -89,6 +89,12 @@ AllocationManager* gAllocManager;
     }                           \
   } while (false)
 
+// Returns whether we are able to use all jvmti features.
+static bool IsFullJvmtiAvailable() {
+  art::Runtime* runtime = art::Runtime::Current();
+  return runtime->GetInstrumentation()->IsForcedInterpretOnly() || runtime->IsJavaDebuggable();
+}
+
 class JvmtiFunctions {
  private:
   static jvmtiError getEnvironmentError(jvmtiEnv* env) {
@@ -1468,7 +1474,13 @@ extern "C" bool ArtPlugin_Initialize() {
   FieldUtil::Register(gEventHandler);
   BreakpointUtil::Register(gEventHandler);
   Transformer::Register(gEventHandler);
-  gDeoptManager->FinishSetup();
+
+  {
+    // Make sure we can deopt anything we need to.
+    art::ScopedSuspendAll ssa(__FUNCTION__);
+    gDeoptManager->FinishSetup();
+  }
+
   runtime->GetJavaVM()->AddEnvironmentHook(GetEnvHandler);
 
   return true;

diff --git a/openjdkjvmti/art_jvmti.h b/openjdkjvmti/art_jvmti.h
@@ -47,11 +47,9 @@
 #include "base/strlcpy.h"
 #include "base/mutex.h"
 #include "events.h"
-#include "instrumentation.h"
 #include "jni/java_vm_ext.h"
 #include "jni/jni_env_ext.h"
 #include "jvmti.h"
-#include "runtime.h"
 #include "ti_breakpoint.h"
 
 namespace art {
@@ -71,13 +69,6 @@ class ObjectTagTable;
 // This is the value 0x70010200.
 static constexpr jint kArtTiVersion = JVMTI_VERSION_1_2 | 0x40000000;
 
-// Returns whether we are able to use all jvmti features.
-static inline bool IsFullJvmtiAvailable() {
-  art::Runtime* runtime = art::Runtime::Current();
-  return runtime->GetInstrumentation()->IsForcedInterpretOnly() ||
-         runtime->IsJavaDebuggableAtInit();
-}
-
 // A structure that is a jvmtiEnv with additional information for the runtime.
 struct ArtJvmTiEnv : public jvmtiEnv {
   art::JavaVMExt* art_vm;

diff --git a/openjdkjvmti/deopt_manager.cc b/openjdkjvmti/deopt_manager.cc
@@ -47,12 +47,10 @@
 #include "gc/scoped_gc_critical_section.h"
 #include "instrumentation.h"
 #include "jit/jit.h"
-#include "jit/jit_code_cache.h"
 #include "jni/jni_internal.h"
 #include "mirror/class-inl.h"
 #include "mirror/object_array-inl.h"
 #include "nativehelper/scoped_local_ref.h"
-#include "oat_file_manager.h"
 #include "read_barrier_config.h"
 #include "runtime_callbacks.h"
 #include "scoped_thread_state_change-inl.h"
@@ -63,8 +61,6 @@
 
 namespace openjdkjvmti {
 
-static constexpr const char* kInstrumentationKey = "JVMTI_DeoptRequester";
-
 // We could make this much more selective in the future so we only return true when we
 // actually care about the method at this time (ie active frames had locals changed). For now we
 // just assume that if anything has changed any frame's locals we care about all methods. This only
@@ -95,6 +91,14 @@ void DeoptManager::Setup() {
   callbacks->AddMethodInspectionCallback(&inspection_callback_);
 }
 
+void DeoptManager::Shutdown() {
+  art::ScopedThreadStateChange stsc(art::Thread::Current(),
+                                    art::ThreadState::kWaitingForDebuggerToAttach);
+  art::ScopedSuspendAll ssa("remove method Inspection Callback");
+  art::RuntimeCallbacks* callbacks = art::Runtime::Current()->GetRuntimeCallbacks();
+  callbacks->RemoveMethodInspectionCallback(&inspection_callback_);
+}
+
 void DeoptManager::DumpDeoptInfo(art::Thread* self, std::ostream& stream) {
   art::ScopedObjectAccess soa(self);
   art::MutexLock mutll(self, *art::Locks::thread_list_lock_);
@@ -144,59 +148,48 @@ void DeoptManager::DumpDeoptInfo(art::Thread* self, std::ostream& stream) {
 
 void DeoptManager::FinishSetup() {
   art::Thread* self = art::Thread::Current();
-  art::Runtime* runtime = art::Runtime::Current();
-  if (runtime->IsJavaDebuggable()) {
-    return;
-  }
+  art::MutexLock mu(self, deoptimization_status_lock_);
 
-  // See if we can enable all JVMTI functions.
-  if (PhaseUtil::GetPhaseUnchecked() == JVMTI_PHASE_ONLOAD) {
-    // We are still early enough to change the compiler options and get full JVMTI support.
-    LOG(INFO) << "Openjdkjvmti plugin loaded on a non-debuggable runtime. Changing runtime to "
-              << "debuggable state. Please pass '--debuggable' to dex2oat and "
-              << "'-Xcompiler-option --debuggable' to dalvikvm in the future.";
-    DCHECK(runtime->GetJit() == nullptr) << "Jit should not be running yet!";
-    art::ScopedSuspendAll ssa(__FUNCTION__);
-    // TODO check if we need to hold deoptimization_status_lock_ here.
-    art::MutexLock mu(self, deoptimization_status_lock_);
-    runtime->AddCompilerOption("--debuggable");
-    runtime->SetRuntimeDebugState(art::Runtime::RuntimeDebugState::kJavaDebuggableAtInit);
+  art::Runtime* runtime = art::Runtime::Current();
+  // See if we need to do anything.
+  if (!runtime->IsJavaDebuggable()) {
+    // See if we can enable all JVMTI functions. If this is false, only kArtTiVersion agents can be
+    // retrieved and they will all be best-effort.
+    if (PhaseUtil::GetPhaseUnchecked() == JVMTI_PHASE_ONLOAD) {
+      // We are still early enough to change the compiler options and get full JVMTI support.
+      LOG(INFO) << "Openjdkjvmti plugin loaded on a non-debuggable runtime. Changing runtime to "
+                << "debuggable state. Please pass '--debuggable' to dex2oat and "
+                << "'-Xcompiler-option --debuggable' to dalvikvm in the future.";
+      DCHECK(runtime->GetJit() == nullptr) << "Jit should not be running yet!";
+      runtime->AddCompilerOption("--debuggable");
+      runtime->SetJavaDebuggable(true);
+    } else {
+      LOG(WARNING) << "Openjdkjvmti plugin was loaded on a non-debuggable Runtime. Plugin was "
+                   << "loaded too late to change runtime state to DEBUGGABLE. Only kArtTiVersion "
+                   << "(0x" << std::hex << kArtTiVersion << ") environments are available. Some "
+                   << "functionality might not work properly.";
+      if (runtime->GetJit() == nullptr &&
+          runtime->GetJITOptions()->UseJitCompilation() &&
+          !runtime->GetInstrumentation()->IsForcedInterpretOnly()) {
+        // If we don't have a jit we should try to start the jit for performance reasons. We only
+        // need to do this for late attach on non-debuggable processes because for debuggable
+        // processes we already rely on jit and we cannot force this jit to start if we are still in
+        // OnLoad since the runtime hasn't started up sufficiently. This is only expected to happen
+        // on userdebug/eng builds.
+        LOG(INFO) << "Attempting to start jit for openjdkjvmti plugin.";
+        // Note: use rwx allowed = true, because if this is the system server, we will not be
+        //       allowed to allocate any JIT code cache, anyways.
+        runtime->CreateJitCodeCache(/*rwx_memory_allowed=*/true);
+        runtime->CreateJit();
+        if (runtime->GetJit() == nullptr) {
+          LOG(WARNING) << "Could not start jit for openjdkjvmti plugin. This process might be "
+                       << "quite slow as it is running entirely in the interpreter. Try running "
+                       << "'setenforce 0' and restarting this process.";
+        }
+      }
+    }
     runtime->DeoptimizeBootImage();
-    return;
   }
-
-  // Runtime has already started in non-debuggable mode. Only kArtTiVersion agents can be
-  // retrieved and they will all be best-effort.
-  LOG(WARNING) << "Openjdkjvmti plugin was loaded on a non-debuggable Runtime. Plugin was "
-               << "loaded too late to change runtime state to support all capabilities. Only "
-               << "kArtTiVersion (0x" << std::hex << kArtTiVersion << ") environments are "
-               << "available. Some functionality might not work properly.";
-
-  // Transition the runtime to debuggable:
-  // 1. Wait for any background verification tasks to finish. We don't support
-  // background verification after moving to debuggable state.
-  runtime->GetOatFileManager().WaitForBackgroundVerificationTasksToFinish();
-
-  // Do the transition in ScopedJITSuspend, so we don't start any JIT compilations
-  // before the transition to debuggable is finished.
-  art::jit::ScopedJitSuspend suspend_jit;
-  art::ScopedSuspendAll ssa(__FUNCTION__);
-
-  // 2. Discard any JITed code that was generated before, since they would be
-  // compiled without debug support.
-  art::jit::Jit* jit = runtime->GetJit();
-  if (jit != nullptr) {
-    jit->GetCodeCache()->InvalidateAllCompiledCode();
-    jit->GetCodeCache()->TransitionToDebuggable();
-    jit->GetJitCompiler()->SetDebuggableCompilerOption(true);
-  }
-
-  // 3. Change the state to JavaDebuggable, so that debug features can be
-  // enabled from now on.
-  runtime->SetRuntimeDebugState(art::Runtime::RuntimeDebugState::kJavaDebuggable);
-
-  // 4. Update all entrypoints to avoid using any AOT code.
-  runtime->GetInstrumentation()->UpdateEntrypointsForDebuggable();
 }
 
 bool DeoptManager::MethodHasBreakpoints(art::ArtMethod* method) {
@@ -369,39 +362,6 @@ void DeoptManager::AddDeoptimizeAllMethodsLocked(art::Thread* self) {
   }
 }
 
-void DeoptManager::Shutdown() {
-  art::Thread* self = art::Thread::Current();
-  art::Runtime* runtime = art::Runtime::Current();
-
-  // Do the transition in ScopedJITSuspend, so we don't start any JIT compilations
-  // before the transition to debuggable is finished.
-  art::jit::ScopedJitSuspend suspend_jit;
-
-  art::ScopedThreadStateChange sts(self, art::ThreadState::kSuspended);
-  deoptimization_status_lock_.ExclusiveLock(self);
-  ScopedDeoptimizationContext sdc(self, this);
-
-  art::jit::Jit* jit = runtime->GetJit();
-  if (jit != nullptr && !runtime->IsShuttingDown(self)) {
-    jit->GetCodeCache()->InvalidateAllCompiledCode();
-    jit->GetCodeCache()->TransitionToDebuggable();
-    jit->GetJitCompiler()->SetDebuggableCompilerOption(false);
-  }
-
-  art::RuntimeCallbacks* callbacks = runtime->GetRuntimeCallbacks();
-  callbacks->RemoveMethodInspectionCallback(&inspection_callback_);
-  if (!runtime->IsJavaDebuggableAtInit()) {
-    runtime->SetRuntimeDebugState(art::Runtime::RuntimeDebugState::kNonJavaDebuggable);
-  }
-  // TODO(mythria): DeoptManager should use only one key. Merge
-  // kInstrumentationKey and kDeoptManagerInstrumentationKey.
-  if (!runtime->IsShuttingDown(self)) {
-    art::Runtime::Current()->GetInstrumentation()->DisableDeoptimization(kInstrumentationKey);
-    art::Runtime::Current()->GetInstrumentation()->DisableDeoptimization(
-        kDeoptManagerInstrumentationKey);
-  }
-}
-
 void DeoptManager::RemoveDeoptimizeAllMethodsLocked(art::Thread* self) {
   DCHECK_GT(global_deopt_count_, 0u) << "Request to remove non-existent global deoptimization!";
   global_deopt_count_--;
@@ -475,6 +435,7 @@ jvmtiError DeoptManager::RemoveDeoptimizeThreadMethods(art::ScopedObjectAccessUn
   return OK;
 }
 
+static constexpr const char* kInstrumentationKey = "JVMTI_DeoptRequester";
 
 void DeoptManager::RemoveDeoptimizationRequester() {
   art::Thread* self = art::Thread::Current();

diff --git a/openjdkjvmti/deopt_manager.h b/openjdkjvmti/deopt_manager.h
@@ -110,7 +110,9 @@ class DeoptManager {
       REQUIRES_SHARED(art::Locks::mutator_lock_);
   void DeoptimizeAllThreads() REQUIRES_SHARED(art::Locks::mutator_lock_);
 
-  void FinishSetup() REQUIRES(!deoptimization_status_lock_, !art::Roles::uninterruptible_);
+  void FinishSetup()
+      REQUIRES(!deoptimization_status_lock_, !art::Roles::uninterruptible_)
+      REQUIRES(art::Locks::mutator_lock_);
 
   static DeoptManager* Get();
 

diff --git a/openjdkjvmti/ti_extension.cc b/openjdkjvmti/ti_extension.cc
@@ -398,7 +398,8 @@ jvmtiError ExtensionUtil::GetExtensionFunctions(jvmtiEnv* env,
 
   // These require index-ids and debuggable to function
   art::Runtime* runtime = art::Runtime::Current();
-  if (runtime->GetJniIdType() == art::JniIdType::kIndices && IsFullJvmtiAvailable()) {
+  if (runtime->GetJniIdType() == art::JniIdType::kIndices &&
+      (runtime->GetInstrumentation()->IsForcedInterpretOnly() || runtime->IsJavaDebuggable())) {
     // IsStructurallyModifiableClass
     error = add_extension(
         reinterpret_cast<jvmtiExtensionFunction>(Redefiner::IsStructurallyModifiableClass),
@@ -702,7 +703,8 @@ jvmtiError ExtensionUtil::GetExtensionEvents(jvmtiEnv* env,
     return error;
   }
   art::Runtime* runtime = art::Runtime::Current();
-  if (runtime->GetJniIdType() == art::JniIdType::kIndices && IsFullJvmtiAvailable()) {
+  if (runtime->GetJniIdType() == art::JniIdType::kIndices &&
+      (runtime->GetInstrumentation()->IsForcedInterpretOnly() || runtime->IsJavaDebuggable())) {
     error = add_extension(
         ArtJvmtiEvent::kStructuralDexFileLoadHook,
         "com.android.art.class.structural_dex_file_load_hook",

diff --git a/runtime/hidden_api.h b/runtime/hidden_api.h
@@ -123,7 +123,7 @@ class AccessContext {
 
     if (domain == Domain::kApplication &&
         klass->ShouldSkipHiddenApiChecks() &&
-        Runtime::Current()->IsJavaDebuggableAtInit()) {
+        Runtime::Current()->IsJavaDebuggable()) {
       // Class is known, it is marked trusted and we are in debuggable mode.
       domain = ComputeDomain(/* is_trusted= */ true);
     }

diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
@@ -170,6 +170,7 @@ bool Instrumentation::ProcessMethodUnwindCallbacks(Thread* self,
   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
@@ -449,14 +450,6 @@ void Instrumentation::InstallStubsForMethod(ArtMethod* method) {
   UpdateEntryPoints(method, GetOptimizedCodeFor(method));
 }
 
-void Instrumentation::UpdateEntrypointsForDebuggable() {
-  Runtime* runtime = Runtime::Current();
-  // If we are transitioning from non-debuggable to debuggable, we patch
-  // entry points of methods to remove any aot / JITed entry points.
-  InstallStubsClassVisitor visitor(this);
-  runtime->GetClassLinker()->VisitClasses(&visitor);
-}
-
 // Places the instrumentation exit pc as the return PC for every quick frame. This also allows
 // deoptimization of quick frames to interpreter frames. When force_deopt is
 // true the frames have to be deoptimized. If the frame has a deoptimization
@@ -1005,15 +998,17 @@ void Instrumentation::UpdateStubs() {
   Locks::mutator_lock_->AssertExclusiveHeld(self);
   Locks::thread_list_lock_->AssertNotHeld(self);
   UpdateInstrumentationLevel(requested_level);
-  InstallStubsClassVisitor visitor(this);
-  runtime->GetClassLinker()->VisitClasses(&visitor);
   if (requested_level > InstrumentationLevel::kInstrumentNothing) {
+    InstallStubsClassVisitor visitor(this);
+    runtime->GetClassLinker()->VisitClasses(&visitor);
     instrumentation_stubs_installed_ = true;
     MutexLock mu(self, *Locks::thread_list_lock_);
     for (Thread* thread : Runtime::Current()->GetThreadList()->GetList()) {
       InstrumentThreadStack(thread, /* deopt_all_frames= */ false);
     }
   } else {
+    InstallStubsClassVisitor visitor(this);
+    runtime->GetClassLinker()->VisitClasses(&visitor);
     MaybeRestoreInstrumentationStack();
   }
 }
@@ -1228,14 +1223,6 @@ void Instrumentation::Undeoptimize(ArtMethod* method) {
     return;
   }
 
-  if (method->IsObsolete()) {
-    // Don't update entry points for obsolete methods. The entrypoint should
-    // have been set to InvokeObsoleteMethoStub.
-    DCHECK_EQ(method->GetEntryPointFromQuickCompiledCodePtrSize(kRuntimePointerSize),
-              GetInvokeObsoleteMethodStub());
-    return;
-  }
-
   // We are not using interpreter stubs for deoptimization. Restore the code of the method.
   // We still retain interpreter bridge if we need it for other reasons.
   if (InterpretOnly(method)) {

diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
@@ -550,8 +550,6 @@ class Instrumentation {
 
   void InstallStubsForMethod(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_);
 
-  void UpdateEntrypointsForDebuggable() REQUIRES(art::Locks::mutator_lock_);
-
   // Install instrumentation exit stub on every method of the stack of the given thread.
   // This is used by:
   //  - the debugger to cause a deoptimization of the all frames in thread's stack (for

diff --git a/runtime/jit/jit.h b/runtime/jit/jit.h
@@ -196,7 +196,6 @@ class JitCompilerInterface {
   virtual bool GenerateDebugInfo() = 0;
   virtual void ParseCompilerOptions() = 0;
   virtual bool IsBaselineCompiler() const = 0;
-  virtual void SetDebuggableCompilerOption(bool value) = 0;
 
   virtual std::vector<uint8_t> PackElfFileForJIT(ArrayRef<const JITCodeEntry*> elf_files,
                                                  ArrayRef<const void*> removed_symbols,

diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
@@ -1763,18 +1763,6 @@ void JitCodeCache::InvalidateAllCompiledCode() {
       Runtime::Current()->GetInstrumentation()->InitializeMethodsCode(meth, /*aot_code=*/ nullptr);
     }
   }
-
-  for (auto it : zygote_map_) {
-    if (it.method == nullptr) {
-      continue;
-    }
-    if (it.method->IsPreCompiled()) {
-      it.method->ClearPreCompiled();
-    }
-    Runtime::Current()->GetInstrumentation()->InitializeMethodsCode(it.method,
-                                                                    /*aot_code=*/nullptr);
-  }
-
   saved_compiled_methods_map_.clear();
   osr_code_map_.clear();
 }