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

6 files changed, 49 insertions, 6 deletions

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 1710e788ef..c374e03ed7 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -4440,8 +4440,8 @@ void ClassLinker::CreateProxyConstructor(Handle<mirror::Class> klass, ArtMethod*
 
   // 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 2b1623ac5a..dc89c27685 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -1101,6 +1101,10 @@ extern "C" const void* artInstrumentationMethodEntryFromCode(ArtMethod* method,
   // 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 d7f33d5e43..cf1d79773b 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -154,8 +154,16 @@ void Instrumentation::InstallStubsForMethod(ArtMethod* method) {
     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 @@ void Instrumentation::UpdateMethodsCodeImpl(ArtMethod* method, const void* quick
       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 946ea018f3..36dea60367 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 c64e7bbca1..206418fbc6 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_Long_valueOf;
 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 @@ void WellKnownClasses::LateInit(JNIEnv* env) {
       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 @@ void WellKnownClasses::Clear() {
   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 c81062f594..ce5ab1df84 100644
--- a/runtime/well_known_classes.h
+++ b/runtime/well_known_classes.h
@@ -108,6 +108,7 @@ struct WellKnownClasses {
   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;