Ensure we never instrument Proxy.<init> entrypoint
Due to the way we implement Proxy classes we need to be very careful
when modifying proxy classes and the (non-proxy)
java.lang.reflect.Proxy class and its methods. In particular we always
avoid installing an instrumentation entrypoint into the Proxy.<init>
method since we copy it for each proxy class. Failing to do this
causes problems as the instrumentation entrypoint bounces to the Proxy
entrypoint which gets confused since the copied init method is not
really a proxy method. Unfortunately if one starts the profiling
process early enough it was possible for the Proxy.<init> method to
get instrumented as it is being loaded. This CL ensures that the
method is skipped just like it would be if profiling was started
later.
NB Test requires several other patches to actually run far enough to
observe this issue.
Test: ./test/testrunner/testrunner.py --host --runtime-option=-Xplugin:libtracefast-trampolined.so
Test: ./test/testrunner/testrunner.py --host --run-test-option='--with-agent libtifast.so=MethodEntry,MethodExit'
Change-Id: I18fb381d18d7100b5ec843b3cddd387f2d033776
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 1710e78..c374e03 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -4440,8 +4440,8 @@
// Find the <init>(InvocationHandler)V method. The exact method offset varies depending
// on which front-end compiler was used to build the libcore DEX files.
- ArtMethod* proxy_constructor = proxy_class->FindConstructor(
- "(Ljava/lang/reflect/InvocationHandler;)V", image_pointer_size_);
+ ArtMethod* proxy_constructor =
+ jni::DecodeArtMethod(WellKnownClasses::java_lang_reflect_Proxy_init);
DCHECK(proxy_constructor != nullptr)
<< "Could not find <init> method in java.lang.reflect.Proxy";
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index 2b1623a..dc89c27 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -1101,6 +1101,10 @@
// that part.
ScopedQuickEntrypointChecks sqec(self, kIsDebugBuild, false);
instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
+ DCHECK(!method->IsProxyMethod())
+ << "Proxy method " << method->PrettyMethod()
+ << " (declaring class: " << method->GetDeclaringClass()->PrettyClass() << ")"
+ << " should not hit instrumentation entrypoint.";
if (instrumentation->IsDeoptimized(method)) {
result = GetQuickToInterpreterBridge();
} else {
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index d7f33d5..cf1d797 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -154,8 +154,16 @@
return;
}
// Don't stub Proxy.<init>. Note that the Proxy class itself is not a proxy class.
- if (method->IsConstructor() &&
- method->GetDeclaringClass()->DescriptorEquals("Ljava/lang/reflect/Proxy;")) {
+ // TODO We should remove the need for this since it means we cannot always correctly detect calls
+ // to Proxy.<init>
+ // 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;
}
const void* new_quick_code;
@@ -785,7 +793,13 @@
if (class_linker->IsQuickResolutionStub(quick_code) ||
class_linker->IsQuickToInterpreterBridge(quick_code)) {
new_quick_code = quick_code;
- } else if (entry_exit_stubs_installed_) {
+ } else if (entry_exit_stubs_installed_ &&
+ // 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)) {
new_quick_code = GetQuickInstrumentationEntryPoint();
} else {
new_quick_code = quick_code;
diff --git a/runtime/proxy_test.cc b/runtime/proxy_test.cc
index 946ea01..36dea60 100644
--- a/runtime/proxy_test.cc
+++ b/runtime/proxy_test.cc
@@ -23,11 +23,24 @@
#include "mirror/field-inl.h"
#include "proxy_test.h"
#include "scoped_thread_state_change-inl.h"
+#include "well_known_classes.h"
namespace art {
namespace proxy_test {
-class ProxyTest : public CommonRuntimeTest {};
+class ProxyTest : public CommonRuntimeTest {
+ protected:
+ void SetUp() OVERRIDE {
+ CommonRuntimeTest::SetUp();
+ // The creation of a Proxy class uses WellKnownClasses. These are not normally initialized by
+ // CommonRuntimeTest so we need to do that now.
+ WellKnownClasses::Clear();
+ WellKnownClasses::Init(art::Thread::Current()->GetJniEnv());
+ // Since we aren't actually calling any of the native functions we can just immediately call
+ // LateInit after calling Init.
+ WellKnownClasses::LateInit(art::Thread::Current()->GetJniEnv());
+ }
+};
// Creates a proxy class and check ClassHelper works correctly.
TEST_F(ProxyTest, ProxyClassHelper) {
diff --git a/runtime/well_known_classes.cc b/runtime/well_known_classes.cc
index c64e7bb..206418f 100644
--- a/runtime/well_known_classes.cc
+++ b/runtime/well_known_classes.cc
@@ -26,6 +26,7 @@
#include "base/enums.h"
#include "class_linker.h"
#include "entrypoints/quick/quick_entrypoints_enum.h"
+#include "entrypoints/runtime_asm_entrypoints.h"
#include "hidden_api.h"
#include "jni/jni_internal.h"
#include "mirror/class.h"
@@ -98,6 +99,7 @@
jmethodID WellKnownClasses::java_lang_ref_FinalizerReference_add;
jmethodID WellKnownClasses::java_lang_ref_ReferenceQueue_add;
jmethodID WellKnownClasses::java_lang_reflect_Parameter_init;
+jmethodID WellKnownClasses::java_lang_reflect_Proxy_init;
jmethodID WellKnownClasses::java_lang_reflect_Proxy_invoke;
jmethodID WellKnownClasses::java_lang_Runtime_nativeLoad;
jmethodID WellKnownClasses::java_lang_Short_valueOf;
@@ -418,6 +420,14 @@
CacheMethod(env, java_lang_Runtime.get(), true, "nativeLoad",
"(Ljava/lang/String;Ljava/lang/ClassLoader;)"
"Ljava/lang/String;");
+ java_lang_reflect_Proxy_init =
+ CacheMethod(env, java_lang_reflect_Proxy, false, "<init>",
+ "(Ljava/lang/reflect/InvocationHandler;)V");
+ // This invariant is important since otherwise we will have the entire proxy invoke system
+ // confused.
+ DCHECK_NE(
+ jni::DecodeArtMethod(java_lang_reflect_Proxy_init)->GetEntryPointFromQuickCompiledCode(),
+ GetQuickInstrumentationEntryPoint());
java_lang_reflect_Proxy_invoke =
CacheMethod(env, java_lang_reflect_Proxy, true, "invoke",
"(Ljava/lang/reflect/Proxy;Ljava/lang/reflect/Method;"
@@ -484,6 +494,7 @@
java_lang_ref_FinalizerReference_add = nullptr;
java_lang_ref_ReferenceQueue_add = nullptr;
java_lang_reflect_Parameter_init = nullptr;
+ java_lang_reflect_Proxy_init = nullptr;
java_lang_reflect_Proxy_invoke = nullptr;
java_lang_Runtime_nativeLoad = nullptr;
java_lang_Short_valueOf = nullptr;
diff --git a/runtime/well_known_classes.h b/runtime/well_known_classes.h
index c81062f..ce5ab1d 100644
--- a/runtime/well_known_classes.h
+++ b/runtime/well_known_classes.h
@@ -108,6 +108,7 @@
static jmethodID java_lang_ref_FinalizerReference_add;
static jmethodID java_lang_ref_ReferenceQueue_add;
static jmethodID java_lang_reflect_Parameter_init;
+ static jmethodID java_lang_reflect_Proxy_init;
static jmethodID java_lang_reflect_Proxy_invoke;
static jmethodID java_lang_Runtime_nativeLoad;
static jmethodID java_lang_Short_valueOf;