[JSC] StringAt, StringCharCodeAt and StringCodePointAt must survive DCE

JSTests/stress/string-index-dead-result-keeps-bounds-check.js

@@ -0,0 +1,47 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual + ' expected: ' + expected);
+}
+
+// The result of each string access is only compared against a constant. The abstract
+// interpreter folds the compare, so the access node has no users and DCE removes it. The
+// in-bounds speculation must survive that removal.
+
+function codePointAtIsUndefined(string, index) {
+    return string.codePointAt(index) === undefined;
+}
+noInline(codePointAtIsUndefined);
+
+function atIsUndefined(string, index) {
+    return string.at(index) === undefined;
+}
+noInline(atIsUndefined);
+
+function charCodeAtIsNaN(string, index) {
+    var c = string.charCodeAt(index);
+    return c !== c;
+}
+noInline(charCodeAtIsNaN);
+
+var str8bit = "Hello, World!";
+var str16bit = "こんにちは世界";
+
+for (var i = 0; i < testLoopCount; ++i) {
+    var index = i % str8bit.length;
+    shouldBe(codePointAtIsUndefined(str8bit, index), false);
+    shouldBe(atIsUndefined(str8bit, index), false);
+    shouldBe(charCodeAtIsNaN(str8bit, index), false);
+    index = i % str16bit.length;
+    shouldBe(codePointAtIsUndefined(str16bit, index), false);
+    shouldBe(atIsUndefined(str16bit, index), false);
+    shouldBe(charCodeAtIsNaN(str16bit, index), false);
+}
+
+for (var i = 0; i < 10; ++i) {
+    shouldBe(codePointAtIsUndefined(str8bit, str8bit.length), true);
+    shouldBe(atIsUndefined(str8bit, str8bit.length), true);
+    shouldBe(charCodeAtIsNaN(str8bit, str8bit.length), true);
+    shouldBe(codePointAtIsUndefined(str16bit, str16bit.length), true);
+    shouldBe(atIsUndefined(str16bit, str16bit.length), true);
+    shouldBe(charCodeAtIsNaN(str16bit, str16bit.length), true);
+}

Source/JavaScriptCore/dfg/DFGFixupPhase.cpp

@@ -1141,9 +1141,12 @@ class FixupPhase : public Phase {
         case StringCharCodeAt:
         case StringCodePointAt: {
             // Currently we have no good way of refining these.
-            if (op == StringAt)
+            if (op == StringAt) {
                 ASSERT(node->arrayMode() == ArrayMode(Array::String, Array::Read, Array::OutOfBounds) || node->arrayMode() == ArrayMode(Array::String, Array::Read, Array::InBounds));
-            else
+                // An out-of-bounds StringAt returns undefined instead of exiting, so nothing depends on it running.
+                if (node->arrayMode().isOutOfBounds())
+                    node->clearFlags(NodeMustGenerate);
+            } else
                 ASSERT(node->arrayMode() == ArrayMode(Array::String, Array::Read));
             blessArrayOperation(node->child1(), node->child2(), node->child1()); // Rewrite child1 with ResolveRope.
             fixEdge<KnownStringUse>(node->child1());

Source/JavaScriptCore/dfg/DFGNodeType.h

@@ -378,9 +378,12 @@ namespace JSC { namespace DFG {
     macro(StringSearch, NodeResultJS | NodeMustGenerate) \
     \
     /* Optimizations for string access */ \
-    macro(StringAt, NodeResultJS) \
-    macro(StringCharCodeAt, NodeResultInt32) \
-    macro(StringCodePointAt, NodeResultInt32) \
+    /* StringAt (InBounds), StringCharCodeAt and StringCodePointAt speculate that the index is in bounds, and the */ \
+    /* abstract interpreter relies on that speculation for their result type. They must survive DCE so the exit */ \
+    /* survives too. Fixup clears the flag for StringAt with an OutOfBounds array mode, which does not exit. */ \
+    macro(StringAt, NodeResultJS | NodeMustGenerate) \
+    macro(StringCharCodeAt, NodeResultInt32 | NodeMustGenerate) \
+    macro(StringCodePointAt, NodeResultInt32 | NodeMustGenerate) \
     macro(StringCharAt, NodeResultJS) \
     macro(StringIteratorNext, 0) \
     macro(StringIteratorNextWithUndefined, 0) \