projects / profile / ivi / webkit-efl.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 243928f)
Unsigned bit shift fails under certain conditions in 32 bit builds
authorbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 17 Sep 2011 00:20:24 +0000 (00:20 +0000)
committerbarraclough@apple.com <barraclough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 17 Sep 2011 00:20:24 +0000 (00:20 +0000)
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:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@95338 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog patch | blob | history
LayoutTests/fast/js/floating-point-truncate-rshift-expected.txt patch | blob | history
LayoutTests/fast/js/floating-point-truncate-rshift.html patch | blob | history
Source/JavaScriptCore/ChangeLog patch | blob | history
Source/JavaScriptCore/jit/JITArithmetic32_64.cpp patch | blob | history

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index fcf9fa4..0dd4863 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2011-09-16  Gavin Barraclough  <barraclough@apple.com>
+
+        Unsigned bit shift fails under certain conditions in 32 bit builds
+        https://bugs.webkit.org/show_bug.cgi?id=68166
+
+        Reviewed by Geoff Garen.
+
+        Added layout test.
+
+        * fast/js/floating-point-truncate-rshift-expected.txt:
+        * fast/js/floating-point-truncate-rshift.html:
+
 2011-09-16  Ryosuke Niwa  <rniwa@webkit.org>
 
         Add the forgotten test, also suppress a test in Chromium as it needs a rebaseline.
diff --git a/LayoutTests/fast/js/floating-point-truncate-rshift.html b/LayoutTests/fast/js/floating-point-truncate-rshift.html
index d2d0625..3074663 100644 (file)
--- a/LayoutTests/fast/js/floating-point-truncate-rshift.html
+++ b/LayoutTests/fast/js/floating-point-truncate-rshift.html
@@ -1,11 +1,21 @@
 <body>
 <script>
+
 if (window.layoutTestController)
     layoutTestController.dumpAsText();
+
 var largeNeg = -2715228072;
 if ((largeNeg >>> 5) == 49366850)
-    document.write("PASS");
+    document.write("PASS<br>");
 else
     document.write(largeNeg >>> 5);
+
+var lhs = -1699021058.1;
+var rhs = 0;
+if ((lhs >>> rhs) == 2595946238)
+    document.write("PASS<br>");
+else
+    document.write(lhs >>> rhs);
+
 </script>
 </body>
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index bd1516b..9041867 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,20 @@
+2011-09-16  Gavin Barraclough  <barraclough@apple.com>
+
+        Unsigned bit shift fails under certain conditions in 32 bit builds
+        https://bugs.webkit.org/show_bug.cgi?id=68166
+
+        Reviewed by Geoff Garen.
+
+        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):
+
 2011-09-16  Geoffrey Garen  <ggaren@apple.com>
 
         Removed undetectable style.filter.
diff --git a/Source/JavaScriptCore/jit/JITArithmetic32_64.cpp b/Source/JavaScriptCore/jit/JITArithmetic32_64.cpp
index 3c5569f..c9a143d 100644 (file)
--- a/Source/JavaScriptCore/jit/JITArithmetic32_64.cpp
+++ b/Source/JavaScriptCore/jit/JITArithmetic32_64.cpp
@@ -216,32 +216,27 @@ void JIT::emitRightShift(Instruction* currentInstruction, bool isUnsigned)
     if (isOperandConstantImmediateInt(op2)) {
         emitLoad(op1, regT1, regT0);
         addSlowCase(branch32(NotEqual, regT1, TrustedImm32(JSValue::Int32Tag)));
-        int shift = getConstantOperand(op2).asInt32();
+        int shift = getConstantOperand(op2).asInt32() & 0x1f;
+        if (shift) {
+            if (isUnsigned)
+                urshift32(Imm32(shift), regT0);
+            else
+                rshift32(Imm32(shift), regT0);
+        } else if (isUnsigned) // signed right shift by zero is simply toInt conversion
+            addSlowCase(branch32(LessThan, regT0, TrustedImm32(0)));
+        emitStoreAndMapInt32(dst, regT1, regT0, dst == op1, OPCODE_LENGTH(op_rshift));
+    } else {
+        emitLoad2(op1, regT1, regT0, op2, regT3, regT2);
+        if (!isOperandConstantImmediateInt(op1))
+            addSlowCase(branch32(NotEqual, regT1, TrustedImm32(JSValue::Int32Tag)));
+        addSlowCase(branch32(NotEqual, regT3, TrustedImm32(JSValue::Int32Tag)));
         if (isUnsigned) {
-            if (shift)
-                urshift32(Imm32(shift & 0x1f), regT0);
-            // unsigned shift < 0 or shift = k*2^32 may result in (essentially)
-            // a toUint conversion, which can result in a value we can represent
-            // as an immediate int.
-            if (shift < 0 || !(shift & 31))
-                addSlowCase(branch32(LessThan, regT0, TrustedImm32(0)));
-        } else if (shift) { // signed right shift by zero is simply toInt conversion
-            rshift32(Imm32(shift & 0x1f), regT0);
-        }
+            urshift32(regT2, regT0);
+            addSlowCase(branch32(LessThan, regT0, TrustedImm32(0)));
+        } else
+            rshift32(regT2, regT0);
         emitStoreAndMapInt32(dst, regT1, regT0, dst == op1, OPCODE_LENGTH(op_rshift));
-        return;
     }
-
-    emitLoad2(op1, regT1, regT0, op2, regT3, regT2);
-    if (!isOperandConstantImmediateInt(op1))
-        addSlowCase(branch32(NotEqual, regT1, TrustedImm32(JSValue::Int32Tag)));
-    addSlowCase(branch32(NotEqual, regT3, TrustedImm32(JSValue::Int32Tag)));
-    if (isUnsigned) {
-        urshift32(regT2, regT0);
-        addSlowCase(branch32(LessThan, regT0, TrustedImm32(0)));
-    } else
-        rshift32(regT2, regT0);
-    emitStoreAndMapInt32(dst, regT1, regT0, dst == op1, OPCODE_LENGTH(op_rshift));
 }
 
 void JIT::emitRightShiftSlowCase(Instruction* currentInstruction, Vector<SlowCaseEntry>::iterator& iter, bool isUnsigned)
@@ -250,7 +245,7 @@ void JIT::emitRightShiftSlowCase(Instruction* currentInstruction, Vector<SlowCas
     unsigned op1 = currentInstruction[2].u.operand;
     unsigned op2 = currentInstruction[3].u.operand;
     if (isOperandConstantImmediateInt(op2)) {
-        int shift = getConstantOperand(op2).asInt32();
+        int shift = getConstantOperand(op2).asInt32() & 0x1f;
         // op1 = regT1:regT0
         linkSlowCase(iter); // int32 check
         if (supportsFloatingPointTruncate()) {
@@ -258,19 +253,19 @@ void JIT::emitRightShiftSlowCase(Instruction* currentInstruction, Vector<SlowCas
             failures.append(branch32(AboveOrEqual, regT1, TrustedImm32(JSValue::LowestTag)));
             emitLoadDouble(op1, fpRegT0);
             failures.append(branchTruncateDoubleToInt32(fpRegT0, regT0));
-            if (isUnsigned) {
-                if (shift)
-                    urshift32(Imm32(shift & 0x1f), regT0);
-                if (shift < 0 || !(shift & 31))
-                    failures.append(branch32(LessThan, regT0, TrustedImm32(0)));
-            } else if (shift)
-                rshift32(Imm32(shift & 0x1f), regT0);
+            if (shift) {
+                if (isUnsigned)
+                    urshift32(Imm32(shift), regT0);
+                else
+                    rshift32(Imm32(shift), regT0);
+            } else if (isUnsigned) // signed right shift by zero is simply toInt conversion
+                failures.append(branch32(LessThan, regT0, TrustedImm32(0)));
             move(TrustedImm32(JSValue::Int32Tag), regT1);
             emitStoreInt32(dst, regT0, false);
             emitJumpSlowToHot(jump(), OPCODE_LENGTH(op_rshift));
             failures.link(this);
         }
-        if (isUnsigned && (shift < 0 || !(shift & 31)))
+        if (isUnsigned && !shift)
             linkSlowCase(iter); // failed to box in hot path
     } else {
         // op1 = regT1:regT0
@@ -278,20 +273,20 @@ void JIT::emitRightShiftSlowCase(Instruction* currentInstruction, Vector<SlowCas
         if (!isOperandConstantImmediateInt(op1)) {
             linkSlowCase(iter); // int32 check -- op1 is not an int
             if (supportsFloatingPointTruncate()) {
-                Jump notDouble = branch32(Above, regT1, TrustedImm32(JSValue::LowestTag)); // op1 is not a double
+                JumpList failures;
+                failures.append(branch32(Above, regT1, TrustedImm32(JSValue::LowestTag))); // op1 is not a double
                 emitLoadDouble(op1, fpRegT0);
-                Jump notInt = branch32(NotEqual, regT3, TrustedImm32(JSValue::Int32Tag)); // op2 is not an int
-                Jump cantTruncate = branchTruncateDoubleToInt32(fpRegT0, regT0);
-                if (isUnsigned)
+                failures.append(branch32(NotEqual, regT3, TrustedImm32(JSValue::Int32Tag))); // op2 is not an int
+                failures.append(branchTruncateDoubleToInt32(fpRegT0, regT0));
+                if (isUnsigned) {
                     urshift32(regT2, regT0);
-                else
+                    failures.append(branch32(LessThan, regT0, TrustedImm32(0)));
+                } else
                     rshift32(regT2, regT0);
                 move(TrustedImm32(JSValue::Int32Tag), regT1);
                 emitStoreInt32(dst, regT0, false);
                 emitJumpSlowToHot(jump(), OPCODE_LENGTH(op_rshift));
-                notDouble.link(this);
-                notInt.link(this);
-                cantTruncate.link(this);
+                failures.link(this);
             }
         }
 
Domain: Web Framework / Uncategorized;