Bug 231321 – EnumeratorGetByVal for IndexedMode+OwnStructureMode doesn't always recover the property name

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

[Lukas Bernhard]

Differential testing identifies the following samples to trigger a miscomputation in FTL.
Tested on e467a9710432ebb3dae9880f897cf93929adc0e6 (Wed Oct 6 16:30:57 2021 +0000)
Sorry I couldn't minimize the testcase further, everything I try to simplify breaks the differential behavior.
Also, the bug description is obviously meaningless due to not having a hunch regarding the root cause.

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 diff.js

function main() {
    let v224;
    const v35 = [0, 0, {b:"AAAAA"}];
    
    async function v36(arr) {
        edenGC();  // removing this break differential
        for (let v198 = 0; v198 < 2; v198++) {
            const v200 = [0, 0]; 
            const v201 = ` 
                for (let v205 = 0; v205 < 60000; v205++) { }

                async function v215() { } // never called but removing breaks differential

                const v222 = {"__proto__":[[]], "a":0, "b":0};
                for (const v223 in v222) {
                    v224 = arr[v223];
                    v222.__proto__ = {};
                }
                v200;
            `;
            eval(v201); // moving code out of eval breaks differential
        }   
    }   
    v35.filter(v36);
    print(v224) // prints undefined in FTL, AAAAA without FTL (also AAAAA in v8)
}
main();

[Radar WebKit Bug Importer]

<rdar://problem/84211697>

[Saam Barati]

Created attachment 443027 [details]
patch

[Yusuke Suzuki]

Comment on attachment 443027 [details]
patch

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

r=me

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:15908
> +            MacroAssembler::JumpList& genericOrRecoverCase = m_graph.varArgChild(node, 1).node() == m_graph.varArgChild(node, 3).node() ? recoverGenericCase : notFastNamedCases;

For readability, let's define `bool indexedModeAndOwnStructureMode = ...` and use it.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:13686
> +        // FIXME: This is a pretty bad way to say we're using IndexedMode+OwnStructureMode mode. 

Ditto for bool variable.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:13755
> +        if (indexEdge.node() == propertyNameEdge.node()) {

Use bool variable which should be defined above.

[Saam Barati]

Created attachment 443034 [details]
patch for landing

[EWS]

Committed r285167 (243803@main): <https://commits.webkit.org/243803@main>

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