Closed Bug 509599 Opened 11 years ago Closed 10 years ago

Arguments objects are not populated when leaving a function on trace

Categories

(Core :: JavaScript Engine, defect)

Other Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: jorendorff, Assigned: dmandelin)

References

Details

Attachments

(1 file, 1 obsolete file)

function f(a) {
    x = arguments;
}
for (var i = 0; i < 9; i++)
    f(123);
assertEq(x[0], 123);

crasher.js:10: TypeError: Assertion failed: got (void 0), expected 123

In the interpreter, we call js_PutArgsObject when leaving a frame that has fp->argsobj (via JSStackFrame::putActivationObjects). On trace we need to do something like that too -- of course we know if there is an argsobj or not, so this is zero-cost in the fast case.
Attached patch Patch for checker bug (obsolete) — Splinter Review
I already had the right feature in there, I just made a wrong assumption about which opcodes do and don't get run on function return.

I'd like to add the test case after bug 505588, because then it's as simple as adding a new file and there are no nesting level change issues to worry about.
Assignee: general → dmandelin
Status: NEW → ASSIGNED
Attachment #393673 - Flags: review?(jorendorff)
Comment on attachment 393673 [details] [diff] [review]
Patch for checker bug

Looks good.
Attachment #393673 - Flags: review?(jorendorff) → review+
To david:

This will badly conflict with the bug 495061. I hope that can be landed today. Could you use that bug as base?
Depends on: 495061
(In reply to comment #3)
> To david:
> 
> This will badly conflict with the bug 495061. I hope that can be landed today.
> Could you use that bug as base?

Yes, this fix is conceptually very easy so it should not be hard to rebase.
Attachment #393673 - Attachment is obsolete: true
Attachment #394585 - Flags: review?(jorendorff)
Attachment #394585 - Flags: review?(jorendorff) → review+
http://hg.mozilla.org/mozilla-central/rev/78d111c4ab84
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.