8225106: C2: Parse::clinit_deopt asserts when holder klass is in error state

Reviewed-by: mdoerr

src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp

@@ -361,8 +361,7 @@ int LIR_Assembler::check_icache() {
 
 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;

src/hotspot/cpu/x86/macroAssembler_x86.cpp

@@ -4602,13 +4602,14 @@ void MacroAssembler::check_klass_subtype_slow_path(Register sub_klass,
   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 @@ void MacroAssembler::clinit_barrier(Register klass, Register thread, Label* L_fa
 
   // 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_slow_path);
+  } else if (L_fast_path == &L_fallthrough) {
+    jcc(Assembler::notEqual, *L_slow_path);
+    bind(*L_fast_path);
+  } else {
+    Unimplemented();
   }
-
-  bind(L_fallthrough);
 }
 
 void MacroAssembler::cmov32(Condition cc, Register dst, Address src) {

src/hotspot/cpu/x86/x86_64.ad

@@ -876,8 +876,7 @@ void MachPrologNode::emit(CodeBuffer &cbuf, PhaseRegAlloc *ra_) const {
 
   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;

src/hotspot/share/ci/ciEnv.cpp

@@ -991,6 +991,11 @@ void ciEnv::register_method(ciMethod* target,
       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.

src/hotspot/share/ci/ciInstanceKlass.hpp

@@ -120,6 +120,10 @@ public:
     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 @@ public:
     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()          {

src/hotspot/share/opto/parse1.cpp

@@ -2112,8 +2112,7 @@ void Parse::clinit_deopt() {
   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);
 

test/hotspot/jtreg/runtime/clinit/ClassInitBarrier.java

@@ -140,8 +140,8 @@ public class ClassInitBarrier {
 
         static void run() {
             execute(ExceptionInInitializerError.class, () -> triggerInitialization(A.class));
-
             ensureFinished();
+            runTests(); // after initialization is over
         }
     }
 
@@ -150,22 +150,30 @@ public class ClassInitBarrier {
     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 @@ public class ClassInitBarrier {
 
     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
+        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());
+        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 @@ public class ClassInitBarrier {
     }
 
     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 @@ public class ClassInitBarrier {
         } catch (InterruptedException e) {
             throw new Error(e);
         }
+
+        if (ex[0] != null) {
+            throw new AssertionError("no exception expected", ex[0]);
+        }
     }
 
     static void maybeThrow() {