Closed Bug 1331444 Opened 3 years ago Closed 3 years ago

Crash [@ JSObject::getClass] or Crash [@ js::CloseIterator] Assertion failure: isObject(), at dist/include/js/Value.h:642

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 --- fixed

People

(Reporter: decoder, Unassigned)

References

(Blocks 1 open bug)

Details

(5 keywords, Whiteboard: [fuzzblocker] [jsbugmon:update])

Crash Data

Attachments

(1 file, 2 obsolete files)

The following testcase crashes on mozilla-central revision 8eaf154b385b (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off --ion-eager):

symbols = [Symbol];
for (comparator of[, ])
  for (a of symbols)
    for (;;) 
      expect;



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x00000000007c5739 in JSObject::getClass (this=<optimized out>) at js/src/jsobj.h:127
#0  0x00000000007c5739 in JSObject::getClass (this=<optimized out>) at js/src/jsobj.h:127
#1  JSObject::getOpsGetProperty (this=0xe) at js/src/jsobj.h:139
#2  js::GetProperty (vp=..., id=..., receiver=..., obj=..., cx=0x7ffff695f000) at js/src/vm/NativeObject.h:1508
#3  js::GetProperty (vp=..., name=<optimized out>, receiver=..., obj=..., cx=0x7ffff695f000) at js/src/jsobj.h:848
#4  js::GetProperty (vp=..., name=<optimized out>, receiver=..., obj=..., cx=0x7ffff695f000) at js/src/jsobj.h:864
#5  js::IteratorCloseForException (cx=cx@entry=0x7ffff695f000, obj=obj@entry=...) at js/src/jsiter.cpp:1254
#6  0x00000000005f2c9c in js::jit::CloseLiveIteratorIon (tn=<optimized out>, frame=..., cx=0x7ffff695f000) at js/src/jit/JitFrames.cpp:368
#7  js::jit::HandleExceptionIon (overrecursed=0x7fffffffcb0f, rfe=0x7fffffffd0e8, frame=..., cx=0x7ffff695f000) at js/src/jit/JitFrames.cpp:449
#8  js::jit::HandleException (rfe=<optimized out>) at js/src/jit/JitFrames.cpp:853
#9  0x00007ffff7e3b15b in ?? ()
#10 0x0000000000000000 in ?? ()
rax	0xe	14
rbx	0x7ffff695f000	140737330409472
rcx	0x7fffffffc9c0	140737488341440
rdx	0x7fffffffc9a0	140737488341408
rsi	0x7ffff695f000	140737330409472
rdi	0x7fffffffc9d0	140737488341456
rbp	0x7fffffffca80	140737488341632
rsp	0x7fffffffc950	140737488341328
r8	0x7fffffffc980	140737488341376
r9	0x7fffffffd2a8	140737488343720
r10	0x7fffffffc990	140737488341392
r11	0x0	0
r12	0x7fffffffc970	140737488341360
r13	0x7fffffffcbd0	140737488341968
r14	0x7fffffffc9b0	140737488341424
r15	0x7ffff69e7a38	140737330969144
rip	0x7c5739 <js::IteratorCloseForException(JSContext*, JS::Handle<JSObject*>)+313>
=> 0x7c5739 <js::IteratorCloseForException(JSContext*, JS::Handle<JSObject*>)+313>:	mov    (%rax),%rax
   0x7c573c <js::IteratorCloseForException(JSContext*, JS::Handle<JSObject*>)+316>:	mov    (%rax),%rax
Flags: needinfo?(shu)
This bug seems to crash in multiple ways, marking fuzzblocker.
Crash Signature: [@ JSObject::getClass] → [@ JSObject::getClass] [@ js::CloseIterator]
Summary: Crash [@ JSObject::getClass] or Assertion failure: isObject(), at dist/include/js/Value.h:642 → Crash [@ JSObject::getClass] or Crash [@ js::CloseIterator] Assertion failure: isObject(), at dist/include/js/Value.h:642
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update,bisect][fuzzblocker]
This is unfortunate. The bug is that the on-stack iterator object was being
optimized out by Ion since it's only used by the exception unwinder to call
IteratorClose.

I had to teach MResumePoint about stack slots that are observable due to the
exception unwinder and I'm afraid it might regress Ion compilation times
(haven't benchmarked). Jan, what do you think?
Attachment #8827733 - Flags: review?(jdemooij)
Flags: needinfo?(shu)
Is it possible to use the IonBuilder::processIterators mechanism here somehow? I think it was added a long time ago to solve a similar problem with for-in loops, bug 759213.

Also, when we have a try block, we have some code to make EliminateDeadResumePointOperands and phi elimination play nice. Maybe we could do something similar here for now? See MIRGraph::hasTryBlock.
Flags: needinfo?(shu)
(In reply to Jan de Mooij [:jandem] from comment #3)
> Is it possible to use the IonBuilder::processIterators mechanism here
> somehow? I think it was added a long time ago to solve a similar problem
> with for-in loops, bug 759213.

Maybe we could add a no-op JSOP to communicate to IonBuilder what the iterator slot is that we have to keep alive? Then we could just append it to iterators_, similar to what we do for JSOP_ITER.
Simply appending the SSA value of JSOP_CALLITER to iterators_ seems to work.
Thanks for the suggestion.
Attachment #8828144 - Flags: review?(jdemooij)
Attachment #8827733 - Attachment is obsolete: true
Attachment #8827733 - Flags: review?(jdemooij)
Flags: needinfo?(shu)
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20170114094823" and the hash "5ce3882eec21be3a70e4afc050959ca2f76bfa76".
The "bad" changeset has the timestamp "20170114144423" and the hash "142dbb4bffc04efdb358df11293079d00af693fc".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5ce3882eec21be3a70e4afc050959ca2f76bfa76&tochange=142dbb4bffc04efdb358df11293079d00af693fc

Guessing this is related to bug 1147371.
Blocks: 1147371
Comment on attachment 8828144 [details] [diff] [review]
Keep iterators alive in Ion in for-of loops for IteratorClose due to exceptions.

Review of attachment 8828144 [details] [diff] [review]:
-----------------------------------------------------------------

Glad it works.
Attachment #8828144 - Flags: review?(jdemooij) → review+
I ran into some assertions where some of the iterator values were phis, and
iterators_ is a vector of MInstructions. Hannes suggested just changing it to
MDefinitions, seems to work.
Attachment #8828624 - Flags: review?(jdemooij)
Attachment #8828144 - Attachment is obsolete: true
Comment on attachment 8828624 [details] [diff] [review]
Keep iterators alive in Ion in for-of loops for IteratorClose due to exceptions.

Review of attachment 8828624 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit/IonBuilder.cpp
@@ +865,5 @@
>  {
>      // Find phis that must directly hold an iterator live.
>      Vector<MPhi*, 0, SystemAllocPolicy> worklist;
>      for (size_t i = 0; i < iterators_.length(); i++) {
> +        MDefinition* def = iterators_[i];

Not sure if it's strictly necessary but, just to be safe, we could add:

if (def->isPhi()) {
    if (!worklist.append(def->toPhi()))
        return abort(AbortReason::Alloc); 
}
Attachment #8828624 - Flags: review?(jdemooij) → review+
Crash Signature: [@ JSObject::getClass] [@ js::CloseIterator] → [@ JSObject::getClass] [@ js::CloseIterator]
Whiteboard: [jsbugmon:update,bisect][fuzzblocker] → [fuzzblocker] [jsbugmon:update]
JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20170113151323" and the hash "0431a0ab907d53646d359ad10507efe7f89dd487".
The "bad" changeset has the timestamp "20170114194423" and the hash "18ab7878c67ba22b2f98bcc7a1e94b826061dd19".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0431a0ab907d53646d359ad10507efe7f89dd487&tochange=18ab7878c67ba22b2f98bcc7a1e94b826061dd19
Pushed by shu@rfrn.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/287d743f3b70
Keep iterators alive in Ion in for-of loops for IteratorClose due to exceptions. (r=jandem)
https://hg.mozilla.org/mozilla-central/rev/287d743f3b70
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
See Also: → 1333946
You need to log in before you can comment on or make changes to this bug.