Do not create MethodType objects in compiled code...

... if there is a risk of initialization cycle.

Extended approach from aosp/2828246.
Failure was observed in art-no-image build from ab/12309579.

Probably the right fix is to implement Atomic* classes on top
of Unsafe, and not VarHandle-s.

Bug: 297147201
Test: ./art/test/testrunner/testrunner.py --host --64 --no-image --no-relocate --prebuild -b
Change-Id: Icdd6585c3b17aa14f7b5a741fc00486ce004db29

4 files changed, 104 insertions, 63 deletions

diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index 822774e73c..cf4ba6e5b7 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -2380,14 +2380,6 @@ extern "C" uint64_t artInvokePolymorphic(mirror::Object* raw_receiver, Thread* s
   ArtMethod* resolved_method = linker->ResolveMethod<ClassLinker::ResolveMode::kCheckICCEAndIAE>(
       self, inst.VRegB(), caller_method, kVirtual);
 
-  Handle<mirror::MethodType> method_type(
-      hs.NewHandle(linker->ResolveMethodType(self, proto_idx, caller_method)));
-  if (UNLIKELY(method_type.IsNull())) {
-    // This implies we couldn't resolve one or more types in this method handle.
-    CHECK(self->IsExceptionPending());
-    return 0UL;
-  }
-
   DCHECK_EQ(ArtMethod::NumArgRegisters(shorty) + 1u, (uint32_t)inst.VRegA());
   DCHECK_EQ(resolved_method->IsStatic(), kMethodIsStatic);
 
@@ -2420,6 +2412,14 @@ extern "C" uint64_t artInvokePolymorphic(mirror::Object* raw_receiver, Thread* s
   JValue result;
   bool success = false;
   if (resolved_method->GetDeclaringClass() == GetClassRoot<mirror::MethodHandle>(linker)) {
+    Handle<mirror::MethodType> method_type(
+        hs.NewHandle(linker->ResolveMethodType(self, proto_idx, caller_method)));
+    if (UNLIKELY(method_type.IsNull())) {
+      // This implies we couldn't resolve one or more types in this method handle.
+      CHECK(self->IsExceptionPending());
+      return 0UL;
+    }
+
     Handle<mirror::MethodHandle> method_handle(hs.NewHandle(
         ObjPtr<mirror::MethodHandle>::DownCast(receiver_handle.Get())));
     if (intrinsic == Intrinsics::kMethodHandleInvokeExact) {
@@ -2445,10 +2445,12 @@ extern "C" uint64_t artInvokePolymorphic(mirror::Object* raw_receiver, Thread* s
         ObjPtr<mirror::VarHandle>::DownCast(receiver_handle.Get())));
     mirror::VarHandle::AccessMode access_mode =
         mirror::VarHandle::GetAccessModeByIntrinsic(intrinsic);
+
     success = VarHandleInvokeAccessor(self,
                                       *shadow_frame,
+                                      caller_method,
                                       var_handle,
-                                      method_type,
+                                      proto_idx,
                                       access_mode,
                                       &operands,
                                       &result);
diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index 1fcace6d3b..c5e54436e1 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -439,7 +439,7 @@ static bool DoVarHandleInvokeCommon(Thread* self,
   bool is_var_args = inst->HasVarArgs();
   const uint32_t vRegC = is_var_args ? inst->VRegC_45cc() : inst->VRegC_4rcc();
   const uint16_t vRegH = is_var_args ? inst->VRegH_45cc() : inst->VRegH_4rcc();
-  StackHandleScope<4> hs(self);
+  StackHandleScope<3> hs(self);
   Handle<mirror::VarHandle> var_handle = hs.NewHandle(
       ObjPtr<mirror::VarHandle>::DownCast(shadow_frame.GetVRegReference(vRegC)));
   ArtMethod* method = shadow_frame.GetMethod();
@@ -458,63 +458,12 @@ static bool DoVarHandleInvokeCommon(Thread* self,
     all_operands = &range_operands.value();
   }
   NoReceiverInstructionOperands operands(all_operands);
-  ClassLinker* const class_linker = Runtime::Current()->GetClassLinker();
 
-  // If the `ThreadLocalRandom` class is not yet initialized, do the `VarHandle` operation
-  // without creating a managed `MethodType` object. This avoids a circular initialization
-  // issue when `ThreadLocalRandom.<clinit>` indirectly calls `AtomicLong.compareAndSet()`
-  // (implemented with a `VarHandle`) and the `MethodType` caching circles back to the
-  // `ThreadLocalRandom` with uninitialized `seeder` and throws NPE.
-  //
-  // Do a quick test for "visibly initialized" without a read barrier and, if that fails,
-  // do a thorough test for "initialized" (including load acquire) with the read barrier.
-  ArtField* field = WellKnownClasses::java_util_concurrent_ThreadLocalRandom_seeder;
-  if (LIKELY(field->GetDeclaringClass<kWithoutReadBarrier>()->IsVisiblyInitialized()) ||
-      field->GetDeclaringClass()->IsInitialized()) {
-    Handle<mirror::MethodType> callsite_type(hs.NewHandle(
-        class_linker->ResolveMethodType(self, dex::ProtoIndex(vRegH), dex_cache, class_loader)));
-    if (LIKELY(callsite_type != nullptr)) {
-      return VarHandleInvokeAccessor(self,
-                                     shadow_frame,
-                                     var_handle,
-                                     callsite_type,
-                                     access_mode,
-                                     &operands,
-                                     result);
-    }
-    // This implies we couldn't resolve one or more types in this VarHandle,
-    // or we could not allocate the `MethodType` object.
-    CHECK(self->IsExceptionPending());
-    if (self->GetException()->GetClass() != WellKnownClasses::java_lang_OutOfMemoryError.Get()) {
-      return false;
-    }
-    // Clear the OOME and retry without creating an actual `MethodType` object.
-    // This prevents unexpected OOME for trivial `VarHandle` operations.
-    // It also prevents odd situations where a `VarHandle` operation succeeds but the same
-    // operation fails later because the `MethodType` object was evicted from the `DexCache`
-    // and we suddenly run out of memory to allocate a new one.
-    //
-    // We have previously seen OOMEs in the run-test `183-rmw-stress-test` with
-    // `--optimizng --no-image` (boot class path methods run in interpreter without JIT)
-    // but it probably happened on the first execution of a trivial `VarHandle` operation
-    // and not due to the `DexCache` eviction mentioned above.
-    self->ClearException();
-  }
-
-  VariableSizedHandleScope callsite_type_hs(self);
-  mirror::RawMethodType callsite_type(&callsite_type_hs);
-  if (!class_linker->ResolveMethodType(self,
-                                       dex::ProtoIndex(vRegH),
-                                       dex_cache,
-                                       class_loader,
-                                       callsite_type)) {
-    CHECK(self->IsExceptionPending());
-    return false;
-  }
   return VarHandleInvokeAccessor(self,
                                  shadow_frame,
+                                 method,
                                  var_handle,
-                                 callsite_type,
+                                 dex::ProtoIndex(vRegH),
                                  access_mode,
                                  &operands,
                                  result);
diff --git a/runtime/var_handles.cc b/runtime/var_handles.cc
index 872e780612..d2cc564c66 100644
--- a/runtime/var_handles.cc
+++ b/runtime/var_handles.cc
@@ -16,6 +16,7 @@
 
 #include "var_handles.h"
 
+#include "art_method.h"
 #include "common_throws.h"
 #include "dex/dex_instruction.h"
 #include "handle.h"
@@ -155,4 +156,79 @@ bool VarHandleInvokeAccessor(Thread* self,
       self, shadow_frame, var_handle, callsite_type, access_mode, operands, result);
 }
 
+bool VarHandleInvokeAccessor(Thread* self,
+                             ShadowFrame& shadow_frame,
+                             ArtMethod* caller_method,
+                             Handle<mirror::VarHandle> var_handle,
+                             const dex::ProtoIndex callsite_type_id,
+                             const mirror::VarHandle::AccessMode access_mode,
+                             const InstructionOperands* const operands,
+                             JValue* result) {
+  StackHandleScope<3> hs(self);
+  ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+
+  Handle<mirror::DexCache> dex_cache = hs.NewHandle(caller_method->GetDexCache());
+  Handle<mirror::ClassLoader> class_loader = hs.NewHandle(caller_method->GetClassLoader());
+
+  // If the `ThreadLocalRandom` class is not yet initialized, do the `VarHandle` operation
+  // without creating a managed `MethodType` object. This avoids a circular initialization
+  // issue when `ThreadLocalRandom.<clinit>` indirectly calls `AtomicLong.compareAndSet()`
+  // (implemented with a `VarHandle`) and the `MethodType` caching circles back to the
+  // `ThreadLocalRandom` with uninitialized `seeder` and throws NPE.
+  //
+  // Do a quick test for "visibly initialized" without a read barrier and, if that fails,
+  // do a thorough test for "initialized" (including load acquire) with the read barrier.
+  ArtField* field = WellKnownClasses::java_util_concurrent_ThreadLocalRandom_seeder;
+  if (LIKELY(field->GetDeclaringClass<kWithoutReadBarrier>()->IsVisiblyInitialized()) ||
+      field->GetDeclaringClass()->IsInitialized()) {
+    Handle<mirror::MethodType> callsite_type(hs.NewHandle(
+        class_linker->ResolveMethodType(self, callsite_type_id, dex_cache, class_loader)));
+
+    if (LIKELY(callsite_type != nullptr)) {
+      return VarHandleInvokeAccessor(self,
+                                     shadow_frame,
+                                     var_handle,
+                                     callsite_type,
+                                     access_mode,
+                                     operands,
+                                     result);
+    }
+    // This implies we couldn't resolve one or more types in this VarHandle,
+    // or we could not allocate the `MethodType` object.
+    CHECK(self->IsExceptionPending());
+    if (self->GetException()->GetClass() != WellKnownClasses::java_lang_OutOfMemoryError.Get()) {
+      return false;
+    }
+    // Clear the OOME and retry without creating an actual `MethodType` object.
+    // This prevents unexpected OOME for trivial `VarHandle` operations.
+    // It also prevents odd situations where a `VarHandle` operation succeeds but the same
+    // operation fails later because the `MethodType` object was evicted from the `DexCache`
+    // and we suddenly run out of memory to allocate a new one.
+    //
+    // We have previously seen OOMEs in the run-test `183-rmw-stress-test` with
+    // `--optimizng --no-image` (boot class path methods run in interpreter without JIT)
+    // but it probably happened on the first execution of a trivial `VarHandle` operation
+    // and not due to the `DexCache` eviction mentioned above.
+    self->ClearException();
+  }
+
+  VariableSizedHandleScope callsite_type_hs(self);
+  mirror::RawMethodType callsite_type(&callsite_type_hs);
+  if (!class_linker->ResolveMethodType(self,
+                                       callsite_type_id,
+                                       dex_cache,
+                                       class_loader,
+                                       callsite_type)) {
+    CHECK(self->IsExceptionPending());
+    return false;
+  }
+  return VarHandleInvokeAccessor(self,
+                                 shadow_frame,
+                                 var_handle,
+                                 callsite_type,
+                                 access_mode,
+                                 operands,
+                                 result);
+}
+
 }  // namespace art
diff --git a/runtime/var_handles.h b/runtime/var_handles.h
index 82056ca153..acaa6e3cee 100644
--- a/runtime/var_handles.h
+++ b/runtime/var_handles.h
@@ -18,6 +18,10 @@
 #define ART_RUNTIME_VAR_HANDLES_H_
 
 #include "base/macros.h"
+#include "dex/dex_file_types.h"
+#include "dex/dex_instruction.h"
+#include "interpreter/shadow_frame.h"
+#include "jvalue.h"
 #include "mirror/var_handle.h"
 
 namespace art HIDDEN {
@@ -44,6 +48,16 @@ bool VarHandleInvokeAccessor(Thread* self,
                              JValue* result)
     REQUIRES_SHARED(Locks::mutator_lock_);
 
+bool VarHandleInvokeAccessor(Thread* self,
+                             ShadowFrame& shadow_frame,
+                             ArtMethod* caller_method,
+                             Handle<mirror::VarHandle> var_handle,
+                             const dex::ProtoIndex callsite_type,
+                             const mirror::VarHandle::AccessMode access_mode,
+                             const InstructionOperands* const operands,
+                             JValue* result)
+    REQUIRES_SHARED(Locks::mutator_lock_);
+
 }  // namespace art
 
 #endif  // ART_RUNTIME_VAR_HANDLES_H_