[JSC] StringFromCharCode fast path should accept 0xff in DFG and FTL
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Mar 2019 01:53:35 +0000 (01:53 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Mar 2019 01:53:35 +0000 (01:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195429

Reviewed by Saam Barati.

JSTests:

* stress/must-handled-values-should-not-be-used-as-proven-constants-in-cfa.js: Added.
(foo):
* stress/string-from-char-code-255.js: Added.

Source/JavaScriptCore:

We can create single characters without allocation up to 0xff character code. But currently, DFGSpeculativeJIT and FTLLowerDFGToB3 go to the slow path
for 0xff case. On the other hand, DFG DoesGC phase says GC won't happen if the child is int32 constant and it is <= 0xff. So, if you have `String.fromCharCode(0xff)`,
this breaks the assumption in DFG DoesGC. The correct fix is changing the check in DFGSpeculativeJIT and FTLLowerDFGToB3 from AboveOrEqual to Above.
Note that ThunkGenerators's StringFromCharCode thunk was correct.

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileFromCharCode):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileStringFromCharCode):

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

JSTests/ChangeLog
JSTests/stress/must-handled-values-should-not-be-used-as-proven-constants-in-cfa.js [new file with mode: 0644]
JSTests/stress/string-from-char-code-255.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

index 2dea08f..bba5f1c 100644 (file)
@@ -1,3 +1,14 @@
+2019-03-07  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] StringFromCharCode fast path should accept 0xff in DFG and FTL
+        https://bugs.webkit.org/show_bug.cgi?id=195429
+
+        Reviewed by Saam Barati.
+
+        * stress/must-handled-values-should-not-be-used-as-proven-constants-in-cfa.js: Added.
+        (foo):
+        * stress/string-from-char-code-255.js: Added.
+
 2019-03-06  Mark Lam  <mark.lam@apple.com>
 
         Fix incorrect handling of try-finally completion values.
diff --git a/JSTests/stress/must-handled-values-should-not-be-used-as-proven-constants-in-cfa.js b/JSTests/stress/must-handled-values-should-not-be-used-as-proven-constants-in-cfa.js
new file mode 100644 (file)
index 0000000..c90febd
--- /dev/null
@@ -0,0 +1,17 @@
+//@ runDefault("--forceEagerCompilation=1")
+
+function foo() {
+    let array = [];
+    for (let a = 0; a < 4; a++) {
+        array[a + 1] = 0;
+    }
+    gc();
+    array.length=0;
+    gc();
+    var bar = 0;
+    for (var i = 0; i < 1000; i++) {
+        bar[0] = String.fromCharCode(i)[0];
+    }
+}
+
+foo();
diff --git a/JSTests/stress/string-from-char-code-255.js b/JSTests/stress/string-from-char-code-255.js
new file mode 100644 (file)
index 0000000..a631fb4
--- /dev/null
@@ -0,0 +1,4 @@
+for (var i = 0; i <= 1e6; ++i) {
+    if (String.fromCharCode(0xff) != '\u00ff')
+        throw new Error("out");
+}
index 2a75797..347a80a 100644 (file)
@@ -1,3 +1,20 @@
+2019-03-07  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] StringFromCharCode fast path should accept 0xff in DFG and FTL
+        https://bugs.webkit.org/show_bug.cgi?id=195429
+
+        Reviewed by Saam Barati.
+
+        We can create single characters without allocation up to 0xff character code. But currently, DFGSpeculativeJIT and FTLLowerDFGToB3 go to the slow path
+        for 0xff case. On the other hand, DFG DoesGC phase says GC won't happen if the child is int32 constant and it is <= 0xff. So, if you have `String.fromCharCode(0xff)`,
+        this breaks the assumption in DFG DoesGC. The correct fix is changing the check in DFGSpeculativeJIT and FTLLowerDFGToB3 from AboveOrEqual to Above.
+        Note that ThunkGenerators's StringFromCharCode thunk was correct.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileFromCharCode):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileStringFromCharCode):
+
 2019-03-07  Mark Lam  <mark.lam@apple.com>
 
         Follow up refactoring in try-finally code after r242591.
index 95041b4..2fae9ed 100644 (file)
@@ -2296,7 +2296,7 @@ void SpeculativeJIT::compileFromCharCode(Node* node)
     GPRReg smallStringsReg = smallStrings.gpr();
 
     JITCompiler::JumpList slowCases;
-    slowCases.append(m_jit.branch32(MacroAssembler::AboveOrEqual, propertyReg, TrustedImm32(maxSingleCharacterString)));
+    slowCases.append(m_jit.branch32(MacroAssembler::Above, propertyReg, TrustedImm32(maxSingleCharacterString)));
     m_jit.move(TrustedImmPtr(m_jit.vm()->smallStrings.singleCharacterStrings()), smallStringsReg);
     m_jit.loadPtr(MacroAssembler::BaseIndex(smallStringsReg, propertyReg, MacroAssembler::ScalePtr, 0), scratchReg);
 
index 9587df8..444e669 100644 (file)
@@ -6811,7 +6811,7 @@ private:
         LBasicBlock continuation = m_out.newBlock();
 
         m_out.branch(
-            m_out.aboveOrEqual(value, m_out.constInt32(maxSingleCharacterString)),
+            m_out.above(value, m_out.constInt32(maxSingleCharacterString)),
             rarely(slowCase), usually(smallIntCase));
 
         LBasicBlock lastNext = m_out.appendTo(smallIntCase, slowCase);