Changeset 285452 in webkit

Timestamp:

Nov 8, 2021, 5:08:35 PM (5 years ago)

Author:

Kocsen Chung

Message:

Cherry-pick r282212. rdar://problem/85165960

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):

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@282212 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:

branches/safari-612-branch

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)

branches/safari-612-branch/JSTests/ChangeLog

@@ +1 @@
+2021-11-08  Kocsen Chung  <kocsen_chung@apple.com>
+
+        Cherry-pick r282212. rdar://problem/85165960
+
+    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):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@282212 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-11-04  Russell Epstein  <repstein@apple.com>
 

branches/safari-612-branch/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-11-08  Kocsen Chung  <kocsen_chung@apple.com>
+
+        Cherry-pick r282212. rdar://problem/85165960
+
+    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):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@282212 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-11-04  Russell Epstein  <repstein@apple.com>
 

branches/safari-612-branch/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

@@ -17858 +17858 @@
     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();
@@ -17874 +17885 @@
                 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));
                             
@@ -17880 +17891 @@
                             
                 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;
@@ -17916 +17927 @@
                             
                 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;
         }
@@ -17926 +17937 @@
         }
         
-        return intValue;
+        return valueAsInt32;
     }