1034477 - SavedStacks doesn't save the full stack

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

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.

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

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?

Comment 2

[Boris Zbarsky [:bzbarsky]]

> 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 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: save-cross-context-stacks.patch

https://tbpl.mozilla.org/?tree=Try&rev=fe52923a8e59

Comment 4

[Shu-yu Guo [:shu]]

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.

Comment 5

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Correct.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5048d0b0b825

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5048d0b0b825