Crash [@ js::gc::MovingTracer::updateEdge<js::RegExpShared>] or Assertion failure: thing, at gc/Marking.cpp:196 with wasm

RESOLVED FIXED in Firefox 61

Status

()

defect
P1
critical
RESOLVED FIXED
Last year
Last year

People

(Reporter: decoder, Assigned: luke)

Tracking

(Blocks 1 bug, 5 keywords)

Trunk
mozilla61
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox59 unaffected, firefox60 unaffected, firefox61 fixed)

Details

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

Attachments

(2 attachments)

The following testcase crashes on mozilla-central revision 8063b0c54b88 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --disable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off):

gczeal(9, 10);
function wasmEvalText(str, imports) {
    let binary = wasmTextToBinary(str);
    m = new WebAssembly.Module(binary);
    return new WebAssembly.Instance(m, imports);
}
assertEq(wasmEvalText(`(module
    (global (import "a" "b") (mut i32))
    (func (export "get") (result i32) get_global 0)
)`, {  a: { b: 42 }}).exports.get(), 42);
for (let v of []) {}
function testInitExpr(type, initialValue, nextValue, coercion, assertFunc = assertEq) {
    var module = wasmEvalText(`(module
        (import "globals" "a" (global ${type}))
        (global $glob_imm ${type} (get_global 0))
        (export "global_imm" (global $glob_imm))
    )`, {
        globals: {
            a: coercion(initialValue)
        }
    }).exports;
}
testInitExpr('i32', 13, 37, x => x | 0);


Backtrace:

received signal SIGSEGV, Segmentation fault.
js::gc::MovingTracer::updateEdge<js::RegExpShared> (thingp=0x7fffffffcb60, this=0x7fffffffc080) at js/src/gc/GC.cpp:2448
#0  js::gc::MovingTracer::updateEdge<js::RegExpShared> (thingp=0x7fffffffcb60, this=0x7fffffffc080) at js/src/gc/GC.cpp:2448
#1  js::gc::MovingTracer::onRegExpSharedEdge (this=0x7fffffffc080, sharedp=0x7fffffffcb60) at js/src/gc/GC.cpp:2459
#2  0x0000000000be9538 in JS::CallbackTracer::dispatchToOnEdge (objp=0x7fffffffcb60, this=0x7fffffffc080) at dist/include/js/TracingAPI.h:240
#3  DoCallback<JSObject*> (trc=0x7fffffffc080, thingp=0x7fffffffcb60, name=0xea580b "vector element") at js/src/gc/Tracer.cpp:47
#4  0x0000000000ab2746 in JS::GCPolicy<js::HeapPtr<js::WasmGlobalObject*> >::trace (name=0xea580b "vector element", thingp=0x7fffffffcb60, trc=0x7fffffffc088) at js/src/gc/Policy.h:149
#5  JS::GCVector<js::HeapPtr<js::WasmGlobalObject*>, 8ul, js::SystemAllocPolicy>::trace (trc=0x7fffffffc088, this=<optimized out>) at dist/include/js/GCVector.h:135
#6  JS::StructGCPolicy<JS::GCVector<js::HeapPtr<js::WasmGlobalObject*>, 8ul, js::SystemAllocPolicy> >::trace (trc=0x7fffffffc088, tp=<optimized out>, name=<optimized out>) at dist/include/js/GCPolicyAPI.h:79
#7  0x0000000000bcb311 in js::DispatchWrapper<ConcreteTraceable>::TraceWrapped (name=0xebb482 "Traceable", thingp=<optimized out>, trc=0x7fffffffc088) at dist/include/js/RootingAPI.h:782
#8  TraceExactStackRootList<ConcreteTraceable, js::DispatchWrapper<T>::TraceWrapped<ConcreteTraceable> > (name=0xebb482 "Traceable", rooter=0x7fffffffcb30, trc=0x7fffffffc088) at js/src/gc/RootMarking.cpp:57
#9  TraceStackRoots (trc=trc@entry=0x7fffffffc088, stackRoots=...) at js/src/gc/RootMarking.cpp:73
#10 0x0000000000bcb95d in JS::RootingContext::traceStackRoots (trc=0x7fffffffc088, this=<optimized out>) at js/src/gc/RootMarking.cpp:79
#11 TraceExactStackRoots (trc=0x7fffffffc088, cx=<optimized out>) at js/src/gc/RootMarking.cpp:85
#12 js::gc::GCRuntime::traceRuntimeCommon (this=this@entry=0x7ffff5f19498, trc=trc@entry=0x7fffffffc088, traceOrMark=traceOrMark@entry=js::gc::GCRuntime::MarkRuntime, session=...) at js/src/gc/RootMarking.cpp:332
#13 0x0000000000bcbdbb in js::gc::GCRuntime::traceRuntimeForMajorGC (this=this@entry=0x7ffff5f19498, trc=trc@entry=0x7fffffffc088, session=...) at js/src/gc/RootMarking.cpp:261
#14 0x0000000000b81f50 in js::gc::GCRuntime::updateRuntimePointersToRelocatedCells (this=this@entry=0x7ffff5f19498, session=...) at js/src/gc/GC.cpp:2888
#15 0x0000000000b9937d in js::gc::GCRuntime::compactPhase (this=0x7ffff5f19498, reason=JS::gcreason::DEBUG_GC, sliceBudget=..., session=...) at js/src/gc/GC.cpp:6626
#16 0x0000000000ba2e58 in js::gc::GCRuntime::incrementalCollectSlice (this=0x7ffff5f19498, budget=..., reason=JS::gcreason::DEBUG_GC, session=...) at js/src/gc/GC.cpp:7113
#17 0x0000000000ba3177 in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffff5f19498, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/gc/GC.cpp:7394
#18 0x0000000000ba3715 in js::gc::GCRuntime::collect (this=this@entry=0x7ffff5f19498, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/gc/GC.cpp:7537
#19 0x0000000000ba49bc in js::gc::GCRuntime::runDebugGC (this=0x7ffff5f19498) at js/src/gc/GC.cpp:8151
#20 0x0000000000ba4ad8 in js::gc::GCRuntime::gcIfNeededAtAllocation (this=0x7ffff5f19498, cx=0x7ffff5f14000) at js/src/gc/Allocator.cpp:310
#21 0x0000000000bbd6f9 in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=<optimized out>, cx=0x7ffff5f14000, kind=<optimized out>) at js/src/gc/Allocator.cpp:271
#22 0x0000000000bbda16 in js::Allocate<JSObject, (js::AllowGC)1> (cx=cx@entry=0x7ffff5f14000, kind=kind@entry=js::gc::AllocKind::OBJECT8, nDynamicSlots=nDynamicSlots@entry=0, heap=<optimized out>, clasp=clasp@entry=0x1c32b00 <js::WasmInstanceObject::class_>) at js/src/gc/Allocator.cpp:52
#23 0x0000000000975534 in js::NativeObject::create (group=..., shape=..., heap=<optimized out>, kind=js::gc::AllocKind::OBJECT8, cx=0x7ffff5f14000) at js/src/vm/NativeObject-inl.h:538
#24 NewObject (cx=cx@entry=0x7ffff5f14000, group=group@entry=..., kind=kind@entry=js::gc::AllocKind::OBJECT8, newKind=newKind@entry=js::GenericObject, initialShapeFlags=initialShapeFlags@entry=0) at js/src/vm/JSObject.cpp:728
#25 0x0000000000975a6e in js::NewObjectWithGivenTaggedProto (cx=0x7ffff5f14000, clasp=0x1c32b00 <js::WasmInstanceObject::class_>, proto=..., allocKind=<optimized out>, newKind=js::GenericObject, initialShapeFlags=<optimized out>) at js/src/vm/JSObject.cpp:789
#26 0x0000000000b080b2 in js::NewObjectWithGivenTaggedProto<js::WasmInstanceObject> (initialShapeFlags=0, newKind=js::GenericObject, proto=..., cx=0x7ffff5f14000) at js/src/vm/JSObject-inl.h:621
#27 js::NewObjectWithGivenProto<js::WasmInstanceObject> (newKind=js::GenericObject, proto=..., cx=0x7ffff5f14000) at js/src/vm/JSObject-inl.h:654
#28 js::WasmInstanceObject::create(JSContext*, RefPtr<js::wasm::Code const>, mozilla::UniquePtr<js::wasm::DebugState, JS::DeletePolicy<js::wasm::DebugState> >, mozilla::UniquePtr<js::wasm::TlsData, js::wasm::TlsDataDeleter>, JS::Handle<js::WasmMemoryObject*>, mozilla::Vector<RefPtr<js::wasm::Table>, 0ul, js::SystemAllocPolicy>&&, JS::Handle<JS::GCVector<JSFunction*, 0ul, js::TempAllocPolicy> >, mozilla::Vector<js::wasm::GlobalDesc, 0ul, js::SystemAllocPolicy> const&, mozilla::Vector<js::wasm::Val, 0ul, js::SystemAllocPolicy> const&, JS::GCVector<js::HeapPtr<js::WasmGlobalObject*>, 8ul, js::SystemAllocPolicy> const&, JS::Handle<JSObject*>) (cx=cx@entry=0x7ffff5f14000, code=..., debug=..., tlsData=..., memory=..., tables=tables@entry=<unknown type in /mnt/LangFuzz/work/builds/opt64/dist/bin/js, CU 0x5fbff5a, DIE 0x62d4d6b>, funcImports=..., globals=..., globalImportValues=..., globalObjs=..., proto=...) at js/src/wasm/WasmJS.cpp:1052
#29 0x0000000000b0abc9 in js::wasm::Module::instantiate (this=this@entry=0x7ffff4982d00, cx=cx@entry=0x7ffff5f14000, funcImports=funcImports@entry=..., tableImport=..., tableImport@entry=..., memoryImport=..., memoryImport@entry=..., globalImportValues=..., globalObjs=..., instanceProto=..., instance=...) at js/src/wasm/WasmModule.cpp:1301
#30 0x0000000000b0c3ef in Instantiate (cx=cx@entry=0x7ffff5f14000, module=..., importObj=..., importObj@entry=..., instanceObj=..., instanceObj@entry=...) at js/src/wasm/WasmJS.cpp:1124
#31 0x0000000000b0ceb5 in js::WasmInstanceObject::construct (cx=0x7ffff5f14000, argc=2, vp=0x7ffff49ca260) at js/src/wasm/WasmJS.cpp:1149
#32 0x0000000000530413 in js::CallJSNative (args=..., native=<optimized out>, cx=0x7ffff5f14000) at js/src/vm/JSContext-inl.h:290
[...]
#45 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:9137
rax	0x0	0
rbx	0x7fffffffc080	140737488339072
rcx	0x7ffff5f19000	140737319636992
rdx	0x0	0
rsi	0x7fffffffcb60	140737488341856
rdi	0x7fffffffc080	140737488339072
rbp	0x7fffffffcb60	140737488341856
rsp	0x7fffffffbe88	140737488338568
r8	0x7ffff49909f0	140737297058288
r9	0x7ffff4990970	140737297058160
r10	0x7ffff4990940	140737297058112
r11	0x0	0
r12	0x0	0
r13	0xea580b	15357963
r14	0x7ffff5f32000	140737319739392
r15	0x7fffffffc140	140737488339264
rip	0xb7a761 <js::gc::MovingTracer::onRegExpSharedEdge(js::RegExpShared**)+17>
=> 0xb7a761 <js::gc::MovingTracer::onRegExpSharedEdge(js::RegExpShared**)+17>:	cmp    %rcx,0xffff8(%rdx)
   0xb7a768 <js::gc::MovingTracer::onRegExpSharedEdge(js::RegExpShared**)+24>:	je     0xb7a770 <js::gc::MovingTracer::onRegExpSharedEdge(js::RegExpShared**)+32>


Assertion and crash involving GC, marking s-s.
This is an automated crash issue comment:

Summary: Crash [@ ShouldMark<JSString*>]
Build version: mozilla-central revision ef717c03ff54
Build flags: --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --disable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu

Backtrace:

received signal SIGSEGV, Segmentation fault.
ShouldMark<JSString*> (gcmarker=0xf6e09e04, str=0x0) at js/src/gc/Marking.cpp:804
#0  ShouldMark<JSString*> (gcmarker=0xf6e09e04, str=0x0) at js/src/gc/Marking.cpp:804
#1  0x0879cfd6 in DoMarking<JSObject> (gcmarker=0xf6e09e04, thing=0x0) at js/src/gc/Marking.cpp:816
#2  0x0868aa31 in JS::GCPolicy<js::HeapPtr<js::WasmGlobalObject*> >::trace (name=0x883d712 "vector element", thingp=0xffffc644, trc=0xf6e09e04) at js/src/gc/Policy.h:149
#3  JS::GCVector<js::HeapPtr<js::WasmGlobalObject*>, 8u, js::SystemAllocPolicy>::trace (trc=0xf6e09e04, this=0xffffc638) at dist/include/js/GCVector.h:135
#4  JS::StructGCPolicy<JS::GCVector<js::HeapPtr<js::WasmGlobalObject*>, 8u, js::SystemAllocPolicy> >::trace (trc=0xf6e09e04, tp=0xffffc638, name=0x8852a9c "Traceable") at dist/include/js/GCPolicyAPI.h:79
#5  0x08797adc in js::DispatchWrapper<ConcreteTraceable>::TraceWrapped (name=0x8852a9c "Traceable", thingp=<optimized out>, trc=0xf6e09e04) at dist/include/js/RootingAPI.h:782
#6  TraceExactStackRootList<ConcreteTraceable, js::DispatchWrapper<T>::TraceWrapped<ConcreteTraceable> > (name=0x8852a9c "Traceable", rooter=0xffffc628, trc=0xf6e09e04) at js/src/gc/RootMarking.cpp:57
#7  TraceStackRoots (trc=trc@entry=0xf6e09e04, stackRoots=...) at js/src/gc/RootMarking.cpp:73
#8  0x087980f5 in JS::RootingContext::traceStackRoots (trc=0xf6e09e04, this=<optimized out>) at js/src/gc/RootMarking.cpp:79
#9  TraceExactStackRoots (trc=0xf6e09e04, cx=<optimized out>) at js/src/gc/RootMarking.cpp:85
#10 js::gc::GCRuntime::traceRuntimeCommon (this=0xf6e092b8, trc=0xf6e09e04, traceOrMark=js::gc::GCRuntime::MarkRuntime, session=...) at js/src/gc/RootMarking.cpp:332
#11 0x087984d4 in js::gc::GCRuntime::traceRuntimeForMajorGC (this=0xf6e092b8, trc=0xf6e09e04, session=...) at js/src/gc/RootMarking.cpp:261
#12 0x0876729e in js::gc::GCRuntime::beginMarkPhase (this=0xf6e092b8, reason=JS::gcreason::DEBUG_GC, session=...) at js/src/gc/GC.cpp:4357
#13 0x08770c44 in js::gc::GCRuntime::incrementalCollectSlice (this=0xf6e092b8, budget=..., reason=JS::gcreason::DEBUG_GC, session=...) at js/src/gc/GC.cpp:7007
#14 0x0877133f in js::gc::GCRuntime::gcCycle (this=0xf6e092b8, nonincrementalByAPI=true, budget=..., reason=JS::gcreason::DEBUG_GC) at js/src/gc/GC.cpp:7394
#15 0x087717ff in js::gc::GCRuntime::collect (this=0xf6e092b8, nonincrementalByAPI=true, budget=..., reason=JS::gcreason::DEBUG_GC) at js/src/gc/GC.cpp:7537
#16 0x08771a0e in js::gc::GCRuntime::gc (this=0xf6e092b8, gckind=GC_NORMAL, reason=JS::gcreason::DEBUG_GC) at js/src/gc/GC.cpp:7607
#17 0x08772be5 in js::gc::GCRuntime::runDebugGC (this=0xf6e092b8) at js/src/gc/GC.cpp:8167
#18 0x08772ca1 in js::gc::GCRuntime::gcIfNeededAtAllocation (this=0xf6e092b8, cx=0xf6e17800) at js/src/gc/Allocator.cpp:310
#19 0x0878a3de in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=0xf6e092b8, cx=0xf6e17800, kind=js::gc::AllocKind::BASE_SHAPE) at js/src/gc/Allocator.cpp:271
#20 0x0878ab3c in js::Allocate<js::BaseShape, (js::AllowGC)1> (cx=0xf6e17800) at js/src/gc/Allocator.cpp:222
#21 0x085d06e4 in js::BaseShape::getUnowned (cx=0xf6e17800, base=...) at js/src/vm/Shape.cpp:1435
#22 0x085d695b in js::EmptyShape::getInitialShape (cx=0xf6e17800, clasp=0x8b02090 <js::WasmInstanceObject::class_>, proto=..., nfixed=8, objectFlags=0) at js/src/vm/Shape.cpp:2111
#23 0x0857b107 in NewObject (cx=cx@entry=0xf6e17800, group=..., group@entry=..., kind=kind@entry=js::gc::AllocKind::OBJECT8, newKind=js::GenericObject, initialShapeFlags=0) at js/src/vm/JSObject.cpp:719
#24 0x0857cc01 in js::NewObjectWithGivenTaggedProto (cx=0xf6e17800, clasp=0x8b02090 <js::WasmInstanceObject::class_>, proto=..., allocKind=<optimized out>, newKind=js::GenericObject, initialShapeFlags=0) at js/src/vm/JSObject.cpp:789
#25 0x083a3a28 in js::NewObjectWithGivenTaggedProto (cx=0xf6e17800, clasp=0x8b02090 <js::WasmInstanceObject::class_>, proto=..., newKind=js::GenericObject, initialShapeFlags=0) at js/src/vm/JSObject-inl.h:611
#26 0x086da4f5 in js::NewObjectWithGivenTaggedProto<js::WasmInstanceObject> (initialShapeFlags=0, newKind=js::GenericObject, proto=..., cx=0xf6e17800) at js/src/vm/JSObject-inl.h:621
#27 js::NewObjectWithGivenProto<js::WasmInstanceObject> (newKind=js::GenericObject, proto=..., cx=0xf6e17800) at js/src/vm/JSObject-inl.h:654
#28 js::WasmInstanceObject::create(JSContext*, RefPtr<js::wasm::Code const>, mozilla::UniquePtr<js::wasm::DebugState, JS::DeletePolicy<js::wasm::DebugState> >, mozilla::UniquePtr<js::wasm::TlsData, js::wasm::TlsDataDeleter>, JS::Handle<js::WasmMemoryObject*>, mozilla::Vector<RefPtr<js::wasm::Table>, 0u, js::SystemAllocPolicy>&&, JS::Handle<JS::GCVector<JSFunction*, 0u, js::TempAllocPolicy> >, mozilla::Vector<js::wasm::GlobalDesc, 0u, js::SystemAllocPolicy> const&, mozilla::Vector<js::wasm::Val, 0u, js::SystemAllocPolicy> const&, JS::GCVector<js::HeapPtr<js::WasmGlobalObject*>, 8u, js::SystemAllocPolicy> const&, JS::Handle<JSObject*>) (cx=0xf6e17800, code=..., debug=..., tlsData=..., memory=..., tables=<unknown type in /home/ubuntu/build/dist/bin/js, CU 0x5b601ec, DIE 0x5e5106f>, funcImports=..., globals=..., globalImportValues=..., globalObjs=..., proto=...) at js/src/wasm/WasmJS.cpp:1052
#29 0x086dcbdb in js::wasm::Module::instantiate (this=0xf59e4ac0, cx=0xf6e17800, funcImports=..., tableImport=..., memoryImport=..., globalImportValues=..., globalObjs=..., instanceProto=..., instance=...) at js/src/wasm/WasmModule.cpp:1301
#30 0x086ddfc7 in Instantiate (cx=cx@entry=0xf6e17800, module=..., importObj=..., importObj@entry=..., instanceObj=...) at js/src/wasm/WasmJS.cpp:1124
#31 0x086de8cf in js::WasmInstanceObject::construct (cx=0xf6e17800, argc=2, vp=0xf59ee210) at js/src/wasm/WasmJS.cpp:1149
#32 0x08161aa2 in js::CallJSNative (args=..., native=<optimized out>, cx=0xf6e17800) at js/src/vm/JSContext-inl.h:290
#33 js::CallJSNativeConstructor (args=..., native=<optimized out>, cx=0xf6e17800) at js/src/vm/JSContext-inl.h:323
#34 InternalConstruct (cx=0xf6e17800, args=...) at js/src/vm/Interpreter.cpp:562
#35 0x0815bdf0 in Interpret (cx=0xf6e17800, state=...) at js/src/vm/Interpreter.cpp:3076
#36 0x081608e9 in js::RunScript (cx=0xf6e17800, state=...) at js/src/vm/Interpreter.cpp:417
#37 0x0816241c in js::ExecuteKernel (cx=<optimized out>, script=..., envChainArg=..., newTargetValue=..., evalInFrame=..., result=<optimized out>) at js/src/vm/Interpreter.cpp:700
#38 0x0816255b in js::Execute (cx=0xf6e17800, script=..., envChainArg=..., rval=0xf59ee058) at js/src/vm/Interpreter.cpp:733
#39 0x08440430 in ExecuteScript (cx=cx@entry=0xf6e17800, scope=..., scope@entry=..., script=script@entry=..., rval=0xf59ee058) at js/src/jsapi.cpp:4702
#40 0x084405db in ExecuteScript (cx=0xf6e17800, envChain=..., scriptArg=..., scriptArg@entry=..., rval=0xf59ee058) at js/src/jsapi.cpp:4721
#41 0x08448b99 in JS_ExecuteScript (cx=<optimized out>, envChain=..., scriptArg=..., rval=...) at js/src/jsapi.cpp:4742
#42 0x0809e073 in Evaluate (cx=0xf6e17800, argc=1, vp=0xf59ee058) at js/src/shell/js.cpp:1976
#43 0x08160c9e in js::CallJSNative (args=..., native=0x809db10 <Evaluate(JSContext*, unsigned int, JS::Value*)>, cx=0xf6e17800) at js/src/vm/JSContext-inl.h:290
#44 js::InternalCallOrConstruct (cx=0xf6e17800, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:467
#45 0x08161052 in InternalCall (cx=<optimized out>, args=...) at js/src/vm/Interpreter.cpp:516
#46 0x081543e2 in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:522
#47 Interpret (cx=0xf6e17800, state=...) at js/src/vm/Interpreter.cpp:3084
#48 0x081608e9 in js::RunScript (cx=0xf6e17800, state=...) at js/src/vm/Interpreter.cpp:417
#49 0x0816241c in js::ExecuteKernel (cx=<optimized out>, script=..., envChainArg=..., newTargetValue=..., evalInFrame=..., result=<optimized out>) at js/src/vm/Interpreter.cpp:700
#50 0x0816255b in js::Execute (cx=0xf6e17800, script=..., envChainArg=..., rval=0x0) at js/src/vm/Interpreter.cpp:733
#51 0x08440430 in ExecuteScript (cx=cx@entry=0xf6e17800, scope=..., scope@entry=..., script=script@entry=..., rval=0x0) at js/src/jsapi.cpp:4702
#52 0x08448b4e in JS_ExecuteScript (cx=0xf6e17800, scriptArg=...) at js/src/jsapi.cpp:4735
#53 0x0806f270 in RunFile (compileOnly=<optimized out>, file=<optimized out>, filename=<optimized out>, cx=0xf6e17800) at js/src/shell/js.cpp:833
#54 Process (cx=0xf6e17800, filename=0xffffda4e "min.js", forceTTY=false, kind=FileScript) at js/src/shell/js.cpp:1277
#55 0x0807934c in ProcessArgs (op=0xffffd684, cx=0xf6e17800) at js/src/shell/js.cpp:8280
#56 Shell (envp=<optimized out>, op=0xffffd684, cx=0xf6e17800) at js/src/shell/js.cpp:8672
#57 main (argc=3, argv=0xffffd8d4, envp=0xffffd8e4) at js/src/shell/js.cpp:9137
eax	0xf6e09e04	-153051644
ebx	0x0	0
ecx	0x0	0
edx	0x0	0
esi	0xf6e09e04	-153051644
edi	0x883d712	142858002
ebp	0xffffc648	4294952520
esp	0xffffbab8	4294949560
eip	0x878af69 <ShouldMark<JSString*>(js::GCMarker*, JSString*)+9>
=> 0x878af69 <ShouldMark<JSString*>(js::GCMarker*, JSString*)+9>:	mov    0xffffc(%ecx),%ebx
   0x878af6f <ShouldMark<JSString*>(js::GCMarker*, JSString*)+15>:	cmp    %ebx,(%eax)


Also found this crash with a similar testcase, I assume this is the same bug?
Flags: needinfo?(luke)
Ah hah, it seems GCVector doesn't like null pointers (which we really need).  I'll see if there is a GCPolicy we can use here that does a null check.  On a side note, I noticed we're using a GCVector<HeapPtr<T>> which is overkill; all we need is a GCVector<T*>.
Flags: needinfo?(luke)
So while GCPolicy<T*> is implemented by GCPointerPolicy for the public pointer types, and GCPointerPolicy null checks in trace() et al, when T* is an internal type (i.e., in FOR_EACH_INTERNAL_GC_POINTER_TYPE), GCPolicy<T*> instead is implemented by InternalGCPointerPolicy which does not.  This seems like a pretty subtle discrepancy, so this patch does the obvious thing and adds null checks to the latter.  I would assume these are not significant.

Another option I considered first was to have GCVector do the null check in trace(), since null pointers seem somewhat inherent to GCVectors (they have resize() and grow(), after all), but afaics this requires access to InternalBarrierMethods<T>::isMarkable() (to null check) which is not public.

Anyhow, happy to have a replacement patch if there is a better way.  Either way, it seems like GCVector<T*> should support null T* for all T, not just public T.
Assignee: nobody → luke
Attachment #8964662 - Flags: review?(sphink)
GCVector<T*> handles all the barrier-ing and HeapPtr<T> is relatively heavyweight, so this patch switches from GCVector<HeapPtr<WasmGlobalObject*>> to GCVector<WasmGlobalObject*>.

Also, since this vector is long-lived and often empty, I changed the 8 inline capacity to 0.
Attachment #8964667 - Flags: review?(lhansen)
Comment on attachment 8964662 [details] [diff] [review]
check-null-in-trace

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

I have a distinct memory of terrence (maybe jonco, but I thought it was terrence) carefully adding the null-check to a limited set of T because it *was* perf-sensitive. But in all my spelunking through history, I see no evidence for that; it seems like the null-check was always there for the public types.

So, I guess we may as well land this and see if the talos people come beating on our door before thinking any further.
Attachment #8964662 - Flags: review?(sphink) → review+
Comment on attachment 8964667 [details] [diff] [review]
fix-wasm-global-ptr-type

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

OK.  The HeapPtr predated a lot of the work and is, so far as I can tell, now redundant.

I probably don't really believe that the vector is often empty but I doubt the inline capacity will make or break performance here.  However, since "0" is frequently the wrong inline capacity, maybe a comment justifying it is in order.

::: js/src/jit-test/tests/wasm/regress/bug1450800.js
@@ +1,1 @@
> +gczeal(9, 10);

This TC needs a guard on WasmGlobalObject being defined, or the import of the mutable global below will permafail the TC once we move past early beta.
Attachment #8964667 - Flags: review?(lhansen) → review+
(In reply to Lars T Hansen [:lth] from comment #6)
> I probably don't really believe that the vector is often empty but I doubt
> the inline capacity will make or break performance here.

For the stack-local, I think you're right we'll usually have N>0 globals; I was thinking about the persistently-stored WasmGlobalObject::indirectGlobals() which I do think should be usually 0.

> However, since "0"
> is frequently the wrong inline capacity, maybe a comment justifying it is in
> order.

I agree for a stack-bound vector, but for a persistent heap structure, I think 0 is the safe default with burden of proof on non-zero given that, if you grow past the inline storage, the inline storage is permanently wasted.

Maybe "this is stored in the heap" deserves a comment, but I think SystemAllocPolicy (as opposed to the default TempAllocPolicy) more-or-less says this in code.
Priority: -- → P1
Group: javascript-core-security
JSBugMon: This bug has been automatically verified fixed.
You need to log in before you can comment on or make changes to this bug.