Revert "Simplify and document entrypoint toggling in instrumentation."
This reverts commit 76ec6f6562229033b2fecc08c01bef58a9488c92.
Reason for revert: Test failure
Change-Id: Ibb14e7a2830276b4f0a68b7025f9c394041e52d6
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index bd392f6..3488cef 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -1036,21 +1036,24 @@
<< "Proxy method " << method->PrettyMethod()
<< " (declaring class: " << method->GetDeclaringClass()->PrettyClass() << ")"
<< " should not hit instrumentation entrypoint.";
- DCHECK(!instrumentation->IsDeoptimized(method));
- // This will get the entry point either from the oat file, the JIT or the appropriate bridge
- // method if none of those can be found.
- result = instrumentation->GetCodeForInvoke(method);
- jit::Jit* jit = Runtime::Current()->GetJit();
- DCHECK_NE(result, GetQuickInstrumentationEntryPoint()) << method->PrettyMethod();
- DCHECK(jit == nullptr ||
- // Native methods come through here in Interpreter entrypoints. We might not have
- // disabled jit-gc but that is fine since we won't return jit-code for native methods.
- method->IsNative() ||
- !jit->GetCodeCache()->GetGarbageCollectCode());
- DCHECK(!method->IsNative() ||
- jit == nullptr ||
- !jit->GetCodeCache()->ContainsPc(result))
- << method->PrettyMethod() << " code will jump to possibly cleaned up jit code!";
+ if (instrumentation->IsDeoptimized(method)) {
+ result = GetQuickToInterpreterBridge();
+ } else {
+ // This will get the entry point either from the oat file, the JIT or the appropriate bridge
+ // method if none of those can be found.
+ result = instrumentation->GetCodeForInvoke(method);
+ jit::Jit* jit = Runtime::Current()->GetJit();
+ DCHECK_NE(result, GetQuickInstrumentationEntryPoint()) << method->PrettyMethod();
+ DCHECK(jit == nullptr ||
+ // Native methods come through here in Interpreter entrypoints. We might not have
+ // disabled jit-gc but that is fine since we won't return jit-code for native methods.
+ method->IsNative() ||
+ !jit->GetCodeCache()->GetGarbageCollectCode());
+ DCHECK(!method->IsNative() ||
+ jit == nullptr ||
+ !jit->GetCodeCache()->ContainsPc(result))
+ << method->PrettyMethod() << " code will jump to possibly cleaned up jit code!";
+ }
bool interpreter_entry = (result == GetQuickToInterpreterBridge());
bool is_static = method->IsStatic();
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 78a77a3..8ef5ef4 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -47,7 +47,6 @@
#include "mirror/object-inl.h"
#include "mirror/object_array-inl.h"
#include "nth_caller_visitor.h"
-#include "oat_file_manager.h"
#include "oat_quick_method_header.h"
#include "runtime-inl.h"
#include "thread.h"
@@ -217,13 +216,15 @@
method->SetEntryPointFromQuickCompiledCode(quick_code);
}
-bool Instrumentation::InterpretOnly(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) {
- if (method->IsNative()) {
- return false;
- }
- return InterpretOnly() ||
- IsDeoptimized(method) ||
- Runtime::Current()->GetRuntimeCallbacks()->MethodNeedsDebugVersion(method);
+bool Instrumentation::NeedDebugVersionFor(ArtMethod* method) const
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ art::Runtime* runtime = Runtime::Current();
+ // If anything says we need the debug version or we are debuggable we will need the debug version
+ // of the method.
+ return (runtime->GetRuntimeCallbacks()->MethodNeedsDebugVersion(method) ||
+ runtime->IsJavaDebuggable()) &&
+ !method->IsNative() &&
+ !method->IsProxyMethod();
}
bool Instrumentation::CodeNeedsEntryExitStub(const void* code, ArtMethod* method) {
@@ -233,17 +234,6 @@
return true;
}
- // If the methods needs an initialization check, we need to keep the current
- // entrypoint.
- if (!method->GetDeclaringClass()->IsInitialized() && NeedsClinitCheckBeforeCall(method)) {
- return false;
- }
-
- // Code running in the interpreter doesn't need entry/exit stubs.
- if (Runtime::Current()->GetClassLinker()->IsQuickToInterpreterBridge(code)) {
- 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.
@@ -265,39 +255,6 @@
return true;
}
-static bool IsProxyInit(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) {
- // Annoyingly this can be called before we have actually initialized WellKnownClasses so therefore
- // we also need to check this based on the declaring-class descriptor. The check is valid because
- // Proxy only has a single constructor.
- ArtMethod* well_known_proxy_init = jni::DecodeArtMethod(
- WellKnownClasses::java_lang_reflect_Proxy_init);
- if (well_known_proxy_init == method) {
- return true;
- }
-
- if (well_known_proxy_init != nullptr) {
- return false;
- }
-
- return method->IsConstructor() &&
- method->GetDeclaringClass()->DescriptorEquals("Ljava/lang/reflect/Proxy;");
-}
-
-static const void* GetOptimizedCodeFor(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) {
- DCHECK(!Runtime::Current()->GetInstrumentation()->InterpretOnly(method));
- // We need to have the resolution stub still if the class is not initialized.
- if (NeedsClinitCheckBeforeCall(method) && !method->GetDeclaringClass()->IsInitialized()) {
- return GetQuickResolutionStub();
- }
- // TODO We could have JIT compiled native entrypoints. It might be worth it to find these.
- if (!method->IsNative() && Runtime::Current()->IsJavaDebuggable()) {
- // TODO: Return nterp.
- return GetQuickToInterpreterBridge();
- }
- return Runtime::Current()->GetClassLinker()->GetQuickOatCodeFor(method);
-}
-
-
void Instrumentation::InstallStubsForMethod(ArtMethod* method) {
if (!method->IsInvokable() || method->IsProxyMethod()) {
// Do not change stubs for these methods.
@@ -306,28 +263,60 @@
// Don't stub Proxy.<init>. Note that the Proxy class itself is not a proxy class.
// TODO We should remove the need for this since it means we cannot always correctly detect calls
// to Proxy.<init>
- if (IsProxyInit(method)) {
+ // Annoyingly this can be called before we have actually initialized WellKnownClasses so therefore
+ // we also need to check this based on the declaring-class descriptor. The check is valid because
+ // Proxy only has a single constructor.
+ ArtMethod* well_known_proxy_init = jni::DecodeArtMethod(
+ WellKnownClasses::java_lang_reflect_Proxy_init);
+ if ((LIKELY(well_known_proxy_init != nullptr) && UNLIKELY(method == well_known_proxy_init)) ||
+ UNLIKELY(method->IsConstructor() &&
+ method->GetDeclaringClass()->DescriptorEquals("Ljava/lang/reflect/Proxy;"))) {
return;
}
-
- // If the instrumentation needs to go through the interpreter, just update the
- // entrypoint to interpreter.
- if (InterpretOnly(method)) {
- UpdateEntrypoints(method, GetQuickToInterpreterBridge());
- return;
- }
-
- if (EntryExitStubsInstalled()) {
- // Install the instrumentation entry point if needed.
- if (CodeNeedsEntryExitStub(method->GetEntryPointFromQuickCompiledCode(), method)) {
- UpdateEntrypoints(method, GetQuickInstrumentationEntryPoint());
+ const void* new_quick_code;
+ bool uninstall = (instrumentation_level_ == InstrumentationLevel::kInstrumentNothing);
+ Runtime* const runtime = Runtime::Current();
+ ClassLinker* const class_linker = runtime->GetClassLinker();
+ bool is_class_initialized = method->GetDeclaringClass()->IsInitialized();
+ if (uninstall) {
+ if ((forced_interpret_only_ || IsDeoptimized(method)) && !method->IsNative()) {
+ new_quick_code = GetQuickToInterpreterBridge();
+ } else if (is_class_initialized || !method->IsStatic() || method->IsConstructor()) {
+ new_quick_code = GetCodeForInvoke(method);
+ } else {
+ new_quick_code = GetQuickResolutionStub();
}
- return;
+ } else { // !uninstall
+ if ((InterpretOnly() || IsDeoptimized(method)) && !method->IsNative()) {
+ new_quick_code = GetQuickToInterpreterBridge();
+ } else {
+ // Do not overwrite resolution trampoline. When the trampoline initializes the method's
+ // class, all its static methods code will be set to the instrumentation entry point.
+ // For more details, see ClassLinker::FixupStaticTrampolines.
+ if (is_class_initialized || !method->IsStatic() || method->IsConstructor()) {
+ if (EntryExitStubsInstalled()) {
+ // This needs to be checked first since the instrumentation entrypoint will be able to
+ // find the actual JIT compiled code that corresponds to this method.
+ const void* code = method->GetEntryPointFromQuickCompiledCodePtrSize(kRuntimePointerSize);
+ if (CodeNeedsEntryExitStub(code, method)) {
+ new_quick_code = GetQuickInstrumentationEntryPoint();
+ } else {
+ new_quick_code = code;
+ }
+ } else if (NeedDebugVersionFor(method)) {
+ // It would be great to search the JIT for its implementation here but we cannot due to
+ // the locks we hold. Instead just set to the interpreter bridge and that code will search
+ // the JIT when it gets called and replace the entrypoint then.
+ new_quick_code = GetQuickToInterpreterBridge();
+ } else {
+ new_quick_code = class_linker->GetQuickOatCodeFor(method);
+ }
+ } else {
+ new_quick_code = GetQuickResolutionStub();
+ }
+ }
}
-
- // We're being asked to restore the entrypoints after instrumentation.
- CHECK_EQ(instrumentation_level_, InstrumentationLevel::kInstrumentNothing);
- UpdateEntrypoints(method, GetOptimizedCodeFor(method));
+ UpdateEntrypoints(method, new_quick_code);
}
// Places the instrumentation exit pc as the return PC for every quick frame. This also allows
@@ -936,77 +925,44 @@
}
}
-static std::string EntrypointString(const void* code) {
- ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
- jit::Jit* jit = Runtime::Current()->GetJit();
- if (class_linker->IsQuickToInterpreterBridge(code)) {
- return "interpreter";
- } else if (class_linker->IsQuickResolutionStub(code)) {
- return "resolution";
- } else if (code == GetQuickInstrumentationEntryPoint()) {
- return "instrumentation";
- } else if (jit != nullptr && jit->GetCodeCache()->ContainsPc(code)) {
- return "jit";
- } else if (code == GetInvokeObsoleteMethodStub()) {
- return "obsolete";
- } else if (code == interpreter::GetNterpEntryPoint()) {
- return "nterp";
- } else if (class_linker->IsQuickGenericJniStub(code)) {
- return "generic jni";
- } else if (Runtime::Current()->GetOatFileManager().ContainsPc(code)) {
- return "oat";
- }
- return "unknown";
-}
-
void Instrumentation::UpdateMethodsCodeImpl(ArtMethod* method, const void* quick_code) {
- if (!AreExitStubsInstalled()) {
- // Fast path: no instrumentation.
- DCHECK(!EntryExitStubsInstalled());
- DCHECK(!IsDeoptimized(method));
- UpdateEntrypoints(method, quick_code);
- return;
- }
-
- ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
- if (class_linker->IsQuickResolutionStub(quick_code) ||
- class_linker->IsQuickToInterpreterBridge(quick_code)) {
- // It's always OK to update to these entrypoints.
- UpdateEntrypoints(method, quick_code);
- return;
- }
-
- if (IsDeoptimized(method)) {
- DCHECK(class_linker->IsQuickToInterpreterBridge(method->GetEntryPointFromQuickCompiledCode()));
- // Don't update, stay deoptimized.
- return;
- }
-
- if (EntryExitStubsInstalled()) {
- if (CodeNeedsEntryExitStub(quick_code, method)) {
- DCHECK(method->GetEntryPointFromQuickCompiledCode() == GetQuickInstrumentationEntryPoint() ||
- class_linker->IsQuickGenericJniStub(method->GetEntryPointFromQuickCompiledCode()) ||
- class_linker->IsQuickToInterpreterBridge(method->GetEntryPointFromQuickCompiledCode()) ||
- class_linker->IsQuickResolutionStub(method->GetEntryPointFromQuickCompiledCode()))
- << EntrypointString(method->GetEntryPointFromQuickCompiledCode());
- // If the code we want to update the method with still needs entry/exit stub, just skip.
- return;
+ const void* new_quick_code;
+ if (LIKELY(!instrumentation_stubs_installed_)) {
+ new_quick_code = quick_code;
+ } else {
+ if ((InterpreterStubsInstalled() || IsDeoptimized(method)) && !method->IsNative()) {
+ new_quick_code = GetQuickToInterpreterBridge();
+ } else {
+ ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+ if (class_linker->IsQuickResolutionStub(quick_code) ||
+ class_linker->IsQuickToInterpreterBridge(quick_code)) {
+ new_quick_code = quick_code;
+ } else if (EntryExitStubsInstalled() &&
+ // We need to make sure not to replace anything that InstallStubsForMethod
+ // wouldn't. Specifically we cannot stub out Proxy.<init> since subtypes copy the
+ // implementation directly and this will confuse the instrumentation trampolines.
+ // TODO We should remove the need for this since it makes it impossible to profile
+ // Proxy.<init> correctly in all cases.
+ method != jni::DecodeArtMethod(WellKnownClasses::java_lang_reflect_Proxy_init) &&
+ CodeNeedsEntryExitStub(quick_code, method)) {
+ new_quick_code = GetQuickInstrumentationEntryPoint();
+ } else {
+ new_quick_code = quick_code;
+ }
}
}
-
- // At this point, we can update as asked.
- UpdateEntrypoints(method, quick_code);
+ UpdateEntrypoints(method, new_quick_code);
}
void Instrumentation::UpdateNativeMethodsCodeToJitCode(ArtMethod* method, const void* quick_code) {
// We don't do any read barrier on `method`'s declaring class in this code, as the JIT might
// enter here on a soon-to-be deleted ArtMethod. Updating the entrypoint is OK though, as
// the ArtMethod is still in memory.
- if (EntryExitStubsInstalled()) {
- // If stubs are installed don't update.
- return;
+ const void* new_quick_code = quick_code;
+ if (UNLIKELY(instrumentation_stubs_installed_) && EntryExitStubsInstalled()) {
+ new_quick_code = GetQuickInstrumentationEntryPoint();
}
- UpdateEntrypoints(method, quick_code);
+ UpdateEntrypoints(method, new_quick_code);
}
void Instrumentation::UpdateMethodsCode(ArtMethod* method, const void* quick_code) {
@@ -1075,7 +1031,7 @@
<< " is already deoptimized";
}
if (!InterpreterStubsInstalled()) {
- UpdateEntrypoints(method, GetQuickToInterpreterBridge());
+ UpdateEntrypoints(method, GetQuickInstrumentationEntryPoint());
// Install instrumentation exit stub and instrumentation frames. We may already have installed
// these previously so it will only cover the newly created frames.
@@ -1108,10 +1064,15 @@
// Restore code and possibly stack only if we did not deoptimize everything.
if (!InterpreterStubsInstalled()) {
// Restore its code or resolution trampoline.
- if (InterpretOnly(method)) {
- UpdateEntrypoints(method, GetQuickToInterpreterBridge());
+ ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+ if (method->IsStatic() && !method->IsConstructor() &&
+ !method->GetDeclaringClass()->IsInitialized()) {
+ UpdateEntrypoints(method, GetQuickResolutionStub());
} else {
- UpdateEntrypoints(method, GetOptimizedCodeFor(method));
+ const void* quick_code = NeedDebugVersionFor(method)
+ ? GetQuickToInterpreterBridge()
+ : class_linker->GetQuickOatCodeFor(method);
+ UpdateEntrypoints(method, quick_code);
}
// If there is no deoptimized method left, we can restore the stack of each thread.
@@ -1192,26 +1153,46 @@
ConfigureStubs(key, InstrumentationLevel::kInstrumentNothing);
}
-const void* Instrumentation::GetCodeForInvoke(ArtMethod* method) {
+const void* Instrumentation::GetCodeForInvoke(ArtMethod* method) const {
// This is called by instrumentation entry only and that should never be getting proxy methods.
DCHECK(!method->IsProxyMethod()) << method->PrettyMethod();
ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
- const void* code = method->GetEntryPointFromQuickCompiledCodePtrSize(kRuntimePointerSize);
- // If we don't have the instrumentation, the resolution stub, or the
- // interpreter as entrypoint, just return the current entrypoint, assuming
- // it's the most optimized.
- if (code != GetQuickInstrumentationEntryPoint() &&
- !class_linker->IsQuickResolutionStub(code) &&
- !class_linker->IsQuickToInterpreterBridge(code)) {
- return code;
+ if (LIKELY(!InterpreterStubsInstalled())) {
+ // In general we just return whatever the method thinks its entrypoint is here. The only
+ // exception is if it still has the instrumentation entrypoint. That means we are racing another
+ // thread getting rid of instrumentation which is unexpected but possible. In that case we want
+ // to wait and try to get it from the oat file or jit.
+ const void* code = method->GetEntryPointFromQuickCompiledCodePtrSize(kRuntimePointerSize);
+ DCHECK(code != nullptr);
+ if (code != GetQuickInstrumentationEntryPoint()) {
+ return code;
+ }
+ // We don't know what it is. Fallthough to try to find the code from the JIT or Oat file.
}
- if (InterpretOnly(method)) {
- // If we're forced into interpreter just use it.
+ if (method->IsNative()) {
+ // TODO We could have JIT compiled native entrypoints. It might be worth it to find these.
+ return class_linker->GetQuickOatCodeFor(method);
+ } else if (!NeedDebugVersionFor(method) && !InterpreterStubsInstalled()) {
+ return class_linker->GetQuickOatCodeFor(method);
+ } else {
return GetQuickToInterpreterBridge();
}
+}
- return GetOptimizedCodeFor(method);
+const void* Instrumentation::GetQuickCodeFor(ArtMethod* method, PointerSize pointer_size) const {
+ ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+ if (LIKELY(!instrumentation_stubs_installed_)) {
+ const void* code = method->GetEntryPointFromQuickCompiledCodePtrSize(pointer_size);
+ DCHECK(code != nullptr);
+ if (LIKELY(!class_linker->IsQuickResolutionStub(code) &&
+ !class_linker->IsQuickToInterpreterBridge(code)) &&
+ !class_linker->IsQuickResolutionStub(code) &&
+ !class_linker->IsQuickToInterpreterBridge(code)) {
+ return code;
+ }
+ }
+ return class_linker->GetQuickOatCodeFor(method);
}
void Instrumentation::MethodEnterEventImpl(Thread* thread, ArtMethod* method) const {
diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
index 2f0594f..a505ce3 100644
--- a/runtime/instrumentation.h
+++ b/runtime/instrumentation.h
@@ -322,7 +322,14 @@
REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!GetDeoptimizedMethodsLock());
// Return the code that we can execute for an invoke including from the JIT.
- const void* GetCodeForInvoke(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_);
+ const void* GetCodeForInvoke(ArtMethod* method) const
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
+ // Get the quick code for the given method. More efficient than asking the class linker as it
+ // will short-cut to GetCode if instrumentation and static method resolution stubs aren't
+ // installed.
+ const void* GetQuickCodeFor(ArtMethod* method, PointerSize pointer_size) const
+ REQUIRES_SHARED(Locks::mutator_lock_);
void ForceInterpretOnly() {
forced_interpret_only_ = true;
@@ -342,12 +349,15 @@
return forced_interpret_only_ || InterpreterStubsInstalled();
}
- bool InterpretOnly(ArtMethod* m) REQUIRES_SHARED(Locks::mutator_lock_);
-
bool IsForcedInterpretOnly() const {
return forced_interpret_only_;
}
+ // Code is in boot image oat file which isn't compiled as debuggable.
+ // Need debug version (interpreter or jitted) if that's the case.
+ bool NeedDebugVersionFor(ArtMethod* method) const
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
bool AreExitStubsInstalled() const {
return instrumentation_stubs_installed_;
}
@@ -574,8 +584,7 @@
// Returns true if we need entry exit stub to call entry hooks. JITed code
// directly call entry / exit hooks and don't need the stub.
- bool CodeNeedsEntryExitStub(const void* code, ArtMethod* method)
- REQUIRES_SHARED(Locks::mutator_lock_);
+ bool CodeNeedsEntryExitStub(const void* code, ArtMethod* method);
// Update the current instrumentation_level_.
void UpdateInstrumentationLevel(InstrumentationLevel level);
diff --git a/runtime/oat_file_manager.cc b/runtime/oat_file_manager.cc
index 3bcd9bb..3449919 100644
--- a/runtime/oat_file_manager.cc
+++ b/runtime/oat_file_manager.cc
@@ -849,15 +849,4 @@
}
}
-bool OatFileManager::ContainsPc(const void* code) {
- ReaderMutexLock mu(Thread::Current(), *Locks::oat_file_manager_lock_);
- std::vector<const OatFile*> boot_oat_files = GetBootOatFiles();
- for (const std::unique_ptr<const OatFile>& oat_file : oat_files_) {
- if (oat_file->Contains(code)) {
- return true;
- }
- }
- return false;
-}
-
} // namespace art
diff --git a/runtime/oat_file_manager.h b/runtime/oat_file_manager.h
index b73ac58..0c1a69b 100644
--- a/runtime/oat_file_manager.h
+++ b/runtime/oat_file_manager.h
@@ -140,8 +140,6 @@
// Maximum number of anonymous vdex files kept in the process' data folder.
static constexpr size_t kAnonymousVdexCacheSize = 8u;
- bool ContainsPc(const void* pc) REQUIRES(!Locks::oat_file_manager_lock_);
-
private:
std::vector<std::unique_ptr<const DexFile>> OpenDexFilesFromOat_Impl(
std::vector<MemMap>&& dex_mem_maps,
diff --git a/runtime/stack.cc b/runtime/stack.cc
index 7f1f47f..eb0fe56 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -787,7 +787,8 @@
DCHECK(method->IsNative());
if (kIsDebugBuild && !method->IsCriticalNative()) {
ClassLinker* class_linker = runtime->GetClassLinker();
- const void* entry_point = runtime->GetInstrumentation()->GetCodeForInvoke(method);
+ const void* entry_point = runtime->GetInstrumentation()->GetQuickCodeFor(method,
+ kRuntimePointerSize);
CHECK(class_linker->IsQuickGenericJniStub(entry_point) ||
// The current entrypoint (after filtering out trampolines) may have changed
// from GenericJNI to JIT-compiled stub since we have entered this frame.