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
diff --git a/runtime/common_dex_operations.h b/runtime/common_dex_operations.h
index 22057c9..882e3ce 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 @@
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 @@
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 fc77110..9c8c220 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -687,18 +687,25 @@
BuildQuickShadowFrameVisitor shadow_frame_builder(sp, method->IsStatic(), shorty, shorty_len,
shadow_frame, first_arg_reg);
shadow_frame_builder.VisitArguments();
- 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;
- }
-
// 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;
+ }
+ }
+ }
+
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 0ffc38b..5aa2fb6 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -389,6 +389,7 @@
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 @@
}
}
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 @@
}
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 @@
// 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 c6b2ddc..4ee4cb5 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -262,6 +262,25 @@
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 449f415..cec6f1d 100644
--- a/runtime/interpreter/unstarted_runtime.cc
+++ b/runtime/interpreter/unstarted_runtime.cc
@@ -2263,9 +2263,6 @@
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 @@
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 e69de29..0000000
--- 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 e69de29..0000000
--- 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 36cbb6f..0000000
--- 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 52fc128..0000000
--- 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);
- }
- }
-}
-