Bug 232966 – GetMyArgumentByValOutOfBounds needs to check for negative indices

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

[Lukas Bernhard]

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();
```

[Lukas Bernhard]

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)
```

[Radar WebKit Bug Importer]

<rdar://problem/85519898>

[Saam Barati]

Created attachment 445366 [details]
Patch

[Saam Barati]

Created attachment 445367 [details]
patch

[Yusuke Suzuki]

Comment on attachment 445367 [details]
patch

r=me

[Saam Barati]

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.

[Saam Barati]

Created attachment 445370 [details]
patch for landing

[EWS]

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].