United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-8031743 : C2: loadI2L_immI broken for negative memory values

Details
Type:
Bug
Submit Date:
2014-01-14
Status:
Closed
Updated Date:
2014-09-04
Project Name:
JDK
Resolved Date:
2014-01-23
Component:
hotspot
OS:
generic
Sub-Component:
compiler
CPU:
generic
Priority:
P1
Resolution:
Fixed
Affected Versions:
hs25
Fixed Versions:
hs25 (b68)

Related Reports
Backport:
Backport:
Backport:
Backport:
Backport:
Backport:
Backport:
Backport:
Backport:
Backport:

Sub Tasks

Description
instruct loadI2L_immI(iRegL dst, memory mem, immI mask, iRegI tmp) %{
  match(Set dst (ConvI2L (AndI (LoadI mem) mask)));

From inspection, it appears that this rule (on all platforms) only works if the result of the AndI
doesn't need to be sign-extended (>= 0).  My first attempt at a test program didn't work.  For
some reason when using an array, C2 only takes a fast path if the value is positive:

    static long foo(int[] x) {
        return x[0] & 0xfffffffe;
    }

However, using a field instead doesn't have that problem.

% cat a.java
public class a {
    static int x;
    static long foo() {
        return x & 0xfffffffe;
    }

    public static void main(String[] args) {
        x = -1;
        long l = 0;
        for (int i = 0; i < 100000; ++i) {
            l = foo();
        }
        System.out.println(l);
    }
}
% ./bin/java -Xint a
-2
% ./bin/java -XX:-TieredCompilation -server a
4294967294

                                    

Comments
Uh-oh, that's extremely bad! This should be a P1.
                                     
2014-01-18
The optimization is incorrect, it would work only with mask <= 0x7fffffff. Predicating it off based on that should do the trick.
                                     
2014-01-18
The bug exists on all platforms. x86, x64, sparc, arm
                                     
2014-01-18
Suggest fix: http://cr.openjdk.java.net/~iveresov/8031743/webrev.00/
                                     
2014-01-18
ILW=HHH=P1 

Impact: Incorrect result (which is way worse than a crash), the test case is very simple. 
Likelihood: This reproduces all the time. 
Workaround: None 

Justification: This affects any code that does a bitwise AND with a integer loaded from memory and constant mask value with conversion to a long.
The optimization transforms (ConvI2L (AndI (LoadI mem) mask)) to (AndI (LoadUI2L (mem) mask). If the mask constant has the highest bit set and the value is also negative the optimization behaves incorrectly and produces wrong result because the value is not sign-extended.

The fix is low risk since it conservatively doesn't introduce any new code generation but rather restricts when the existing optimizations are being applied.
                                     
2014-01-19
Release team: Approved for fixing
                                     
2014-01-21
I'll push it today, I was waiting to check the nightlies in 9 first.
                                     
2014-01-23
URL:   http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/rev/984401824c5e
User:  iveresov
Date:  2014-01-23 19:55:26 +0000

                                     
2014-01-23
URL:   http://hg.openjdk.java.net/hsx/hsx25/hotspot/rev/984401824c5e
User:  amurillo
Date:  2014-01-23 23:09:27 +0000

                                     
2014-01-23
7u60-critical-justification:  A related issue to JDK-8032207 that the OpsCenter team is having.  Fixing both issues is needed for a proper fix.
                                     
2014-02-14



Hardware and Software, Engineered to Work Together