Fix tests flakiness with jit when using Proxy classes.

We cannot copy the entry point between ArtMethod when
the entry point has been JITted. We put the interpreter
bridge instead.

bug:25334878

Change-Id: I65a50cc1f10a5a152733807f8c85fb3ed81c5829

8 files changed, 56 insertions, 32 deletions

diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index fb116bb3da..d055b37ea7 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -697,6 +697,9 @@ void CompilerDriver::CompileOne(Thread* self, ArtMethod* method, TimingLogger* t
 }
 
 CompiledMethod* CompilerDriver::CompileArtMethod(Thread* self, ArtMethod* method) {
+  DCHECK_EQ(method,
+            method->GetInterfaceMethodIfProxy(
+                Runtime::Current()->GetClassLinker()->GetImagePointerSize()));
   const uint32_t method_idx = method->GetDexMethodIndex();
   const uint32_t access_flags = method->GetAccessFlags();
   const InvokeType invoke_type = method->GetInvokeType();
diff --git a/compiler/jit/jit_compiler.cc b/compiler/jit/jit_compiler.cc
index c1b87c9cd0..d520208d32 100644
--- a/compiler/jit/jit_compiler.cc
+++ b/compiler/jit/jit_compiler.cc
@@ -192,7 +192,10 @@ bool JitCompiler::CompileMethod(Thread* self, ArtMethod* method) {
   CompiledMethod* compiled_method = nullptr;
   {
     TimingLogger::ScopedTiming t2("Compiling", &logger);
-    compiled_method = compiler_driver_->CompileArtMethod(self, method);
+    // If we get a request to compile a proxy method, we pass the actual Java method
+    // of that proxy method, as the compiler does not expect a proxy method.
+    ArtMethod* method_to_compile = method->GetInterfaceMethodIfProxy(sizeof(void*));
+    compiled_method = compiler_driver_->CompileArtMethod(self, method_to_compile);
   }
 
   // Trim maps to reduce memory usage.
diff --git a/runtime/art_method-inl.h b/runtime/art_method-inl.h
index f741732046..cf548ada33 100644
--- a/runtime/art_method-inl.h
+++ b/runtime/art_method-inl.h
@@ -468,12 +468,6 @@ void ArtMethod::VisitRoots(RootVisitorType& visitor, size_t pointer_size) {
   }
 }
 
-inline void ArtMethod::CopyFrom(const ArtMethod* src, size_t image_pointer_size) {
-  memcpy(reinterpret_cast<void*>(this), reinterpret_cast<const void*>(src),
-         Size(image_pointer_size));
-  declaring_class_ = GcRoot<mirror::Class>(const_cast<ArtMethod*>(src)->GetDeclaringClass());
-}
-
 }  // namespace art
 
 #endif  // ART_RUNTIME_ART_METHOD_INL_H_
diff --git a/runtime/art_method.cc b/runtime/art_method.cc
index c1279bf6b1..f4a5f233ff 100644
--- a/runtime/art_method.cc
+++ b/runtime/art_method.cc
@@ -367,7 +367,7 @@ const uint8_t* ArtMethod::GetQuickenedInfo() {
 }
 
 const OatQuickMethodHeader* ArtMethod::GetOatQuickMethodHeader(uintptr_t pc) {
-  if (IsRuntimeMethod() || IsProxyMethod()) {
+  if (IsRuntimeMethod()) {
     return nullptr;
   }
 
@@ -381,6 +381,12 @@ const OatQuickMethodHeader* ArtMethod::GetOatQuickMethodHeader(uintptr_t pc) {
     return nullptr;
   }
 
+  if (existing_entry_point == GetQuickProxyInvokeHandler()) {
+    DCHECK(IsProxyMethod() && !IsConstructor());
+    // The proxy entry point does not have any method header.
+    return nullptr;
+  }
+
   // Check whether the current entry point contains this pc.
   if (!class_linker->IsQuickResolutionStub(existing_entry_point) &&
       !class_linker->IsQuickToInterpreterBridge(existing_entry_point)) {
@@ -452,4 +458,28 @@ const OatQuickMethodHeader* ArtMethod::GetOatQuickMethodHeader(uintptr_t pc) {
   return method_header;
 }
 
+
+void ArtMethod::CopyFrom(ArtMethod* src, size_t image_pointer_size) {
+  memcpy(reinterpret_cast<void*>(this), reinterpret_cast<const void*>(src),
+         Size(image_pointer_size));
+  declaring_class_ = GcRoot<mirror::Class>(const_cast<ArtMethod*>(src)->GetDeclaringClass());
+
+  // If the entry point of the method we are copying from is from JIT code, we just
+  // put the entry point of the new method to interpreter. We could set the entry point
+  // to the JIT code, but this would require taking the JIT code cache lock to notify
+  // it, which we do not want at this level.
+  Runtime* runtime = Runtime::Current();
+  if (runtime->GetJit() != nullptr) {
+    if (runtime->GetJit()->GetCodeCache()->ContainsPc(GetEntryPointFromQuickCompiledCode())) {
+      SetEntryPointFromQuickCompiledCodePtrSize(GetQuickToInterpreterBridge(), image_pointer_size);
+    }
+  }
+  // Clear the profiling info for the same reasons as the JIT code.
+  if (!src->IsNative()) {
+    SetProfilingInfoPtrSize(nullptr, image_pointer_size);
+  }
+  // Clear hotness to let the JIT properly decide when to compile this method.
+  hotness_count_ = 0;
+}
+
 }  // namespace art
diff --git a/runtime/art_method.h b/runtime/art_method.h
index 551989d182..ce9f2025ce 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -49,8 +49,8 @@ class ArtMethod FINAL {
   ArtMethod() : access_flags_(0), dex_code_item_offset_(0), dex_method_index_(0),
       method_index_(0) { }
 
-  ArtMethod(const ArtMethod& src, size_t image_pointer_size) {
-    CopyFrom(&src, image_pointer_size);
+  ArtMethod(ArtMethod* src, size_t image_pointer_size) {
+    CopyFrom(src, image_pointer_size);
   }
 
   static ArtMethod* FromReflectedMethod(const ScopedObjectAccessAlreadyRunnable& soa,
@@ -313,6 +313,10 @@ class ArtMethod FINAL {
     SetEntryPointFromJniPtrSize(info, sizeof(void*));
   }
 
+  ALWAYS_INLINE void SetProfilingInfoPtrSize(ProfilingInfo* info, size_t pointer_size) {
+    SetEntryPointFromJniPtrSize(info, pointer_size);
+  }
+
   static MemberOffset ProfilingInfoOffset() {
     return EntryPointFromJniOffset(sizeof(void*));
   }
@@ -429,7 +433,7 @@ class ArtMethod FINAL {
     return pointer_size;
   }
 
-  void CopyFrom(const ArtMethod* src, size_t image_pointer_size)
+  void CopyFrom(ArtMethod* src, size_t image_pointer_size)
       SHARED_REQUIRES(Locks::mutator_lock_);
 
   ALWAYS_INLINE GcRoot<mirror::Class>* GetDexCacheResolvedTypes(size_t pointer_size)
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 5de1cacba8..da70456369 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -5279,7 +5279,7 @@ bool ClassLinker::LinkInterfaceMethods(
             miranda_method = reinterpret_cast<ArtMethod*>(allocator.Alloc(method_size));
             CHECK(miranda_method != nullptr);
             // Point the interface table at a phantom slot.
-            new(miranda_method) ArtMethod(*interface_method, image_pointer_size_);
+            new(miranda_method) ArtMethod(interface_method, image_pointer_size_);
             miranda_methods.push_back(miranda_method);
           }
           method_array->SetElementPtrSize(j, miranda_method, image_pointer_size_);
diff --git a/runtime/jit/jit_instrumentation.cc b/runtime/jit/jit_instrumentation.cc
index 8aaa5fa304..7931306ff6 100644
--- a/runtime/jit/jit_instrumentation.cc
+++ b/runtime/jit/jit_instrumentation.cc
@@ -102,15 +102,13 @@ void JitInstrumentationCache::AddSamples(Thread* self, ArtMethod* method, size_t
     } else {
       // We failed allocating. Instead of doing the collection on the Java thread, we push
       // an allocation to a compiler thread, that will do the collection.
-      thread_pool_->AddTask(self, new JitCompileTask(
-          method->GetInterfaceMethodIfProxy(sizeof(void*)), JitCompileTask::kAllocateProfile));
+      thread_pool_->AddTask(self, new JitCompileTask(method, JitCompileTask::kAllocateProfile));
       thread_pool_->StartWorkers(self);
     }
   }
 
   if (sample_count == hot_method_threshold_) {
-    thread_pool_->AddTask(self, new JitCompileTask(
-        method->GetInterfaceMethodIfProxy(sizeof(void*)), JitCompileTask::kCompile));
+    thread_pool_->AddTask(self, new JitCompileTask(method, JitCompileTask::kCompile));
     thread_pool_->StartWorkers(self);
   }
 }
diff --git a/runtime/stack.cc b/runtime/stack.cc
index b0727daa15..d7edfade15 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -958,26 +958,18 @@ QuickMethodFrameInfo StackVisitor::GetCurrentQuickFrameInfo() const {
     return runtime->GetRuntimeMethodFrameInfo(method);
   }
 
-  // For Proxy method we add special handling for the direct method case  (there is only one
-  // direct method - constructor). Direct method is cloned from original
-  // java.lang.reflect.Proxy class together with code and as a result it is executed as usual
-  // quick compiled method without any stubs. So the frame info should be returned as it is a
-  // quick method not a stub. However, if instrumentation stubs are installed, the
-  // instrumentation->GetQuickCodeFor() returns the artQuickProxyInvokeHandler instead of an
-  // oat code pointer, thus we have to add a special case here.
   if (method->IsProxyMethod()) {
-    if (method->IsDirect()) {
-      CHECK(method->IsConstructor());
-      const void* code_pointer =
-          EntryPointToCodePointer(method->GetEntryPointFromQuickCompiledCode());
-      return reinterpret_cast<const OatQuickMethodHeader*>(code_pointer)[-1].frame_info_;
-    } else {
-      return runtime->GetCalleeSaveMethodFrameInfo(Runtime::kRefsAndArgs);
-    }
+    // There is only one direct method of a proxy class: the constructor. A direct method is
+    // cloned from the original java.lang.reflect.Proxy and is executed as usual quick
+    // compiled method without any stubs. Therefore the method must have a OatQuickMethodHeader.
+    DCHECK(!method->IsDirect() && !method->IsConstructor())
+        << "Constructors of proxy classes must have a OatQuickMethodHeader";
+    return runtime->GetCalleeSaveMethodFrameInfo(Runtime::kRefsAndArgs);
   }
 
-  ClassLinker* class_linker = runtime->GetClassLinker();
+  // The only remaining case is if the method is native and uses the generic JNI stub.
   DCHECK(method->IsNative());
+  ClassLinker* class_linker = runtime->GetClassLinker();
   const void* entry_point = runtime->GetInstrumentation()->GetQuickCodeFor(method, sizeof(void*));
   DCHECK(class_linker->IsQuickGenericJniStub(entry_point)) << PrettyMethod(method);
   // Generic JNI frame.