Bug 232966

Summary: GetMyArgumentByValOutOfBounds needs to check for negative indices
Product: WebKit Reporter: Lukas Bernhard <lukas.bernhard>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, lukas.bernhard, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
patch
ysuzuki: review+
patch for landing none

Description Lukas Bernhard 2021-11-10 13:56:56 PST
During differential testing of webkit I found a sample triggering a miscomputation in FTL.

JSC on git commit: 93d2e7bf275b
Build options: ./Tools/Scripts/build-jsc --jsc-only --release --cmakeargs="-ENABLE_STATIC_JSC=ON -DCMAKE_C_COMPILER='/usr/bin/clang-12' -DCMAKE_CXX_COMPILER='/usr/bin/clang++-12' -DCMAKE_CXX_FLAGS='-fsanitize-coverage=trace-pc-guard -O3 -lrt -fuse-ld=lld'"

WebKitBuild/Debug/bin/jsc --validateOptions=true --useConcurrentJIT=false --useConcurrentGC=false --thresholdForJITSoon=10 --thresholdForJITAfterWarmUp=10 --thresholdForOptimizeAfterWarmUp=100 --thresholdForOptimizeAfterLongWarmUp=100 --thresholdForOptimizeSoon=100 --thresholdForFTLOptimizeAfterWarmUp=1000 --thresholdForFTLOptimizeSoon=1000 --validateBCE=true --useFTLJIT=true sample.js

```
function main() {
    let v26;

    const v13 = [0, 0]; 
    v16 = [0];
    const v18 = v16.__proto__;

    const v23 = (v24,...v25) => {
        v26 = v25[-80887344];
    };  
    v18[-80887344] = v13;
    for (let v30 = -256; v30 < 100; v30++) {
        const v31 = v23(0);
    }   
    print(v26);  // 0,0 without FTL, undefined without FTL
}
noDFG(main);
main();
```
Comment 1 Lukas Bernhard 2021-11-10 23:53:42 PST
The comment in the sample is incorrect, instead of
```
// 0,0 without FTL, undefined without FTL
```

it should be:
```
// 0,0 without FTL, undefined with FTL. (also 0,0 in spidermonkey)
```
Comment 2 Radar WebKit Bug Importer 2021-11-17 13:57:21 PST
<rdar://problem/85519898>
Comment 3 Saam Barati 2021-11-29 17:36:51 PST
Created attachment 445366 [details]
Patch
Comment 4 Saam Barati 2021-11-29 17:42:27 PST
Created attachment 445367 [details]
patch
Comment 5 Yusuke Suzuki 2021-11-29 17:48:38 PST
Comment on attachment 445367 [details]
patch

r=me
Comment 6 Saam Barati 2021-11-29 17:50:12 PST
Comment on attachment 445367 [details]
patch

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

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:5668
> +        LValue isOutOfBounds = m_out.aboveOrEqual(indexToCheck, numberOfArgs);

I'll clean up the scope of the numberOfArgumentsToSkip since we no longer use it here.
Comment 7 Saam Barati 2021-11-29 17:51:30 PST
Created attachment 445370 [details]
patch for landing
Comment 8 EWS 2021-11-30 12:00:53 PST
Committed r286312 (244671@main): <https://commits.webkit.org/244671@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 445370 [details].