Crash [@ IsInsideNursery] with --unboxed-objects

RESOLVED FIXED in Firefox 39



4 years ago
4 years ago


(Reporter: decoder, Assigned: bhackett)


(Blocks: 1 bug, {crash, regression, testcase})

crash, regression, testcase

Firefox Tracking Flags

(firefox38 affected, firefox39 fixed)


(Whiteboard: [jsbugmon:update,bisect], crash signature)


(1 attachment)



4 years ago
The following testcase crashes on mozilla-central revision 09f4968d5f42 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --unboxed-objects):

function Foo(a, b) {
    b = {};
    this.b = b;
var a = [];
for (var i = 0; i < 50; i++)
    a.push(new Foo(i, i + 1));
i = 0;
a[i].c = i;


Program received signal SIGSEGV, Segmentation fault.
IsInsideNursery (cell=0x1ad4fd0) at ../../dist/include/js/HeapAPI.h:317
#0  IsInsideNursery (cell=0x1ad4fd0) at ../../dist/include/js/HeapAPI.h:317
#1  GetGCThingTraceKind (thing=0x1ad4fd0) at js/src/jsgcinlines.h:43
#2  js::gc::StoreBuffer::CellPtrEdge::mark (this=this@entry=0x19d0b78, trc=trc@entry=0x7fffffffdf30) at js/src/gc/StoreBuffer.cpp:70
#3  0x00000000007a7d11 in js::gc::StoreBuffer::MonoTypeBuffer<js::gc::StoreBuffer::CellPtrEdge>::mark (this=this@entry=0x19b08c0, owner=owner@entry=0x19af868, trc=trc@entry=0x7fffffffdf30) at js/src/gc/StoreBuffer.cpp:94
#4  0x00000000005b5701 in markCells (trc=0x7fffffffdf30, this=0x19af868) at js/src/gc/StoreBuffer.h:474
#5  js::Nursery::collect (this=this@entry=0x19af770, rt=0x19af3f0, reason=reason@entry=JS::gcreason::DESTROY_CONTEXT, pretenureGroups=pretenureGroups@entry=0x0) at js/src/gc/Nursery.cpp:804
#6  0x0000000000a431e7 in js::gc::GCRuntime::minorGCImpl (this=this@entry=0x19af718, reason=reason@entry=JS::gcreason::DESTROY_CONTEXT, pretenureGroups=pretenureGroups@entry=0x0) at js/src/jsgc.cpp:6358
#7  0x00000000005c8ce4 in js::gc::GCRuntime::evictNursery (this=0x19af718, reason=JS::gcreason::DESTROY_CONTEXT) at js/src/gc/GCRuntime.h:618
#8  0x0000000000a85728 in js::gc::GCRuntime::gcCycle (this=this@entry=0x19af718, incremental=incremental@entry=false, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:5976
#9  0x0000000000a85c1d in js::gc::GCRuntime::collect (this=this@entry=0x19af718, incremental=incremental@entry=false, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6149
#10 0x0000000000a85ef5 in js::gc::GCRuntime::gc (this=this@entry=0x19af718, gckind=gckind@entry=GC_NORMAL, reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6210
#11 0x00000000009fcd17 in js::DestroyContext (cx=0x19db900, mode=js::DCM_FORCE_GC) at js/src/jscntxt.cpp:185
#12 0x00000000009fd04e in JS_DestroyContext (cx=<optimized out>) at js/src/jsapi.cpp:589
#13 0x0000000000451de2 in DestroyContext (withGC=true, cx=0x19db900) at js/src/shell/js.cpp:5361
#14 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:6092
rax	0x1ad4fd0	28135376
rbx	0x19	25
rcx	0x80	128
rdx	0x1afffe8	28311528
rsi	0x7ffff505a110	140737304174864
rdi	0x19d0b78	27069304
rbp	0x7fffffffdc60	140737488346208
rsp	0x7fffffffdc60	140737488346208
r8	0x7fffffffdf30	140737488346928
r9	0x7ffff6f787b8	140737336805304
r10	0x40	64
r11	0x8	8
r12	0x0	0
r13	0x19d0b70	27069296
r14	0x19b08c0	26937536
r15	0x19d0c30	27069488
rip	0x755167 <js::gc::StoreBuffer::CellPtrEdge::mark(JSTracer*) const+39>
=> 0x755167 <js::gc::StoreBuffer::CellPtrEdge::mark(JSTracer*) const+39>:	mov    (%rdx),%edx
   0x755169 <js::gc::StoreBuffer::CellPtrEdge::mark(JSTracer*) const+41>:	test   %edx,%edx

Not marking s-s because --unboxed-objects is not enabled yet by default.
Flags: needinfo?(bhackett1024)

Comment 1

4 years ago
Created attachment 8567736 [details] [diff] [review]

Treating pointers in an unboxed object as HeapPtr can end up adding edge specific StoreBuffer entries which will be messed up if the unboxed object is later converted to its native representation.  This patch uses whole cell store buffer edges for unboxed object updates (which the JIT already uses in all cases).
Assignee: nobody → bhackett1024
Flags: needinfo?(bhackett1024)
Attachment #8567736 - Flags: review?(jdemooij)
Comment on attachment 8567736 [details] [diff] [review]

Review of attachment 8567736 [details] [diff] [review]:

This makes sense, but isn't there a similar problem when we convert the initial objects from native to unboxed?

::: js/src/gc/StoreBuffer.h
@@ +226,5 @@
>      {
>          Cell **edge;
>          CellPtrEdge() : edge(nullptr) {}
> +        explicit CellPtrEdge(Cell **v) : edge(v) { printf("DERP %p %p\n", v, *v); }

Remove the printf.
Attachment #8567736 - Flags: review?(jdemooij) → review+

Comment 3

4 years ago
Store buffer entries for slots/elements in native objects already need to account for the object's structure mutating due to property insertion/deletion, JSObject::swap, etc.
Last Resolved: 4 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.