8225106: C2: Parse::clinit_deopt asserts when holder klass is in error state
Reviewed-by: mdoerr
--- a/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp Thu Jun 06 13:45:59 2019 +0300
+++ b/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp Thu Jun 06 13:46:01 2019 +0300
@@ -361,8 +361,7 @@
void LIR_Assembler::clinit_barrier(ciMethod* method) {
assert(VM_Version::supports_fast_class_init_checks(), "sanity");
- assert(method->holder()->is_being_initialized() || method->holder()->is_initialized(),
- "initialization should have been started");
+ assert(!method->holder()->is_not_initialized(), "initialization should have been started");
Label L_skip_barrier;
Register klass = rscratch1;
--- a/src/hotspot/cpu/x86/macroAssembler_x86.cpp Thu Jun 06 13:45:59 2019 +0300
+++ b/src/hotspot/cpu/x86/macroAssembler_x86.cpp Thu Jun 06 13:46:01 2019 +0300
@@ -4602,13 +4602,14 @@
bind(L_fallthrough);
}
-
void MacroAssembler::clinit_barrier(Register klass, Register thread, Label* L_fast_path, Label* L_slow_path) {
assert(L_fast_path != NULL || L_slow_path != NULL, "at least one is required");
Label L_fallthrough;
if (L_fast_path == NULL) {
L_fast_path = &L_fallthrough;
+ } else if (L_slow_path == NULL) {
+ L_slow_path = &L_fallthrough;
}
// Fast path check: class is fully initialized
@@ -4617,13 +4618,15 @@
// Fast path check: current thread is initializer thread
cmpptr(thread, Address(klass, InstanceKlass::init_thread_offset()));
- if (L_slow_path != NULL) {
- jcc(Assembler::notEqual, *L_slow_path);
- } else {
+ if (L_slow_path == &L_fallthrough) {
jcc(Assembler::equal, *L_fast_path);
- }
-
- bind(L_fallthrough);
+ bind(*L_slow_path);
+ } else if (L_fast_path == &L_fallthrough) {
+ jcc(Assembler::notEqual, *L_slow_path);
+ bind(*L_fast_path);
+ } else {
+ Unimplemented();
+ }
}
void MacroAssembler::cmov32(Condition cc, Register dst, Address src) {
--- a/src/hotspot/cpu/x86/x86_64.ad Thu Jun 06 13:45:59 2019 +0300
+++ b/src/hotspot/cpu/x86/x86_64.ad Thu Jun 06 13:46:01 2019 +0300
@@ -876,8 +876,7 @@
if (C->clinit_barrier_on_entry()) {
assert(VM_Version::supports_fast_class_init_checks(), "sanity");
- assert(C->method()->holder()->is_being_initialized() || C->method()->holder()->is_initialized(),
- "initialization should have been started");
+ assert(!C->method()->holder()->is_not_initialized(), "initialization should have been started");
Label L_skip_barrier;
Register klass = rscratch1;
--- a/src/hotspot/share/ci/ciEnv.cpp Thu Jun 06 13:45:59 2019 +0300
+++ b/src/hotspot/share/ci/ciEnv.cpp Thu Jun 06 13:46:01 2019 +0300
@@ -991,6 +991,11 @@
record_failure("DTrace flags change invalidated dependencies");
}
+ if (!failing() && target->needs_clinit_barrier() &&
+ target->holder()->is_in_error_state()) {
+ record_failure("method holder is in error state");
+ }
+
if (!failing()) {
if (log() != NULL) {
// Log the dependencies which this compilation declares.
--- a/src/hotspot/share/ci/ciInstanceKlass.hpp Thu Jun 06 13:45:59 2019 +0300
+++ b/src/hotspot/share/ci/ciInstanceKlass.hpp Thu Jun 06 13:46:01 2019 +0300
@@ -120,6 +120,10 @@
update_if_shared(InstanceKlass::fully_initialized);
return _init_state == InstanceKlass::fully_initialized;
}
+ bool is_not_initialized() {
+ update_if_shared(InstanceKlass::fully_initialized);
+ return _init_state < InstanceKlass::being_initialized;
+ }
// Is this klass being initialized?
bool is_being_initialized() {
update_if_shared(InstanceKlass::being_initialized);
@@ -130,6 +134,11 @@
update_if_shared(InstanceKlass::linked);
return _init_state >= InstanceKlass::linked;
}
+ // Is this klass in error state?
+ bool is_in_error_state() {
+ update_if_shared(InstanceKlass::initialization_error);
+ return _init_state == InstanceKlass::initialization_error;
+ }
// General klass information.
ciFlags flags() {
--- a/src/hotspot/share/opto/parse1.cpp Thu Jun 06 13:45:59 2019 +0300
+++ b/src/hotspot/share/opto/parse1.cpp Thu Jun 06 13:46:01 2019 +0300
@@ -2112,8 +2112,7 @@
assert(C->has_method(), "only for normal compilations");
assert(depth() == 1, "only for main compiled method");
assert(is_normal_parse(), "no barrier needed on osr entry");
- assert(method()->holder()->is_being_initialized() || method()->holder()->is_initialized(),
- "initialization should have been started");
+ assert(!method()->holder()->is_not_initialized(), "initialization should have been started");
set_parse_bci(0);
--- a/test/hotspot/jtreg/runtime/clinit/ClassInitBarrier.java Thu Jun 06 13:45:59 2019 +0300
+++ b/test/hotspot/jtreg/runtime/clinit/ClassInitBarrier.java Thu Jun 06 13:46:01 2019 +0300
@@ -114,7 +114,7 @@
checkBlockingAction(Test::testPutStatic); // putstatic
checkBlockingAction(Test::testNewInstanceA); // new
- A recv = testNewInstanceB(NON_BLOCKING.get()); // trigger B initialization
+ A recv = testNewInstanceB(NON_BLOCKING.get()); // trigger B initialization
checkNonBlockingAction(Test::testNewInstanceB); // new: NO BLOCKING: same thread: A being initialized, B fully initialized
checkNonBlockingAction(recv, Test::testGetField); // getfield
@@ -140,8 +140,8 @@
static void run() {
execute(ExceptionInInitializerError.class, () -> triggerInitialization(A.class));
-
ensureFinished();
+ runTests(); // after initialization is over
}
}
@@ -150,22 +150,30 @@
static void execute(Class<? extends Throwable> expectedExceptionClass, Runnable action) {
try {
action.run();
- if (THROW) throw new AssertionError("no exception thrown");
+ if (THROW) throw failure("no exception thrown");
} catch (Throwable e) {
if (THROW) {
if (e.getClass() == expectedExceptionClass) {
// expected
} else {
String msg = String.format("unexpected exception thrown: expected %s, caught %s",
- expectedExceptionClass.getName(), e.getClass().getName());
- throw new AssertionError(msg, e);
+ expectedExceptionClass.getName(), e);
+ throw failure(msg, e);
}
} else {
- throw new AssertionError("no exception expected", e);
+ throw failure("no exception expected", e);
}
}
}
+ private static AssertionError failure(String msg) {
+ return new AssertionError(phase + ": " + msg);
+ }
+
+ private static AssertionError failure(String msg, Throwable e) {
+ return new AssertionError(phase + ": " + msg, e);
+ }
+
static final List<Thread> BLOCKED_THREADS = Collections.synchronizedList(new ArrayList<>());
static final Consumer<Thread> ON_BLOCK = BLOCKED_THREADS::add;
@@ -293,27 +301,53 @@
static final AtomicInteger NON_BLOCKING_COUNTER = new AtomicInteger(0);
static final AtomicInteger NON_BLOCKING_ACTIONS = new AtomicInteger(0);
- static final Factory<Runnable> NON_BLOCKING = () -> disposableAction(Phase.IN_PROGRESS, NON_BLOCKING_COUNTER, NON_BLOCKING_ACTIONS);
+ static final Factory<Runnable> NON_BLOCKING = () -> disposableAction(phase, NON_BLOCKING_COUNTER, NON_BLOCKING_ACTIONS);
static final AtomicInteger BLOCKING_COUNTER = new AtomicInteger(0);
static final AtomicInteger BLOCKING_ACTIONS = new AtomicInteger(0);
static final Factory<Runnable> BLOCKING = () -> disposableAction(Phase.FINISHED, BLOCKING_COUNTER, BLOCKING_ACTIONS);
static void checkBlockingAction(TestCase0 r) {
- r.run(NON_BLOCKING.get()); // same thread
- checkBlocked(ON_BLOCK, ON_FAILURE, r); // different thread
+ switch (phase) {
+ case IN_PROGRESS: {
+ // Barrier during class initalization.
+ r.run(NON_BLOCKING.get()); // initializing thread
+ checkBlocked(ON_BLOCK, ON_FAILURE, r); // different thread
+ break;
+ }
+ case FINISHED: {
+ // No barrier after class initalization is over.
+ r.run(NON_BLOCKING.get()); // initializing thread
+ checkNotBlocked(r); // different thread
+ break;
+ }
+ case INIT_FAILURE: {
+ // Exception is thrown after class initialization failed.
+ TestCase0 test = action -> execute(NoClassDefFoundError.class, () -> r.run(action));
+
+ test.run(NON_BLOCKING.get()); // initializing thread
+ checkNotBlocked(test); // different thread
+ break;
+ }
+ default: throw new Error("wrong phase: " + phase);
+ }
}
static void checkNonBlockingAction(TestCase0 r) {
- r.run(NON_BLOCKING.get());
- checkNotBlocked(r); // different thread
+ r.run(NON_BLOCKING.get()); // initializing thread
+ checkNotBlocked(r); // different thread
}
static <T> void checkNonBlockingAction(T recv, TestCase1<T> r) {
- r.run(recv, NON_BLOCKING.get()); // same thread
+ r.run(recv, NON_BLOCKING.get()); // initializing thread
checkNotBlocked((action) -> r.run(recv, action)); // different thread
}
+ static void checkFailingAction(TestCase0 r) {
+ r.run(NON_BLOCKING.get()); // initializing thread
+ checkNotBlocked(r); // different thread
+ }
+
static void triggerInitialization(Class<?> cls) {
try {
Class<?> loadedClass = Class.forName(cls.getName(), true, cls.getClassLoader());
@@ -356,7 +390,15 @@
}
static void checkNotBlocked(TestCase0 r) {
- Thread thr = new Thread(() -> r.run(NON_BLOCKING.get()));
+ final Thread thr = new Thread(() -> r.run(NON_BLOCKING.get()));
+ final Throwable[] ex = new Throwable[1];
+ thr.setUncaughtExceptionHandler((t, e) -> {
+ if (thr != t) {
+ ex[0] = new Error("wrong thread: " + thr + " vs " + t);
+ } else {
+ ex[0] = e;
+ }
+ });
thr.start();
try {
@@ -368,6 +410,10 @@
} catch (InterruptedException e) {
throw new Error(e);
}
+
+ if (ex[0] != null) {
+ throw new AssertionError("no exception expected", ex[0]);
+ }
}
static void maybeThrow() {