Revert "Ensure we initialize before pushing the method invoked."

This reverts commit 9f1a9bcfa320de7b4409cfd85cbe9b192c022f06.

Reason for revert: Just to confirm if it resolves presubmit tests
on the userfaultfd GC CLs (topic uffd-gc). This CL definitely causes
art-test failures (with uffd GC) on local tests due to double
reference to same shadow-frame on mutator stack.

Test: test/testrunner/testrunner.py --host --debug --interpreter --gcstress
Change-Id: I69affec203416aceb6f77e1ea2d1ec9bb1add6a6

9 files changed, 74 insertions, 129 deletions

diff --git a/runtime/common_dex_operations.h b/runtime/common_dex_operations.h
index 22057c91b8..882e3ce4c7 100644
--- a/runtime/common_dex_operations.h
+++ b/runtime/common_dex_operations.h
@@ -26,7 +26,6 @@
 #include "dex/code_item_accessors.h"
 #include "dex/dex_file_structs.h"
 #include "dex/primitive.h"
-#include "entrypoints/entrypoint_utils.h"
 #include "handle_scope-inl.h"
 #include "instrumentation.h"
 #include "interpreter/interpreter.h"
@@ -56,33 +55,8 @@ namespace interpreter {
                                           ShadowFrame* shadow_frame,
                                           uint16_t arg_offset,
                                           JValue* result);
-
 }  // namespace interpreter
 
-inline bool EnsureInitialized(Thread* self, ShadowFrame* shadow_frame)
-    REQUIRES_SHARED(Locks::mutator_lock_) {
-  if (!NeedsClinitCheckBeforeCall(shadow_frame->GetMethod())) {
-    return true;
-  }
-  ObjPtr<mirror::Class> declaring_class = shadow_frame->GetMethod()->GetDeclaringClass();
-  if (LIKELY(declaring_class->IsVisiblyInitialized())) {
-    return true;
-  }
-
-  // Save the shadow frame.
-  ScopedStackedShadowFramePusher pusher(
-      self, shadow_frame, StackedShadowFrameType::kShadowFrameUnderConstruction);
-  StackHandleScope<1> hs(self);
-  Handle<mirror::Class> h_class(hs.NewHandle(declaring_class));
-  if (UNLIKELY(!Runtime::Current()->GetClassLinker()->EnsureInitialized(
-                    self, h_class, /*can_init_fields=*/ true, /*can_init_parents=*/ true))) {
-    DCHECK(self->IsExceptionPending());
-    return false;
-  }
-  DCHECK(h_class->IsInitializing());
-  return true;
-}
-
 inline void PerformCall(Thread* self,
                         const CodeItemDataAccessor& accessor,
                         ArtMethod* caller_method,
@@ -91,20 +65,15 @@ inline void PerformCall(Thread* self,
                         JValue* result,
                         bool use_interpreter_entrypoint)
     REQUIRES_SHARED(Locks::mutator_lock_) {
-  if (UNLIKELY(!Runtime::Current()->IsStarted())) {
-    interpreter::UnstartedRuntime::Invoke(self, accessor, callee_frame, result, first_dest_reg);
-    return;
-  }
-
-  if (!EnsureInitialized(self, callee_frame)) {
-    return;
-  }
-
-  if (use_interpreter_entrypoint) {
-    interpreter::ArtInterpreterToInterpreterBridge(self, accessor, callee_frame, result);
+  if (LIKELY(Runtime::Current()->IsStarted())) {
+    if (use_interpreter_entrypoint) {
+      interpreter::ArtInterpreterToInterpreterBridge(self, accessor, callee_frame, result);
+    } else {
+      interpreter::ArtInterpreterToCompiledCodeBridge(
+          self, caller_method, callee_frame, first_dest_reg, result);
+    }
   } else {
-    interpreter::ArtInterpreterToCompiledCodeBridge(
-        self, caller_method, callee_frame, first_dest_reg, result);
+    interpreter::UnstartedRuntime::Invoke(self, accessor, callee_frame, result, first_dest_reg);
   }
 }
 
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index fc7711027f..9c8c2208ba 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -687,18 +687,25 @@ extern "C" uint64_t artQuickToInterpreterBridge(ArtMethod* method, Thread* self,
     BuildQuickShadowFrameVisitor shadow_frame_builder(sp, method->IsStatic(), shorty, shorty_len,
                                                       shadow_frame, first_arg_reg);
     shadow_frame_builder.VisitArguments();
+    // Push a transition back into managed code onto the linked list in thread.
+    self->PushManagedStackFragment(&fragment);
+    self->PushShadowFrame(shadow_frame);
     self->EndAssertNoThreadSuspension(old_cause);
 
-    // Potentially run <clinit> before pushing the shadow frame. We do not want
-    // to have the called method on the stack if there is an exception.
-    if (!EnsureInitialized(self, shadow_frame)) {
-      DCHECK(self->IsExceptionPending());
-      return 0;
+    if (NeedsClinitCheckBeforeCall(method)) {
+      ObjPtr<mirror::Class> declaring_class = method->GetDeclaringClass();
+      if (UNLIKELY(!declaring_class->IsVisiblyInitialized())) {
+        // Ensure static method's class is initialized.
+        StackHandleScope<1> hs(self);
+        Handle<mirror::Class> h_class(hs.NewHandle(declaring_class));
+        if (!Runtime::Current()->GetClassLinker()->EnsureInitialized(self, h_class, true, true)) {
+          DCHECK(Thread::Current()->IsExceptionPending()) << method->PrettyMethod();
+          self->PopManagedStackFragment(fragment);
+          return 0;
+        }
+      }
     }
 
-    // Push a transition back into managed code onto the linked list in thread.
-    self->PushManagedStackFragment(&fragment);
-    self->PushShadowFrame(shadow_frame);
     result = interpreter::EnterInterpreterFromEntryPoint(self, accessor, shadow_frame);
     force_frame_pop = shadow_frame->GetForcePopFrame();
   }
diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index 0ffc38beeb..5aa2fb6eed 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -389,6 +389,7 @@ void EnterInterpreterFromInvoke(Thread* self,
   ShadowFrameAllocaUniquePtr shadow_frame_unique_ptr =
       CREATE_SHADOW_FRAME(num_regs, last_shadow_frame, method, /* dex pc */ 0);
   ShadowFrame* shadow_frame = shadow_frame_unique_ptr.get();
+  self->PushShadowFrame(shadow_frame);
 
   size_t cur_reg = num_regs - num_ins;
   if (!method->IsStatic()) {
@@ -420,10 +421,21 @@ void EnterInterpreterFromInvoke(Thread* self,
     }
   }
   self->EndAssertNoThreadSuspension(old_cause);
-  if (!EnsureInitialized(self, shadow_frame)) {
-    return;
+  // Do this after populating the shadow frame in case EnsureInitialized causes a GC.
+  if (method->IsStatic()) {
+    ObjPtr<mirror::Class> declaring_class = method->GetDeclaringClass();
+    if (UNLIKELY(!declaring_class->IsVisiblyInitialized())) {
+      StackHandleScope<1> hs(self);
+      Handle<mirror::Class> h_class(hs.NewHandle(declaring_class));
+      if (UNLIKELY(!Runtime::Current()->GetClassLinker()->EnsureInitialized(
+                        self, h_class, /*can_init_fields=*/ true, /*can_init_parents=*/ true))) {
+        CHECK(self->IsExceptionPending());
+        self->PopShadowFrame();
+        return;
+      }
+      DCHECK(h_class->IsInitializing());
+    }
   }
-  self->PushShadowFrame(shadow_frame);
   if (LIKELY(!method->IsNative())) {
     JValue r = Execute(self, accessor, *shadow_frame, JValue(), stay_in_interpreter);
     if (result != nullptr) {
@@ -595,6 +607,23 @@ void ArtInterpreterToInterpreterBridge(Thread* self,
   }
 
   self->PushShadowFrame(shadow_frame);
+  ArtMethod* method = shadow_frame->GetMethod();
+  // Ensure static methods are initialized.
+  const bool is_static = method->IsStatic();
+  if (is_static) {
+    ObjPtr<mirror::Class> declaring_class = method->GetDeclaringClass();
+    if (UNLIKELY(!declaring_class->IsVisiblyInitialized())) {
+      StackHandleScope<1> hs(self);
+      Handle<mirror::Class> h_class(hs.NewHandle(declaring_class));
+      if (UNLIKELY(!Runtime::Current()->GetClassLinker()->EnsureInitialized(
+                        self, h_class, /*can_init_fields=*/ true, /*can_init_parents=*/ true))) {
+        DCHECK(self->IsExceptionPending());
+        self->PopShadowFrame();
+        return;
+      }
+      DCHECK(h_class->IsInitializing());
+    }
+  }
 
   if (LIKELY(!shadow_frame->GetMethod()->IsNative())) {
     result->SetJ(Execute(self, accessor, *shadow_frame, JValue()).GetJ());
@@ -602,7 +631,6 @@ void ArtInterpreterToInterpreterBridge(Thread* self,
     // We don't expect to be asked to interpret native code (which is entered via a JNI compiler
     // generated stub) except during testing and image writing.
     CHECK(!Runtime::Current()->IsStarted());
-    bool is_static = shadow_frame->GetMethod()->IsStatic();
     ObjPtr<mirror::Object> receiver = is_static ? nullptr : shadow_frame->GetVRegReference(0);
     uint32_t* args = shadow_frame->GetVRegArgs(is_static ? 0 : 1);
     UnstartedRuntime::Jni(self, shadow_frame->GetMethod(), receiver.Ptr(), args, result);
diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index c6b2ddc964..4ee4cb5a9f 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -262,6 +262,25 @@ void ArtInterpreterToCompiledCodeBridge(Thread* self,
                                         JValue* result)
     REQUIRES_SHARED(Locks::mutator_lock_) {
   ArtMethod* method = shadow_frame->GetMethod();
+  // Ensure static methods are initialized.
+  if (method->IsStatic()) {
+    ObjPtr<mirror::Class> declaringClass = method->GetDeclaringClass();
+    if (UNLIKELY(!declaringClass->IsVisiblyInitialized())) {
+      self->PushShadowFrame(shadow_frame);
+      StackHandleScope<1> hs(self);
+      Handle<mirror::Class> h_class(hs.NewHandle(declaringClass));
+      if (UNLIKELY(!Runtime::Current()->GetClassLinker()->EnsureInitialized(
+                        self, h_class, /*can_init_fields=*/ true, /*can_init_parents=*/ true))) {
+        self->PopShadowFrame();
+        DCHECK(self->IsExceptionPending());
+        return;
+      }
+      self->PopShadowFrame();
+      DCHECK(h_class->IsInitializing());
+      // Reload from shadow frame in case the method moved, this is faster than adding a handle.
+      method = shadow_frame->GetMethod();
+    }
+  }
   // Basic checks for the arg_offset. If there's no code item, the arg_offset must be 0. Otherwise,
   // check that the arg_offset isn't greater than the number of registers. A stronger check is
   // difficult since the frame may contain space for all the registers in the method, or only enough
diff --git a/runtime/interpreter/unstarted_runtime.cc b/runtime/interpreter/unstarted_runtime.cc
index 449f4153b8..cec6f1dc8d 100644
--- a/runtime/interpreter/unstarted_runtime.cc
+++ b/runtime/interpreter/unstarted_runtime.cc
@@ -2263,9 +2263,6 @@ void UnstartedRuntime::Invoke(Thread* self, const CodeItemDataAccessor& accessor
 
   const auto& iter = invoke_handlers_.find(shadow_frame->GetMethod());
   if (iter != invoke_handlers_.end()) {
-    // Note: When we special case the method, we do not ensure initialization.
-    // This has been the behavior since implementation of this feature.
-
     // Clear out the result in case it's not zeroed out.
     result->SetL(nullptr);
 
@@ -2276,9 +2273,6 @@ void UnstartedRuntime::Invoke(Thread* self, const CodeItemDataAccessor& accessor
 
     self->PopShadowFrame();
   } else {
-    if (!EnsureInitialized(self, shadow_frame)) {
-      return;
-    }
     // Not special, continue with regular interpreter execution.
     ArtInterpreterToInterpreterBridge(self, accessor, shadow_frame, result);
   }
diff --git a/test/839-clinit-throw/expected-stderr.txt b/test/839-clinit-throw/expected-stderr.txt
deleted file mode 100644
index e69de29bb2..0000000000
--- a/test/839-clinit-throw/expected-stderr.txt
+++ /dev/null
diff --git a/test/839-clinit-throw/expected-stdout.txt b/test/839-clinit-throw/expected-stdout.txt
deleted file mode 100644
index e69de29bb2..0000000000
--- a/test/839-clinit-throw/expected-stdout.txt
+++ /dev/null
diff --git a/test/839-clinit-throw/info.txt b/test/839-clinit-throw/info.txt
deleted file mode 100644
index 36cbb6f740..0000000000
--- a/test/839-clinit-throw/info.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-Test that a static initializer throwing doesn't contain a static method of the
-same class in the stack trace.
diff --git a/test/839-clinit-throw/src/Main.java b/test/839-clinit-throw/src/Main.java
deleted file mode 100644
index 52fc128d1e..0000000000
--- a/test/839-clinit-throw/src/Main.java
+++ /dev/null
@@ -1,70 +0,0 @@
-/*
- * Copyright (C) 2022 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-public class Main {
-
-  static class NoPreloadHolder {
-    static Object o = null;
-
-    static {
-      o.toString();
-    }
-
-    static void $noinline$doCall() {
-    }
-  }
-
-  public static void main(String[] args) {
-    try {
-      NoPreloadHolder.$noinline$doCall();
-      throw new Error("Expected ExceptionInInitializerError");
-    } catch (ExceptionInInitializerError e) {
-      // expected
-      check(e, mainLine);
-    }
-  }
-
-  public static int mainLine = 32;
-
-  static void check(ExceptionInInitializerError ie, int mainLine) {
-    StackTraceElement[] trace = ie.getStackTrace();
-    assertEquals(trace.length, 1);
-    checkElement(trace[0], "Main", "main", "Main.java", mainLine);
-  }
-
-  static void checkElement(StackTraceElement element,
-                           String declaringClass, String methodName,
-                           String fileName, int lineNumber) {
-    assertEquals(declaringClass, element.getClassName());
-    assertEquals(methodName, element.getMethodName());
-    assertEquals(fileName, element.getFileName());
-    assertEquals(lineNumber, element.getLineNumber());
-  }
-
-  static void assertEquals(Object expected, Object actual) {
-    if (!expected.equals(actual)) {
-      String msg = "Expected \"" + expected + "\" but got \"" + actual + "\"";
-      throw new AssertionError(msg);
-    }
-  }
-
-  static void assertEquals(int expected, int actual) {
-    if (expected != actual) {
-      throw new AssertionError("Expected " + expected + " got " + actual);
-    }
-  }
-}
-