601968 - call(null, args) aborts trace

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

When I log aborts/stats for the attached testcase, I get:

trace stopped: 11346: trying to call native apply or call
Abort recording of tree /Users/bzbarsky/bar.js:2@17 at /Users/bzbarsky/bar.js:3@25: apply.
trace stopped: 11346: trying to call native apply or call
Abort recording of tree /Users/bzbarsky/bar.js:2@17 at /Users/bzbarsky/bar.js:3@25: apply.
trace stopped: 11346: trying to call native apply or call
Abort recording of tree /Users/bzbarsky/bar.js:2@17 at /Users/bzbarsky/bar.js:3@25: apply.
recorder: started(3), aborted(3), completed(0), different header(0), trees trashed(0), slot promoted(0), unstable loop variable(0), breaks(0), returns(0), merged loop exits(0), unstableInnerCalls(0), blacklisted(1)
monitor: exits(0), timeouts(0), type mismatch(0), triggered(0), global mismatch(0), flushed(0)

If I do the same thing but pass a non-null object to call(), things trace fine.
call(null) is reasonably common out there...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

The relevant code seems to be:

    /*
     * We don't trace apply and call with a primitive 'this', which is the
     * first positional parameter.
     */
    if (argc > 0 && !vp[2].isObject())
        return record_JSOP_CALL();

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: . Trace call(null).

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: . Trace call(null).

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 480990 [details] [diff] [review]
.  Trace call(null).

This seems to be safe to me... am I missing something?

Comment 6

[Andreas Gal :gal]

Comment on attachment 480990 [details] [diff] [review]
.  Trace call(null).

Blake, its ok to not call the this object hook here right because the callee will do that if it actually does this.bla?

Comment 7

[Jason Orendorff [:jorendorff]]

Yes, it's OK.

Once we get primitive this where we want it, we can relax this further. But computeThis already does the right thing for null.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 480990 [details] [diff] [review]
.  Trace call(null).

Requesting approval and all

Comment 9

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/tracemonkey/rev/a2d2127aaf61

Comment 10

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/a2d2127aaf61