JDK-4787943 : REGRESSION: Long shift produces incorrect results with -client
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 1.4.0_03,1.4.1
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: solaris,solaris_8
  • CPU: sparc
  • Submitted: 2002-12-03
  • Updated: 2003-02-07
  • Resolved: 2003-01-09
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.
Other Other Other
1.4.0_04 04Fixed 1.4.1_03Fixed 1.4.2Fixed
Description

Name: rmT116609			Date: 12/03/2002


FULL PRODUCT VERSION :
java version "1.4.1_01"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.1_01-b01)
Java HotSpot(TM) Client VM (build 1.4.1_01-b01, mixed mode)


FULL OPERATING SYSTEM VERSION :
SunOS lxor 5.8 Generic_108528-17 sun4u sparc SUNW,Ultra-5_10

A DESCRIPTION OF THE PROBLEM :
Shifting a long by a dynamically calculated integer value
produces the wrong answer when run with the java -client
command line option.

REGRESSION.  Last worked in version 1.3

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
1. javac LongCrash.java
2. java -client LongCrash
3.

EXPECTED VERSUS ACTUAL BEHAVIOR :
When the program works there should be no output (java
-server).

When the program fails (java -client), the output should be
similar to:

ERROR: sent: 80000000000003d7 got: ffffff00000003d7

The value at which the program fails does change, but it
consistently fails; I'm guessing it depends on when a
dynamic optimization occurs for the sizeAdj() method.  In my
environment the failure occurs almost immediately after
running the program.  I ran the working (-server) version
over a weekend and experienced no failures.

ERROR MESSAGES/STACK TRACES THAT OCCUR :
ERROR: sent: 80000000000003d7 got: ffffff00000003d7

This error is printed from within the test program when the arithmetic failure
is detected.  The expected is equal to the sent value.

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
/*
 * This example detects errant arithmetic in the sizeAdj() method.
 * The error occurs on Solaris Sparc JVM 1.4.0 and 1.4.1_01 when
 * running with the java -client command line argument.  This program
 * works as expected on 1.3.1_01.  The J2SE_Solaris_8_Recommended
 * patches were all installed.  This program also works on the 1.4
 * versions when run with the -server command line argument.
 *
 * The failure occurs in the long shifts in the sizeAdj() method.  The
 * error occurs for numbers sligthly larger than MIN_LONG and doesn't
 * always fail on the same number which might be related to how long
 * the program runs before the sizeAdj() method is dynamically
 * optimized.  The error is detected by checking the result of the
 * sizeAdj() method against the input value; they should be identical
 * since the shifts performed in sizeAdj() should always be by 0 since
 * the width argument is set to a constant 64 when sizeAdj() is
 * called.
 *
 * Tinkering with the program has showed that the error doesn't occur
 * when run with -server or if the sizeAdj() local variable "shiftMag"
 * is declared as long (causing the shift amount calculation to
 * produce a long instead of the integer value as coded) or if the
 * "shiftMag" variable declaration is left as an integer but moved
 * after the "ret" variable declaration.  The program will also work
 * if the "shiftMag" variable is removed and replaced with its only
 * value of the "width" variable, or if the shift amount is forced to
 * be a long value by using "64L" in the amount calculation.
 *
 * To reproduce the error:
 *
 * 1. javac LongCrash.java
 * 2. java -client LongCrash
 *
 * The output will be similar to:
 *
 * ERROR: sent: 80000000000003d7 got: ffffff00000003d7
 *
 */
public class LongCrash
{
    public static void main(String[] args)
    {
        while(true)
        {
            for(long i=0x7ffffffffffffff0L; i != 0x80000000000f0000L; i++)
            {
                long answer = 0L;
                
                if((answer = sizeAdj(i,64)) != i)
                {
                    System.out.println("ERROR: sent: " +
                                       Long.toHexString(i) + " got: " +
                                       Long.toHexString(answer));
                    System.exit(1);
                }
            }
        }
    }
    
    private static long sizeAdj (long value, int width)
    {
        // if this variable declaration moves after the next one it
        // works
        // if this variable declaration stays here but is made long it works
        int shiftMag = width;
        
        long ret = value;

        // if the shiftMag variable is replaced with width it works
        // if the constant 64 is replaced with 64L it works
        ret <<= (64 - shiftMag);
        ret >>= (64 - shiftMag);

        return ret;
    }
}

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

CUSTOMER WORKAROUND :
1. Use -server java command line option
2. Force long shifts to be by long types, not integers 
3. Allocate smaller primitive type local variables after long types.

Release Regression From : 1.3.1_06
The above release value was the last known release where this 
bug was known to work. Since then there has been a regression.

(Review ID: 178368) 
======================================================================

Comments
CONVERTED DATA BugTraq+ Release Management Values COMMIT TO FIX: 1.4.0_04 1.4.1_03 mantis-beta FIXED IN: 1.4.0_04 1.4.1_03 mantis-beta INTEGRATED IN: 1.4.0_04 1.4.1_03 mantis-beta
14-06-2004

SUGGESTED FIX An alternative suggested fix is to modify the peephole optimizer to not update the count register for long shifts. This would look like this: *** /tmp/geta19157 Wed Jan 29 15:44:45 2003 --- c1_LIROptimizer_sparc.cpp Wed Jan 29 15:44:44 2003 *************** *** 158,168 **** void LIR_Optimizer::emit_op2(LIR_Op2* op) { switch (op->code()) { case lir_logic_and: case lir_logic_or: case lir_logic_xor: - case lir_shl: - case lir_shr: case lir_add: case lir_sub: case lir_mul: { --- 158,178 ---- void LIR_Optimizer::emit_op2(LIR_Op2* op) { switch (op->code()) { + case lir_shl: + case lir_shr: { + op->_opr1 = maybe_opto(op->in_opr1(), _state.equivalent_register(op->in_opr1())); + // long shift op destroys the count register, so don't optimize in that case + if (op->in_opr1()->is_single_cpu()) { + op->_opr2 = maybe_opto(op->in_opr2(), _state.equivalent_register(op->in_opr2())); + } + result_substitute(); + op->visit(this); + break; + } + case lir_logic_and: case lir_logic_or: case lir_logic_xor: case lir_add: case lir_sub: case lir_mul: { ###@###.### 2003-01-29 fixed suggested fix above. The test should be on opr1 instead of opr2 since opr1 determines whether this is a long shift or not. ###@###.### 2003-02-04
29-01-2003

EVALUATION Long shifts destroy the count register but the peephole optimizer didn't understand this and so tried to substitute a better register. The fix is to propagate the destroyed flag through and check it in the LIROptimizer. The bug has existed since 1.4.0. ###@###.### 2002-12-03 i've added an alternative fix that would be acceptable on earlier releases. ###@###.### 2003-01-29
03-12-2002