Closed Bug 1455599 Opened 4 years ago Closed 4 years ago

Assertion failure: !JS::CurrentThreadIsHeapMinorCollecting(), at js/src/vm/TypeInference.cpp:4191 with wasm

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 --- verified

People

(Reporter: decoder, Assigned: sfink)

References

Details

(4 keywords, Whiteboard: [jsbugmon:update,ignore])

Attachments

(1 file)

The following testcase crashes on mozilla-central revision cc0d7de218cb (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --cpu-count=2 --disable-oom-functions):

for (var i = 0; i < 2000000; ++i, WebAssembly.Instance) {
    var o = {};
    o = Object.create(o);
}


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x0000000000c99d90 in AssertGCStateForSweep (zone=<optimized out>) at js/src/vm/TypeInference.cpp:4191
#0  0x0000000000c99d90 in AssertGCStateForSweep (zone=<optimized out>) at js/src/vm/TypeInference.cpp:4191
#1  0x0000000000cb286c in js::ObjectGroup::sweep (this=this@entry=0x7ffff5889310, oom=<optimized out>, oom@entry=0x0) at js/src/vm/TypeInference.cpp:4332
#2  0x00000000004d6e0f in js::ObjectGroup::maybeSweep (oom=0x0, this=0x7ffff5889310) at js/src/vm/ObjectGroup-inl.h:27
#3  js::ObjectGroup::flags (this=0x7ffff5889310) at js/src/vm/ObjectGroup-inl.h:33
#4  js::ObjectGroup::unknownProperties (this=0x7ffff5889310) at js/src/vm/ObjectGroup-inl.h:68
#5  0x0000000000f2f5c1 in js::ObjectGroup::canPreTenure (this=0x7ffff5889310) at js/src/vm/ObjectGroup-inl.h:82
#6  js::Nursery::collect (this=0x7ffff5f1bd28, reason=reason@entry=JS::gcreason::FULL_CELL_PTR_BUFFER) at js/src/gc/Nursery.cpp:758
#7  0x0000000000ed6704 in js::gc::GCRuntime::minorGC (this=0x7ffff5f19700, reason=JS::gcreason::FULL_CELL_PTR_BUFFER, phase=<optimized out>) at js/src/gc/GC.cpp:7825
#8  0x0000000000ee8dbd in js::gc::GCRuntime::gcIfRequested (this=this@entry=0x7ffff5f19700) at js/src/gc/GC.cpp:7870
#9  0x0000000000eea4b8 in js::gc::GCRuntime::gcIfNeededAtAllocation (this=0x7ffff5f19700, cx=cx@entry=0x7ffff5f15000) at js/src/gc/Allocator.cpp:316
#10 0x0000000000f16628 in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=<optimized out>, cx=0x7ffff5f15000, kind=<optimized out>) at js/src/gc/Allocator.cpp:271
#11 0x0000000000f1683f in js::Allocate<JSObject, (js::AllowGC)1> (cx=cx@entry=0x7ffff5f15000, kind=kind@entry=js::gc::AllocKind::OBJECT4_BACKGROUND, nDynamicSlots=nDynamicSlots@entry=0, heap=heap@entry=js::gc::DefaultHeap, clasp=clasp@entry=0x2071aa0 <js::PlainObject::class_>) at js/src/gc/Allocator.cpp:52
#12 0x0000000000aad52f in js::NativeObject::create (cx=0x7ffff5f15000, kind=<optimized out>, heap=js::gc::DefaultHeap, shape=..., group=...) at js/src/vm/NativeObject-inl.h:539
#13 0x0000000000be5463 in NewObject (cx=<optimized out>, group=..., kind=<optimized out>, newKind=js::GenericObject, initialShapeFlags=<optimized out>) at js/src/vm/JSObject.cpp:731
#14 0x0000000000be583d in js::NewObjectWithGivenTaggedProto (cx=<optimized out>, cx@entry=0x7ffff5f15000, clasp=clasp@entry=0x2071aa0 <js::PlainObject::class_>, proto=..., allocKind=js::gc::AllocKind::OBJECT4_BACKGROUND, allocKind@entry=js::gc::AllocKind::OBJECT4, newKind=newKind@entry=js::GenericObject, initialShapeFlags=initialShapeFlags@entry=0) at js/src/vm/JSObject.cpp:792
#15 0x00000000005fb28f in js::NewObjectWithGivenProto<js::PlainObject> (newKind=js::GenericObject, allocKind=js::gc::AllocKind::OBJECT4, proto=..., cx=0x7ffff5f15000) at js/src/vm/JSObject-inl.h:678
#16 js::ObjectCreateImpl (cx=0x7ffff5f15000, proto=..., proto@entry=..., newKind=newKind@entry=js::GenericObject, group=..., group@entry=...) at js/src/builtin/Object.cpp:973
#17 0x000000000060bb5f in js::obj_create (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/Object.cpp:1062
#18 0x000012c7a80b810f in ?? ()
[...]
#22 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff5889310	140737312756496
rcx	0x7ffff6c282ad	140737333330605
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffbed0	140737488338640
rsp	0x7fffffffbed0	140737488338640
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4780	140737354024832
r10	0x58	88
r11	0x7ffff6b9e7a0	140737332766624
r12	0x7ffff55ec000	140737310015488
r13	0x7ffff55ec000	140737310015488
r14	0x7ffff5889310	140737312756496
r15	0x7ffff5f15000	140737319620608
rip	0xc99d90 <AssertGCStateForSweep(JS::Zone*)+96>
=> 0xc99d90 <AssertGCStateForSweep(JS::Zone*)+96>:	movl   $0x0,0x0
   0xc99d9b <AssertGCStateForSweep(JS::Zone*)+107>:	ud2


Marking s-s because this is a GC assertion.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
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/ebfd78786947
user:        Steve Fink
date:        Thu Apr 05 15:46:59 2018 -0700
summary:     Bug 1446693 - Include discardJitCode in AutoTraceSession for minor GC, r=jonco

This iteration took 279.587 seconds to run.
Steve, is bug 1446693 a likely regressor?
Flags: needinfo?(sphink)
Yes it is. Also for bug 1454925. I was going to look at that today; I'll do it now.
Flags: needinfo?(sphink)
I don't *think* there's any reason not to end a trace session and then restart it.
Attachment #8969837 - Flags: review?(jcoppeard)
Assignee: nobody → sphink
Status: NEW → ASSIGNED
Duplicate of this bug: 1454925
Comment on attachment 8969837 [details] [diff] [review]
Redo bug 1446693, creating separate AutoTraceSessions instead of expanding one

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

This seems fine.

I'm not clear why we don't end the session sooner though.  Is it required for anything after the pretenuring block?  Ideally we could hoist the shouldPretenure check and then dispense with the Maybe.
Attachment #8969837 - Flags: review?(jcoppeard) → review+
(In reply to Jon Coppeard (:jonco) from comment #6)
> Comment on attachment 8969837 [details] [diff] [review]
> Redo bug 1446693, creating separate AutoTraceSessions instead of expanding
> one
> 
> Review of attachment 8969837 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This seems fine.
> 
> I'm not clear why we don't end the session sooner though.  Is it required
> for anything after the pretenuring block?  Ideally we could hoist the
> shouldPretenure check and then dispense with the Maybe.

Oops, there's no reason not to end it sooner. I think it was a holdover from bug 1446693, where it felt a little cleaner to wrap the entire minor GC instead of ending it slightly sooner -- but with this new version, it's just weird to have some extra stuff within it.

The shouldPretenure check is kind of a silly reason to use Maybe, although even if I changed that, then we could easily have a common case of not doing any string pretenuring even though shouldPretenure is true, when everything's already pretenured (so zone->allocNurseryStrings always returns false.)

For the current state and planned future changes, I think leaving it as a Maybe is simplest. Though I'm not sure whether it's better or worse than constructing the session within the innermost if; it's probably fine to be starting and stopping a lot.
Duplicate of this bug: 1454839
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 26e53729a109).
Duplicate of this bug: 1456419
Duplicate of this bug: 1456460
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Group: javascript-core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.