JDK-8184271 : Time related C1 intrinsics produce inconsistent results when floating around
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 8,9,10
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2017-07-10
  • Updated: 2022-05-06
  • Resolved: 2017-07-14
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
10 b21Fixed 8u152Fixed
Related Reports
Duplicate :  
Relates :  
Relates :  
Description
FULL PRODUCT VERSION :
java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)

FULL OS VERSION :
- RHEL 7.3 Linux xxx.gs.com 3.10.0-514.16.1.el7.x86_64 #1 SMP Fri Mar 10 13:12:32 EST 2017 x86_64 x86_64 x86_64 GNU/Linux
- Ubuntu 17.10 with 4.12 kernel
- This is platform-agnostic

A DESCRIPTION OF THE PROBLEM :
C1 emits instructions in a sequence that violates program order when performing OSR + inlining code from the affected method.
This seems to be a subtle edge-case as referring the impacted value in checkResult() or using assert instead of "if" results in correct behavior.
Looking at the sequence of instructions emitted by C1 in this case (with PrintAssembly) clearly shows that C2 code produces valid instruction order whereas C1 does not.
Reducing inline level to 1 or disabling tiered compilation results in correct program behavior.

THE PROBLEM WAS REPRODUCIBLE WITH -Xint FLAG: No

THE PROBLEM WAS REPRODUCIBLE WITH -server FLAG: Yes

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Just run the following class with default java args.
It is also reproducible with manually inlined timeInvocation() + local variables inlined, but the provided example is clearer to work with.


EXPECTED VERSUS ACTUAL BEHAVIOR :
Should exit without printing anything.
Prints "should not get here ..." contrary to the expected behavior. Neither javac nor any of the JIT compilers should reorder capturing nanoTime value as there is a direct data dependency on the result of their subtraction.
The fact that System.nanoTime() return monotonic values guarantees result consistency.
REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
public class C1_OSR_Inline_Bug {

    public static void main(String[] args) {
        for (int i = 0; i < 100_000; i++) {
            timeInvocation();
        }
    }

    private static void timeInvocation() {
        final long start = System.nanoTime();
        final long end = System.nanoTime();
        checkResult(end - start);
    }

    private static void checkResult(final long l) {
        if (l < 0) {
            System.out.println("should not get here " + l); // removing reference to l parameter here "fixes" the bug
            System.exit(0);
        }
    }
}

---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
Reducing inline level to 1 or disabling tiered compilation result in correct program behavior.
The same applies to running in interpreted mode.


Comments
URL: http://hg.openjdk.java.net/jdk10/jdk10/hotspot/rev/cc2bc9993d64 User: jwilhelm Date: 2017-08-18 18:01:35 +0000
18-08-2017

URL: http://hg.openjdk.java.net/jdk10/hs/hotspot/rev/cc2bc9993d64 User: thartmann Date: 2017-07-14 11:44:45 +0000
14-07-2017

Patch: http://cr.openjdk.java.net/~thartmann/8184271/webrev.00/
13-07-2017

Hi Vladimir, thanks for the pointers! I think other intrinsics like _currentTimeMillis and _counterTime are also affected. I'll prepare a patch with regression tests. Thanks, Tobias
13-07-2017

The problem is that all non-trapping intrinsics are not pinned and can float around: {code} LEAF(Intrinsic, StateSplit) ... // some intrinsics can't trap, so don't force them to be pinned if (!can_trap()) { unpin(PinStateSplitConstructor); } {code} A possible fix would be to pin _nanoTime: diff --git a/src/share/vm/c1/c1_Instruction.hpp b/src/share/vm/c1/c1_Instruction.hpp --- a/src/share/vm/c1/c1_Instruction.hpp +++ b/src/share/vm/c1/c1_Instruction.hpp @@ -1566,7 +1566,7 @@ set_needs_null_check(has_receiver); // some intrinsics can't trap, so don't force them to be pinned - if (!can_trap()) { + if (!can_trap() && !vmIntrinsics::should_be_pinned(id)) { unpin(PinStateSplitConstructor); } } diff --git a/src/share/vm/classfile/vmSymbols.cpp b/src/share/vm/classfile/vmSymbols.cpp --- a/src/share/vm/classfile/vmSymbols.cpp +++ b/src/share/vm/classfile/vmSymbols.cpp @@ -398,6 +398,16 @@ } } +bool vmIntrinsics::should_be_pinned(vmIntrinsics::ID id) { + assert(id != vmIntrinsics::_none, "must be a VM intrinsic"); + switch(id) { + case vmIntrinsics::_nanoTime: + return true; + default: + return false; + } +} + bool vmIntrinsics::does_virtual_dispatch(vmIntrinsics::ID id) { assert(id != vmIntrinsics::_none, "must be a VM intrinsic"); switch(id) { diff --git a/src/share/vm/classfile/vmSymbols.hpp b/src/share/vm/classfile/vmSymbols.hpp --- a/src/share/vm/classfile/vmSymbols.hpp +++ b/src/share/vm/classfile/vmSymbols.hpp @@ -1630,6 +1630,7 @@ static bool preserves_state(vmIntrinsics::ID id); static bool can_trap(vmIntrinsics::ID id); + static bool should_be_pinned(vmIntrinsics::ID id); // (2) Information needed by the C2 compiler.
12-07-2017

Workaround: -XX:DisableIntrinsic=_nanoTime
12-07-2017

ILW = C1 generates incorrect code, easy to reproduce with JDK 8 but hidden by ISC in JDK 9, -XX:-TieredCompilation = HMM = P2
12-07-2017

This bug is just hidden by Indify String Concat (JDK-8085796). Disabling this optimization makes it re-appear with latest JDK 9: javac -XDstringConcat=inline C1_OSR_Inline_Bug.java java -XX:TieredStopAtLevel=1 C1_OSR_Inline_Bug should not get here -32
12-07-2017

This issue is not reproducible on 9, below are the results. 7u80 - Pass 8 - Fail // Regression started from here 8u131 - Fail 8u152 - Fail 9 ea b 103 - Fail 9 ea b 104 - Pass // Issue fixed here 9 ea b 174 - Pass Issue is only applicable to 8
12-07-2017

If using the old javac version I can still reproduce this with the latest JDK 9 HotSpot version. javac output from build 104: private static void checkResult(long); Code: 0: lload_0 1: lconst_0 2: lcmp 3: ifge 22 6: getstatic #6 // Field java/lang/System.out:Ljava/io/PrintStream; 9: lload_0 10: invokedynamic #7, 0 // InvokeDynamic #0:makeConcatWithConstants:(J)Ljava/lang/String; 15: invokevirtual #8 // Method java/io/PrintStream.println:(Ljava/lang/String;)V 18: iconst_0 19: invokestatic #9 // Method java/lang/System.exit:(I)V 22: return javac output from build 103: private static void checkResult(long); Code: 0: lload_0 1: lconst_0 2: lcmp 3: ifge 35 6: getstatic #6 // Field java/lang/System.out:Ljava/io/PrintStream; 9: new #7 // class java/lang/StringBuilder 12: dup 13: invokespecial #8 // Method java/lang/StringBuilder."<init>":()V 16: ldc #9 // String should not get here 18: invokevirtual #10 // Method java/lang/StringBuilder.append:(Ljava/lang/String;)Ljava/lang/StringBuilder; 21: lload_0 22: invokevirtual #11 // Method java/lang/StringBuilder.append:(J)Ljava/lang/StringBuilder; 25: invokevirtual #12 // Method java/lang/StringBuilder.toString:()Ljava/lang/String; 28: invokevirtual #13 // Method java/io/PrintStream.println:(Ljava/lang/String;)V 31: iconst_0 32: invokestatic #14 // Method java/lang/System.exit:(I)V 35: return
12-07-2017