Closed Bug 1546157 Opened 6 years ago Closed 6 years ago

Crashing with prototype GC code

Categories

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

x86_64
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
thunderbird_esr60 --- unaffected
geckoview66 --- unaffected
firefox-esr60 --- unaffected
firefox66 --- unaffected
firefox67 --- unaffected
firefox68 --- fixed

People

(Reporter: lth, Assigned: jseward)

References

Details

(Keywords: csectype-other, sec-high, Whiteboard: [post-critsmash-triage])

Attachments

(2 files, 3 obsolete files)

(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

Group: core-security → javascript-core-security

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 ?? ()
Attached file crasher.js (obsolete) (deleted) —

Test case with the necessary adornments.

Attached file crasher.js (obsolete) (deleted) —

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.

Attachment #9060021 - Attachment is obsolete: true
Attached file crasher.js (obsolete) (deleted) —

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.

Attachment #9060024 - Attachment is obsolete: true

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

Attached file crasher.js (deleted) —

Minimal TC.

Attachment #9060028 - Attachment is obsolete: true

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

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

Assignee: lhansen → jseward
Depends on: 1546407

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.

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.

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

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

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.

Group: javascript-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Works now, thanks for the fast fix.

Thanks for the fast feedback!

Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: