[INTL] Implement String.prototype.localeCompare in ECMA-402
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 24 Apr 2016 17:05:51 +0000 (17:05 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 24 Apr 2016 17:05:51 +0000 (17:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=147607

Patch by Filip Pizlo <fpizlo@apple.com> and Andy VanWagoner <thetalecrafter@gmail.com> on 2016-04-24
Reviewed by Darin Adler.
Source/JavaScriptCore:

Part of this change is just rolling 194394 back in.

The other part is making that not a regression on CDjs. Other than the fact that it uses
bound functions, the problem with this new localeCompare implementation is that it uses
the arguments object. It uses it in a way that *seems* like ArgumentsEliminationPhase
ought to handle, but to my surprise it didn't:

- If we have a ForceExit GetByVal on the arguments object, we would previously assume that
  it escaped. That's false since we just exit at ForceExit. On the other hand we probably
  should be pruning unreachable paths before we get here, but that's a separate issue. I
  don't want to play with phase order right now.

- If we have a OutOfBounds GetByVal on the arguments object, then the best that would
  previously happen is that we'd compile it into an in-bounds arguments access. That's quite
  bad, as Andy's localeCompare illustrates: it uses out-of-bounds access on the arguments
  object to detect if an argument was passed. This change introduces an OutOfBounds version
  of GetMyArgumentByVal for this purpose.

This change required registering sane chain watchpoints. In the process, I noticed that the
old way of doing it had a race condition: we might register watchpoints for the structure
that had become insane. This change introduces a double-checking idiom that I believe works
because once the structure becomes insane it can't go back to sane and watchpoints
registration already involves executing the hardest possible fences.

* builtins/StringPrototype.js:
(repeat):
(localeCompare):
(search):
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGArgumentsEliminationPhase.cpp:
* dfg/DFGArrayMode.cpp:
(JSC::DFG::ArrayMode::refine):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGConstantFoldingPhase.cpp:
(JSC::DFG::ConstantFoldingPhase::foldConstants):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGNodeType.h:
* dfg/DFGPreciseLocalClobberize.h:
(JSC::DFG::PreciseLocalClobberizeAdaptor::readTop):
* dfg/DFGPredictionPropagationPhase.cpp:
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileGetByValOnString):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGValidate.cpp:
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):
(JSC::FTL::DFG::LowerDFGToB3::compilePutByVal):
(JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
* ftl/FTLTypedPointer.h:
(JSC::FTL::TypedPointer::TypedPointer):
(JSC::FTL::TypedPointer::operator bool):
(JSC::FTL::TypedPointer::heap):
(JSC::FTL::TypedPointer::operator!): Deleted.
* runtime/StringPrototype.cpp:
(JSC::StringPrototype::finishCreation):

LayoutTests:

* js/dom/script-tests/string-prototype-properties.js:
* js/dom/string-prototype-properties-expected.txt:
* js/regress/locale-compare.html: Added.
* js/regress/locale-compare-expected.txt: Added.
* js/regress/scripts-tests/locale-compare.js: Added.
* js/script-tests/string-localeCompare.js:
* js/string-localeCompare-expected.txt:
* js/string-localeCompare.html:

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

32 files changed:
LayoutTests/ChangeLog
LayoutTests/js/dom/script-tests/string-prototype-properties.js
LayoutTests/js/dom/string-prototype-properties-expected.txt
LayoutTests/js/regress/locale-compare-expected.txt [new file with mode: 0644]
LayoutTests/js/regress/locale-compare.html [new file with mode: 0644]
LayoutTests/js/regress/script-tests/locale-compare.js [new file with mode: 0644]
LayoutTests/js/regress/sortamorphic-load-expected.txt [new file with mode: 0644]
LayoutTests/js/regress/sortamorphic-load.html [new file with mode: 0644]
LayoutTests/js/script-tests/string-localeCompare.js
LayoutTests/js/string-localeCompare-expected.txt
LayoutTests/js/string-localeCompare.html
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/builtins/StringPrototype.js
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp
Source/JavaScriptCore/dfg/DFGArrayMode.cpp
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp
Source/JavaScriptCore/dfg/DFGDoesGC.cpp
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGNodeType.h
Source/JavaScriptCore/dfg/DFGPreciseLocalClobberize.h
Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
Source/JavaScriptCore/dfg/DFGSafeToExecute.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Source/JavaScriptCore/dfg/DFGValidate.cpp
Source/JavaScriptCore/ftl/FTLCapabilities.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/ftl/FTLTypedPointer.h
Source/JavaScriptCore/runtime/StringPrototype.cpp

index 8ff35f5..1c389e8 100644 (file)
@@ -1,3 +1,19 @@
+2016-04-24  Filip Pizlo <fpizlo@apple.com> and Andy VanWagoner <thetalecrafter@gmail.com>
+
+        [INTL] Implement String.prototype.localeCompare in ECMA-402
+        https://bugs.webkit.org/show_bug.cgi?id=147607
+
+        Reviewed by Darin Adler.
+
+        * js/dom/script-tests/string-prototype-properties.js:
+        * js/dom/string-prototype-properties-expected.txt:
+        * js/regress/locale-compare.html: Added.
+        * js/regress/locale-compare-expected.txt: Added.
+        * js/regress/scripts-tests/locale-compare.js: Added.
+        * js/script-tests/string-localeCompare.js:
+        * js/string-localeCompare-expected.txt:
+        * js/string-localeCompare.html:
+
 2016-04-22  Filip Pizlo  <fpizlo@apple.com>
 
         Speed up bound functions a bit
index 019b5c6..24051db 100644 (file)
@@ -20,7 +20,7 @@ shouldThrow("String.prototype.substr.call(undefined, 1, 3)");
 shouldThrow("String.prototype.substring.call(undefined, 1, 3)");
 shouldThrow("String.prototype.toLowerCase.call(undefined)");
 shouldThrow("String.prototype.toUpperCase.call(undefined)");
-shouldThrow("String.prototype.localeCompare.call(undefined, '1224')");
+shouldThrow("String.prototype.localeCompare.call(undefined, '1224')", "'TypeError: String.prototype.localeCompare requires that |this| not be undefined'");
 shouldThrow("String.prototype.toLocaleLowerCase.call(undefined)");
 shouldThrow("String.prototype.toLocaleUpperCase.call(undefined)");
 shouldThrow("String.prototype.trim.call(undefined)");
index fe61241..ef186df 100644 (file)
@@ -20,7 +20,7 @@ PASS String.prototype.substr.call(undefined, 1, 3) threw exception TypeError: Ty
 PASS String.prototype.substring.call(undefined, 1, 3) threw exception TypeError: Type error.
 PASS String.prototype.toLowerCase.call(undefined) threw exception TypeError: Type error.
 PASS String.prototype.toUpperCase.call(undefined) threw exception TypeError: Type error.
-PASS String.prototype.localeCompare.call(undefined, '1224') threw exception TypeError: Type error.
+PASS String.prototype.localeCompare.call(undefined, '1224') threw exception TypeError: String.prototype.localeCompare requires that |this| not be undefined.
 PASS String.prototype.toLocaleLowerCase.call(undefined) threw exception TypeError: Type error.
 PASS String.prototype.toLocaleUpperCase.call(undefined) threw exception TypeError: Type error.
 PASS String.prototype.trim.call(undefined) threw exception TypeError: Type error.
diff --git a/LayoutTests/js/regress/locale-compare-expected.txt b/LayoutTests/js/regress/locale-compare-expected.txt
new file mode 100644 (file)
index 0000000..c11dc9a
--- /dev/null
@@ -0,0 +1,10 @@
+JSRegress/locale-compare
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS no exception thrown
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/js/regress/locale-compare.html b/LayoutTests/js/regress/locale-compare.html
new file mode 100644 (file)
index 0000000..6124920
--- /dev/null
@@ -0,0 +1,12 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="../../resources/regress-pre.js"></script>
+<script src="script-tests/locale-compare.js"></script>
+<script src="../../resources/regress-post.js"></script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/js/regress/script-tests/locale-compare.js b/LayoutTests/js/regress/script-tests/locale-compare.js
new file mode 100644 (file)
index 0000000..e7818c9
--- /dev/null
@@ -0,0 +1,10 @@
+(function(a, b) {
+    var n = 200000;
+    var result = 0;
+    for (var i = 0; i < n; ++i) {
+        result += a.localeCompare(b);
+    }
+    if (result != n)
+        throw "Error: bad result: " + result;
+})("yes", "no");
+
diff --git a/LayoutTests/js/regress/sortamorphic-load-expected.txt b/LayoutTests/js/regress/sortamorphic-load-expected.txt
new file mode 100644 (file)
index 0000000..a4f3c2d
--- /dev/null
@@ -0,0 +1,10 @@
+JSRegress/sortamorphic-load
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS no exception thrown
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/js/regress/sortamorphic-load.html b/LayoutTests/js/regress/sortamorphic-load.html
new file mode 100644 (file)
index 0000000..579d9c1
--- /dev/null
@@ -0,0 +1,12 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="../../resources/regress-pre.js"></script>
+<script src="script-tests/sortamorphic-load.js"></script>
+<script src="../../resources/regress-post.js"></script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index ded7a4c..d50fdf1 100644 (file)
@@ -1,5 +1,28 @@
-description("This test checks String.localeCompare().");
+description("This test checks the behavior of String.prototype.localeCompare as described in the ECMAScript Internationalization API Specification (ECMA-402 2.0).");
 
+shouldBe("String.prototype.localeCompare.length", "1");
+shouldBeFalse("Object.getOwnPropertyDescriptor(String.prototype, 'localeCompare').enumerable");
+shouldBeTrue("Object.getOwnPropertyDescriptor(String.prototype, 'localeCompare').configurable");
+shouldBeTrue("Object.getOwnPropertyDescriptor(String.prototype, 'localeCompare').writable");
+
+// Test RequireObjectCoercible.
+shouldThrow("String.prototype.localeCompare.call()", "'TypeError: String.prototype.localeCompare requires that |this| not be undefined'");
+shouldThrow("String.prototype.localeCompare.call(undefined)", "'TypeError: String.prototype.localeCompare requires that |this| not be undefined'");
+shouldThrow("String.prototype.localeCompare.call(null)", "'TypeError: String.prototype.localeCompare requires that |this| not be null'");
+shouldNotThrow("String.prototype.localeCompare.call({}, '')");
+shouldNotThrow("String.prototype.localeCompare.call([], '')");
+shouldNotThrow("String.prototype.localeCompare.call(NaN, '')");
+shouldNotThrow("String.prototype.localeCompare.call(5, '')");
+shouldNotThrow("String.prototype.localeCompare.call('', '')");
+shouldNotThrow("String.prototype.localeCompare.call(() => {}, '')");
+
+// Test toString fails.
+shouldThrow("''.localeCompare.call({ toString() { throw 'thisFail' } }, '')", "'thisFail'");
+shouldThrow("''.localeCompare({ toString() { throw 'thatFail' } })", "'thatFail'");
+shouldNotThrow("''.localeCompare()");
+shouldNotThrow("''.localeCompare(null)");
+
+// Basic support.
 shouldBeTrue('"a".localeCompare("aa") < 0');
 shouldBeTrue('"a".localeCompare("b") < 0');
 
@@ -8,3 +31,45 @@ shouldBeTrue('"a\u0308\u0323".localeCompare("a\u0323\u0308") === 0');
 
 shouldBeTrue('"aa".localeCompare("a") > 0');
 shouldBeTrue('"b".localeCompare("a") > 0');
+
+// Uses Intl.Collator.
+shouldThrow("'a'.localeCompare('b', '$')", "'RangeError: invalid language tag: $'");
+shouldThrow("'a'.localeCompare('b', 'en', {usage: 'Sort'})", '\'RangeError: usage must be either "sort" or "search"\'');
+
+shouldBe("'ä'.localeCompare('z', 'en')", "-1");
+shouldBe("'ä'.localeCompare('z', 'sv')", "1");
+
+shouldBe("'a'.localeCompare('b', 'en', { sensitivity:'base' })", "-1");
+shouldBe("'a'.localeCompare('ä', 'en', { sensitivity:'base' })", "0");
+shouldBe("'a'.localeCompare('A', 'en', { sensitivity:'base' })", "0");
+shouldBe("'a'.localeCompare('ⓐ', 'en', { sensitivity:'base' })", "0");
+
+shouldBe("'a'.localeCompare('b', 'en', { sensitivity:'accent' })", "-1");
+shouldBe("'a'.localeCompare('ä', 'en', { sensitivity:'accent' })", "-1");
+shouldBe("'a'.localeCompare('A', 'en', { sensitivity:'accent' })", "0");
+shouldBe("'a'.localeCompare('ⓐ', 'en', { sensitivity:'accent' })", "0");
+
+shouldBe("'a'.localeCompare('b', 'en', { sensitivity:'case' })", "-1");
+shouldBe("'a'.localeCompare('ä', 'en', { sensitivity:'case' })", "0");
+shouldBe("'a'.localeCompare('A', 'en', { sensitivity:'case' })", "-1");
+shouldBe("'a'.localeCompare('ⓐ', 'en', { sensitivity:'case' })", "0");
+
+shouldBe("'a'.localeCompare('b', 'en', { sensitivity:'variant' })", "-1");
+shouldBe("'a'.localeCompare('ä', 'en', { sensitivity:'variant' })", "-1");
+shouldBe("'a'.localeCompare('A', 'en', { sensitivity:'variant' })", "-1");
+shouldBe("'a'.localeCompare('ⓐ', 'en', { sensitivity:'variant' })", "-1");
+
+shouldBe("'1'.localeCompare('2', 'en', { numeric:false })", "-1");
+shouldBe("'2'.localeCompare('10', 'en', { numeric:false })", "1");
+shouldBe("'01'.localeCompare('1', 'en', { numeric:false })", "-1");
+shouldBe("'๑'.localeCompare('๒', 'en', { numeric:false })", "-1");
+shouldBe("'๒'.localeCompare('๑๐', 'en', { numeric:false })", "1");
+shouldBe("'๐๑'.localeCompare('๑', 'en', { numeric:false })", "-1");
+
+shouldBe("'1'.localeCompare('2', 'en', { numeric:true })", "-1");
+shouldBe("'2'.localeCompare('10', 'en', { numeric:true })", "-1");
+shouldBe("'01'.localeCompare('1', 'en', { numeric:true })", "0");
+shouldBe("'๑'.localeCompare('๒', 'en', { numeric:true })", "-1");
+shouldBe("'๒'.localeCompare('๑๐', 'en', { numeric:true })", "-1");
+shouldBe("'๐๑'.localeCompare('๑', 'en', { numeric:true })", "0");
+
index 5b0e2ac..d98e2ae 100644 (file)
@@ -1,14 +1,63 @@
-This test checks String.localeCompare().
+This test checks the behavior of String.prototype.localeCompare as described in the ECMAScript Internationalization API Specification (ECMA-402 2.0).
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
+PASS String.prototype.localeCompare.length is 1
+PASS Object.getOwnPropertyDescriptor(String.prototype, 'localeCompare').enumerable is false
+PASS Object.getOwnPropertyDescriptor(String.prototype, 'localeCompare').configurable is true
+PASS Object.getOwnPropertyDescriptor(String.prototype, 'localeCompare').writable is true
+PASS String.prototype.localeCompare.call() threw exception TypeError: String.prototype.localeCompare requires that |this| not be undefined.
+PASS String.prototype.localeCompare.call(undefined) threw exception TypeError: String.prototype.localeCompare requires that |this| not be undefined.
+PASS String.prototype.localeCompare.call(null) threw exception TypeError: String.prototype.localeCompare requires that |this| not be null.
+PASS String.prototype.localeCompare.call({}, '') did not throw exception.
+PASS String.prototype.localeCompare.call([], '') did not throw exception.
+PASS String.prototype.localeCompare.call(NaN, '') did not throw exception.
+PASS String.prototype.localeCompare.call(5, '') did not throw exception.
+PASS String.prototype.localeCompare.call('', '') did not throw exception.
+PASS String.prototype.localeCompare.call(() => {}, '') did not throw exception.
+PASS ''.localeCompare.call({ toString() { throw 'thisFail' } }, '') threw exception thisFail.
+PASS ''.localeCompare({ toString() { throw 'thatFail' } }) threw exception thatFail.
+PASS ''.localeCompare() did not throw exception.
+PASS ''.localeCompare(null) did not throw exception.
 PASS "a".localeCompare("aa") < 0 is true
 PASS "a".localeCompare("b") < 0 is true
 PASS "a".localeCompare("a") === 0 is true
 PASS "ạ̈".localeCompare("ạ̈") === 0 is true
 PASS "aa".localeCompare("a") > 0 is true
 PASS "b".localeCompare("a") > 0 is true
+PASS 'a'.localeCompare('b', '$') threw exception RangeError: invalid language tag: $.
+PASS 'a'.localeCompare('b', 'en', {usage: 'Sort'}) threw exception RangeError: usage must be either "sort" or "search".
+PASS 'ä'.localeCompare('z', 'en') is -1
+PASS 'ä'.localeCompare('z', 'sv') is 1
+PASS 'a'.localeCompare('b', 'en', { sensitivity:'base' }) is -1
+PASS 'a'.localeCompare('ä', 'en', { sensitivity:'base' }) is 0
+PASS 'a'.localeCompare('A', 'en', { sensitivity:'base' }) is 0
+PASS 'a'.localeCompare('ⓐ', 'en', { sensitivity:'base' }) is 0
+PASS 'a'.localeCompare('b', 'en', { sensitivity:'accent' }) is -1
+PASS 'a'.localeCompare('ä', 'en', { sensitivity:'accent' }) is -1
+PASS 'a'.localeCompare('A', 'en', { sensitivity:'accent' }) is 0
+PASS 'a'.localeCompare('ⓐ', 'en', { sensitivity:'accent' }) is 0
+PASS 'a'.localeCompare('b', 'en', { sensitivity:'case' }) is -1
+PASS 'a'.localeCompare('ä', 'en', { sensitivity:'case' }) is 0
+PASS 'a'.localeCompare('A', 'en', { sensitivity:'case' }) is -1
+PASS 'a'.localeCompare('ⓐ', 'en', { sensitivity:'case' }) is 0
+PASS 'a'.localeCompare('b', 'en', { sensitivity:'variant' }) is -1
+PASS 'a'.localeCompare('ä', 'en', { sensitivity:'variant' }) is -1
+PASS 'a'.localeCompare('A', 'en', { sensitivity:'variant' }) is -1
+PASS 'a'.localeCompare('ⓐ', 'en', { sensitivity:'variant' }) is -1
+PASS '1'.localeCompare('2', 'en', { numeric:false }) is -1
+PASS '2'.localeCompare('10', 'en', { numeric:false }) is 1
+PASS '01'.localeCompare('1', 'en', { numeric:false }) is -1
+PASS '๑'.localeCompare('๒', 'en', { numeric:false }) is -1
+PASS '๒'.localeCompare('๑๐', 'en', { numeric:false }) is 1
+PASS '๐๑'.localeCompare('๑', 'en', { numeric:false }) is -1
+PASS '1'.localeCompare('2', 'en', { numeric:true }) is -1
+PASS '2'.localeCompare('10', 'en', { numeric:true }) is -1
+PASS '01'.localeCompare('1', 'en', { numeric:true }) is 0
+PASS '๑'.localeCompare('๒', 'en', { numeric:true }) is -1
+PASS '๒'.localeCompare('๑๐', 'en', { numeric:true }) is -1
+PASS '๐๑'.localeCompare('๑', 'en', { numeric:true }) is 0
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 66f0184..2c4ec40 100644 (file)
@@ -1,6 +1,7 @@
 <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
 <html>
 <head>
+<meta charset="utf-8">
 <script src="../resources/js-test-pre.js"></script>
 </head>
 <body>
index b240899..a584bff 100644 (file)
@@ -1,3 +1,79 @@
+2016-04-24  Filip Pizlo <fpizlo@apple.com> and Andy VanWagoner <thetalecrafter@gmail.com>
+
+        [INTL] Implement String.prototype.localeCompare in ECMA-402
+        https://bugs.webkit.org/show_bug.cgi?id=147607
+
+        Reviewed by Darin Adler.
+        
+        Part of this change is just rolling 194394 back in.
+        
+        The other part is making that not a regression on CDjs. Other than the fact that it uses
+        bound functions, the problem with this new localeCompare implementation is that it uses
+        the arguments object. It uses it in a way that *seems* like ArgumentsEliminationPhase
+        ought to handle, but to my surprise it didn't:
+        
+        - If we have a ForceExit GetByVal on the arguments object, we would previously assume that
+          it escaped. That's false since we just exit at ForceExit. On the other hand we probably
+          should be pruning unreachable paths before we get here, but that's a separate issue. I
+          don't want to play with phase order right now.
+        
+        - If we have a OutOfBounds GetByVal on the arguments object, then the best that would
+          previously happen is that we'd compile it into an in-bounds arguments access. That's quite
+          bad, as Andy's localeCompare illustrates: it uses out-of-bounds access on the arguments
+          object to detect if an argument was passed. This change introduces an OutOfBounds version
+          of GetMyArgumentByVal for this purpose.
+        
+        This change required registering sane chain watchpoints. In the process, I noticed that the
+        old way of doing it had a race condition: we might register watchpoints for the structure
+        that had become insane. This change introduces a double-checking idiom that I believe works
+        because once the structure becomes insane it can't go back to sane and watchpoints
+        registration already involves executing the hardest possible fences.
+
+        * builtins/StringPrototype.js:
+        (repeat):
+        (localeCompare):
+        (search):
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGArgumentsEliminationPhase.cpp:
+        * dfg/DFGArrayMode.cpp:
+        (JSC::DFG::ArrayMode::refine):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGConstantFoldingPhase.cpp:
+        (JSC::DFG::ConstantFoldingPhase::foldConstants):
+        * dfg/DFGDoesGC.cpp:
+        (JSC::DFG::doesGC):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGNodeType.h:
+        * dfg/DFGPreciseLocalClobberize.h:
+        (JSC::DFG::PreciseLocalClobberizeAdaptor::readTop):
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        * dfg/DFGSafeToExecute.h:
+        (JSC::DFG::safeToExecute):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileGetByValOnString):
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGValidate.cpp:
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileNode):
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetMyArgumentByVal):
+        (JSC::FTL::DFG::LowerDFGToB3::compilePutByVal):
+        (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
+        * ftl/FTLTypedPointer.h:
+        (JSC::FTL::TypedPointer::TypedPointer):
+        (JSC::FTL::TypedPointer::operator bool):
+        (JSC::FTL::TypedPointer::heap):
+        (JSC::FTL::TypedPointer::operator!): Deleted.
+        * runtime/StringPrototype.cpp:
+        (JSC::StringPrototype::finishCreation):
+
 2016-04-23  Filip Pizlo  <fpizlo@apple.com>
 
         Unreviewed, unbreak cloop.
index 8fecf00..9822748 100644 (file)
@@ -105,6 +105,40 @@ function repeat(count)
     return @repeatSlowPath(string, count);
 }
 
+function localeCompare(that/*, locales, options */)
+{
+    "use strict";
+
+    // 13.1.1 String.prototype.localeCompare (that [, locales [, options ]]) (ECMA-402 2.0)
+    // http://ecma-international.org/publications/standards/Ecma-402.htm
+
+    // 1. Let O be RequireObjectCoercible(this value).
+    if (this === null)
+        throw new @TypeError("String.prototype.localeCompare requires that |this| not be null");
+    
+    if (this === @undefined)
+        throw new @TypeError("String.prototype.localeCompare requires that |this| not be undefined");
+
+    // 2. Let S be ToString(O).
+    // 3. ReturnIfAbrupt(S).
+    var thisString = @toString(this);
+
+    // 4. Let That be ToString(that).
+    // 5. ReturnIfAbrupt(That).
+    var thatString = @toString(that);
+
+    // Avoid creating a collator for defaults.
+    if (arguments[1] === @undefined && arguments[2] === @undefined)
+        return @Collator.prototype.compare(thisString, thatString);
+
+    // 6. Let collator be Construct(%Collator%, «locales, options»).
+    // 7. ReturnIfAbrupt(collator).
+    var collator = new @Collator(arguments[1], arguments[2]);
+
+    // 8. Return CompareStrings(collator, S, That).
+    return collator.compare(thisString, thatString);
+}
+
 function search(regexp)
 {
     "use strict";
index 1004f91..45da2c2 100644 (file)
@@ -1622,14 +1622,15 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         forNode(node).makeHeapTop();
         break;
         
-    case GetMyArgumentByVal: {
+    case GetMyArgumentByVal:
+    case GetMyArgumentByValOutOfBounds: {
         JSValue index = forNode(node->child2()).m_value;
         InlineCallFrame* inlineCallFrame = node->child1()->origin.semantic.inlineCallFrame;
 
         if (index && index.isInt32()) {
             // This pretends to return TOP for accesses that are actually proven out-of-bounds because
             // that's the conservative thing to do. Otherwise we'd need to write more code to mark such
-            // paths as unreachable, and it's almost certainly not worth the effort.
+            // paths as unreachable, or to return undefined. We could implement that eventually.
             
             if (inlineCallFrame) {
                 if (index.asUInt32() < inlineCallFrame->arguments.size() - 1) {
@@ -1657,8 +1658,12 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
                         virtualRegisterForArgument(i) + inlineCallFrame->stackOffset));
             }
             
+            if (node->op() == GetMyArgumentByValOutOfBounds)
+                result.merge(SpecOther);
+            
             if (result.value())
                 m_state.setFoundConstants(true);
+            
             forNode(node) = result;
             break;
         }
index 66cecd3..9192cf9 100644 (file)
@@ -134,11 +134,29 @@ private:
                     escape(edge, source);
                 break;
             
-            case Array::Int32:
-            case Array::Double:
-            case Array::Contiguous:
-                if (edge->op() != CreateClonedArguments)
+            case Array::Contiguous: {
+                if (edge->op() != CreateClonedArguments) {
                     escape(edge, source);
+                    return;
+                }
+            
+                // Everything is fine if we're doing an in-bounds access.
+                if (mode.isInBounds())
+                    break;
+                
+                // If we're out-of-bounds then we proceed only if the object prototype is
+                // sane (i.e. doesn't have indexed properties).
+                JSGlobalObject* globalObject = m_graph.globalObjectFor(edge->origin.semantic);
+                if (globalObject->objectPrototypeIsSane()) {
+                    m_graph.watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
+                    if (globalObject->objectPrototypeIsSane())
+                        break;
+                }
+                escape(edge, source);
+                break;
+            }
+            
+            case Array::ForceExit:
                 break;
             
             default:
@@ -490,8 +508,13 @@ private:
                     }
                     
                     if (!result) {
+                        NodeType op;
+                        if (node->arrayMode().isInBounds())
+                            op = GetMyArgumentByVal;
+                        else
+                            op = GetMyArgumentByValOutOfBounds;
                         result = insertionSet.insertNode(
-                            nodeIndex, node->prediction(), GetMyArgumentByVal, node->origin,
+                            nodeIndex, node->prediction(), op, node->origin,
                             node->child1(), node->child2());
                     }
                     
index 87ee3ad..162201d 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -198,7 +198,8 @@ ArrayMode ArrayMode::refine(
             && !graph.hasExitSite(node->origin.semantic, OutOfBounds)) {
             graph.watchpoints().addLazily(globalObject->arrayPrototype()->structure()->transitionWatchpointSet());
             graph.watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
-            return withSpeculation(Array::SaneChain);
+            if (globalObject->arrayPrototypeChainIsSane())
+                return withSpeculation(Array::SaneChain);
         }
         return ArrayMode(Array::Generic);
     }
index 71ef98a..2e0528a 100644 (file)
@@ -661,7 +661,8 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
         return;
     }
         
-    case GetMyArgumentByVal: {
+    case GetMyArgumentByVal:
+    case GetMyArgumentByValOutOfBounds: {
         read(Stack);
         // FIXME: It would be trivial to have a def here.
         // https://bugs.webkit.org/show_bug.cgi?id=143077
index c9da359..029d61c 100644 (file)
@@ -288,7 +288,8 @@ private:
                 break;
             }
                 
-            case GetMyArgumentByVal: {
+            case GetMyArgumentByVal:
+            case GetMyArgumentByValOutOfBounds: {
                 JSValue index = m_state.forNode(node->child2()).value();
                 if (!index || !index.isInt32())
                     break;
@@ -338,6 +339,9 @@ private:
                     break;
                 }
                 
+                if (node->op() == GetMyArgumentByValOutOfBounds)
+                    break;
+                
                 Node* length = emitCodeToGetArgumentsArrayLength(
                     m_insertionSet, arguments, indexInBlock, node->origin);
                 m_insertionSet.insertNode(
index 02ef942..294fbf1 100644 (file)
@@ -231,6 +231,7 @@ bool doesGC(Graph& graph, Node* node)
     case PhantomDirectArguments:
     case PhantomClonedArguments:
     case GetMyArgumentByVal:
+    case GetMyArgumentByValOutOfBounds:
     case ForwardVarargs:
     case PutHint:
     case CheckStructureImmediate:
index 28bf040..344827e 100644 (file)
@@ -713,7 +713,8 @@ private:
                                 globalObject->arrayPrototype()->structure()->transitionWatchpointSet());
                             m_graph.watchpoints().addLazily(
                                 globalObject->objectPrototype()->structure()->transitionWatchpointSet());
-                            node->setArrayMode(arrayMode.withSpeculation(Array::SaneChain));
+                            if (globalObject->arrayPrototypeChainIsSane())
+                                node->setArrayMode(arrayMode.withSpeculation(Array::SaneChain));
                         }
                     }
                 }
@@ -1323,6 +1324,7 @@ private:
         case PhantomClonedArguments:
         case ForwardVarargs:
         case GetMyArgumentByVal:
+        case GetMyArgumentByValOutOfBounds:
         case PutHint:
         case CheckStructureImmediate:
         case MaterializeNewObject:
index 4f44749..529f571 100644 (file)
@@ -177,6 +177,7 @@ namespace JSC { namespace DFG {
     /* opcodes use VarArgs beause they may have up to 4 children. */\
     macro(GetByVal, NodeResultJS | NodeMustGenerate) \
     macro(GetMyArgumentByVal, NodeResultJS | NodeMustGenerate) \
+    macro(GetMyArgumentByValOutOfBounds, NodeResultJS | NodeMustGenerate) \
     macro(LoadVarargs, NodeMustGenerate) \
     macro(ForwardVarargs, NodeMustGenerate) \
     macro(PutByValDirect, NodeMustGenerate | NodeHasVarArgs) \
index 3a7d716..3e5a833 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014, 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -109,6 +109,7 @@ private:
     {
         switch (m_node->op()) {
         case GetMyArgumentByVal:
+        case GetMyArgumentByValOutOfBounds:
         case ForwardVarargs:
         case CallForwardVarargs:
         case ConstructForwardVarargs:
index 76ccc0e..f09dc54 100644 (file)
@@ -955,6 +955,7 @@ private:
         case PhantomDirectArguments:
         case PhantomClonedArguments:
         case GetMyArgumentByVal:
+        case GetMyArgumentByValOutOfBounds:
         case ForwardVarargs:
         case PutHint:
         case CheckStructureImmediate:
index 0e5ea46..a88ca5d 100644 (file)
@@ -339,6 +339,7 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node)
     case PhantomDirectArguments:
     case PhantomClonedArguments:
     case GetMyArgumentByVal:
+    case GetMyArgumentByValOutOfBounds:
     case ForwardVarargs:
     case CopyRest:
     case StringReplace:
index 4b304e0..a3d0e56 100644 (file)
@@ -1956,6 +1956,7 @@ void SpeculativeJIT::compileGetByValOnString(Node* node)
 #endif
 
         JSGlobalObject* globalObject = m_jit.globalObjectFor(node->origin.semantic);
+        bool prototypeChainIsSane = false;
         if (globalObject->stringPrototypeChainIsSane()) {
             // FIXME: This could be captured using a Speculation mode that means "out-of-bounds
             // loads return a trivial value". Something like SaneChainOutOfBounds. This should
@@ -1965,6 +1966,11 @@ void SpeculativeJIT::compileGetByValOnString(Node* node)
             // https://bugs.webkit.org/show_bug.cgi?id=144668
             m_jit.graph().watchpoints().addLazily(globalObject->stringPrototype()->structure()->transitionWatchpointSet());
             m_jit.graph().watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
+            prototypeChainIsSane = globalObject->stringPrototypeChainIsSane();
+        }
+        if (prototypeChainIsSane) {
+            m_jit.graph().watchpoints().addLazily(globalObject->stringPrototype()->structure()->transitionWatchpointSet());
+            m_jit.graph().watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
             
 #if USE(JSVALUE64)
             addSlowPathGenerator(std::make_unique<SaneStringGetByValSlowPathGenerator>(
index b52d10c..21f4207 100644 (file)
@@ -5100,6 +5100,7 @@ void SpeculativeJIT::compile(Node* node)
     case KillStack:
     case GetStack:
     case GetMyArgumentByVal:
+    case GetMyArgumentByValOutOfBounds:
         DFG_CRASH(m_jit.graph(), node, "unexpected node in DFG backend");
         break;
     }
index 4d24a55..b52d47d 100644 (file)
@@ -5160,6 +5160,7 @@ void SpeculativeJIT::compile(Node* node)
     case PhantomNewGeneratorFunction:
     case PhantomCreateActivation:
     case GetMyArgumentByVal:
+    case GetMyArgumentByValOutOfBounds:
     case PutHint:
     case CheckStructureImmediate:
     case MaterializeCreateActivation:
index cf86c5d..2b398eb 100644 (file)
@@ -515,6 +515,7 @@ private:
                 case PhantomNewGeneratorFunction:
                 case PhantomCreateActivation:
                 case GetMyArgumentByVal:
+                case GetMyArgumentByValOutOfBounds:
                 case PutHint:
                 case CheckStructureImmediate:
                 case MaterializeCreateActivation:
@@ -662,6 +663,7 @@ private:
                 case TailCallForwardVarargsInlinedCaller:
                 case ConstructForwardVarargs:
                 case GetMyArgumentByVal:
+                case GetMyArgumentByValOutOfBounds:
                     break;
 
                 case Check:
index ee5be8f..d710839 100644 (file)
@@ -217,6 +217,7 @@ inline CapabilityLevel canCompile(Node* node)
     case PhantomDirectArguments:
     case PhantomClonedArguments:
     case GetMyArgumentByVal:
+    case GetMyArgumentByValOutOfBounds:
     case ForwardVarargs:
     case Switch:
     case TypeOf:
index cb03cfe..385bb89 100644 (file)
@@ -645,6 +645,7 @@ private:
             compileGetByVal();
             break;
         case GetMyArgumentByVal:
+        case GetMyArgumentByValOutOfBounds:
             compileGetMyArgumentByVal();
             break;
         case PutByVal:
@@ -2907,26 +2908,45 @@ private:
             limit = m_out.sub(m_out.load32(payloadFor(argumentCountRegister)), m_out.int32One);
         }
         
-        speculate(ExoticObjectMode, noValue(), 0, m_out.aboveOrEqual(index, limit));
+        LValue isOutOfBounds = m_out.aboveOrEqual(index, limit);
+        LBasicBlock continuation = nullptr;
+        LBasicBlock lastNext = nullptr;
+        ValueFromBlock slowResult;
+        if (m_node->op() == GetMyArgumentByValOutOfBounds) {
+            LBasicBlock normalCase = m_out.newBlock();
+            continuation = m_out.newBlock();
+            
+            slowResult = m_out.anchor(m_out.constInt64(JSValue::encode(jsUndefined())));
+            m_out.branch(isOutOfBounds, unsure(continuation), unsure(normalCase));
+            
+            lastNext = m_out.appendTo(normalCase, continuation);
+        } else
+            speculate(ExoticObjectMode, noValue(), 0, isOutOfBounds);
         
         TypedPointer base;
         if (inlineCallFrame) {
-            if (inlineCallFrame->arguments.size() <= 1) {
-                // We should have already exited due to the bounds check, above. Just tell the
-                // compiler that anything dominated by this instruction is not reachable, so
-                // that we don't waste time generating such code. This will also plant some
-                // kind of crashing instruction so that if by some fluke the bounds check didn't
-                // work, we'll crash in an easy-to-see way.
-                didAlreadyTerminate();
-                return;
-            }
-            base = addressFor(inlineCallFrame->arguments[1].virtualRegister());
+            if (inlineCallFrame->arguments.size() > 1)
+                base = addressFor(inlineCallFrame->arguments[1].virtualRegister());
         } else
             base = addressFor(virtualRegisterForArgument(1));
         
-        LValue pointer = m_out.baseIndex(
-            base.value(), m_out.zeroExt(index, m_out.intPtr), ScaleEight);
-        setJSValue(m_out.load64(TypedPointer(m_heaps.variables.atAnyIndex(), pointer)));
+        LValue result;
+        if (base) {
+            LValue pointer = m_out.baseIndex(
+                base.value(), m_out.zeroExt(index, m_out.intPtr), ScaleEight);
+            result = m_out.load64(TypedPointer(m_heaps.variables.atAnyIndex(), pointer));
+        } else
+            result = m_out.constInt64(JSValue::encode(jsUndefined()));
+        
+        if (m_node->op() == GetMyArgumentByValOutOfBounds) {
+            ValueFromBlock normalResult = m_out.anchor(result);
+            m_out.jump(continuation);
+            
+            m_out.appendTo(continuation, lastNext);
+            result = m_out.phi(Int64, slowResult, normalResult);
+        }
+        
+        setJSValue(result);
     }
     
     void compilePutByVal()
@@ -4201,7 +4221,8 @@ private:
             results.append(m_out.anchor(m_out.intPtrZero));
         } else {
             JSGlobalObject* globalObject = m_graph.globalObjectFor(m_node->origin.semantic);
-                
+            
+            bool prototypeChainIsSane = false;
             if (globalObject->stringPrototypeChainIsSane()) {
                 // FIXME: This could be captured using a Speculation mode that means
                 // "out-of-bounds loads return a trivial value", something like
@@ -4211,6 +4232,9 @@ private:
                 m_graph.watchpoints().addLazily(globalObject->stringPrototype()->structure()->transitionWatchpointSet());
                 m_graph.watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
                 
+                prototypeChainIsSane = globalObject->stringPrototypeChainIsSane();
+            }
+            if (prototypeChainIsSane) {
                 LBasicBlock negativeIndex = m_out.newBlock();
                     
                 results.append(m_out.anchor(m_out.constInt64(JSValue::encode(jsUndefined()))));
index e39c3d8..a435f79 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -47,10 +47,10 @@ public:
     {
     }
     
-    bool operator!() const
+    explicit operator bool() const
     {
         ASSERT(!m_heap == !m_value);
-        return !m_heap;
+        return !!m_heap;
     }
     
     const AbstractHeap* heap() const { return m_heap; }
index e765e8b..f0e27a5 100644 (file)
@@ -143,11 +143,12 @@ void StringPrototype::finishCreation(VM& vm, JSGlobalObject* globalObject, JSStr
     JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION("substring", stringProtoFuncSubstring, DontEnum, 2);
     JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION("toLowerCase", stringProtoFuncToLowerCase, DontEnum, 0);
     JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION("toUpperCase", stringProtoFuncToUpperCase, DontEnum, 0);
-    JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION("localeCompare", stringProtoFuncLocaleCompare, DontEnum, 1);
 #if ENABLE(INTL)
+    JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION("localeCompare", stringPrototypeLocaleCompareCodeGenerator, DontEnum);
     JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION("toLocaleLowerCase", stringProtoFuncToLocaleLowerCase, DontEnum, 0);
     JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION("toLocaleUpperCase", stringProtoFuncToLocaleUpperCase, DontEnum, 0);
 #else
+    JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION("localeCompare", stringProtoFuncLocaleCompare, DontEnum, 1);
     JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION("toLocaleLowerCase", stringProtoFuncToLowerCase, DontEnum, 0);
     JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION("toLocaleUpperCase", stringProtoFuncToUpperCase, DontEnum, 0);
 #endif