698101 - TI: Function cloning messes up inline allocation path

Status: RESOLVED DUPLICATE of bug 706110

(Core :: JavaScript Engine, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Test case (run in shell with -m -n -a; works with other options):

function testLambdaCtor() {
    var q;
    for (var x = 0; x < 2; ++x) {
        var f = function(){};
	if (x == 1) gc();
        q = new f;
    }
    return q.__proto__ === f.prototype;
}
assertEq(testLambdaCtor(), true);

I think the problem is as follows. We clone f twice. Normally, each time we call the constructor, we fetch f.prototype, which triggers the fun_resolve hook and creates a new prototype object. We end up with two prototype objects, one for each cloned function. At the end, we're asserting that the second cloned function's .prototype equals the second object's .__proto__.

However, doing the gc triggers a recompilation of the empty lambda in the second loop iteration. Rather than calling the fun_resolve hook, the jit compiler fetches the prototype out of the function's type object. This isn't the cloned function--it's the one stored in the bytecode. So it gets the prototype from the first iteration. After the loop, when it does the ===, it finally generates a new .prototype via fun_resolve. This will be different than the first iteration's .prototype, so we fail.

I have no idea how to fix this. Maybe we could detect that f has been cloned more than once and not do the optimization in this case? Could you take a look, Brian?

Comment 1

[Brian Hackett [Laid off!]]

(In reply to Bill McCloskey (:billm) from comment #0)
> I have no idea how to fix this. Maybe we could detect that f has been cloned
> more than once and not do the optimization in this case? Could you take a
> look, Brian?

Mmm, the problem is that the getSingleton in the function resolve hook is not accounting for future prototypes constructed via the resolve hook.  The right fix is to only inline construction of 'this' for functions with singleton types, i.e. toplevel functions rather than those nested inside another function or in a loop.  (The 'new' Class.create function in the Prototype library is an inner function, but the patch in bug 638794 handles that by giving such inner functions their own singleton types).

Comment 2

[Brian Hackett [Laid off!]]

Fixed by the bug 706110 patch (will include this testcase when pushing that fix).