Bug 232754 - When inlining NewSymbol in the DFG don't universally call ToString on the input
Summary: When inlining NewSymbol in the DFG don't universally call ToString on the input
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: PC Linux
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-11-05 05:43 PDT by Lukas Bernhard
Modified: 2021-11-09 12:50 PST (History)
9 users (show)

See Also:


Attachments
Patch (12.40 KB, patch)
2021-11-08 17:14 PST, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lukas Bernhard 2021-11-05 05:43:20 PDT
During differential testing of webkit I found a sample triggering a miscomputation in the baseline execution. The sample is larger than I'd like it to be, unfortunately all further minimizations I attempted did break the differential behavior.

The sample is invoked as:
WebKitBuild/Release/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 v83;
    let v102;
    const v30 = []; 
    
    for (let v56 = 0; v56 < 80; v56++) {
        let v57 = 0;
        const v59 = []; 
        const v61 = []; 
            
        v63 = [0.0];
        function v65(v66,v67,v68) {
            const v69 = v63 * v30;
            const v70 = Uint16Array;
            let v72 = 0;
            const v76 = [0,0,v61,0];
            const v78 = []; 
                
            v79 = {__proto__:[], length:"a"};
            const v81 = [0,0,v79,v76];
            v83 = [v65];
            Reflect.apply(v81.map,v67,[v65]);
                
            for (const v86 of v68) {
                v87 = v59 << v86;
                const v91 = new Int32Array(0);
                const v92 = undefined;
                const v94 = Symbol(undefined);
                v102 = v94.description;
                v57++;
            }   
        }   
        const v105 = v65(0.0,"aaaa",v63);
    }
        
    print(v102); // undefined with and without FTL
    print(typeof v102); // string without FTL, undefined with FTL (also undefined in spidermonkey)
}
main();
Comment 1 Saam Barati 2021-11-08 09:43:38 PST
I can't reproduce this on r285408. Can you still reproduce it?
Comment 2 Lukas Bernhard 2021-11-08 13:39:24 PST
The issue still reproduces for me on git 016f88c15b9bf0ebae0090babdad6a34e783d1b5
Just in case it somehow depends on build options, here are the ones I used:
./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'"
Comment 3 Saam Barati 2021-11-08 16:06:26 PST
Thanks, I can reproduce it. I must've been running it with the wrong option earlier.
Comment 4 Saam Barati 2021-11-08 17:14:17 PST
Created attachment 443634 [details]
Patch
Comment 5 Robin Morisset 2021-11-08 17:31:47 PST
Comment on attachment 443634 [details]
Patch

r=me
Comment 6 EWS 2021-11-09 12:49:49 PST
Committed r285525 (244042@main): <https://commits.webkit.org/244042@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 443634 [details].
Comment 7 Radar WebKit Bug Importer 2021-11-09 12:50:19 PST
<rdar://problem/85215832>