1144973 - js::StringifySavedFrameStack doesn't put the string in the cx's compartment like it promises

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

It's documentation says:

> The stringified stack out parameter is placed in the cx's compartment.

*Sigh*, but that just is not true.

The first thing it does is maybe enter the SavedFrame's compartment and then the string is built in that possibly different compartment.

Comment 1

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

Attachment: Ensure that StringifySavedFrameStack puts its stack string in the cx's compartment

Comment 2

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

Sorry, I know you aren't doing reviews right now, but you broke this in 04f34e86aee1 and this is a pretty small patch :P

Comment 3

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

Attachment: Same patch but without whitespace changes

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8579711 [details] [diff] [review]
Ensure that StringifySavedFrameStack puts its stack string in the cx's compartment

This works, but couldn't we actually just let the AutoMaybeEnterFrameCompartment go out of scope before calling finishString()?  Seems to me like that would work fine (albeit with a bit more dependency on StringBuffer internals) and avoid the wrap() call.  Maybe I'm just allergic to wrap() calls: they show up in profiles too much.  :(

Either way should document near the opening curly of the scope we're adding what the scope is there for.

Comment 5

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

Attachment: Ensure that StringifySavedFrameStack puts its stack string in the cx's compartment

Comment 6

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

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=17c29e1ad7d1

Comment 7

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

(In reply to Nick Fitzgerald [:fitzgen] from comment #6)
> Try push:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=17c29e1ad7d1

Those failures are fixed in bug 1144108.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f3988f82aefc

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f3988f82aefc