JDK-8175887 : C1 value numbering handling of Unsafe.get*Volatile is incorrect
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 9,10
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2017-02-27
  • Updated: 2019-09-13
  • Resolved: 2017-03-01
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 10 JDK 8 JDK 9
10Fixed 8u152Fixed 9 b161Fixed
Related Reports
Relates :  
JDK-7170145 - C1 doesn't respect the JMM with volatile field loads
Description
This looks eerily similar to JDK-7170145, but now for Unsafe.get*Volatile handling in C1. We seem to miss the case for volatile ops like this:

$ hg diff src/share/vm/c1/c1_ValueMap.hpp
diff -r dedf248e8e3e src/share/vm/c1/c1_ValueMap.hpp
--- a/src/share/vm/c1/c1_ValueMap.hpp	Mon Feb 13 10:37:33 2017 -0500
+++ b/src/share/vm/c1/c1_ValueMap.hpp	Mon Feb 27 10:33:01 2017 +0100
@@ -198,7 +198,11 @@
   void do_ExceptionObject(ExceptionObject* x) { /* nothing to do */ }
   void do_RoundFP        (RoundFP*         x) { /* nothing to do */ }
   void do_UnsafeGetRaw   (UnsafeGetRaw*    x) { /* nothing to do */ }
-  void do_UnsafeGetObject(UnsafeGetObject* x) { /* nothing to do */ }
+  void do_UnsafeGetObject(UnsafeGetObject* x) {
+    if (x->is_volatile()) {
+      kill_memory();
+    }
+  }
   void do_ProfileCall    (ProfileCall*     x) { /* nothing to do */ }
   void do_ProfileReturnType (ProfileReturnType*  x) { /* nothing to do */ }
   void do_ProfileInvoke  (ProfileInvoke*   x) { /* nothing to do */ };

This might explain a few failures in jcstress, like this -- although not confirmed as the actual culprit:
 http://openjdk.linaro.org/jdk9/jcstress-nightly-runs/2017/056/results/org.openjdk.jcstress.tests.coherence.varHandles.fields.opaque.ShortTest.html
Comments
Verified with new added tests.
15-08-2017
ILW=breaking specification, so far observed with a number of jcstress tests, no workaround (or use -XX:-TieredCompilation) = HLH
27-02-2017
RFR: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-February/025732.html
27-02-2017
As in JDK-7170145, we should probably fix the put*Volatile too. jcstress would be amended to test memory effects of unsafe ops too (it does test volatile-keyword ops).
27-02-2017
Confirmed on current jdk9-hs Linux x86_64 with targeted jcstress test: http://hg.openjdk.java.net/code-tools/jcstress/file/0400ed6dd6a6/tests-custom/src/main/java/org/openjdk/jcstress/tests/unsafe/UnsafeReadTwiceOverVolatileReadTest.java Exception in thread "main" java.lang.AssertionError: TEST FAILURES: org.openjdk.jcstress.tests.unsafe.UnsafeReadTwiceOverVolatileReadTest [-XX:+UnlockDiagnosticVMOptions, -XX:-UseNewCode, -XX:TieredStopAtLevel=1]: Observed forbidden state: 0, 1, 0 org.openjdk.jcstress.tests.unsafe.UnsafeReadTwiceOverVolatileReadTest [-XX:+UnlockDiagnosticVMOptions, -XX:-UseNewCode, -server]: Observed forbidden state: 0, 1, 0 org.openjdk.jcstress.tests.unsafe.UnsafeReadTwiceOverVolatileReadTest [-XX:+UnlockDiagnosticVMOptions, -XX:-UseNewCode, -client]: Observed forbidden state: 0, 1, 0 org.openjdk.jcstress.tests.unsafe.UnsafeReadTwiceOverVolatileReadTest [-XX:+UnlockDiagnosticVMOptions, -XX:-UseNewCode, -server, -XX:+UnlockDiagnosticVMOptions, -XX:+StressLCM, -XX:+StressGCM]: Observed forbidden state: 0, 1, 0 Seems fixed with the patch above.
27-02-2017
RFC: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-February/025731.html
27-02-2017