Closed Bug 601968 Opened 14 years ago Closed 14 years ago

call(null, args) aborts trace

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(2 files, 1 obsolete file)

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...
Attached file Testcase
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();
Attached patch . Trace call(null). (obsolete) — Splinter Review
Attachment #480966 - Attachment is obsolete: true
Comment on attachment 480990 [details] [diff] [review]
.  Trace call(null).

This seems to be safe to me... am I missing something?
Attachment #480990 - Flags: review?(gal)
Assignee: general → bzbarsky
OS: Mac OS X → All
Priority: -- → P1
Hardware: x86 → All
Whiteboard: [needs review]
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?
Attachment #480990 - Flags: review?(mrbkap)
Attachment #480990 - Flags: review?(gal)
Attachment #480990 - Flags: review+
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 on attachment 480990 [details] [diff] [review]
.  Trace call(null).

Requesting approval and all
Attachment #480990 - Flags: approval2.0?
Whiteboard: [needs review] → [need approval]
Blocks: 602132
Attachment #480990 - Flags: approval2.0? → approval2.0+
Attachment #480990 - Flags: review?(mrbkap) → review?
Attachment #480990 - Flags: review?
Pushed http://hg.mozilla.org/tracemonkey/rev/a2d2127aaf61
Whiteboard: [need approval] → fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/a2d2127aaf61
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.