Closed
Bug 781660
Opened 12 years ago
Closed 12 years ago
IonMonkey: Crash [@ ExpressionDecompiler::decompilePC] or "Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth)," or "Assertion failure: pcdepth >= nuses,"
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
firefox-esr10 | --- | unaffected |
firefox-esr17 | --- | unaffected |
People
(Reporter: gkw, Assigned: nbp)
References
Details
(4 keywords, Whiteboard: [ion:p1:fx18] [jsbugmon:update,ignore])
Crash Data
Attachments
(3 files, 1 obsolete file)
options('strict') x = 2 try {} catch (u if ({})) {} x.e asserts js debug shell on IonMonkey changeset c0195737650c without any CLI arguments at Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth),
Reporter | ||
Updated•12 years ago
|
Summary: IonMonkey: " Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth)," → IonMonkey: "Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth),"
Reporter | ||
Comment 1•12 years ago
|
||
(not sure if this is correct) autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 103173:0bc212d0183b parent: 103059:b457b592f609 parent: 103172:a7fadfbad932 user: David Anderson date: Fri Aug 03 18:58:30 2012 -0700 summary: Merge from mozilla-central. Quite sure this is IonMonkey, it does not affect mozilla-central changeset ed5c848f50e0 w/ w/o -m or -n.
Assignee | ||
Comment 2•12 years ago
|
||
I added this assertion to make sure that the ReconstructPCStack behave correctly by comparing it to the script analysis. This assertion was added in Bug 780451 and pushed in IonMonkey.
Blocks: 780451
Updated•12 years ago
|
Whiteboard: [ion:p1:fx18]
Reporter | ||
Comment 3•12 years ago
|
||
Related testcases seem also to produce this assertion: "Assertion failure: pcdepth >= nuses,"
Summary: IonMonkey: "Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth)," → IonMonkey: "Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth)," or "Assertion failure: pcdepth >= nuses,"
Reporter | ||
Comment 4•12 years ago
|
||
try { h } catch (x if gc()) {} finally { this.z.z } Run this on 64-bit debug shell on IonMonkey changeset 32b7b76d111c with --no-ti: Assertion failure: pcdepth >= nuses, It switches back to the other assertion in this bug without --no-ti, or if you replace gc with print. s-s because gc seems essential here.
Group: core-security
Reporter | ||
Comment 5•12 years ago
|
||
(function () { try {} catch (x if true) { } finally { 3(__defineSetter__("x", encodeURIComponent)) } })() crashes 64-bit js opt shell on IonMonkey changeset d794f23798f4 without any CLI arguments at ExpressionDecompiler::decompilePC
Reporter | ||
Updated•12 years ago
|
Crash Signature: [@ ExpressionDecompiler::decompilePC]
Summary: IonMonkey: "Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth)," or "Assertion failure: pcdepth >= nuses," → IonMonkey: Crash [@ ExpressionDecompiler::decompilePC] or "Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth)," or "Assertion failure: pcdepth >= nuses,"
Assignee | ||
Updated•12 years ago
|
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
Crash Signature: [@ ExpressionDecompiler::decompilePC]
Summary: IonMonkey: Crash [@ ExpressionDecompiler::decompilePC] or "Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth)," or "Assertion failure: pcdepth >= nuses," → IonMonkey: "Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth)," or "Assertion failure: pcdepth >= nuses,"
Assignee | ||
Comment 6•12 years ago
|
||
Fix first test case reported in the bug by avoiding continuing on a throw instruction.
Attachment #652981 -
Flags: review?(luke)
Updated•12 years ago
|
Attachment #652981 -
Flags: review?(luke) → review+
Reporter | ||
Comment 7•12 years ago
|
||
> crashes 64-bit js opt shell on IonMonkey changeset d794f23798f4 without any
> CLI arguments at ExpressionDecompiler::decompilePC
Was the clearing of the opt crash intentional? I forgot to mention that the testcase causes a similar assertion in debug builds.
Assignee | ||
Comment 8•12 years ago
|
||
(In reply to Gary Kwong [:gkw, :nth10sd] from comment #7) > > crashes 64-bit js opt shell on IonMonkey changeset d794f23798f4 without any > > CLI arguments at ExpressionDecompiler::decompilePC > > Was the clearing of the opt crash intentional? I forgot to mention that the > testcase causes a similar assertion in debug builds. No, it wasn't.
Crash Signature: [@ ExpressionDecompiler::decompilePC]
Summary: IonMonkey: "Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth)," or "Assertion failure: pcdepth >= nuses," → IonMonkey: Crash [@ ExpressionDecompiler::decompilePC] or "Assertion failure: script->analysis()->getCode(pc).stackDepth == ((hpcdepth == unsigned(-1)) ? pcdepth : hpcdepth)," or "Assertion failure: pcdepth >= nuses,"
Assignee | ||
Comment 9•12 years ago
|
||
Comment on attachment 652981 [details] [diff] [review] Follow goto's target if identical to the reconstructed one. https://hg.mozilla.org/projects/ionmonkey/rev/13b1019e04af
Attachment #652981 -
Flags: checkin+
Assignee | ||
Comment 10•12 years ago
|
||
Attachment #654478 -
Flags: review?(luke)
Assignee | ||
Comment 11•12 years ago
|
||
(In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #10) > Created attachment 654478 [details] [diff] [review] > Follow try notes when we cannot follow gotos. This patch should fix the last 2 test cases.
Comment 12•12 years ago
|
||
Comment on attachment 654478 [details] [diff] [review] Follow try notes when we cannot follow gotos. Review of attachment 654478 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsopcode.cpp @@ +6465,5 @@ > + JSTryNote *tnEnd = tn + script->trynotes()->length; > + for (; tn != tnEnd; tn++) { > + jsbytecode *start = script->main() + tn->start; > + jsbytecode *end = start + tn->length; > + if (start < pc && pc <= end && end <= target) Looking at TryNoteIter, it seems like we'd want the inclusive range [start, end]. If not, could you comment as to why not?
Attachment #654478 -
Flags: review?(luke) → review+
Assignee | ||
Comment 13•12 years ago
|
||
(In reply to Luke Wagner [:luke] from comment #12) > Comment on attachment 654478 [details] [diff] [review] > Follow try notes when we cannot follow gotos. > > Review of attachment 654478 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: js/src/jsopcode.cpp > @@ +6465,5 @@ > > + JSTryNote *tnEnd = tn + script->trynotes()->length; > > + for (; tn != tnEnd; tn++) { > > + jsbytecode *start = script->main() + tn->start; > > + jsbytecode *end = start + tn->length; > > + if (start < pc && pc <= end && end <= target) > > Looking at TryNoteIter, it seems like we'd want the inclusive range [start, > end]. If not, could you comment as to why not? Hum … Maybe I should just add another constructor to TryNoteIter and reuse it instead of adding another for loop to iterate over the try notes. What do you think ?
Assignee | ||
Comment 14•12 years ago
|
||
https://hg.mozilla.org/projects/ionmonkey/rev/4c24e4fc3472
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•12 years ago
|
Attachment #654478 -
Flags: checkin+
Updated•12 years ago
|
Status: RESOLVED → VERIFIED
Comment 15•12 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Assignee | ||
Comment 16•12 years ago
|
||
The last patch has been reverted after some test failures on mochitest. https://hg.mozilla.org/projects/ionmonkey/rev/6c7537647873
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Updated•12 years ago
|
Whiteboard: [ion:p1:fx18] → [ion:p1:fx18][jsbugmon:update]
Updated•12 years ago
|
Whiteboard: [ion:p1:fx18][jsbugmon:update] → [ion:p1:fx18] [jsbugmon:update,ignore]
Comment 17•12 years ago
|
||
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 92b9b2840a79).
Reporter | ||
Comment 18•12 years ago
|
||
Comment on attachment 654478 [details] [diff] [review] Follow try notes when we cannot follow gotos. This patch was backed out.
Attachment #654478 -
Flags: checkin+
Reporter | ||
Updated•12 years ago
|
Whiteboard: [ion:p1:fx18] [jsbugmon:update,ignore] → [ion:p1:fx18] [jsbugmon:update,reconfirm]
Updated•12 years ago
|
Whiteboard: [ion:p1:fx18] [jsbugmon:update,reconfirm] → [ion:p1:fx18] [jsbugmon:update,reconfirm,ignore]
Comment 19•12 years ago
|
||
JSBugMon: The testcase found in this bug no longer reproduces (tried revision a26469331bf9).
Updated•12 years ago
|
status-firefox-esr10:
--- → unaffected
Reporter | ||
Comment 20•12 years ago
|
||
Comment on attachment 654478 [details] [diff] [review] Follow try notes when we cannot follow gotos. An updated form of this patch to fix the testcases in comment 4 and comment 5 should be posted in bug 787309 now that Nicolas and I have agreed to spin them off into that bug after a f2f discussion.
Attachment #654478 -
Attachment is obsolete: true
Reporter | ||
Comment 21•12 years ago
|
||
Still resolving FIXED since the original testcase in comment 0 has been fixed by the patch in comment 6. At Nicolas' request, please do not yet open up this bug until mentioned otherwise. (bug may affect m-c, but we are not sure yet)
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Status: RESOLVED → VERIFIED
Comment 22•12 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Reporter | ||
Updated•12 years ago
|
Whiteboard: [ion:p1:fx18] [jsbugmon:update,reconfirm,ignore] → [ion:p1:fx18] [jsbugmon:update,ignore]
Reporter | ||
Updated•12 years ago
|
Flags: in-testsuite?
Updated•11 years ago
|
status-firefox-esr17:
--- → unaffected
Updated•9 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•