Closed Bug 842431 Opened 8 years ago Closed 8 years ago

BaselineCompiler: [jsdbg2] Assertion failure: maybeSnapshot() == __null, at vm/ScopeObject.cpp:1530

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86_64
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED

People

(Reporter: decoder, Assigned: jandem)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase, Whiteboard: [jsbugmon:update])

Attachments

(2 files)

The following testcase asserts on baseline compiler branch revision 1c8552cc6ec4 (run with ):


var g = newGlobal('new-compartment');
g.eval('function f(a,b) { var x = "entablature", y; debugger; return x+y+a+b; }');
var dbg = new Debugger(g);
dbg.onDebuggerStatement = function handleDebugger(frame) {
    frame.onPop = handlePop;
};
function handlePop(c) {
    assertEq(this.eval('y = "cornice"').catch , 'cornice');
}
g.eval('f("frieze", "stylobate")');
Attached patch PatchSplinter Review
Debug-mode-only bug. What happens is: a frame returns, we call DebugEpilogue, its onPop handler throws, we enter the exception handling code, and we call DebugEpilogue again for the same frame, but this is wrong. The attached patch makes us match the interpreter by updating ionTop to "pop" the frame in DebugEpilogue. Also adds a second debugger test that shows the problem.

(And fuzzable debugger tests are really awesome.)
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Attachment #716431 - Flags: review?(kvijayan)
Attached patch Part 2Splinter Review
I just realized there's another edge case: if an onExceptionUnwind hooks performs a forced return, we call DebugEpilogue. If DebugEpilogue then throws, the exception has to be propagated to the caller frame (instead of handling in the current frame), and we also should not call DebugEpilogue twice in this case. The testcase fails without this patch.

The different debugger hooks and how they interact is pretty complicated, and the interpreter "exit:", "forced_return:" and "error:" goto spaghetti is not very clear either. This matches what the interpreter does though and at least we can add shell tests to ensure baseline behaves the same way.
Attachment #716447 - Flags: review?(kvijayan)
Attachment #716431 - Flags: review?(kvijayan) → review+
Attachment #716447 - Flags: review?(kvijayan) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/609c145cbbf3
https://hg.mozilla.org/projects/ionmonkey/rev/44496b0909de
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.