Ensure we initialize before pushing the method invoked.

Test: 839-clinit-throw
Change-Id: I4571d2445bc3f3a35fe27757208cfc7bcee4a801

9 files changed, 129 insertions, 74 deletions

diff --git a/runtime/common_dex_operations.h b/runtime/common_dex_operations.h
index 882e3ce4c7..22057c91b8 100644
--- a/runtime/common_dex_operations.h
+++ b/runtime/common_dex_operations.h
@@ -26,6 +26,7 @@
 #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"
@@ -55,8 +56,33 @@ 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,
@@ -65,15 +91,20 @@ inline void PerformCall(Thread* self,
                         JValue* result,
                         bool use_interpreter_entrypoint)
     REQUIRES_SHARED(Locks::mutator_lock_) {
-  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 {
+  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);
+  } else {
+    interpreter::ArtInterpreterToCompiledCodeBridge(
+        self, caller_method, callee_frame, first_dest_reg, result);
   }
 }
 
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index 0c7402c8fd..5995d5b0c5 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -688,25 +688,18 @@ 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);
 
-    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;
-        }
-      }
+    // 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;
     }
 
+    // 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 7a4ff5f692..311cb23510 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -389,7 +389,6 @@ 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()) {
@@ -421,21 +420,10 @@ void EnterInterpreterFromInvoke(Thread* self,
     }
   }
   self->EndAssertNoThreadSuspension(old_cause);
-  // 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());
-    }
+  if (!EnsureInitialized(self, shadow_frame)) {
+    return;
   }
+  self->PushShadowFrame(shadow_frame);
   if (LIKELY(!method->IsNative())) {
     JValue r = Execute(self, accessor, *shadow_frame, JValue(), stay_in_interpreter);
     if (result != nullptr) {
@@ -607,23 +595,6 @@ 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());
@@ -631,6 +602,7 @@ 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 4ee4cb5a9f..c6b2ddc964 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -262,25 +262,6 @@ 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 cec6f1dc8d..449f4153b8 100644
--- a/runtime/interpreter/unstarted_runtime.cc
+++ b/runtime/interpreter/unstarted_runtime.cc
@@ -2263,6 +2263,9 @@ 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);
 
@@ -2273,6 +2276,9 @@ 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
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/839-clinit-throw/expected-stderr.txt
diff --git a/test/839-clinit-throw/expected-stdout.txt b/test/839-clinit-throw/expected-stdout.txt
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/839-clinit-throw/expected-stdout.txt
diff --git a/test/839-clinit-throw/info.txt b/test/839-clinit-throw/info.txt
new file mode 100644
index 0000000000..36cbb6f740
--- /dev/null
+++ b/test/839-clinit-throw/info.txt
@@ -0,0 +1,2 @@
+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
new file mode 100644
index 0000000000..52fc128d1e
--- /dev/null
+++ b/test/839-clinit-throw/src/Main.java
@@ -0,0 +1,70 @@
+/*
+ * 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);
+    }
+  }
+}
+