Bug 236775 – [JSC] Substring resolving should check 8bit / 16bit again

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Lukas Bernhard]

The attached sample triggers an assertion in webkit on git commit d96b38bfed8b

Build command: ./Tools/Scripts/build-jsc --jsc-only --debug --cmakeargs="-DENABLE_STATIC_JSC=ON -DCMAKE_C_COMPILER='/usr/bin/clang-12' -DCMAKE_CXX_COMPILER='/usr/bin/clang++-12' -DCMAKE_CXX_FLAGS='-O3 -lrt -latomic -fuse-ld=lld'"
Run command: build/Debug/bin/jsc --validateOptions=true --useConcurrentJIT=false --useConcurrentGC=false --thresholdForJITSoon=10 --thresholdForJITAfterWarmUp=10 --thresholdForOptimizeAfterWarmUp=100 --thresholdForOptimizeAfterLongWarmUp=100 --thresholdForOptimizeAfterLongWarmUp=100 --thresholdForFTLOptimizeAfterWarmUp=1000 --thresholdForFTLOptimizeSoon=100

// STDERR:
// ASSERTION FAILED: !is8Bit()
// WTF/Headers/wtf/text/StringImpl.h(297) : const UChar *WTF::StringImpl::characters16() const

sample.js:
```
function main() {
    for (let v27 = 0; v27 < 100; v27++) {
        const v44 = [0,0,1.1];
        const v61 = v44.toLocaleString();
        const v62 = eval(Math);
        v63 = v61.substring(v62,v27);

        function v64() {
            if (v62) {
                Math[v61] = [];
            }
            const v82 = (-1.0).__proto__;
            delete v82[v63];
        }
        v64();
    }
}
main();
```

Full backtrace:
#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=140737314615232) at pthread_kill.c:44
#1  __pthread_kill_internal (signo=6, threadid=140737314615232) at pthread_kill.c:80
#2  __GI___pthread_kill (threadid=140737314615232, signo=signo@entry=6) at pthread_kill.c:91
#3  0x00007ffff5a96476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4  0x00007ffff5a7c7b7 in __GI_abort () at abort.c:79
#5  0x0000000000cd501a in WTFCrashWithInfo () at WTF/Headers/wtf/Assertions.h:741
#6  0x0000000001e6d1b9 in WTF::StringImpl::characters16 (this=<optimized out>) at WTF/Headers/wtf/text/StringImpl.h:297
#7  WTF::String::characters16 (this=<optimized out>) at WTF/Headers/wtf/text/WTFString.h:129
#8  JSC::JSRopeString::resolveRopeInternal16 (this=this@entry=0x7fffaa4f0020, buffer=buffer@entry=0x7fffffffc140 u"쓼\xffff翿")
    at ../../Source/JavaScriptCore/runtime/JSString.cpp:169
#9  0x0000000001e6d42a in JSC::JSRopeString::resolveRopeToAtomString (this=0x7fffaa4f0020, globalObject=<optimized out>)
    at ../../Source/JavaScriptCore/runtime/JSString.cpp:217
#10 0x0000000000cd6a84 in JSC::JSRopeString::toIdentifier (this=0x6, this@entry=0x7fffaa4f0020,
    globalObject=globalObject@entry=0x7fffaa460a68) at ../../Source/JavaScriptCore/runtime/JSString.h:771
#11 0x0000000000cd654c in JSC::JSString::toIdentifier (this=0x7fffaa4f0020, globalObject=globalObject@entry=0x7fffaa460a68)
    at ../../Source/JavaScriptCore/runtime/JSString.h:794
#12 0x0000000000cd5498 in JSC::JSValue::toPropertyKey (this=<optimized out>, globalObject=0x7fffaa460a68)
    at ../../Source/JavaScriptCore/runtime/JSCJSValueInlines.h:808
#13 0x0000000001a72664 in JSC::deleteByVal (globalObject=globalObject@entry=0x7fffaa460a68, vm=..., slot=..., base=..., key=...,
    ecmaMode=ecmaMode@entry=...) at ../../Source/JavaScriptCore/jit/JITOperations.cpp:2614
#14 0x0000000001a71fd8 in JSC::operationDeleteByValOptimize (globalObject=0x7fffaa460a68, stubInfo=0x7fffec059ca8,
    encodedBase=140737152821896, encodedSubscript=140736050692128, ecmaMode=...) at ../../Source/JavaScriptCore/jit/JITOperations.cpp:2636

[Radar WebKit Bug Importer]

<rdar://problem/89253391>

[Yusuke Suzuki]

Created attachment 456800 [details]
Patch

[Yusuke Suzuki]

Created attachment 456802 [details]
Patch

[Saam Barati]

Comment on attachment 456802 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456802&action=review

> Source/JavaScriptCore/ChangeLog:10
> +        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.

Why does the underlying string change here?

[Yusuke Suzuki]

Comment on attachment 456802 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456802&action=review

>> Source/JavaScriptCore/ChangeLog:10
>> +        becomes different from substring JSString wrapper's bit. We should not assume they are the same.
> 
> Why does the underlying string change here?

One possible case is that, underlying string was 16bit rope, and after resolving that, it was converted into the existing 8bit atom string.

[Yusuke Suzuki]

Committed r292484 (249335@trunk): <https://commits.webkit.org/249335@trunk>

[Brent Fulgham]

This fix shipped with Safari 15.5 (all platforms).