OpenJDK / amber / amber

changeset 56635:3c905e67e380

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
8225106: C2: Parse::clinit_deopt asserts when holder klass is in error state Reviewed-by: mdoerr
author vlivanov
date Thu, 06 Jun 2019 13:46:01 +0300
parents 6502d6a92fe2
children 36cb654a690f
files src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp src/hotspot/cpu/x86/macroAssembler_x86.cpp src/hotspot/cpu/x86/x86_64.ad src/hotspot/share/ci/ciEnv.cpp src/hotspot/share/ci/ciInstanceKlass.hpp src/hotspot/share/opto/parse1.cpp test/hotspot/jtreg/runtime/clinit/ClassInitBarrier.java
diffstat 7 files changed, 86 insertions(+), 26 deletions(-) [+]
line wrap: on
line diff
--- 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() {
© 2007, Oracle and/or its affiliates
Terms of Use · Privacy · Trademarks