Closed Bug 769754 Opened 8 years ago Closed 7 years ago

Crash while using the debugger

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

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)

Paul, any specific STR?
Crash Signature: [@ Str ]
Keywords: crash
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);
    }
  }
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.
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
(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.
Assignee: nobody → general
Component: Developer Tools: Debugger → JavaScript Engine
Product: Firefox → Core
QA Contact: developer.tools.debugger → general
Whiteboard: [js:p1]
does this occur in Aurora as well?
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();
I can work on this.
Assignee: general → jimb
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.
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.
(In reply to Rob Campbell [:rc] (:robcee) from comment #7)
> does this occur in Aurora as well?

Yep.
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.
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*/));
Paul, were your crashes from a browser built with DEBUG?
(In reply to Jim Blandy :jimb from comment #16)
> Paul, were your crashes from a browser built with DEBUG?

No.
Try: https://tbpl.mozilla.org/?tree=Try&rev=f80cdcb6cc4c
Status: NEW → ASSIGNED
Fixes the problem in the browser.
"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.
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)
OS: All → Mac OS X
Hardware: x86 → x86_64
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+
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86_64 → All
Target Milestone: --- → mozilla16
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?
Can't reproduce with inbound. \o/
https://hg.mozilla.org/mozilla-central/rev/a5dacb3e58bd
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Depends on: 772770
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+
(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!
You need to log in before you can comment on or make changes to this bug.