Changeset 95338 in webkit


Ignore:
Timestamp:
Sep 16, 2011 5:20:24 PM (13 years ago)
Author:
barraclough@apple.com
Message:

Unsigned bit shift fails under certain conditions in 32 bit builds
https://bugs.webkit.org/show_bug.cgi?id=68166

Reviewed by Geoff Garen.

Source/JavaScriptCore:

The major bug here is that the slow case (which handles shifts of
doubles) doesn't check for negative results from an unsigned shift
(which should be unsigned, and as such can't be represented by a
signed integer immediate). The implementation is also flawed for
shifts by negative shift amounts (treats as shift by zero).

  • jit/JITArithmetic32_64.cpp:

(JSC::JIT::emitRightShift):
(JSC::JIT::emitRightShiftSlowCase):

LayoutTests:

Added layout test.

  • fast/js/floating-point-truncate-rshift-expected.txt:
  • fast/js/floating-point-truncate-rshift.html:
Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r95337 r95338  
     12011-09-16  Gavin Barraclough  <barraclough@apple.com>
     2
     3        Unsigned bit shift fails under certain conditions in 32 bit builds
     4        https://bugs.webkit.org/show_bug.cgi?id=68166
     5
     6        Reviewed by Geoff Garen.
     7
     8        Added layout test.
     9
     10        * fast/js/floating-point-truncate-rshift-expected.txt:
     11        * fast/js/floating-point-truncate-rshift.html:
     12
    1132011-09-16  Ryosuke Niwa  <rniwa@webkit.org>
    214
  • trunk/LayoutTests/fast/js/floating-point-truncate-rshift-expected.txt

    r94622 r95338  
    11PASS
     2PASS
     3
  • trunk/LayoutTests/fast/js/floating-point-truncate-rshift.html

    r94622 r95338  
    11<body>
    22<script>
     3
    34if (window.layoutTestController)
    45    layoutTestController.dumpAsText();
     6
    57var largeNeg = -2715228072;
    68if ((largeNeg >>> 5) == 49366850)
    7     document.write("PASS");
     9    document.write("PASS<br>");
    810else
    911    document.write(largeNeg >>> 5);
     12
     13var lhs = -1699021058.1;
     14var rhs = 0;
     15if ((lhs >>> rhs) == 2595946238)
     16    document.write("PASS<br>");
     17else
     18    document.write(lhs >>> rhs);
     19
    1020</script>
    1121</body>
  • trunk/Source/JavaScriptCore/ChangeLog

    r95331 r95338  
     12011-09-16  Gavin Barraclough  <barraclough@apple.com>
     2
     3        Unsigned bit shift fails under certain conditions in 32 bit builds
     4        https://bugs.webkit.org/show_bug.cgi?id=68166
     5
     6        Reviewed by Geoff Garen.
     7
     8        The major bug here is that the slow case (which handles shifts of
     9        doubles) doesn't check for negative results from an unsigned shift
     10        (which should be unsigned, and as such can't be represented by a
     11        signed integer immediate).  The implementation is also flawed for
     12        shifts by negative shift amounts (treats as shift by zero).
     13
     14        * jit/JITArithmetic32_64.cpp:
     15        (JSC::JIT::emitRightShift):
     16        (JSC::JIT::emitRightShiftSlowCase):
     17
    1182011-09-16  Geoffrey Garen  <ggaren@apple.com>
    219
  • trunk/Source/JavaScriptCore/jit/JITArithmetic32_64.cpp

    r95324 r95338  
    217217        emitLoad(op1, regT1, regT0);
    218218        addSlowCase(branch32(NotEqual, regT1, TrustedImm32(JSValue::Int32Tag)));
    219         int shift = getConstantOperand(op2).asInt32();
     219        int shift = getConstantOperand(op2).asInt32() & 0x1f;
     220        if (shift) {
     221            if (isUnsigned)
     222                urshift32(Imm32(shift), regT0);
     223            else
     224                rshift32(Imm32(shift), regT0);
     225        } else if (isUnsigned) // signed right shift by zero is simply toInt conversion
     226            addSlowCase(branch32(LessThan, regT0, TrustedImm32(0)));
     227        emitStoreAndMapInt32(dst, regT1, regT0, dst == op1, OPCODE_LENGTH(op_rshift));
     228    } else {
     229        emitLoad2(op1, regT1, regT0, op2, regT3, regT2);
     230        if (!isOperandConstantImmediateInt(op1))
     231            addSlowCase(branch32(NotEqual, regT1, TrustedImm32(JSValue::Int32Tag)));
     232        addSlowCase(branch32(NotEqual, regT3, TrustedImm32(JSValue::Int32Tag)));
    220233        if (isUnsigned) {
    221             if (shift)
    222                 urshift32(Imm32(shift & 0x1f), regT0);
    223             // unsigned shift < 0 or shift = k*2^32 may result in (essentially)
    224             // a toUint conversion, which can result in a value we can represent
    225             // as an immediate int.
    226             if (shift < 0 || !(shift & 31))
    227                 addSlowCase(branch32(LessThan, regT0, TrustedImm32(0)));
    228         } else if (shift) { // signed right shift by zero is simply toInt conversion
    229             rshift32(Imm32(shift & 0x1f), regT0);
    230         }
     234            urshift32(regT2, regT0);
     235            addSlowCase(branch32(LessThan, regT0, TrustedImm32(0)));
     236        } else
     237            rshift32(regT2, regT0);
    231238        emitStoreAndMapInt32(dst, regT1, regT0, dst == op1, OPCODE_LENGTH(op_rshift));
    232         return;
    233     }
    234 
    235     emitLoad2(op1, regT1, regT0, op2, regT3, regT2);
    236     if (!isOperandConstantImmediateInt(op1))
    237         addSlowCase(branch32(NotEqual, regT1, TrustedImm32(JSValue::Int32Tag)));
    238     addSlowCase(branch32(NotEqual, regT3, TrustedImm32(JSValue::Int32Tag)));
    239     if (isUnsigned) {
    240         urshift32(regT2, regT0);
    241         addSlowCase(branch32(LessThan, regT0, TrustedImm32(0)));
    242     } else
    243         rshift32(regT2, regT0);
    244     emitStoreAndMapInt32(dst, regT1, regT0, dst == op1, OPCODE_LENGTH(op_rshift));
     239    }
    245240}
    246241
     
    251246    unsigned op2 = currentInstruction[3].u.operand;
    252247    if (isOperandConstantImmediateInt(op2)) {
    253         int shift = getConstantOperand(op2).asInt32();
     248        int shift = getConstantOperand(op2).asInt32() & 0x1f;
    254249        // op1 = regT1:regT0
    255250        linkSlowCase(iter); // int32 check
     
    259254            emitLoadDouble(op1, fpRegT0);
    260255            failures.append(branchTruncateDoubleToInt32(fpRegT0, regT0));
    261             if (isUnsigned) {
    262                 if (shift)
    263                     urshift32(Imm32(shift & 0x1f), regT0);
    264                 if (shift < 0 || !(shift & 31))
    265                     failures.append(branch32(LessThan, regT0, TrustedImm32(0)));
    266             } else if (shift)
    267                 rshift32(Imm32(shift & 0x1f), regT0);
     256            if (shift) {
     257                if (isUnsigned)
     258                    urshift32(Imm32(shift), regT0);
     259                else
     260                    rshift32(Imm32(shift), regT0);
     261            } else if (isUnsigned) // signed right shift by zero is simply toInt conversion
     262                failures.append(branch32(LessThan, regT0, TrustedImm32(0)));
    268263            move(TrustedImm32(JSValue::Int32Tag), regT1);
    269264            emitStoreInt32(dst, regT0, false);
     
    271266            failures.link(this);
    272267        }
    273         if (isUnsigned && (shift < 0 || !(shift & 31)))
     268        if (isUnsigned && !shift)
    274269            linkSlowCase(iter); // failed to box in hot path
    275270    } else {
     
    279274            linkSlowCase(iter); // int32 check -- op1 is not an int
    280275            if (supportsFloatingPointTruncate()) {
    281                 Jump notDouble = branch32(Above, regT1, TrustedImm32(JSValue::LowestTag)); // op1 is not a double
     276                JumpList failures;
     277                failures.append(branch32(Above, regT1, TrustedImm32(JSValue::LowestTag))); // op1 is not a double
    282278                emitLoadDouble(op1, fpRegT0);
    283                 Jump notInt = branch32(NotEqual, regT3, TrustedImm32(JSValue::Int32Tag)); // op2 is not an int
    284                 Jump cantTruncate = branchTruncateDoubleToInt32(fpRegT0, regT0);
    285                 if (isUnsigned)
     279                failures.append(branch32(NotEqual, regT3, TrustedImm32(JSValue::Int32Tag))); // op2 is not an int
     280                failures.append(branchTruncateDoubleToInt32(fpRegT0, regT0));
     281                if (isUnsigned) {
    286282                    urshift32(regT2, regT0);
    287                 else
     283                    failures.append(branch32(LessThan, regT0, TrustedImm32(0)));
     284                } else
    288285                    rshift32(regT2, regT0);
    289286                move(TrustedImm32(JSValue::Int32Tag), regT1);
    290287                emitStoreInt32(dst, regT0, false);
    291288                emitJumpSlowToHot(jump(), OPCODE_LENGTH(op_rshift));
    292                 notDouble.link(this);
    293                 notInt.link(this);
    294                 cantTruncate.link(this);
     289                failures.link(this);
    295290            }
    296291        }
Note: See TracChangeset for help on using the changeset viewer.