Changeset 282212 in webkit

Timestamp:

Sep 9, 2021, 8:30:01 AM (5 years ago)

Author:

Justin Michaud

Message:

Differential testing: incorrect constant propagation around Uint8ClampedArray
​https://bugs.webkit.org/show_bug.cgi?id=229869

JSTests:

Reviewed by Saam Barati.

• stress/Uint8ClampedArrayClampsInt52Positive.js: Added.

(let.x.123.test):
(noInline.test.int32pos1):
(255.int32pos2):
(1.int32neg1):
(0.int32neg2):
(0.int52pos1):
(255.int52pos2):
(255.int52neg1):
(0.int52neg2):
(0.int52neg3):
(0.int52pos3):
(255.int8):

Source/JavaScriptCore:

We casted int52 values to int32 before clamping, which caused any value with the 32nd bit
set to be interpreted as negative. The fix is to check the full-size value when deciding to clamp.

Reviewed by Saam Barati.

• ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/Uint8ClampedArrayClampsInt52Positive.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (modified) (5 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2021-09-09  Justin Michaud  <justin_michaud@apple.com>
+
+        Differential testing: incorrect constant propagation around Uint8ClampedArray
+        https://bugs.webkit.org/show_bug.cgi?id=229869
+
+        Reviewed by Saam Barati.
+
+        * stress/Uint8ClampedArrayClampsInt52Positive.js: Added.
+        (let.x.123.test):
+        (noInline.test.int32pos1):
+        (255.int32pos2):
+        (1.int32neg1):
+        (0.int32neg2):
+        (0.int52pos1):
+        (255.int52pos2):
+        (255.int52neg1):
+        (0.int52neg2):
+        (0.int52neg3):
+        (0.int52pos3):
+        (255.int8):
+
 2021-09-09  Robin Morisset  <rmorisset@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-09-09  Justin Michaud  <justin_michaud@apple.com>
+
+        Differential testing: incorrect constant propagation around Uint8ClampedArray
+        https://bugs.webkit.org/show_bug.cgi?id=229869
+
+        We casted int52 values to int32 before clamping, which caused any value with the 32nd bit
+        set to be interpreted as negative. The fix is to check the full-size value when deciding to clamp.
+
+        Reviewed by Saam Barati.
+
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
+
 2021-09-09  Robin Morisset  <rmorisset@apple.com>
 

trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

@@ -17798 +17798 @@
     LValue getIntTypedArrayStoreOperand(Edge edge, bool isClamped = false)
     {
-        LValue intValue;
+        LValue valueAsInt32;
+        LValue value;
+        LValue zero;
+        LValue byteMax;
+        
         switch (edge.useKind()) {
         case Int52RepUse:
         case Int32Use: {
-            if (edge.useKind() == Int32Use)
-                intValue = lowInt32(edge);
-            else
-                intValue = m_out.castToInt32(lowStrictInt52(edge));
-
+            if (edge.useKind() == Int32Use) {
+                value = lowInt32(edge);
+                valueAsInt32 = value;
+                zero = m_out.int32Zero;
+                byteMax = m_out.constInt32(255);
+            } else {
+                value = lowStrictInt52(edge);
+                valueAsInt32 = m_out.castToInt32(value);
+                zero = m_out.int64Zero;
+                byteMax = m_out.constInt64(255);
+            }
+            
             if (isClamped) {
                 LBasicBlock atLeastZero = m_out.newBlock();
@@ -17814 +17825 @@
                 intValues.append(m_out.anchor(m_out.int32Zero));
                 m_out.branch(
-                    m_out.lessThan(intValue, m_out.int32Zero),
+                    m_out.lessThan(value, zero),
                     unsure(continuation), unsure(atLeastZero));
                             
@@ -17820 +17831 @@
                             
                 intValues.append(m_out.anchor(m_out.select(
-                    m_out.greaterThan(intValue, m_out.constInt32(255)),
+                    m_out.greaterThan(value, byteMax),
                     m_out.constInt32(255),
-                    intValue)));
+                    valueAsInt32)));
                 m_out.jump(continuation);
                             
                 m_out.appendTo(continuation, lastNext);
-                intValue = m_out.phi(Int32, intValues);
+                valueAsInt32 = m_out.phi(Int32, intValues);
             }
             break;
@@ -17856 +17867 @@
                             
                 m_out.appendTo(continuation, lastNext);
-                intValue = m_out.phi(Int32, intValues);
+                valueAsInt32 = m_out.phi(Int32, intValues);
             } else
-                intValue = doubleToInt32(doubleValue);
+                valueAsInt32 = doubleToInt32(doubleValue);
             break;
         }
@@ -17866 +17877 @@
         }
         
-        return intValue;
+        return valueAsInt32;
     }