1546157 - Crashing with prototype GC code

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P2)

Description

[Lars T Hansen [:lth]]

(Moved from github.)

Sorry, but I have the next problem. It look like a crash of SpiderMonkey. I want test the behavior of the GC and have create a large linked list. The follow Java code:

@Export
static int simple() {
    Abc2 val = new Abc2();
    for( int i = 0; i < 100_000; i++ ) {
        val.abc = new Abc2();
    }
    val.a = 63;
    return val.a;
}

compile to the follow WAT code:

(module (gc_feature_opt_in 3)
  (export "simple" (func $simple))
  (type $Abc2 (struct
    (field $Abc2.a (mut i32))
    (field $Abc2.b (mut i64))
    (field $Abc2.abc (mut anyref))
  ))
  (func $simple (result i32) (local $val (ref $Abc2)) (local $i i32) (local (ref $Abc2))
    i32.const 0
    i64.const 0
    ref.null
    struct.new $Abc2
    local.set $val
    i32.const 0
    local.set $i
    block
      loop
        local.get $i
        i32.const 100000
        i32.ge_s
        br_if 1
        local.get $val
        i32.const 0
        i64.const 0
        ref.null
        struct.new $Abc2
        struct.set $Abc2 2 ;; $abc
        local.get $i
        i32.const 1
        i32.add
        local.set $i
        br 0
      end
    end
    local.get $val
    i32.const 63
    struct.set $Abc2 0 ;; $a
    local.get $val
    struct.get $Abc2 0 ;; $a
    return
  )
)

If I run this code then SpiderMonkey terminate with the exit code -1073741819 (0xC0000005). I think this is a Access Violation.
I run https://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central/jsshell-win64.zip under Windows 10.
The error is reproducible on every run. I have use the command line options --wasm-gc --wasm-verbose

Comment 1

[Lars T Hansen [:lth]]

Easily reproduces on Linux64 in a standard debug build of the shell, run with --wasm-gc. Crashes during tenuring GC from a wasm frame

#0  0x0000556579f594f0 in js::shadow::Object::numFixedSlots (this=0x15e8a5200788)
    at /home/lhansen/m-cl/js/src/jsfriendapi.h:607
#1  0x0000556579f61ac5 in js::NativeObject::numFixedSlots (this=0x15e8a5200788)
    at /home/lhansen/m-cl/js/src/vm/NativeObject.h:728
#2  0x000055657a42e269 in js::NativeObject::allocKindForTenure (this=0x15e8a5200788)
    at /home/lhansen/m-cl/js/src/vm/NativeObject-inl.h:622
#3  0x000055657a407a2e in JSObject::allocKindForTenure (this=0x15e8a5200788, nursery=...)
    at /home/lhansen/m-cl/js/src/vm/JSObject.cpp:3916
#4  0x000055657ab1caff in js::TenuringTracer::moveToTenuredSlow (this=0x7fff5c712588, src=0x15e8a5200788)
    at /home/lhansen/m-cl/js/src/gc/Marking.cpp:2889
#5  0x000055657ab1c97e in js::TenuringTracer::traverse<JSObject> (this=0x7fff5c712588, objp=0x7fff5c7130f0)
    at /home/lhansen/m-cl/js/src/gc/Marking.cpp:2637
#6  0x000055657ab5c33d in js::gc::TraceEdgeInternal<JSObject*> (trc=0x7fff5c712588, thingp=0x7fff5c7130f0, 
    name=0x55657bc1dde2 "Instance::traceWasmFrame: normal word") at /home/lhansen/m-cl/js/src/gc/Marking.cpp:565
#7  0x000055657a176301 in js::TraceRoot<JSObject*> (trc=0x7fff5c712588, thingp=0x7fff5c7130f0, 
    name=0x55657bc1dde2 "Instance::traceWasmFrame: normal word") at /home/lhansen/m-cl/js/src/gc/Tracer.h:152
#8  0x000055657b26f36b in js::wasm::Instance::traceFrame (this=0x7f49d844f9b0, trc=0x7fff5c712588, wfi=..., 
    nextPC=0x3ec937aa116f "L\213t$0M\213>M\213V M\213f\030M\211\242\220", highestByteVisitedInPrevFrame=0)
    at /home/lhansen/m-cl/js/src/wasm/WasmInstance.cpp:1438
#9  0x000055657aff0e14 in js::jit::TraceJitActivation (trc=0x7fff5c712588, activation=0x7fff5c7133f8)
    at /home/lhansen/m-cl/js/src/jit/JitFrames.cpp:1286
#10 0x000055657aff0acd in js::jit::TraceJitActivations (cx=0x7f49d8f19000, trc=0x7fff5c712588)
    at /home/lhansen/m-cl/js/src/jit/JitFrames.cpp:1295
#11 0x000055657abc427f in js::gc::GCRuntime::traceRuntimeCommon (this=0x7f49d8f1b6d8, trc=0x7fff5c712588, 
    traceOrMark=js::gc::GCRuntime::TraceRuntime) at /home/lhansen/m-cl/js/src/gc/RootMarking.cpp:354
#12 0x000055657abbf9d4 in js::gc::GCRuntime::traceRuntimeForMinorGC (this=0x7f49d8f1b6d8, trc=0x7fff5c712588, 
    session=...) at /home/lhansen/m-cl/js/src/gc/RootMarking.cpp:303
#13 0x000055657abbee87 in js::Nursery::doCollection (this=0x7f49d8f1e230, reason=JS::GCReason::OUT_OF_NURSERY, 
    tenureCounts=...) at /home/lhansen/m-cl/js/src/gc/Nursery.cpp:952
#14 0x000055657abbdffe in js::Nursery::collect (this=0x7f49d8f1e230, reason=JS::GCReason::OUT_OF_NURSERY)
    at /home/lhansen/m-cl/js/src/gc/Nursery.cpp:785
#15 0x000055657ab0f83f in js::gc::GCRuntime::minorGC (this=0x7f49d8f1b6d8, reason=JS::GCReason::OUT_OF_NURSERY, 
    phase=js::gcstats::PhaseKind::MINOR_GC) at /home/lhansen/m-cl/js/src/gc/GC.cpp:7859
#16 0x000055657ab384c0 in js::gc::GCRuntime::tryNewNurseryObject<(js::AllowGC)1> (this=0x7f49d8f1b6d8, 
    cx=0x7f49d8f19000, thingSize=48, nDynamicSlots=0, clasp=0x55657c2988d0 <js::InlineOpaqueTypedObject::class_>)
    at /home/lhansen/m-cl/js/src/gc/Allocator.cpp:112
#17 0x000055657ab37ffc in js::AllocateObject<(js::AllowGC)1> (cx=0x7f49d8f19000, 
    kind=js::gc::AllocKind::OBJECT2_BACKGROUND, nDynamicSlots=0, heap=js::gc::InitialHeap::DefaultHeap, 
    clasp=0x55657c2988d0 <js::InlineOpaqueTypedObject::class_>) at /home/lhansen/m-cl/js/src/gc/Allocator.cpp:64
#18 0x000055657a7d74c8 in js::TypedObject::create (cx=0x7f49d8f19000, kind=js::gc::AllocKind::OBJECT2_BACKGROUND, 
    heap=js::gc::InitialHeap::DefaultHeap, shape=..., group=...)
    at /home/lhansen/m-cl/js/src/builtin/TypedObject.cpp:2320
#19 0x000055657a3f6bca in NewObject (cx=0x7f49d8f19000, group=..., kind=js::gc::AllocKind::OBJECT2_BACKGROUND, 
    newKind=js::GenericObject, initialShapeFlags=0) at /home/lhansen/m-cl/js/src/vm/JSObject.cpp:807
#20 0x000055657a3f7477 in js::NewObjectWithGroupCommon (cx=0x7f49d8f19000, group=..., 
    allocKind=js::gc::AllocKind::OBJECT2_BACKGROUND, newKind=js::GenericObject)
    at /home/lhansen/m-cl/js/src/vm/JSObject.cpp:986
#21 0x000055657a7f1f63 in js::NewObjectWithGroup<js::InlineTypedObject> (cx=0x7f49d8f19000, group=..., 
    allocKind=js::gc::AllocKind::OBJECT2, newKind=js::GenericObject)
    at /home/lhansen/m-cl/js/src/vm/JSObject-inl.h:541
#22 0x000055657a7d39f6 in js::InlineTypedObject::create (cx=0x7f49d8f19000, descr=..., 
    heap=js::gc::InitialHeap::DefaultHeap) at /home/lhansen/m-cl/js/src/builtin/TypedObject.cpp:2147
#23 0x000055657a7d35c5 in js::TypedObject::createZeroed (cx=0x7f49d8f19000, descr=..., 
    heap=js::gc::InitialHeap::DefaultHeap) at /home/lhansen/m-cl/js/src/builtin/TypedObject.cpp:1643
#24 0x000055657b26c846 in js::wasm::Instance::structNew (instance=0x7f49d844f9b0, typeIndex=0)
    at /home/lhansen/m-cl/js/src/wasm/WasmInstance.cpp:937
#25 0x00003ec937ac23b9 in ?? ()

Comment 2

[Lars T Hansen [:lth]]

Attachment: crasher.js

Test case with the necessary adornments.

Comment 3

[Lars T Hansen [:lth]]

Attachment: crasher.js

Better test case. In the body of the function notice two sections of code, "original" and "rewritten". The only difference is that in original, there is a reference to the value of $val that is live on the stack across the allocation call that causes GC. The original crashes; the rewritten does not.

Given that this program does not in fact create a linked list but only has a static node (that becomes tenured after a time presumably) that it keeps updating a pointer field of, there is something fishy about that static node, or about the previous node that was stored into its field.

So this could be a stack tracing problem or a write barrier problem in struct.set.

Comment 4

[Lars T Hansen [:lth]]

Attachment: crasher.js

Added a third case that just keeps a value on the stack across the call to struct.new but does not use it for anything, just discards it after. This brings back the crash. This suggests that the tracing machinery itself is somehow at fault here, that the pointer that is followed from the frame is not interpreted correctly.

Comment 5

[Lars T Hansen [:lth]]

Indeed the final struct.set is not needed either, so it's definitely not the write barrier, it can only be a tracing issue.

Comment 6

[Lars T Hansen [:lth]]

Attachment: crasher.js

Minimal TC.

Comment 7

[Lars T Hansen [:lth]]

Crashes on the second minor GC, the first one also triggered by struct.new.

Comment 8

[Lars T Hansen [:lth]]

Looks like beginFunction() does not set up the stack map machinery to track locals properly. Assigning to Mr Stack Maps.

Comment 9

[Julian Seward [:jseward]]

beginFunction() only marks in the stackmap, reftyped locals which also happen to be arguments. It should also (or, really, instead) visit the locals independently of the arguments. Fix in progress.

Comment 10

[Julian Seward [:jseward]]

A possibly-important question is whether we can mash around the test case so that the ion pipeline will accept it, so we can use it to check that the same problem doesn't exist there.

Comment 11

[Lars T Hansen [:lth]]

Configure with --enable-debug --disable-optimize --without-intl-api (that last one probably incidental).

Run with --wasm-gc, no compiler selection required.

Comment 12

[Julian Seward [:jseward]]

Attachment: Bug 1546157 - Wasm/Baseline: ensure stackmaps take into account reftyped locals.

Prior to this patch, Wasm/Baseline's stackmap creation logic failed to take
into account reftyped locals which are not also parameters. This patch fixes
that. It also adds a new test that reliably exposes the failure on all 4
primary targets. The test case also runs on Ion, and it appears that Wasm/Ion
does not suffer from an analogous problem.

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/93e4873ffd1dd508057bd1049750fcf08ad2b1be
https://hg.mozilla.org/mozilla-central/rev/93e4873ffd1d

Comment 14

[Volker Berlin]

Works now, thanks for the fast fix.

Comment 15

[Julian Seward [:jseward]]

Thanks for the fast feedback!