Closed Bug 894854 Opened 7 years ago Closed 7 years ago

Add env var that disables censoring of self-hosted script frames

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: pnkfelix, Assigned: pnkfelix)

References

(Blocks 1 open bug)

Details

(Whiteboard: [js:t])

Attachments

(1 file, 3 obsolete files)

We currently take pains to skip over the frames from self-hosted javascript code when building up stack traces.

That makes sense for inter-operation with tools that want to inspect the stack, where we want to hide our internal implementation details.

But when one is debugging the self-hosted code itself, skipping over those frames is throwing out useful information.

So we should provide some way for that information to be reported; not by default, but when used in tandem with some combination of:

* A debug-build of spidermonkey, and/or

* A shell build, and/or

* Some environment variable setting.
Attached patch patch A v1: DEBUG guard alone (obsolete) — Splinter Review
This is probably not what we actually want.  I know I'm inviting a bike shed discussion, but since I don't actually care what constraints we add to this "functionality", go ahead, pile on what shades of paint you want to see here.
Attachment #777034 - Flags: feedback?(till)
Sorry, should've filed this yesterday.

We can't make it hinge on debug mode, as that would make testing of at least some addons (such as GreaseMonkey) impossible. And in general, would be a pretty severe change of behavior between release and debug.

Enabling it only in the shell would work, but it would also be pretty unfortunate: there'd be no way to debug self-hosting issues that one can reproduce only in the browser. Plus, again, introduce a noticeable difference in behavior between two environments: the shell and the browser, in this case. We already have too many of those.

Thus, an env var is probably the way to go.
Blocks: 784288
OS: Mac OS X → All
Hardware: x86 → All
Summary: self-hosted errors should be identified by line number in debug (and/or shell?) → Add env var that disables censoring of self-hosted script frames
Whiteboard: [js:t]
Comment on attachment 777034 [details] [diff] [review]
patch A v1: DEBUG guard alone

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

Clearing feedback based on the previous comment.
Attachment #777034 - Flags: feedback?(till)
Attached patch patch A v2: env var guard alone (obsolete) — Splinter Review
Attachment #777034 - Attachment is obsolete: true
Attachment #777075 - Flags: review?(till)
Assignee: general → pnkfelix
Not asking for review yet because this is not tested beyond checking it compile; just checkpointing here; will sanity-check later today.
Attachment #777075 - Attachment is obsolete: true
Attachment #777075 - Flags: review?(till)
Comment on attachment 777092 [details] [diff] [review]
patch A v3: cached env var and debug guard both

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

I know you didn't ask for a review, but if your testing shows this to work as expected, I'm fine with it.

My only quibble is the long name for the env var: I'm 100% certain I won't be able to remember that. How about MOZ_SHOW_SH_FRAMES, or even just SHOW_SH_FRAMES? (Little danger of that colliding with anything, I'd say.)
Attachment #777092 - Flags: review+
till notes that it would be good to update this page:

  https://developer.mozilla.org/en-US/docs/SpiderMonkey/Internals/self-hosting

in tandem with landing any patch for this bug.
inherited r+ from till.

a bit of bikeshedding in pjs: http://logbot.glob.com.au/?c=mozilla%23pjs#c35581
yielded the environment variable name that we settled on: MOZ_SHOW_ALL_JS_FRAMES
Attachment #777092 - Attachment is obsolete: true
Attachment #777177 - Flags: review+
try run:

try: -b do -p all -u all[x86] -t none

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

(all green)
https://hg.mozilla.org/mozilla-central/rev/5e29b812885c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.