[JSC] jsSubstring should resolve rope before calling JSRopeString::create
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Mar 2019 17:50:41 +0000 (17:50 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Mar 2019 17:50:41 +0000 (17:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195840

Reviewed by Geoffrey Garen.

jsSubstring always ends up resolving rope of the base string because substring JSRopeString only accepts non-rope JSString
as its base. Instead of resolving ropes in finishCreationSubstring, we should resolve before passing it to JSRopeString.
So that, we can access string data before creating JSRopeString, and we can introduce optimizations like avoiding creation
of single character substrings.

We can find that a lot of substrings for length = 1 are allocated in RAMification regexp tests. This patch avoids creation of these
strings to save memory.

This patch also strengthen error checks caused by rope resolution for base of substrings. Previously we sometimes miss this checks.

* dfg/DFGOperations.cpp:
* runtime/JSString.cpp:
(JSC::JSString::dumpToStream):
* runtime/JSString.h:
(JSC::jsSubstring):
(JSC::jsSubstringOfResolved):
(JSC::jsSingleCharacterString):
* runtime/RegExpCachedResult.cpp:
(JSC::RegExpCachedResult::lastResult): We no longer need to have length = 0 path since jsSubstring returns an empty string if length == 0.
(JSC::RegExpCachedResult::leftContext):
(JSC::RegExpCachedResult::rightContext):
(JSC::RegExpCachedResult::setInput):
* runtime/RegExpGlobalData.cpp:
(JSC::RegExpGlobalData::getBackref):
(JSC::RegExpGlobalData::getLastParen):
* runtime/StringObject.h:
(JSC::jsStringWithReuse):
(JSC::jsSubstring):
* runtime/StringPrototype.cpp:
(JSC::replaceUsingRegExpSearch):
(JSC::operationStringProtoFuncReplaceRegExpEmptyStr):
(JSC::replaceUsingStringSearch):
(JSC::stringProtoFuncSlice):
(JSC::splitStringByOneCharacterImpl):
(JSC::stringProtoFuncSplitFast):
(JSC::stringProtoFuncSubstr):
(JSC::stringProtoFuncSubstring):
(JSC::stringProtoFuncToLowerCase):
(JSC::stringProtoFuncToUpperCase):
Some `const String& value = string->value(exec)` is dangerous if GC happens later. Changed to getting `String` instead of `const String&` here.

* runtime/StringPrototypeInlines.h:
(JSC::stringSlice):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGOperations.cpp
Source/JavaScriptCore/runtime/JSString.cpp
Source/JavaScriptCore/runtime/JSString.h
Source/JavaScriptCore/runtime/RegExpCachedResult.cpp
Source/JavaScriptCore/runtime/RegExpGlobalData.cpp
Source/JavaScriptCore/runtime/StringObject.h
Source/JavaScriptCore/runtime/StringPrototype.cpp
Source/JavaScriptCore/runtime/StringPrototypeInlines.h

index 459ef3c..a7f6537 100644 (file)
@@ -1,3 +1,54 @@
+2019-03-18  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] jsSubstring should resolve rope before calling JSRopeString::create
+        https://bugs.webkit.org/show_bug.cgi?id=195840
+
+        Reviewed by Geoffrey Garen.
+
+        jsSubstring always ends up resolving rope of the base string because substring JSRopeString only accepts non-rope JSString
+        as its base. Instead of resolving ropes in finishCreationSubstring, we should resolve before passing it to JSRopeString.
+        So that, we can access string data before creating JSRopeString, and we can introduce optimizations like avoiding creation
+        of single character substrings.
+
+        We can find that a lot of substrings for length = 1 are allocated in RAMification regexp tests. This patch avoids creation of these
+        strings to save memory.
+
+        This patch also strengthen error checks caused by rope resolution for base of substrings. Previously we sometimes miss this checks.
+
+        * dfg/DFGOperations.cpp:
+        * runtime/JSString.cpp:
+        (JSC::JSString::dumpToStream):
+        * runtime/JSString.h:
+        (JSC::jsSubstring):
+        (JSC::jsSubstringOfResolved):
+        (JSC::jsSingleCharacterString):
+        * runtime/RegExpCachedResult.cpp:
+        (JSC::RegExpCachedResult::lastResult): We no longer need to have length = 0 path since jsSubstring returns an empty string if length == 0.
+        (JSC::RegExpCachedResult::leftContext):
+        (JSC::RegExpCachedResult::rightContext):
+        (JSC::RegExpCachedResult::setInput):
+        * runtime/RegExpGlobalData.cpp:
+        (JSC::RegExpGlobalData::getBackref):
+        (JSC::RegExpGlobalData::getLastParen):
+        * runtime/StringObject.h:
+        (JSC::jsStringWithReuse):
+        (JSC::jsSubstring):
+        * runtime/StringPrototype.cpp:
+        (JSC::replaceUsingRegExpSearch):
+        (JSC::operationStringProtoFuncReplaceRegExpEmptyStr):
+        (JSC::replaceUsingStringSearch):
+        (JSC::stringProtoFuncSlice):
+        (JSC::splitStringByOneCharacterImpl):
+        (JSC::stringProtoFuncSplitFast):
+        (JSC::stringProtoFuncSubstr):
+        (JSC::stringProtoFuncSubstring):
+        (JSC::stringProtoFuncToLowerCase):
+        (JSC::stringProtoFuncToUpperCase):
+        Some `const String& value = string->value(exec)` is dangerous if GC happens later. Changed to getting `String` instead of `const String&` here.
+
+        * runtime/StringPrototypeInlines.h:
+        (JSC::stringSlice):
+
 2019-03-18  Mark Lam  <mark.lam@apple.com>
 
         Missing a ThrowScope release in JSObject::toString().
index 7fb246e..4cfac1d 100644 (file)
@@ -2168,7 +2168,7 @@ JSCell* JIT_OPERATION operationStringSubstr(ExecState* exec, JSCell* cell, int32
 
     auto string = jsCast<JSString*>(cell)->value(exec);
     RETURN_IF_EXCEPTION(scope, nullptr);
-    return jsSubstring(exec, string, from, span);
+    return jsSubstring(&vm, string, from, span);
 }
 
 JSCell* JIT_OPERATION operationStringSlice(ExecState* exec, JSCell* cell, int32_t start, int32_t end)
@@ -2181,8 +2181,7 @@ JSCell* JIT_OPERATION operationStringSlice(ExecState* exec, JSCell* cell, int32_
     RETURN_IF_EXCEPTION(scope, nullptr);
     static_assert(static_cast<uint64_t>(JSString::MaxLength) <= static_cast<uint64_t>(std::numeric_limits<int32_t>::max()), "");
 
-    scope.release();
-    return stringSlice(exec, WTFMove(string), start, end);
+    return stringSlice(vm, WTFMove(string), start, end);
 }
 
 JSString* JIT_OPERATION operationToLowerCase(ExecState* exec, JSString* string, uint32_t failingIndex)
index c836fa2..dcaead9 100644 (file)
@@ -75,9 +75,12 @@ void JSString::dumpToStream(const JSCell* cell, PrintStream& out)
     const JSString* thisObject = jsCast<const JSString*>(cell);
     out.printf("<%p, %s, [%u], ", thisObject, thisObject->className(vm), thisObject->length());
     uintptr_t pointer = thisObject->m_fiber;
-    if (pointer & isRopeInPointer)
-        out.printf("[rope]");
-    else {
+    if (pointer & isRopeInPointer) {
+        if (pointer & JSRopeString::isSubstringInPointer)
+            out.printf("[substring]");
+        else
+            out.printf("[rope]");
+    } else {
         if (WTF::StringImpl* ourImpl = bitwise_cast<StringImpl*>(pointer)) {
             if (ourImpl->is8Bit())
                 out.printf("[8 %p]", ourImpl->characters8());
index 455540a..b893e04 100644 (file)
@@ -49,7 +49,6 @@ JSString* jsString(ExecState*, const String&); // returns empty string if passed
 JSString* jsSingleCharacterString(VM*, UChar);
 JSString* jsSingleCharacterString(ExecState*, UChar);
 JSString* jsSubstring(VM*, const String&, unsigned offset, unsigned length);
-JSString* jsSubstring(ExecState*, const String&, unsigned offset, unsigned length);
 
 // Non-trivial strings are two or more characters long.
 // These functions are faster than just calling jsString.
@@ -507,40 +506,10 @@ private:
         initializeIsSubstring(true);
         initializeLength(length);
         initializeIs8Bit(base->is8Bit());
-        if (base->isSubstring()) {
-            JSRopeString* baseRope = jsCast<JSRopeString*>(base);
-            initializeSubstringBase(baseRope->substringBase());
-            initializeSubstringOffset(baseRope->substringOffset() + offset);
-        } else {
-            initializeSubstringBase(base);
-            initializeSubstringOffset(offset);
-        }
-        ASSERT(length == this->length());
-    }
-
-    enum SubstringOfResolvedTag { SubstringOfResolved };
-    JSRopeString(SubstringOfResolvedTag, VM& vm, JSString* base, unsigned offset, unsigned length)
-        : JSString(vm)
-    {
-        RELEASE_ASSERT(!sumOverflows<int32_t>(offset, length));
-        RELEASE_ASSERT(offset + length <= base->length());
-        initializeIsSubstring(true);
-        initializeLength(length);
-        initializeIs8Bit(base->is8Bit());
         initializeSubstringBase(base);
         initializeSubstringOffset(offset);
         ASSERT(length == this->length());
-    }
-
-    ALWAYS_INLINE void finishCreationSubstring(VM& vm, ExecState* exec)
-    {
-        Base::finishCreation(vm);
-        JSString* updatedBase = substringBase();
-        // For now, let's not allow substrings with a rope base.
-        // Resolve non-substring rope bases so we don't have to deal with it.
-        // FIXME: Evaluate if this would be worth adding more branches.
-        if (updatedBase->isRope())
-            jsCast<JSRopeString*>(updatedBase)->resolveRope(exec);
+        ASSERT(!base->isRope());
     }
 
     ALWAYS_INLINE void finishCreationSubstringOfResolved(VM& vm)
@@ -591,18 +560,9 @@ private:
         return newString;
     }
 
-    static JSRopeString* create(VM& vm, ExecState* exec, JSString* base, unsigned offset, unsigned length)
-    {
-        JSRopeString* newString = new (NotNull, allocateCell<JSRopeString>(vm.heap)) JSRopeString(vm, base, offset, length);
-        newString->finishCreationSubstring(vm, exec);
-        ASSERT(newString->length());
-        ASSERT(newString->isRope());
-        return newString;
-    }
-
     ALWAYS_INLINE static JSRopeString* createSubstringOfResolved(VM& vm, GCDeferralContext* deferralContext, JSString* base, unsigned offset, unsigned length)
     {
-        JSRopeString* newString = new (NotNull, allocateCell<JSRopeString>(vm.heap, deferralContext)) JSRopeString(SubstringOfResolved, vm, base, offset, length);
+        JSRopeString* newString = new (NotNull, allocateCell<JSRopeString>(vm.heap, deferralContext)) JSRopeString(vm, base, offset, length);
         newString->finishCreationSubstringOfResolved(vm);
         ASSERT(newString->length());
         ASSERT(newString->isRope());
@@ -844,16 +804,31 @@ inline JSString* jsString(VM* vm, const String& s)
     return JSString::create(*vm, *s.impl());
 }
 
-inline JSString* jsSubstring(VM& vm, ExecState* exec, JSString* s, unsigned offset, unsigned length)
+inline JSString* jsSubstring(VM& vm, ExecState* exec, JSString* base, unsigned offset, unsigned length)
 {
-    ASSERT(offset <= s->length());
-    ASSERT(length <= s->length());
-    ASSERT(offset + length <= s->length());
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    ASSERT(offset <= base->length());
+    ASSERT(length <= base->length());
+    ASSERT(offset + length <= base->length());
     if (!length)
         return vm.smallStrings.emptyString();
-    if (!offset && length == s->length())
-        return s;
-    return JSRopeString::create(vm, exec, s, offset, length);
+    if (!offset && length == base->length())
+        return base;
+
+    // For now, let's not allow substrings with a rope base.
+    // Resolve non-substring rope bases so we don't have to deal with it.
+    // FIXME: Evaluate if this would be worth adding more branches.
+    if (base->isSubstring()) {
+        JSRopeString* baseRope = jsCast<JSRopeString*>(base);
+        base = baseRope->substringBase();
+        offset = baseRope->substringOffset() + offset;
+        ASSERT(!base->isRope());
+    } else if (base->isRope()) {
+        jsCast<JSRopeString*>(base)->resolveRope(exec);
+        RETURN_IF_EXCEPTION(scope, nullptr);
+    }
+    return jsSubstringOfResolved(vm, nullptr, base, offset, length);
 }
 
 inline JSString* jsSubstringOfResolved(VM& vm, GCDeferralContext* deferralContext, JSString* s, unsigned offset, unsigned length)
@@ -861,10 +836,17 @@ inline JSString* jsSubstringOfResolved(VM& vm, GCDeferralContext* deferralContex
     ASSERT(offset <= s->length());
     ASSERT(length <= s->length());
     ASSERT(offset + length <= s->length());
+    ASSERT(!s->isRope());
     if (!length)
         return vm.smallStrings.emptyString();
     if (!offset && length == s->length())
         return s;
+    if (length == 1) {
+        auto& base = s->valueInternal();
+        UChar character = base.characterAt(offset);
+        if (character <= maxSingleCharacterString)
+            return vm.smallStrings.singleCharacterString(character);
+    }
     return JSRopeString::createSubstringOfResolved(vm, deferralContext, s, offset, length);
 }
 
@@ -912,7 +894,6 @@ inline JSString* jsOwnedString(VM* vm, const String& s)
 inline JSString* jsEmptyString(ExecState* exec) { return jsEmptyString(&exec->vm()); }
 inline JSString* jsString(ExecState* exec, const String& s) { return jsString(&exec->vm(), s); }
 inline JSString* jsSingleCharacterString(ExecState* exec, UChar c) { return jsSingleCharacterString(&exec->vm(), c); }
-inline JSString* jsSubstring(ExecState* exec, const String& s, unsigned offset, unsigned length) { return jsSubstring(&exec->vm(), s, offset, length); }
 inline JSString* jsNontrivialString(ExecState* exec, const String& s) { return jsNontrivialString(&exec->vm(), s); }
 inline JSString* jsNontrivialString(ExecState* exec, String&& s) { return jsNontrivialString(&exec->vm(), WTFMove(s)); }
 inline JSString* jsOwnedString(ExecState* exec, const String& s) { return jsOwnedString(&exec->vm(), s); }
index 9e100d2..a610509 100644 (file)
@@ -46,14 +46,21 @@ void RegExpCachedResult::visitAggregate(SlotVisitor& visitor)
 JSArray* RegExpCachedResult::lastResult(ExecState* exec, JSObject* owner)
 {
     VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
     if (!m_reified) {
         m_reifiedInput.set(vm, owner, m_lastInput.get());
         if (!m_lastRegExp)
             m_lastRegExp.set(vm, owner, vm.regExpCache()->ensureEmptyRegExp(vm));
+
+        JSArray* result = nullptr;
         if (m_result)
-            m_reifiedResult.setWithoutWriteBarrier(createRegExpMatchesArray(exec, exec->lexicalGlobalObject(), m_lastInput.get(), m_lastRegExp.get(), m_result.start));
+            result = createRegExpMatchesArray(exec, exec->lexicalGlobalObject(), m_lastInput.get(), m_lastRegExp.get(), m_result.start);
         else
-            m_reifiedResult.setWithoutWriteBarrier(createEmptyRegExpMatchesArray(exec->lexicalGlobalObject(), m_lastInput.get(), m_lastRegExp.get()));
+            result = createEmptyRegExpMatchesArray(exec->lexicalGlobalObject(), m_lastInput.get(), m_lastRegExp.get());
+        RETURN_IF_EXCEPTION(scope, nullptr);
+
+        m_reifiedResult.setWithoutWriteBarrier(result);
         m_reifiedLeftContext.clear();
         m_reifiedRightContext.clear();
         m_reified = true;
@@ -65,19 +72,34 @@ JSArray* RegExpCachedResult::lastResult(ExecState* exec, JSObject* owner)
 JSString* RegExpCachedResult::leftContext(ExecState* exec, JSObject* owner)
 {
     // Make sure we're reified.
+    VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
     lastResult(exec, owner);
-    if (!m_reifiedLeftContext)
-        m_reifiedLeftContext.set(exec->vm(), owner, m_result.start ? jsSubstring(exec, m_reifiedInput.get(), 0, m_result.start) : jsEmptyString(exec));
+    RETURN_IF_EXCEPTION(scope, nullptr);
+
+    if (!m_reifiedLeftContext) {
+        JSString* leftContext = jsSubstring(exec, m_reifiedInput.get(), 0, m_result.start);
+        RETURN_IF_EXCEPTION(scope, nullptr);
+        m_reifiedLeftContext.set(vm, owner, leftContext);
+    }
     return m_reifiedLeftContext.get();
 }
 
 JSString* RegExpCachedResult::rightContext(ExecState* exec, JSObject* owner)
 {
     // Make sure we're reified.
+    VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
     lastResult(exec, owner);
+    RETURN_IF_EXCEPTION(scope, nullptr);
+
     if (!m_reifiedRightContext) {
         unsigned length = m_reifiedInput->length();
-        m_reifiedRightContext.set(exec->vm(), owner, m_result.end != length ? jsSubstring(exec, m_reifiedInput.get(), m_result.end, length - m_result.end) : jsEmptyString(exec));
+        JSString* rightContext = jsSubstring(exec, m_reifiedInput.get(), m_result.end, length - m_result.end);
+        RETURN_IF_EXCEPTION(scope, nullptr);
+        m_reifiedRightContext.set(vm, owner, rightContext);
     }
     return m_reifiedRightContext.get();
 }
@@ -85,11 +107,17 @@ JSString* RegExpCachedResult::rightContext(ExecState* exec, JSObject* owner)
 void RegExpCachedResult::setInput(ExecState* exec, JSObject* owner, JSString* input)
 {
     // Make sure we're reified, otherwise m_reifiedInput will be ignored.
+    VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
     lastResult(exec, owner);
+    RETURN_IF_EXCEPTION(scope, void());
     leftContext(exec, owner);
+    RETURN_IF_EXCEPTION(scope, void());
     rightContext(exec, owner);
+    RETURN_IF_EXCEPTION(scope, void());
     ASSERT(m_reified);
-    m_reifiedInput.set(exec->vm(), owner, input);
+    m_reifiedInput.set(vm, owner, input);
 }
 
 } // namespace JSC
index c36daf2..9cbf4a7 100644 (file)
@@ -35,7 +35,11 @@ void RegExpGlobalData::visitAggregate(SlotVisitor& visitor)
 
 JSValue RegExpGlobalData::getBackref(ExecState* exec, JSGlobalObject* owner, unsigned i)
 {
+    VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
     JSArray* array = m_cachedResult.lastResult(exec, owner);
+    RETURN_IF_EXCEPTION(scope, { });
 
     if (i < array->length()) {
         JSValue result = JSValue(array).get(exec, i);
@@ -43,12 +47,17 @@ JSValue RegExpGlobalData::getBackref(ExecState* exec, JSGlobalObject* owner, uns
         if (!result.isUndefined())
             return result;
     }
-    return jsEmptyString(exec);
+    return jsEmptyString(&vm);
 }
 
 JSValue RegExpGlobalData::getLastParen(ExecState* exec, JSGlobalObject* owner)
 {
+    VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
     JSArray* array = m_cachedResult.lastResult(exec, owner);
+    RETURN_IF_EXCEPTION(scope, { });
+
     unsigned length = array->length();
     if (length > 1) {
         JSValue result = JSValue(array).get(exec, length - 1);
@@ -56,7 +65,7 @@ JSValue RegExpGlobalData::getLastParen(ExecState* exec, JSGlobalObject* owner)
         if (!result.isUndefined())
             return result;
     }
-    return jsEmptyString(exec);
+    return jsEmptyString(&vm);
 }
 
 JSValue RegExpGlobalData::getLeftContext(ExecState* exec, JSGlobalObject* owner)
index 8aedca6..6f2b909 100644 (file)
@@ -85,7 +85,7 @@ static inline JSString* jsStringWithReuse(ExecState* exec, JSValue originalValue
         ASSERT(asString(originalValue)->value(exec) == string);
         return asString(originalValue);
     }
-    return jsString(exec, string);
+    return jsString(&exec->vm(), string);
 }
 
 // Helper that tries to use the JSString substring sharing mechanism if 'originalValue' is a JSString.
@@ -99,7 +99,7 @@ static inline JSString* jsSubstring(ExecState* exec, JSValue originalValue, cons
         ASSERT(asString(originalValue)->value(exec) == string);
         return jsSubstring(exec, asString(originalValue), offset, length);
     }
-    return jsSubstring(exec, string, offset, length);
+    return jsSubstring(&exec->vm(), string, offset, length);
 }
 
 } // namespace JSC
index 8dccd01..348bf0d 100644 (file)
@@ -527,7 +527,7 @@ static ALWAYS_INLINE JSString* replaceUsingRegExpSearch(
 {
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    const String& source = string->value(exec);
+    String source = string->value(exec);
     unsigned sourceLen = source.length();
     RETURN_IF_EXCEPTION(scope, nullptr);
     RegExpObject* regExpObject = jsCast<RegExpObject*>(searchValue);
@@ -656,8 +656,10 @@ static ALWAYS_INLINE JSString* replaceUsingRegExpSearch(
 
                     if (matchStart < 0)
                         patternValue = jsUndefined();
-                    else
-                        patternValue = jsSubstring(exec, source, matchStart, matchLen);
+                    else {
+                        patternValue = jsSubstring(&vm, source, matchStart, matchLen);
+                        RETURN_IF_EXCEPTION(scope, nullptr);
+                    }
 
                     args.append(patternValue);
 
@@ -735,7 +737,7 @@ JSCell* JIT_OPERATION operationStringProtoFuncReplaceRegExpEmptyStr(
         // ES5.1 15.5.4.10 step 8.a.
         searchValue->setLastIndex(exec, 0);
         RETURN_IF_EXCEPTION(scope, nullptr);
-        const String& source = thisValue->value(exec);
+        String source = thisValue->value(exec);
         RETURN_IF_EXCEPTION(scope, nullptr);
         RELEASE_AND_RETURN(scope, removeUsingRegExpSearch(vm, exec, thisValue, source, regExp));
     }
@@ -778,7 +780,7 @@ static ALWAYS_INLINE JSString* replaceUsingStringSearch(VM& vm, ExecState* exec,
 {
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    const String& string = jsString->value(exec);
+    String string = jsString->value(exec);
     RETURN_IF_EXCEPTION(scope, nullptr);
     String searchString = searchValue.toWTFString(exec);
     RETURN_IF_EXCEPTION(scope, nullptr);
@@ -792,7 +794,9 @@ static ALWAYS_INLINE JSString* replaceUsingStringSearch(VM& vm, ExecState* exec,
     CallType callType = getCallData(vm, replaceValue, callData);
     if (callType != CallType::None) {
         MarkedArgumentBuffer args;
-        args.append(jsSubstring(exec, string, matchStart, searchString.impl()->length()));
+        auto* substring = jsSubstring(&vm, string, matchStart, searchString.impl()->length());
+        RETURN_IF_EXCEPTION(scope, nullptr);
+        args.append(substring);
         args.append(jsNumber(matchStart));
         args.append(jsString);
         ASSERT(!args.hasOverflowed());
@@ -1153,8 +1157,7 @@ EncodedJSValue JSC_HOST_CALL stringProtoFuncSlice(ExecState* exec)
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
     double end = a1.isUndefined() ? len : a1.toInteger(exec);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    scope.release();
-    return JSValue::encode(stringSlice(exec, WTFMove(s), start, end));
+    return JSValue::encode(stringSlice(vm, WTFMove(s), start, end));
 }
 
 // Return true in case of early return (resultLength got to limitLength).
@@ -1176,7 +1179,9 @@ static ALWAYS_INLINE bool splitStringByOneCharacterImpl(ExecState* exec, JSArray
         //    through q (exclusive).
         // 2. Call the [[DefineOwnProperty]] internal method of A with arguments ToString(lengthA),
         //    Property Descriptor {[[Value]]: T, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true}, and false.
-        result->putDirectIndex(exec, resultLength, jsSubstring(exec, originalValue, input, position, matchPosition - position));
+        auto* substring = jsSubstring(exec, originalValue, input, position, matchPosition - position);
+        RETURN_IF_EXCEPTION(scope, false);
+        result->putDirectIndex(exec, resultLength, substring);
         RETURN_IF_EXCEPTION(scope, false);
         // 3. Increment lengthA by 1.
         // 4. If lengthA == lim, return A.
@@ -1300,8 +1305,10 @@ EncodedJSValue JSC_HOST_CALL stringProtoFuncSplitFast(ExecState* exec)
             // 1. Let T be a String value equal to the substring of S consisting of the characters at positions p (inclusive)
             //    through q (exclusive).
             // 2. Call CreateDataProperty(A, ToString(lengthA), T).
-            result->putDirectIndex(exec, resultLength, jsSubstring(exec, thisValue, input, position, matchPosition - position));
-            RETURN_IF_EXCEPTION(scope, encodedJSValue());
+            auto* substring = jsSubstring(exec, thisValue, input, position, matchPosition - position);
+            RETURN_IF_EXCEPTION(scope, { });
+            result->putDirectIndex(exec, resultLength, substring);
+            RETURN_IF_EXCEPTION(scope, { });
             // 3. Increment lengthA by 1.
             // 4. If lengthA == lim, return A.
             if (++resultLength == limit)
@@ -1316,8 +1323,10 @@ EncodedJSValue JSC_HOST_CALL stringProtoFuncSplitFast(ExecState* exec)
     // 15. Let T be a String value equal to the substring of S consisting of the characters at positions p (inclusive)
     //     through s (exclusive).
     // 16. Call CreateDataProperty(A, ToString(lengthA), T).
+    auto* substring = jsSubstring(exec, thisValue, input, position, input.length() - position);
+    RETURN_IF_EXCEPTION(scope, { });
     scope.release();
-    result->putDirectIndex(exec, resultLength++, jsSubstring(exec, thisValue, input, position, input.length() - position));
+    result->putDirectIndex(exec, resultLength++, substring);
 
     // 17. Return A.
     return JSValue::encode(result);
@@ -1361,9 +1370,10 @@ EncodedJSValue JSC_HOST_CALL stringProtoFuncSubstr(ExecState* exec)
         length = len - start;
     unsigned substringStart = static_cast<unsigned>(start);
     unsigned substringLength = static_cast<unsigned>(length);
+    scope.release();
     if (jsString)
         return JSValue::encode(jsSubstring(exec, jsString, substringStart, substringLength));
-    return JSValue::encode(jsSubstring(exec, uString, substringStart, substringLength));
+    return JSValue::encode(jsSubstring(&vm, uString, substringStart, substringLength));
 }
 
 EncodedJSValue JSC_HOST_CALL builtinStringSubstrInternal(ExecState* exec)
@@ -1421,7 +1431,7 @@ EncodedJSValue JSC_HOST_CALL stringProtoFuncSubstring(ExecState* exec)
     }
     unsigned substringStart = static_cast<unsigned>(start);
     unsigned substringLength = static_cast<unsigned>(end) - substringStart;
-    return JSValue::encode(jsSubstring(exec, jsString, substringStart, substringLength));
+    RELEASE_AND_RETURN(scope, JSValue::encode(jsSubstring(exec, jsString, substringStart, substringLength)));
 }
 
 EncodedJSValue JSC_HOST_CALL stringProtoFuncToLowerCase(ExecState* exec)
@@ -1434,7 +1444,7 @@ EncodedJSValue JSC_HOST_CALL stringProtoFuncToLowerCase(ExecState* exec)
         return throwVMTypeError(exec, scope);
     JSString* sVal = thisValue.toString(exec);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    const String& s = sVal->value(exec);
+    String s = sVal->value(exec);
     String lowercasedString = s.convertToLowercaseWithoutLocale();
     if (lowercasedString.impl() == s.impl())
         return JSValue::encode(sVal);
@@ -1451,7 +1461,7 @@ EncodedJSValue JSC_HOST_CALL stringProtoFuncToUpperCase(ExecState* exec)
         return throwVMTypeError(exec, scope);
     JSString* sVal = thisValue.toString(exec);
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    const String& s = sVal->value(exec);
+    String s = sVal->value(exec);
     String uppercasedString = s.convertToUppercaseWithoutLocale();
     if (uppercasedString.impl() == s.impl())
         return JSValue::encode(sVal);
index 4d06113..75ce8d1 100644 (file)
@@ -30,7 +30,7 @@
 namespace JSC {
 
 template<typename NumberType>
-ALWAYS_INLINE JSString* stringSlice(ExecState* exec, String&& string, NumberType start, NumberType end)
+ALWAYS_INLINE JSString* stringSlice(VM& vm, String&& string, NumberType start, NumberType end)
 {
     int32_t length = string.length();
     NumberType from = start < 0 ? length + start : start;
@@ -40,9 +40,9 @@ ALWAYS_INLINE JSString* stringSlice(ExecState* exec, String&& string, NumberType
             from = 0;
         if (to > length)
             to = length;
-        return jsSubstring(exec, WTFMove(string), static_cast<unsigned>(from), static_cast<unsigned>(to) - static_cast<unsigned>(from));
+        return jsSubstring(&vm, WTFMove(string), static_cast<unsigned>(from), static_cast<unsigned>(to) - static_cast<unsigned>(from));
     }
-    return jsEmptyString(exec);
+    return jsEmptyString(&vm);
 }
 
 } // namespace JSC