TestingFunctions.cpp's SaveStack can assert on an empty stack

RESOLVED FIXED in mozilla33

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jimb, Assigned: fitzgen)

Tracking

unspecified
mozilla33
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Calling the 'saveStack' JavaScript function from js/src/buitin/TestingFunctions.cpp will assert if there are no JS frames on the stack.

If there are no JS frames on the stack, then JS::CaptureCurrentStack returns nullptr, which the call to args.rval().setObject in SaveStack will choke on. If that call is changed to setObjectOrNull, it should be fine.

I don't have steps to reproduce this, because the JS shell has no way to invoke functions directly, without the global script's stack frame at the base of the stack. However, in the browser, one can do things like:

  setTimeout(fn, 0)

to call fn with no older frames.
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Created attachment 8449748 [details] [diff] [review]
save-stack-no-frames.patch

This should hopefully be a quick and easy review ;)

Try push: https://tbpl.mozilla.org/?tree=Try&rev=81982a7ca2cd
Attachment #8449748 - Flags: review?(jimb)
(Reporter)

Comment 2

5 years ago
Comment on attachment 8449748 [details] [diff] [review]
save-stack-no-frames.patch

Review of attachment 8449748 [details] [diff] [review]:
-----------------------------------------------------------------

Well, the solution seems kind of involved, and it'll be hard to anticipate the repercussions, but I don't see that we have any choice but to see how the impact on Firefox's overall architecture plays out.
Attachment #8449748 - Flags: review?(jimb) → review+
Keywords: checkin-needed
(Reporter)

Comment 3

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5c3ab97e944
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla33
https://hg.mozilla.org/mozilla-central/rev/e5c3ab97e944
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.