1196210 - Assertion failure: !isInside(newData), at js/src/gc/Nursery.cpp:305

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 90d9b7c391d3 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-check-range-analysis --baseline-eager):

function foo() {
  function testGeneratorDeepBail() {
    gc();
    for (let i = 0; Error(); i++)
      for each(var x in [{n: 1}, {n: 1}]) {
        x[0] = 0;
        Object.freeze(x);
      }
  }
  testGeneratorDeepBail();
} foo();



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x000000000083fc01 in js::Nursery::setForwardingPointer (this=0x7ffff693c460, oldData=0x7ffff48aacb0, newData=0x7ffff4800000, direct=<optimized out>) at js/src/gc/Nursery.cpp:305
#0  0x000000000083fc01 in js::Nursery::setForwardingPointer (this=0x7ffff693c460, oldData=0x7ffff48aacb0, newData=0x7ffff4800000, direct=<optimized out>) at js/src/gc/Nursery.cpp:305
#1  0x0000000000621025 in js::TenuringTracer::moveElementsToTenured (this=this@entry=0x7fffffffabc0, dst=dst@entry=0x7ffff4574700, src=src@entry=0x7ffff48aac40, dstKind=dstKind@entry=js::gc::OBJECT2_BACKGROUND) at js/src/gc/Marking.cpp:2207
#2  0x00000000006212ce in js::TenuringTracer::moveObjectToTenured (this=this@entry=0x7fffffffabc0, dst=dst@entry=0x7ffff4574700, src=src@entry=0x7ffff48aac40, dstKind=dstKind@entry=js::gc::OBJECT2_BACKGROUND) at js/src/gc/Marking.cpp:2125
#3  0x00000000006214e0 in js::TenuringTracer::moveToTenured (this=0x7fffffffabc0, src=0x7ffff48aac40) at js/src/gc/Marking.cpp:2020
#4  0x000000000062171b in js::TenuringTracer::traverse<JSObject*> (this=<optimized out>, objp=0x7fffffffa988) at js/src/gc/Marking.cpp:1870
#5  0x0000000000625db9 in traverse<JS::Value> (valp=0x7ffff4531ab0, this=<optimized out>) at js/src/gc/Marking.cpp:1881
#6  traceSlots (end=0x7ffff4531ac0, vp=0x7ffff4531ab0, this=0x7fffffffabc0) at js/src/gc/Marking.cpp:2099
#7  js::TenuringTracer::traceObject (this=this@entry=0x7fffffffabc0, obj=obj@entry=0x7ffff4531a80) at js/src/gc/Marking.cpp:2075
#8  0x00000000006260c8 in js::Nursery::collectToFixedPoint (this=this@entry=0x7ffff693c460, mover=..., tenureCounts=...) at js/src/gc/Marking.cpp:2039
#9  0x0000000000840e79 in js::Nursery::collect (this=this@entry=0x7ffff693c460, rt=0x7ffff693c000, reason=reason@entry=JS::gcreason::OUT_OF_NURSERY, pretenureGroups=pretenureGroups@entry=0x7fffffffaf30) at js/src/gc/Nursery.cpp:473
#10 0x0000000000b3d085 in js::gc::GCRuntime::minorGCImpl (this=this@entry=0x7ffff693c408, reason=reason@entry=JS::gcreason::OUT_OF_NURSERY, pretenureGroups=pretenureGroups@entry=0x7fffffffaf30) at js/src/jsgc.cpp:6474
#11 0x0000000000b3d9cb in js::gc::GCRuntime::minorGC (this=this@entry=0x7ffff693c408, cx=cx@entry=0x7ffff6907000, reason=reason@entry=JS::gcreason::OUT_OF_NURSERY) at js/src/jsgc.cpp:6486
#12 0x0000000000658032 in tryNewNurseryObject<(js::AllowGC)1> (clasp=0x1aefbe0 <js::PlainObject::class_>, nDynamicSlots=0, thingSize=48, cx=0x7ffff6907000, this=0x7ffff693c408) at js/src/gc/Allocator.cpp:159
#13 js::Allocate<JSObject, (js::AllowGC)1> (cx=cx@entry=0x7ffff6907000, kind=kind@entry=js::gc::OBJECT2_BACKGROUND, nDynamicSlots=0, heap=heap@entry=js::gc::DefaultHeap, clasp=clasp@entry=0x1aefbe0 <js::PlainObject::class_>) at js/src/gc/Allocator.cpp:125
#14 0x0000000000752d13 in JSObject::create (cx=0x7ffff6907000, kind=js::gc::OBJECT2_BACKGROUND, heap=js::gc::DefaultHeap, shape=..., group=...) at js/src/jsobjinlines.h:314
#15 0x0000000000b516ff in NewObject (cx=0x7ffff6907000, group=..., kind=js::gc::OBJECT2_BACKGROUND, newKind=js::GenericObject, initialShapeFlags=<optimized out>) at js/src/jsobj.cpp:684
#16 0x0000000000b52386 in js::NewObjectWithClassProtoCommon (cxArg=cxArg@entry=0x7ffff6907000, clasp=0x1aefbe0 <js::PlainObject::class_>, protoArg=..., protoArg@entry=..., allocKind=allocKind@entry=js::gc::OBJECT2_BACKGROUND, newKind=<optimized out>) at js/src/jsobj.cpp:817
#17 0x0000000000693fc6 in NewObjectWithClassProto (newKind=<optimized out>, allocKind=js::gc::OBJECT2_BACKGROUND, proto=..., clasp=<optimized out>, cx=0x7ffff6907000) at js/src/jsobjinlines.h:706
#18 NewBuiltinClassInstance (newKind=<optimized out>, allocKind=js::gc::OBJECT2_BACKGROUND, clasp=<optimized out>, cx=0x7ffff6907000) at js/src/jsobjinlines.h:725
#19 NewBuiltinClassInstance<js::PlainObject> (newKind=<optimized out>, allocKind=js::gc::OBJECT2_BACKGROUND, cx=0x7ffff6907000) at js/src/jsobjinlines.h:747
#20 js::CopyInitializerObject (cx=0x7ffff6907000, baseobj=..., newKind=<optimized out>) at js/src/vm/NativeObject-inl.h:297
#21 0x0000000000694983 in js::NewObjectOperationWithTemplate (cx=0x7ffff6907000, templateObject=...) at js/src/vm/Interpreter.cpp:4780
#22 0x00007ffff7fed071 in ?? ()
#23 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff693c460	140737330267232
rcx	0x7ffff6ca53cd	140737333842893
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffffa820	140737488332832
rsp	0x7fffffffa7b0	140737488332720
r8	0x7ffff7fe0780	140737354008448
r9	0x6372732f736a2f6c	7165916604736876396
r10	0x7fffffffa570	140737488332144
r11	0x7ffff6c27960	140737333328224
r12	0x7ffff48aaca0	140737296116896
r13	0x7ffff4574700	140737292748544
r14	0x7fffffffabc0	140737488333760
r15	0x10	16
rip	0x83fc01 <js::Nursery::setForwardingPointer(void*, void*, bool)+433>
=> 0x83fc01 <js::Nursery::setForwardingPointer(void*, void*, bool)+433>:	movl   $0x131,0x0
   0x83fc0c <js::Nursery::setForwardingPointer(void*, void*, bool)+444>:	callq  0x499bc0 <abort()>


The testcase might be intermittent and should take less than a second to reproduce. Marking s-s because GC is involved.

Comment 1

[Terrence Cole [:terrence]]

This is an incomplete assertion.

The malloc for the header (16 bytes) + target buffer (0 bytes) happens to give us the last 2 words in the 1MiB jemalloc chunk that just happens to be mapped directly before the nursery. The zero sized data allocation starts two words after the header, i.e. at the same address as the start of the nursery. When we go to write out the forwarding pointer, we assert (fairly reasonably) that the new allocation isn't inside the nursery. But in this case it is. It just happens to be a zero size allocation.

I think the right solution is to update the assertion to be aware that that allocation can be zero sized. I'll draft a patch Monday morning.

Comment 2

[Terrence Cole [:terrence]]

Attachment: bug-1196210-v0.diff

Comment 3

[Jon Coppeard (:jonco)]

Comment on attachment 8651850 [details] [diff] [review]
bug-1196210-v0.diff

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

Heh, this is a nice find!

::: js/src/gc/Nursery.cpp
@@ +305,5 @@
> +
> +    // Bug 1196210: If a zero-capacity header lands in the last 2 words of the
> +    // jemalloc chunk abutting the start of the nursery, the (invalid) newData
> +    // pointer will appear to be "inside" the nursery.
> +    MOZ_ASSERT_IF(direct, !isInside(newData));

We could assert !isInside() || newData == startAddress.

Comment 4

[Terrence Cole [:terrence]]

Uhg, yes, I totally forgot that we had the startAddress handy, despite printing it while debugging. Fixed.

Comment 5

[Jesse Ruderman]

I'd leave the bug number out of the comment.

Comment 6

[Martijn Wargers (dead)]

This effectively prevents us from running automation on the Flame builds.

Comment 7

[Terrence Cole [:terrence]]

(In reply to Jesse Ruderman from comment #5)
> I'd leave the bug number out of the comment.

Why?

(In reply to Martijn Wargers [:mwargers] (QA) from comment #6)
> This effectively prevents us from running automation on the Flame builds.

Sorry, tree kept closing yesterday. Will try to get it in today.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/eff1a4594f3f

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/eff1a4594f3f