Closed
Bug 769754
Opened 12 years ago
Closed 12 years ago
Crash while using the debugger
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla16
Tracking | Status | |
---|---|---|
firefox15 | --- | fixed |
People
(Reporter: paul, Assigned: jimb)
References
Details
(Keywords: crash, Whiteboard: [js:p1])
Crash Data
Attachments
(1 file)
2.16 KB,
patch
|
jorendorff
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Comment 1•12 years ago
|
||
Paul, any specific STR?
Updated•12 years ago
|
Crash Signature: [@ Str ]
Reporter | ||
Comment 2•12 years ago
|
||
More crashes: - https://crash-stats.mozilla.com/report/index/bp-125ab2b3-88b3-4cd9-87f3-536bb2120701 - https://crash-stats.mozilla.com/report/index/b368d8e5-c164-48df-8b9e-6755b2120701 - https://crash-stats.mozilla.com/report/index/bp-7a18e055-9dbe-4a4b-8568-ad50a2120701 This happens almost every time I use the debugger. STR: Stepping through this code crashes Firefox: function run() { let e = "f o o o b b b b b b b b a a a r".split(" "); for (let c of e) { console.log(e); } }
Comment 3•12 years ago
|
||
I've added the code from comment 2 in a test page: http://htmlpad.org/step-crash/ After setting a breakpoint at line 7 and stepping over 4 times, I get an assertion: Assertion failure: !cx->iterValue.isMagic(JS_NO_ITER_VALUE), at /Users/past/src/fx-team/js/src/jsiter.cpp:1280 The full trace is here, but it doesn't look quite like Paul's: http://past.pastebin.mozilla.org/1687533 Paul, if you can reproduce your exact crash/trace with this test page, please add the STR here and I'll file a different bug.
Reporter | ||
Comment 4•12 years ago
|
||
Break point line 6. Step in 4 time. Crash: https://crash-stats.mozilla.com/report/index/bp-d86fd7b3-4338-4f9f-8e74-66a032120702
Reporter | ||
Comment 5•12 years ago
|
||
With Past's STR (line 7, step over 4 times), I get this crash: https://crash-stats.mozilla.com/report/index/bp-f53c6b06-1222-42bc-9e16-438d22120702
Comment 6•12 years ago
|
||
(In reply to Paul Rouget [:paul] from comment #4) > Break point line 6. Step in 4 time. Crash: > https://crash-stats.mozilla.com/report/index/bp-d86fd7b3-4338-4f9f-8e74- > 66a032120702 Following these steps in my fx-team tip debug build results in the same crash as in comment 3, so the different stacks could probably be explained by paul's optimized build versus my own debug one.
Updated•12 years ago
|
Assignee: nobody → general
Component: Developer Tools: Debugger → JavaScript Engine
Product: Firefox → Core
QA Contact: developer.tools.debugger → general
Updated•12 years ago
|
Whiteboard: [js:p1]
Comment 7•12 years ago
|
||
does this occur in Aurora as well?
Comment 8•12 years ago
|
||
Here's what I tried to reproduce this in the shell, but it runs correctly. var g = newGlobal('new-compartment'); g.eval("var line0 = Error().lineNumber;\n" + "function run() {\n" + " let e = 'f o o o b b b b b b b b a a a r'.split(' ');\n" + " for (let c of e) {\n" + " print(e);\n" + " }\n" + "}\n"); var dbg = new Debugger; var gw = dbg.addDebuggee(g); var runw = gw.getOwnPropertyDescriptor('run').value; var bp = { hit: function (frame) { print("hit"); frame.onStep = function () { print("stepped, line " + this.script.getOffsetLine(this.offset)); }; } }; for (let off of runw.script.getLineOffsets(g.line0 + 2)) runw.script.setBreakpoint(off, bp); g.run();
Assignee | ||
Comment 9•12 years ago
|
||
I can work on this.
Assignee | ||
Updated•12 years ago
|
Assignee: general → jimb
Assignee | ||
Comment 10•12 years ago
|
||
All Paul's crashes are in the JSON.stringify method, as called from an onStep handler; the debug server is probably preparing a packet to send to the client. That's probably why Jason's code doesn't reproduce the problem. Panos's crash appears to be entirely unrelated.
Assignee | ||
Comment 11•12 years ago
|
||
I can reproduce Panos's crash. It is suspicious that the bad value Panos's crash finds in cx->iterValue (0x02) is the same as the bad pointer being followed in Paul's crash, so let's not split out the crashes into separate bugs just yet.
Comment 12•12 years ago
|
||
(In reply to Rob Campbell [:rc] (:robcee) from comment #7) > does this occur in Aurora as well? Yep.
Assignee | ||
Comment 13•12 years ago
|
||
Panos' crash, at least, is caused by Debugger::onStepHandler not preserving the value of cx->iterValue, a temporary slot used when invoking iterators. The debuggee sets it to a string, but then Debugger::onStepHandler runs; does something with iterators that wipes out cx->iterValue with its own value, which is eventually MagicValue(JS_NO_ITER_VALUE); and then returns to the debuggee, which (correctly) asserts that it has a legitimate value in there. This should be easy to reproduce in the shell.
Assignee | ||
Comment 14•12 years ago
|
||
This reproduces Panos's crash. var g = newGlobal('new-compartment'); var dbg = new Debugger; var gw = dbg.addDebuggee(g); var log; var a = []; dbg.onDebuggerStatement = function (frame) { print('d'); log += 'd'; frame.onStep = function () { print('s'); // This handler must not wipe out the debuggee's value in JSContext::iterValue. log += 's'; // This will use JSContext::iterValue in the debugger. for (let i of a) log += 'i'; }; }; log = ''; g.eval("debugger; for (let i of [1,2,3]) print(i);"); assertEq(log.match(/ds*/));
Assignee | ||
Comment 15•12 years ago
|
||
A patch; testing now.
Assignee | ||
Comment 16•12 years ago
|
||
Paul, were your crashes from a browser built with DEBUG?
Reporter | ||
Comment 17•12 years ago
|
||
(In reply to Jim Blandy :jimb from comment #16) > Paul, were your crashes from a browser built with DEBUG? No.
Assignee | ||
Comment 18•12 years ago
|
||
Try: https://tbpl.mozilla.org/?tree=Try&rev=f80cdcb6cc4c
Status: NEW → ASSIGNED
Assignee | ||
Comment 19•12 years ago
|
||
Fixes the problem in the browser.
Assignee | ||
Comment 20•12 years ago
|
||
"The problem" may be too optimistic. Paul, when you get a chance, could you see if the attached patch fixes the crash for you? I can't get a crash any more. You said on IRC that you were using a non-debug build, so hopefully the difference in backtraces is due to Panos's and my debug builds catching the problem earlier, and the attached patch will address both manifestations.
Assignee | ||
Comment 21•12 years ago
|
||
Comment on attachment 639146 [details] [diff] [review] Debugger handler functions should not corrupt the debuggee's JSContext::iterValue. Review of attachment 639146 [details] [diff] [review]: ----------------------------------------------------------------- Try run looks good.
Attachment #639146 -
Flags: review?(jorendorff)
Assignee | ||
Updated•12 years ago
|
OS: All → Mac OS X
Hardware: x86 → x86_64
Comment 22•12 years ago
|
||
Comment on attachment 639146 [details] [diff] [review] Debugger handler functions should not corrupt the debuggee's JSContext::iterValue. This is the grossest thing ever. We've got to get that opcode sequence changed. Barf.
Attachment #639146 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 23•12 years ago
|
||
Pushed to inbound. https://hg.mozilla.org/integration/mozilla-inbound/rev/a5dacb3e58bd
Assignee | ||
Updated•12 years ago
|
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86_64 → All
Target Milestone: --- → mozilla16
Assignee | ||
Comment 24•12 years ago
|
||
Comment on attachment 639146 [details] [diff] [review] Debugger handler functions should not corrupt the debuggee's JSContext::iterValue. [Approval Request Comment] Bug caused by (feature/regressing bug #): single-stepping support in Debugger User impact if declined: Users: none. Web developers: stepping through a 'for-in' loop in script debugger crashes Firefox. (Embarrassing. We have many single-step unit tests, but none in which the handler itself also executes a 'for-in' loop. This patch adds such a test.) Testing completed (on m-c, etc.): Try server run linked to in bug. Landed on inbound; will let it bake a bit. Risk to taking this patch (and alternatives if risky): Most of the affected code paths are only executed by the script debugger. The sole exception is a newly added assertion, which is debug-only. String or UUID changes made by this patch: none
Attachment #639146 -
Flags: approval-mozilla-aurora?
Reporter | ||
Comment 25•12 years ago
|
||
Can't reproduce with inbound. \o/
Comment 26•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a5dacb3e58bd
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 27•12 years ago
|
||
Comment on attachment 639146 [details] [diff] [review] Debugger handler functions should not corrupt the debuggee's JSContext::iterValue. [Triage Comment] Given user risk vs dev reward, approving for Aurora 15.
Attachment #639146 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 28•12 years ago
|
||
I landed it in Aurora to make sure we don't miss the uplift: https://hg.mozilla.org/releases/mozilla-aurora/rev/018027a303aa
status-firefox15:
--- → fixed
Assignee | ||
Comment 29•12 years ago
|
||
(In reply to Panos Astithas [:past] from comment #28) > I landed it in Aurora to make sure we don't miss the uplift: > https://hg.mozilla.org/releases/mozilla-aurora/rev/018027a303aa Thanks, Panos!
status-firefox15:
fixed → ---
Assignee | ||
Updated•12 years ago
|
status-firefox15:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•