SavedStacks doesn't save the full stack

RESOLVED FIXED in mozilla33

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bzbarsky, Assigned: fitzgen)

Tracking

unspecified
mozilla33
x86
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Compare what js::ComputeStackString does:

  NonBuiltinFrameIter i(cx, FrameIter::ALL_CONTEXTS, FrameIter::GO_THROUGH_SAVED,
                        cx->compartment()->principals);

or what JS::DescribeStack does:

    NonBuiltinFrameIter i(cx, FrameIter::ALL_CONTEXTS,
                          FrameIter::GO_THROUGH_SAVED,
                          cx->compartment()->principals);

with what SavedStacks does:

    ScriptFrameIter iter(cx);

This causes js/xpconnect/tests/mochitest/test_bug960820.html to fail if I switch over XPConnect stack walking from JS::DescribeStack to SavedStacks, because in that case we do in fact have to cross JSContexts _and_ go through saved stacks to get the right answer, as far as I can tell.
Bug 1030945 switches to plain old FrameIter (and we still use principals to show/hide self hosted frames). After that, it seems we would just need to add the ALL_CONTEXTS and GO_THROUGH_SAVED flags, right?
Depends on: 1030945
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
> After that, it seems we would just need to add the ALL_CONTEXTS and GO_THROUGH_SAVED
> flags, right?

Yes, indeed.  That can even happen independently of bug 1030945, modulo merge issues.
Comment on attachment 8453969 [details] [diff] [review]
save-cross-context-stacks.patch

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

::: js/src/vm/SavedStacks.cpp
@@ +398,5 @@
>  SavedStacks::saveCurrentStack(JSContext *cx, MutableHandleSavedFrame frame, unsigned maxFrameCount)
>  {
>      JS_ASSERT(initialized());
>      JS_ASSERT(&cx->compartment()->savedStacks() == this);
> +    FrameIter iter(cx, FrameIter::ALL_CONTEXTS, FrameIter::GO_THROUGH_SAVED);

Just to make sure: we don't pass in principals here because that filtering is done at stringifying time.
Attachment #8453969 - Flags: review?(shu) → review+
Correct.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5048d0b0b825
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.