Closed Bug 967234 Opened 6 years ago Closed 5 years ago

Debugger no longer needs to explicitly ignore self-hosted scripts

Categories

(DevTools :: Debugger, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 37

People

(Reporter: jimb, Assigned: jimb)

References

Details

Attachments

(1 file)

There's code scattered about in the developer tools to deal with self-hosted code, whose source isn't normally available. However, for a while now Debugger has automatically avoided exposing self-hosted code as debuggable at all, so the special handling in the upper layers is no longer necessary.
The mochitest-browser debugger tests still pass with this patch applied. I need to run the rest of the tests...
Assignee: nobody → jimb
Status: NEW → ASSIGNED
We have a test failure, fantastic:

18:01:20     INFO -  JS frame :: /builds/slave/test/build/tests/xpcshell/tests/toolkit/devtools/server/tests/unit/test_trace_actor-09.js :: check_trace :: line 76
18:01:20     INFO -  JS frame :: /builds/slave/test/build/tests/xpcshell/tests/toolkit/devtools/server/tests/unit/test_trace_actor-09.js :: test_frame_depths/< :: line 35
18:01:20     INFO -  JS frame :: resource://gre/modules/devtools/dbg-client.jsm :: eventSource/aProto.notify :: line 166
18:01:20     INFO -  JS frame :: resource://gre/modules/devtools/dbg-client.jsm :: DebuggerClient.prototype.onPacket/< :: line 697
18:01:20     INFO -  JS frame :: resource://gre/modules/commonjs/sdk/core/promise.js :: resolve :: line 118
18:01:20     INFO -  JS frame :: resource://gre/modules/commonjs/sdk/core/promise.js :: then :: line 43
18:01:20     INFO -  JS frame :: resource://gre/modules/commonjs/sdk/core/promise.js :: then :: line 153
18:01:20     INFO -  JS frame :: resource://gre/modules/devtools/dbg-client.jsm :: DebuggerClient.prototype.onPacket :: line 705
18:01:20     INFO -  JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js -> resource://gre/modules/devtools/server/transport.js :: LDT_send/< :: line 258
18:01:20     INFO -  JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js -> resource://gre/modules/devtools/DevToolsUtils.js :: makeInfallible/< :: line 80
Component: Developer Tools → Developer Tools: Debugger
Priority: -- → P3
So the debugger won't step through self hosted functions anymore? Because even if we could show the source and all that, people would get really annoyed if they had to step through it.
It shouldn't be able to touch them at all. Look for checks of 'selfHosted()' in js/src/vm/Debugger.cpp and .h.
I kind of think there should be a flag on a Debugger saying whether or not it should ignore self-hosted code --- after all, don't the developers of self-hosted code need tools too?
Here is a shell-only test case:

$ cat ~/moz/trace-self-hosted.js
var g = newGlobal();
var dbg = new Debugger(g);
dbg.onEnterFrame = function (frame) {
  print("> " + frame.script.url);
}

g.eval("(" + function iife() {
  [1].forEach(function noop() {});
  for (let x of [1]) {}
} + ")()");
$ obj~/js/src/js -f ~/moz/trace-self-hosted.js
> /home/jimb/moz/trace-self-hosted.js
> /home/jimb/moz/trace-self-hosted.js
> self-hosted
> /home/jimb/moz/trace-self-hosted.js
> self-hosted
> self-hosted
> self-hosted
> self-hosted
> self-hosted
> self-hosted
$
Filed bug 967718 for the Debugger bug; when that's fixed, perhaps we can land this patch.
Depends on: 967718
Now that bug 967718 is fixed, a fresh try push here:
https://tbpl.mozilla.org/?tree=Try&rev=b174c4bf345a
That Try push expired, here's another:
https://tbpl.mozilla.org/?tree=Try&rev=87cc599dc920
If we automatically avoid exposing self-hosted code, how is it possible that we can see the code for Function.prototype in 1023460? (assuming that is indeed self-hosted code, which jimb hasn't confirmed yet).
Jim, bug 967718 has since been fixed. Are there any plans on your side to finish this up?
(In reply to Eddy Bruel [:ejpbruel] from comment #11)
> If we automatically avoid exposing self-hosted code, how is it possible that
> we can see the code for Function.prototype in 1023460? (assuming that is
> indeed self-hosted code, which jimb hasn't confirmed yet).

It's not self-hosted code as such. It just gets some fairly meaningless source set during initialization because that's required by ... something, but it's entirely unrelated to the other self-hosting stuff.
(In reply to Till Schneidereit [:till] from comment #13)
> (In reply to Eddy Bruel [:ejpbruel] from comment #11)
> > If we automatically avoid exposing self-hosted code, how is it possible that
> > we can see the code for Function.prototype in 1023460? (assuming that is
> > indeed self-hosted code, which jimb hasn't confirmed yet).
> 
> It's not self-hosted code as such. It just gets some fairly meaningless
> source set during initialization because that's required by ... something,
> but it's entirely unrelated to the other self-hosting stuff.

Hey Till, thanks for clearing that up! That makes me wonder if 1023460 should actually be considered a bug, since those weird scripts are apparently *supposed* to be there.
Summary: devtools should no longer need to explicitly ignore self-hosted scripts → Debugger no longer needs to explicitly ignore self-hosted scripts
Hi Jim, could you please unassign yourself from this bug if you're not longer working on it?
Flags: needinfo?(jimb)
I think the patch there is correct, and worth landing; I'd just forgotten about it.

Fresh try push:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=6f8a8874d874
Flags: needinfo?(jimb)
Attachment #8369750 - Flags: review?(ejpbruel)
Comment on attachment 8369750 [details] [diff] [review]
Don't block self-hosted code in the devtool client code; Debugger takes care of this.

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

Patch looks good to me. Thank you for getting back to this Jim!
Attachment #8369750 - Flags: review?(ejpbruel) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/87d51f6a3c29
Flags: in-testsuite-
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → Firefox 37
https://hg.mozilla.org/mozilla-central/rev/87d51f6a3c29
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.