Bug 141881 – Function name scope is only created on the function instance that triggered parsing rather than on every function instance that needs it

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

[Filip Pizlo]

Our bytecode generation has a tendency to mistreat the reexecutability of closures.  For example, we have this silly tendency to use the scope of the current execution of a function to find the symbol tables of all future executions.  Of course, this just barely works.  And because of the tendency to mistreat the reexecutability of closures, and our silly tendency to use the current scope for predicting future resolutions, we have a hilarious bug where only the first execution of a function that needs a name scope actually gets that name scope.

[Filip Pizlo]

Test case below.  Note that this passes on the first execution of check(); the failure is experienced on execution #2.  This has nothing to do with JITing; it's just our mistreatment of the reexecutability of closures.

function foo() {
    return function bar(str) {
        var barBefore = bar;
        var result = eval(str);
        return [
            barBefore,
            bar,
            function () {
                return bar;
            },
            result
        ];
    }
}

function check() {
    var bar = foo();
    
    function verify(result, barAfter, evalResult) {
        if (result[0] !== bar)
            throw "Error: bad first entry: " + result[0];
        if (result[1] !== barAfter)
            throw "Error: bad first entry: " + result[1];
        var subResult = result[2]();
        if (subResult !== barAfter)
            throw "Error: bad second entry: " + result[2] + "; returned: " + subResult;
        if (result[3] !== evalResult)
            throw "Error: bad third entry: " + result[3] + "; expected: " + evalResult;
    }
    
    verify(bar("42"), bar, 42);
    verify(bar("bar"), bar, bar);
    verify(bar("var bar = 42; function fuzz() { return bar; }; fuzz()"), 42, 42);
}

for (var i = 0; i < 100; ++i)
    check();

[Filip Pizlo]

Ideally, the way this would work is that we'd have the bytecode create the scope. But that would break the current hack where we create the name scope just before bytecode linking so that linking sees the scope in the way.

Maybe a correct solution would be:

1) Bytecode creates the scope at the prologue.
2) Bytecode linking is given a phony function name scope.

[Filip Pizlo]

Created attachment 247090 [details]
the basic idea

This code is pretty shameful, but it's less wrong than trunk.

[Filip Pizlo]

Created attachment 247092 [details]
the patch

[Filip Pizlo]

Created attachment 247093 [details]
the patch

New patch because I realized that the bug title was incoherent.

[Filip Pizlo]

Created attachment 247094 [details]
the patch

It was still incoherent. Maybe it's more coherent now.

[Michael Saboff]

Comment on attachment 247094 [details]
the patch

r=me

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/180506