Closed
Bug 1001368
Opened 10 years ago
Closed 10 years ago
[jsdbg2] Assertion failure: is<T>(), at jsobj.h:1128 or Crash [@ settle]
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla32
Tracking | Status | |
---|---|---|
firefox31 | --- | affected |
People
(Reporter: decoder, Assigned: shu)
Details
(Keywords: assertion, crash, testcase, Whiteboard: [jsbugmon:update])
Crash Data
Attachments
(3 files, 1 obsolete file)
1009 bytes,
text/plain
|
Details | |
1.97 KB,
patch
|
jimb
:
review+
|
Details | Diff | Splinter Review |
7.46 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
The following testcase asserts on mozilla-central revision 5ecd532a167e (run with --fuzzing-safe --ion-eager): var g = newGlobal(); g.debuggeeGlobal = this; g.eval("(" + function () { dbg = new Debugger(debuggeeGlobal); } + ")();"); for (var i4 = 0; i4 < 16; i4++) { try { var pattern = eval("/" + xx + "/"); } catch (e) { $ERROR('#' + uu + ' '); } }
Reporter | ||
Comment 1•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
Crash Signature: [@ settle]
status-firefox31:
--- → affected
Keywords: crash
Whiteboard: [jsbugmon:update,bisect]
Reporter | ||
Updated•10 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Reporter | ||
Comment 3•10 years ago
|
||
JSBugMon: Bisection requested, result: === Tinderbox Build Bisection Results by autoBisect === The "good" changeset has the timestamp "20140424014842" and the hash "25dd3d17a19a". The "bad" changeset has the timestamp "20140424020245" and the hash "079f4f0ed6a6". Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=25dd3d17a19a&tochange=079f4f0ed6a6
Assignee | ||
Comment 4•10 years ago
|
||
This is an existing bug in the Baseline implementation of Debugger hooks. When handling exceptions in baseline, we call UnwindScope. DebugEpilogue also calls UnwindScope. This can cause inconsistencies, because we're unwinding scope without changing the pc. In the interpreter, after unwinding the scope to a certain pc, we jump to that pc. In the baseline, if we're handling an exception that has no catch/finally, we call DebugEpilogue and terminate immediately, while the BaselineFrame's pc is still settled on the instruction that caused the exception. This breaks a bunch of Debugger assumptions.
Flags: needinfo?(shu)
Assignee | ||
Comment 5•10 years ago
|
||
Tests to reproduce this without any debug mode OSR stuff.
Attachment #8414204 -
Flags: review?(jimb)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → shu
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•10 years ago
|
||
See comment 4 for explanation. I'm not entirely happy with this approach, but I can't think of a better way.
Attachment #8414205 -
Flags: review?(jdemooij)
Assignee | ||
Comment 7•10 years ago
|
||
v2. Turns out the override pc can very well coexist with a DebugModeOSRInfo on the same frame. Use the previously unused padding to store the override pc offset.
Attachment #8414205 -
Attachment is obsolete: true
Attachment #8414205 -
Flags: review?(jdemooij)
Attachment #8414209 -
Flags: review?(jdemooij)
Comment 8•10 years ago
|
||
Comment on attachment 8414209 [details] [diff] [review] Fix UnwindScope logic in BaselineHandleException to account for Debugger. Review of attachment 8414209 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/BaselineFrame.h @@ -81,4 @@ > uint32_t flags_; > -#if JS_BITS_PER_WORD == 32 > - uint32_t padding_; // Pad to 8-byte alignment. > -#endif Ah good this change doesn't affect the frame size.
Attachment #8414209 -
Flags: review?(jdemooij) → review+
Comment 9•10 years ago
|
||
Comment on attachment 8414204 [details] [diff] [review] Tests Review of attachment 8414204 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit-test/tests/debug/Frame-onPop-error-scope-unwind-01.js @@ +6,5 @@ > +dbg.onEnterFrame = function (f) { > + if (f.callee && f.callee.name == "f") { > + f.onPop = function() { > + // The scope at the point of onPop is the outermost. > + correct = f.environment.getVariable("e") === undefined; Would it make sense to also check for a binding we *do* expect to be there? Because "doesn't contain a binding for 'e'" could probably describe most environments floating around in the system. @@ +23,5 @@ > + > +try { > + g.eval("f();"); > +} catch (e) { > + // Don't crash. I think you mean, "The above is expected to throw." Saying "don't crash" is ambiguous, since this was a crasher bug.
Attachment #8414204 -
Flags: review?(jimb) → review+
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0619a71579d https://hg.mozilla.org/integration/mozilla-inbound/rev/ef50feec881a
Comment 11•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ef50feec881a https://hg.mozilla.org/mozilla-central/rev/a0619a71579d
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•