Community
Participate
Working Groups
Build ID: M20071023-1652 Steps To Reproduce: Hi, We found a problem with the way the Java compiler inside Eclipse works. The test case below shows a problem with doing subtractions with negative values if one of the variables is final. If I run the same test outside of Eclipse it works fine! It fails inside Eclipse regardless of which JDK I use for running the test (JDK 5,6, JRockit 5, IBM JDK 5). The only difference is a "final". Looking at the bytecode it seems both versions result in different bytecode, where one is missing a "times -1" and thus results in the incorrect result. Thanks... Dominik. public class TestSimpleThings extends TestCase { // case that works public void testShort1() { short min = Short.MIN_VALUE; int num = -32638; num = num - min; // -32638 - (-32768) should be 130, which it is here assertEquals(130, num); } // case that should work, but doesn't!! public void testShort2() { final short min = Short.MIN_VALUE; int num = -32638; num = num - min; // -32638 - (-32768) should be equal to 130, even if "min" is final! assertEquals("Here the problem manifests, the compiler incorrectly optimizes! " + "-32638 - (-32768) should still be 130, but is reported as -65406 here!", 130, num); } } More information: The following is a detailed analysis from one of our developers: ----------------------------------------------------------- In fact, not short, or the conversion is the problem – the problem is the combination of the final keyword with an integer or short value <= -32768! Having a look at the bytecode, short min = Short.MIN_VALUE; int num = -32638; num = num - min; System.out.println(num); L0 (0) LINENUMBER 3 L0 SIPUSH -32768 ISTORE 1: min L1 (3) LINENUMBER 4 L1 SIPUSH -32638 ISTORE 2: num L2 (6) LINENUMBER 5 L2 ILOAD 2: num ILOAD 1: min ISUB ISTORE 2: num L3 (11) LINENUMBER 6 L3 GETSTATIC System.out : PrintStream ILOAD 2: num INVOKEVIRTUAL PrintStream.println(int) : void L4 (15) LINENUMBER 7 L4 RETURN L5 (17) LOCALVARIABLE args String[] L0 L5 0 LOCALVARIABLE min short L1 L5 1 LOCALVARIABLE num int L2 L5 2 MAXSTACK = 2 MAXLOCALS = 3 final short min = Short.MIN_VALUE; int num = -32638; num = num - min; System.out.println(num); L0 (0) LINENUMBER 3 L0 SIPUSH -32768 ISTORE 1: min L1 (3) LINENUMBER 4 L1 SIPUSH -32638 ISTORE 2: num L2 (6) LINENUMBER 5 L2 IINC 2: num -32768 L3 (8) LINENUMBER 6 L3 GETSTATIC System.out : PrintStream ILOAD 2: num INVOKEVIRTUAL PrintStream.println(int) : void L4 (12) LINENUMBER 7 L4 RETURN L5 (14) LOCALVARIABLE args String[] L0 L5 0 LOCALVARIABLE min short L1 L5 1 LOCALVARIABLE num int L2 L5 2 MAXSTACK = 2 MAXLOCALS = 3 As you see, without final, Java is using ISUB, with final it‘s using IINC – a valid optimization – however, it must use IINC with the number *-1, which for some reason doesn’t work for numbers <= -32768 To illustrate it further: final short min = Short.MAX_VALUE; int num = -32638; num = num - min; System.out.println(num); L0 (0) LINENUMBER 3 L0 SIPUSH 32767 ISTORE 1: min L1 (3) LINENUMBER 4 L1 SIPUSH -32638 ISTORE 2: num L2 (6) LINENUMBER 5 L2 IINC 2: num -32767 L3 (8) LINENUMBER 6 L3 GETSTATIC System.out : PrintStream ILOAD 2: num INVOKEVIRTUAL PrintStream.println(int) : void L4 (12) LINENUMBER 7 L4 RETURN L5 (14) LOCALVARIABLE args String[] L0 L5 0 LOCALVARIABLE min short L1 L5 1 LOCALVARIABLE num int L2 L5 2 MAXSTACK = 2 MAXLOCALS = 3 Here the compiler optimizes correctly and uses IINC with Short.MAX_VALUE * -1. final int min = Short.MIN_VALUE; final int num = -32638; System.out.println(num - min); L0 (0) LINENUMBER 3 L0 SIPUSH -32768 ISTORE 1: min L1 (3) LINENUMBER 4 L1 SIPUSH -32638 ISTORE 2: num L2 (6) LINENUMBER 5 L2 GETSTATIC System.out : PrintStream SIPUSH 130 INVOKEVIRTUAL PrintStream.println(int) : void L3 (10) LINENUMBER 6 L3 RETURN L4 (12) LOCALVARIABLE args String[] L0 L4 0 LOCALVARIABLE min int L1 L4 1 LOCALVARIABLE num int L2 L4 2 MAXSTACK = 2 MAXLOCALS = 3 Another time, the compiler optimizes correctly and inserts the result (130) in the bytecode. final short min = Short.MIN_VALUE; int num = -32638; System.out.println(num - min); L0 (0) LINENUMBER 3 L0 SIPUSH -32768 ISTORE 1: min L1 (3) LINENUMBER 4 L1 SIPUSH -32638 ISTORE 2: num L2 (6) LINENUMBER 6 L2 GETSTATIC System.out : PrintStream ILOAD 2: num SIPUSH -32768 ISUB INVOKEVIRTUAL PrintStream.println(int) : void L3 (12) LINENUMBER 7 L3 RETURN L4 (14) LOCALVARIABLE args String[] L0 L4 0 LOCALVARIABLE min short L1 L4 1 LOCALVARIABLE num int L2 L4 2 MAXSTACK = 3 MAXLOCALS = 3 final int min = -32768; int num = -32638; int num2 = num - min; System.out.println(num2); L0 (0) LINENUMBER 3 L0 SIPUSH -32768 ISTORE 1: min L1 (3) LINENUMBER 4 L1 SIPUSH -32638 ISTORE 2: num L2 (6) LINENUMBER 5 L2 ILOAD 2: num SIPUSH -32768 ISUB ISTORE 3: num2 L3 (11) LINENUMBER 6 L3 GETSTATIC System.out : PrintStream ILOAD 3: num2 INVOKEVIRTUAL PrintStream.println(int) : void L4 (15) LINENUMBER 7 L4 RETURN L5 (17) LOCALVARIABLE args String[] L0 L5 0 LOCALVARIABLE min int L1 L5 1 LOCALVARIABLE num int L2 L5 2 LOCALVARIABLE num2 int L3 L5 3 MAXSTACK = 2 MAXLOCALS = 4 final int min = -32767; int num = -32638; num = num - min; System.out.println(num); L0 (0) LINENUMBER 3 L0 SIPUSH -32767 ISTORE 1: min L1 (3) LINENUMBER 4 L1 SIPUSH -32638 ISTORE 2: num L2 (6) LINENUMBER 5 L2 IINC 2: num 32767 L3 (8) LINENUMBER 6 L3 GETSTATIC System.out : PrintStream ILOAD 2: num INVOKEVIRTUAL PrintStream.println(int) : void L4 (12) LINENUMBER 7 L4 RETURN L5 (14) LOCALVARIABLE args String[] L0 L5 0 LOCALVARIABLE min int L1 L5 1 LOCALVARIABLE num int L2 L5 2 MAXSTACK = 2 MAXLOCALS = 3 You can also replace the short-keyword by int, the result stays the same. However, it works with long, byte and char as they use other operations - the only affected data types are short and int. Conclusion: Java optimizes additions incorrectly if someone subtracts a negative final value <= -32768 from a non-final value. The following conditions must hold: 1. The subtrahend must be final 2. The subtrahend must be negative 3. The subtrahend must be <=-32768 4. The minuend must be non-final 5. The result must be assigned to the minuend (either num = num – min or num -= min)
Reproduced with 3.4RC1. The problem is that we take iinc of -increment where increment is Short.MIN_VALUE.
Created attachment 100988 [details] Proposed fix This seems to fix the problem.
Created attachment 101716 [details] Proposed patch I would rather suggest this version of the fix. It doesn't single out Short.MIN_VALUE, and simply treats it as a 16bit representation problem. The problem was that for minus operator, we didn't consider -increment early enough for 16bit representation check.
Created attachment 101718 [details] Proposed patch Same patch augmented with a testcase
Added NumericTest#test005
Fix is quite trivial, and consequences are quite nasty from compiler standpoint (wrong arithmetics !?). Suggest we try to address it for 3.4RC3, and backport to 3.3.x.
Patch looks good. Pointed a small glitch it test case to Philippe (the testInt series of methods should declare an int), other than that +1.
Created attachment 101733 [details] Patch for 3.3.x 3.3.x patch, including Maxime's test correction
Patch looks good. Note that the following test looks unnecessary: (assignConstant.typeID() != TypeIds.T_float) // only for integral types && (assignConstant.typeID() != TypeIds.T_double) as this would have been caught before (one cannot assign a float or a double to an int local). But it is harmless and it could be removed post 3.4.
Yes, this extra test has always been in, and I agree it feels useless.
+1
Released for 3.4RC3. Fixed
Released in 3.3.x maintenance branch.
Verified for 3.4RC4 using build I20080530-0100
