517640 - TM: we don't substitute the global object for apply(null, ...)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Comment 1

[Andreas Gal :gal]

function f() {
}

for (var i = 0; i < 10; ++i)
    f.apply(null);

This aborts trace right now.

We could run a quick imacro that pops the null and pushes the global object (maybe JSOP_GLOBAL)? If we do it before we unpack the arguments, we could use 1 imacro instead of 8 for the individual argument lengths we support. But then if we fall off trace there is a global object where a null should be. Thats a bit odd. It should be effect-free though when the interpreter resumes.

Comment 2

[Andreas Gal :gal]

Attachment: minimal fix

Actually, this minimal fix should be sufficient.

Comment 3

[Andreas Gal :gal]

This is perf, not correctness.

bz, can you try this?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Makes the abort go away.  Doesn't help my testcase on its own, since we hit other aborts (due to inner trees trying to grow) instead...

Comment 5

[Andreas Gal :gal]

Comment on attachment 401651 [details] [diff] [review]
minimal fix

Well its a step in the right direction.

Comment 6

[Andreas Gal :gal]

Could someone land this on TM?

Comment 7

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/tracemonkey/rev/8d90c8c461e8

Comment 8

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/8d90c8c461e8

Comment 9

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/13e613dc1d51