Crash while using the debugger

RESOLVED FIXED in Firefox 15

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: paul, Assigned: jimb)

Tracking

({crash})

Trunk
mozilla16
crash
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox15 fixed)

Details

(Whiteboard: [js:p1], crash signature)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
https://crash-stats.mozilla.com/report/index/bp-0321f53e-1399-4347-80ec-25f4d2120629
Paul, any specific STR?

Updated

5 years ago
Crash Signature: [@ Str ]

Updated

5 years ago
Keywords: crash
(Reporter)

Comment 2

5 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);
    }
  }
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

5 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

5 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
(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();
(Assignee)

Comment 9

5 years ago
I can work on this.
(Assignee)

Updated

5 years ago
Assignee: general → jimb
(Assignee)

Comment 10

5 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

5 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.
(In reply to Rob Campbell [:rc] (:robcee) from comment #7)
> does this occur in Aurora as well?

Yep.
(Assignee)

Comment 13

5 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

5 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

5 years ago
Created attachment 639146 [details] [diff] [review]
Debugger handler functions should not corrupt the debuggee's JSContext::iterValue.

A patch; testing now.
(Assignee)

Comment 16

5 years ago
Paul, were your crashes from a browser built with DEBUG?
(Reporter)

Comment 17

5 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

5 years ago
Try: https://tbpl.mozilla.org/?tree=Try&rev=f80cdcb6cc4c
Status: NEW → ASSIGNED
(Assignee)

Comment 19

5 years ago
Fixes the problem in the browser.
(Assignee)

Comment 20

5 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

5 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

5 years ago
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+
(Assignee)

Comment 23

5 years ago
Pushed to inbound.
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5dacb3e58bd
(Assignee)

Updated

5 years ago
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86_64 → All
Target Milestone: --- → mozilla16
(Assignee)

Comment 24

5 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

5 years ago
Can't reproduce with inbound. \o/
https://hg.mozilla.org/mozilla-central/rev/a5dacb3e58bd
Status: ASSIGNED → RESOLVED
Last Resolved: 5 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+
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

5 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

5 years ago
status-firefox15: --- → fixed
You need to log in before you can comment on or make changes to this bug.