Closed
Bug 1483183
Opened 5 years ago
Closed 5 years ago
Assertion failure: !JS_IsExceptionPending(cx), at js/src/jsexn.h:130 with stackTest
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox-esr60 | --- | wontfix |
firefox61 | --- | wontfix |
firefox62 | --- | wontfix |
firefox63 | --- | fixed |
People
(Reporter: decoder, Assigned: mgaudet)
References
(Blocks 1 open bug)
Details
(4 keywords, Whiteboard: [jsbugmon:update])
Attachments
(2 files, 1 obsolete file)
1.31 KB,
patch
|
tcampbell
:
review+
|
Details | Diff | Splinter Review |
5.30 KB,
patch
|
tcampbell
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision bf79440c1376 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --baseline-eager): stackTest(new Function(` newGlobal({ sameZoneAs: [] }).frame; `)); Backtrace: received signal SIGSEGV, Segmentation fault. 0x000000000079a4d0 in js::AutoAssertNoPendingException::~AutoAssertNoPendingException (this=<optimized out>, __in_chrg=<optimized out>) at js/src/jsexn.h:130 #0 0x000000000079a4d0 in js::AutoAssertNoPendingException::~AutoAssertNoPendingException (this=<optimized out>, __in_chrg=<optimized out>) at js/src/jsexn.h:130 #1 0x0000000000798c7b in js::jit::GetPropIRGenerator::tryAttachStub (this=this@entry=0x7fffffffb690) at js/src/jit/CacheIR.cpp:166 #2 0x0000000000705e15 in js::jit::DoGetPropFallback (cx=<optimized out>, frame=0x7fffffffb908, stub_=<optimized out>, val=..., res=...) at js/src/jit/BaselineIC.cpp:1522 #3 0x00000c4f40e341a8 in ?? () [...] #24 0x0000000000000000 in ?? () rax 0x0 0 rbx 0x7fffffffb510 140737488336144 rcx 0x7ffff6c1c2dd 140737333281501 rdx 0x0 0 rsi 0x7ffff6eeb770 140737336227696 rdi 0x7ffff6eea540 140737336223040 rbp 0x7fffffffb4d0 140737488336080 rsp 0x7fffffffb4d0 140737488336080 r8 0x7ffff6eeb770 140737336227696 r9 0x7ffff7fe6780 140737354033024 r10 0x58 88 r11 0x7ffff6b927a0 140737332717472 r12 0x7fffffffb540 140737488336192 r13 0xf4d99201 4107899393 r14 0xf4d99201 4107899393 r15 0x7fffffffb690 140737488336528 rip 0x79a4d0 <js::AutoAssertNoPendingException::~AutoAssertNoPendingException()+48> => 0x79a4d0 <js::AutoAssertNoPendingException::~AutoAssertNoPendingException()+48>: movl $0x0,0x0 0x79a4db <js::AutoAssertNoPendingException::~AutoAssertNoPendingException()+59>: ud2
Updated•5 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Comment 1•5 years ago
|
||
JSBugMon: Bisection requested, result: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/6ffbdf27a930 user: Matthew Gaudet date: Mon Apr 23 17:25:38 2018 -0400 summary: Bug 1438556: [Part 1] Use shape guards on the prototype chain for cross-compartment ICs r=tcampbell This iteration took 294.694 seconds to run.
Updated•5 years ago
|
Flags: needinfo?(mgaudet)
Assignee | ||
Comment 2•5 years ago
|
||
To be perfectly frank, I have no idea if this is the right answer. It passes the test case, but all I have really done is hoisted the stack check that exists in Compartment::wrap into tryAttachCrossCompartment wrapper, and replaced with the DontReport variant. (Alternative answers could include changing the check in Compartment::wrap to be a non-reporting version)
Attachment #9001418 -
Flags: review?(tcampbell)
Assignee | ||
Updated•5 years ago
|
Assignee: nobody → mgaudet
Status: NEW → ASSIGNED
Comment 3•5 years ago
|
||
Comment on attachment 9001418 [details] [diff] [review] Check stack before attempting to create CCW in GetPropIRGenerator Review of attachment 9001418 [details] [diff] [review]: ----------------------------------------------------------------- So, the issue here is actually that classic SpiderMonkey problem where bool return value can indicate either predicate-false or operation-failed-and-pending-exception set. The problem here is that tryAttach functions return false to indicate a failure to attach, while the wrap() call returns false to indicate an exception. Here is example of what to do: https://searchfox.org/mozilla-central/rev/5dbfd833bbb114afe758db4d4bdbc5b13bcc33ef/js/src/jit/CacheIR.cpp#214-217 (We really need to boil the ocean and use type system to mark return values that indicate pending exceptions)
Attachment #9001418 -
Flags: review?(tcampbell) → review-
Assignee | ||
Updated•5 years ago
|
Attachment #9001418 -
Attachment is obsolete: true
Flags: needinfo?(mgaudet)
Assignee | ||
Comment 4•5 years ago
|
||
Thanks for the pointer! That's very helpful.
Attachment #9001449 -
Flags: review?(tcampbell)
Comment 5•5 years ago
|
||
Comment on attachment 9001449 [details] [diff] [review] Discard pending exception during CCW GetProp IC failure Review of attachment 9001449 [details] [diff] [review]: ----------------------------------------------------------------- Include testcase too.
Attachment #9001449 -
Flags: review?(tcampbell) → review+
Comment 6•5 years ago
|
||
Can you also make a patch to add AutoAssertNoPendingException to any CacheIR generators that are missing it? (Probably move the CallIC ones to the top tryAttachStub too)
Flags: needinfo?(mgaudet)
Updated•5 years ago
|
Priority: -- → P1
Assignee | ||
Comment 7•5 years ago
|
||
Should we maybe just add a AutoAssertNoPendingException member to the IRGenerator base class?
Attachment #9002330 -
Flags: review?(tcampbell)
Assignee | ||
Updated•5 years ago
|
Flags: needinfo?(mgaudet)
Comment 8•5 years ago
|
||
Comment on attachment 9002330 [details] [diff] [review] Complete covereage with AutoAssertNoPendingException in CacheIR stub attach code Review of attachment 9002330 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for doing this. I think the tryAttach methods are probably the right place to do it since we use these to communicate and test that tryAttach (which are main interface boundary from CacheIR.cpp to actual instantiations) mean |return false| as failed-to-attach and not operation-failed-with-exception.
Attachment #9002330 -
Flags: review?(tcampbell) → review+
Pushed by mgaudet@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4e417850332a Discard pending exception during CCW GetProp IC failure r=tcampbell https://hg.mozilla.org/integration/mozilla-inbound/rev/9370dc79ddfc Complete coverage with AutoAssertNoPendingException in CacheIR stub attach code r=tcampbell
Comment 10•5 years ago
|
||
Backed out for bustages on bug1483183.js Push link: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=9370dc79ddfc94fdb20f3ae6036c9fcbd3c3d6c1 Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/56856b94af05f97c1ef66b0bbb71ce8be890a156 Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=194875448&repo=mozilla-inbound&lineNumber=33755
Flags: needinfo?(mgaudet)
Comment 11•5 years ago
|
||
Pushed by mgaudet@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/79082cb95664 Discard pending exception during CCW GetProp IC failure r=tcampbell https://hg.mozilla.org/integration/mozilla-inbound/rev/a6c928f38da7 Complete coverage with AutoAssertNoPendingException in CacheIR stub attach code r=tcampbell
Assignee | ||
Updated•5 years ago
|
Flags: needinfo?(mgaudet)
Comment 12•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/79082cb95664 https://hg.mozilla.org/mozilla-central/rev/a6c928f38da7
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Comment 13•5 years ago
|
||
Is there a user impact which justifies backport consideration here or can this ride the trains?
Blocks: 1438556
status-firefox61:
--- → wontfix
status-firefox62:
--- → affected
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → affected
Flags: needinfo?(mgaudet)
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•