Changeset 292484 in webkit

Timestamp:

Apr 6, 2022 11:48:46 AM (2 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] Substring resolving should check 8bit / 16bit again
​https://bugs.webkit.org/show_bug.cgi?id=236775
<rdar://problem/89253391>

Reviewed by Saam Barati.

JSTests:

• stress/8bit-16bit-atomize-conversion.js: Added.

(main.v64):
(main):

Source/JavaScriptCore:

Substring JSString is wrapping JSString. Thus it is possible that underlying JSString's 8Bit / 16Bit status
becomes different from substring JSString wrapper's bit. We should not assume they are the same.

• runtime/JSString.cpp:

(JSC::JSRopeString::resolveRopeInternal const):
(JSC::JSRopeString::resolveRopeToAtomString const):
(JSC::JSRopeString::resolveRopeToExistingAtomString const):
(JSC::JSRopeString::resolveRopeInternal8 const): Deleted.
(JSC::JSRopeString::resolveRopeInternal16 const): Deleted.

• runtime/JSString.h:

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/8bit-16bit-atomize-conversion.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSString.cpp (modified) (4 diffs)
• Source/JavaScriptCore/runtime/JSString.h (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2022-04-06  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Substring resolving should check 8bit / 16bit again
+        https://bugs.webkit.org/show_bug.cgi?id=236775
+        <rdar://problem/89253391>
+
+        Reviewed by Saam Barati.
+
+        * stress/8bit-16bit-atomize-conversion.js: Added.
+        (main.v64):
+        (main):
+
 2022-04-06  Alexey Shvayka  <ashvayka@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2022-04-06  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Substring resolving should check 8bit / 16bit again
+        https://bugs.webkit.org/show_bug.cgi?id=236775
+        <rdar://problem/89253391>
+
+        Reviewed by Saam Barati.
+
+        Substring JSString is wrapping JSString. Thus it is possible that underlying JSString's 8Bit / 16Bit status
+        becomes different from substring JSString wrapper's bit. We should not assume they are the same.
+
+        * runtime/JSString.cpp:
+        (JSC::JSRopeString::resolveRopeInternal const):
+        (JSC::JSRopeString::resolveRopeToAtomString const):
+        (JSC::JSRopeString::resolveRopeToExistingAtomString const):
+        (JSC::JSRopeString::resolveRopeInternal8 const): Deleted.
+        (JSC::JSRopeString::resolveRopeInternal16 const): Deleted.
+        * runtime/JSString.h:
+
 2022-04-06  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/JavaScriptCore/runtime/JSString.cpp

@@ -153 +153 @@
 static constexpr unsigned maxLengthForOnStackResolve = 2048;
 
-void JSRopeString::resolveRopeInternal8(LChar* buffer) const
+template<typename CharacterType>
+void JSRopeString::resolveRopeInternal(CharacterType* buffer) const
 {
     if (isSubstring()) {
-        StringImpl::copyCharacters(buffer, substringBase()->valueInternal().characters8() + substringOffset(), length());
-        return;
-    }
-    
-    resolveRopeInternalNoSubstring(buffer);
-}
-
-void JSRopeString::resolveRopeInternal16(UChar* buffer) const
-{
-    if (isSubstring()) {
-        StringImpl::copyCharacters(
-            buffer, substringBase()->valueInternal().characters16() + substringOffset(), length());
+        // It is possible that underlying string becomes 8bit/16bit while wrapper substring is saying it is 16bit/8bit.
+        // But It is definitely true that substring part can be represented as its parent's status 8bit/16bit, which is described as CharacterType.
+        auto& string = substringBase()->valueInternal();
+        if (string.is8Bit())
+            StringImpl::copyCharacters(buffer, string.characters8() + substringOffset(), length());
+        else
+            StringImpl::copyCharacters(buffer, string.characters16() + substringOffset(), length());
         return;
     }
@@ -211 +207 @@
     if (is8Bit()) {
         LChar buffer[maxLengthForOnStackResolve];
-        resolveRopeInternal8(buffer);
+        resolveRopeInternal(buffer);
         convertToNonRope(AtomStringImpl::add(buffer, length()));
     } else {
         UChar buffer[maxLengthForOnStackResolve];
-        resolveRopeInternal16(buffer);
+        resolveRopeInternal(buffer);
         convertToNonRope(AtomStringImpl::add(buffer, length()));
     }
@@ -256 +252 @@
     if (is8Bit()) {
         LChar buffer[maxLengthForOnStackResolve];
-        resolveRopeInternal8(buffer);
+        resolveRopeInternal(buffer);
         if (RefPtr<AtomStringImpl> existingAtomString = AtomStringImpl::lookUp(buffer, length())) {
             convertToNonRope(*existingAtomString);
@@ -263 +259 @@
     } else {
         UChar buffer[maxLengthForOnStackResolve];
-        resolveRopeInternal16(buffer);
+        resolveRopeInternal(buffer);
         if (RefPtr<AtomStringImpl> existingAtomString = AtomStringImpl::lookUp(buffer, length())) {
             convertToNonRope(*existingAtomString);

trunk/Source/JavaScriptCore/runtime/JSString.h

@@ -609 +609 @@
     Identifier toIdentifier(JSGlobalObject*) const;
     void outOfMemory(JSGlobalObject* nullOrGlobalObjectForOOM) const;
-    void resolveRopeInternal8(LChar*) const;
-    void resolveRopeInternal16(UChar*) const;
+    template<typename CharacterType> void resolveRopeInternal(CharacterType*) const;
     StringView unsafeView(JSGlobalObject*) const;
     StringViewWithUnderlyingString viewWithUnderlyingString(JSGlobalObject*) const;