Attachment 361705 Details for Bug 193912 – Patch

[patch] Patch

bug-193912-20190211220356.patch (text/plain), 6.42 KB, created by Guillaume Emont on 2019-02-11 13:03:57 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Guillaume Emont

Created: 2019-02-11 13:03:57 PST

Size: 6.42 KB

patch

obsolete

>Subversion Revision: 241276
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 34959447a3efc250451546ad1e70338d52262976..b17ab2cd44c45703640118392d5a7047353eacf3 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,20 @@
>+2019-02-11  Guillaume Emont  <guijemont@igalia.com>
>+
>+        [JSC] Repeat string created from Array.prototype.join() take too much memory
>+        https://bugs.webkit.org/show_bug.cgi?id=193912
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added a fast case in Array.prototype.join when the array is
>+        uninitialized.
>+
>+        * runtime/ArrayPrototype.cpp:
>+        (JSC::canUseFastJoin):
>+        (JSC::fastJoin):
>+        * runtime/JSStringInlines.h:
>+        (JSC::repeatCharacter): moved from StringPrototype.cpp
>+        * runtime/StringPrototype.cpp:
>+
> 2019-02-10  Mark Lam  <mark.lam@apple.com>
> 
>         Remove the RELEASE_ASSERT check for duplicate cases in the BinarySwitch constructor.
>diff --git a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>index c37389aa857f89ce217d773f73d663ba2ea1728b..1a37b3f0cb1eb5e4341abefc82a736d3c167d2dc 100644
>--- a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>+++ b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>@@ -390,6 +390,7 @@ inline bool canUseFastJoin(const JSObject* thisObject)
>     case ALL_CONTIGUOUS_INDEXING_TYPES:
>     case ALL_INT32_INDEXING_TYPES:
>     case ALL_DOUBLE_INDEXING_TYPES:
>+    case ALL_UNDECIDED_INDEXING_TYPES:
>         return true;
>     default:
>         break;
>@@ -503,6 +504,21 @@ inline JSValue fastJoin(ExecState& state, JSObject* thisObject, StringView separ
>         }
>         RELEASE_AND_RETURN(scope, joiner.join(state));
>     }
>+    case ALL_UNDECIDED_INDEXING_TYPES: {
>+        if (length && holesMustForwardToPrototype(vm, thisObject))
>+            goto generalCase;
>+        switch (separator.length()) {
>+        case 0:
>+            RELEASE_AND_RETURN(scope, jsEmptyString(&state));
>+        case 1: {
>+            if (length <= 1)
>+                RELEASE_AND_RETURN(scope, jsEmptyString(&state));
>+            if (separator.is8Bit())
>+                RELEASE_AND_RETURN(scope, repeatCharacter(state, separator.characters8()[0], length - 1));
>+            RELEASE_AND_RETURN(scope, repeatCharacter(state, separator.characters16()[0], length - 1));
>+        }
>+        }
>+    }
>     }
> 
> generalCase:
>diff --git a/Source/JavaScriptCore/runtime/JSStringInlines.h b/Source/JavaScriptCore/runtime/JSStringInlines.h
>index 19d4756e0c1a9bcee73bb8a5b0cae6e03373074c..a27cf50670f18054876891e359700c6775b1bb1e 100644
>--- a/Source/JavaScriptCore/runtime/JSStringInlines.h
>+++ b/Source/JavaScriptCore/runtime/JSStringInlines.h
>@@ -54,4 +54,22 @@ inline JSValue jsMakeNontrivialString(ExecState* exec, StringType&& string, Stri
>     return jsNontrivialString(exec, WTFMove(result));
> }
> 
>+template <typename CharacterType>
>+inline JSString* repeatCharacter(ExecState& exec, CharacterType character, unsigned repeatCount)
>+{
>+    VM& vm = exec.vm();
>+    auto scope = DECLARE_THROW_SCOPE(vm);
>+
>+    CharacterType* buffer = nullptr;
>+    auto impl = StringImpl::tryCreateUninitialized(repeatCount, buffer);
>+    if (!impl) {
>+        throwOutOfMemoryError(&exec, scope);
>+        return nullptr;
>+    }
>+
>+    std::fill_n(buffer, repeatCount, character);
>+
>+    RELEASE_AND_RETURN(scope, jsString(&exec, WTFMove(impl)));
>+}
>+
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/runtime/StringPrototype.cpp b/Source/JavaScriptCore/runtime/StringPrototype.cpp
>index 28b862d52138bba90a65c5011ecedef91f148696..431e3b321a1ad0e18851a30a339a3a937e1197a7 100644
>--- a/Source/JavaScriptCore/runtime/StringPrototype.cpp
>+++ b/Source/JavaScriptCore/runtime/StringPrototype.cpp
>@@ -830,24 +830,6 @@ static inline bool checkObjectCoercible(JSValue thisValue)
>     return true;
> }
> 
>-template <typename CharacterType>
>-static inline JSString* repeatCharacter(ExecState& exec, CharacterType character, unsigned repeatCount)
>-{
>-    VM& vm = exec.vm();
>-    auto scope = DECLARE_THROW_SCOPE(vm);
>-
>-    CharacterType* buffer = nullptr;
>-    auto impl = StringImpl::tryCreateUninitialized(repeatCount, buffer);
>-    if (!impl) {
>-        throwOutOfMemoryError(&exec, scope);
>-        return nullptr;
>-    }
>-
>-    std::fill_n(buffer, repeatCount, character);
>-
>-    RELEASE_AND_RETURN(scope, jsString(&exec, WTFMove(impl)));
>-}
>-
> EncodedJSValue JSC_HOST_CALL stringProtoFuncRepeatCharacter(ExecState* exec)
> {
>     VM& vm = exec->vm();
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 86ee2061ea146a6cfd328682d23b15d64e62a276..94a60b0f40fb85f4e6a453e28a9f290549c287d8 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-02-11  Guillaume Emont  <guijemont@igalia.com>
>+
>+        [JSC] Repeat string created from Array.prototype.join() take too much memory
>+        https://bugs.webkit.org/show_bug.cgi?id=193912
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added a test for corner cases of Array.prototype.join() with an
>+        uninitialized array.
>+
>+        * stress/array-prototype-join-uninitialized.js: Added.
>+        (testArray):
>+        (testABC):
>+        (B):
>+        (C):
>+
> 2019-02-08  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] String.fromCharCode's slow path always generates 16bit string
>diff --git a/JSTests/stress/array-prototype-join-uninitialized.js b/JSTests/stress/array-prototype-join-uninitialized.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..702d53d0ddbd97594b47973c2c8804021f7b3394
>--- /dev/null
>+++ b/JSTests/stress/array-prototype-join-uninitialized.js
>@@ -0,0 +1,34 @@
>+function testArray(array, expected) {
>+    var s = array.join('M');
>+    if (s !== expected)
>+        throw("Bad result for array " + array + " expected: \"" + expected + "\" but got: \"" + s + "\"");
>+}
>+
>+function testABC(n, resA, resB, resC) {
>+    testArray(new Array(n), resA);
>+    testArray(new B(n), resB);
>+    testArray(new C(n), resC);
>+}
>+
>+class B extends Array { }
>+class C extends B { }
>+
>+
>+testABC(0, "", "", "");
>+testABC(1, "", "", "");
>+testABC(3, "MM", "MM", "MM")
>+
>+B.prototype[0] = "foo";
>+testABC(0, "", "", "");
>+testABC(1, "", "foo", "foo");
>+testABC(3, "MM", "fooMM", "fooMM");
>+
>+C.prototype[1] = "bar";
>+testABC(0, "", "", "");
>+testABC(1, "", "foo", "foo");
>+testABC(3, "MM", "fooMM", "fooMbarM");
>+
>+Array.prototype[1] = "baz";
>+testABC(0, "", "", "");
>+testABC(1, "", "foo", "foo");
>+testABC(3, "MbazM", "fooMbazM", "fooMbarM");

Actions: View | Formatted Diff | Diff

Attachments on bug 193912: 360761 | 361132 | 361157 | 361213 | 361705 | 362913