Assertion failure: !rt->mainContextFromOwnThread()->suppressGC, at js/src/gc/GC.cpp:7230 or Crash [@ js::VerifyPreTracer::onChild] with wasm and GC

RESOLVED FIXED in Firefox 61

Status

()

defect
--
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: decoder, Assigned: bbouvier)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla61
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

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

Details

(Whiteboard: [jsbugmon:], crash signature)

Attachments

(1 attachment)

Reporter

Description

a year ago
The following testcase crashes on mozilla-central revision cc0d7de218cb (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --wasm-gc):

var lfModule = new WebAssembly.Module(wasmTextToBinary(`
    (module
        (import "global" "func" (result i32))
        (func (export "func_0") (result i32)
         call 0 ;; calls the import, which is func #0
        )
    )
`));
gczeal(4)
processModule(lfModule, `
  gczeal(4)
`);
function processModule(module, jscode) {
    imports = {}
    for (let descriptor of WebAssembly.Module.imports(module)) {
        imports[descriptor.module] = {}
        imports[descriptor.module][descriptor.name] = new Function("x", "y", "z", jscode);
        instance = new WebAssembly.Instance(module, imports);
        for (let descriptor of WebAssembly.Module.exports(module)) {
            print(instance.exports[descriptor.name]())
        }
    }
}


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x0000000000ebf9d0 in js::gc::IsIncrementalGCUnsafe (rt=0x7ffff5f19000) at js/src/gc/GC.cpp:7230
#0  0x0000000000ebf9d0 in js::gc::IsIncrementalGCUnsafe (rt=0x7ffff5f19000) at js/src/gc/GC.cpp:7230
#1  0x0000000000f591bf in js::gc::GCRuntime::endVerifyPreBarriers (this=0x7ffff5f19700) at js/src/gc/Verifier.cpp:359
#2  0x0000000000f5c32f in js::gc::GCRuntime::verifyPreBarriers (this=<optimized out>) at js/src/gc/Verifier.cpp:403
#3  js::gc::VerifyBarriers (rt=<optimized out>, type=type@entry=js::gc::PreBarrierVerifier) at js/src/gc/Verifier.cpp:412
#4  0x0000000000ee934f in js::gc::GCRuntime::setZeal (this=0x7ffff5f19700, zeal=<optimized out>, frequency=100) at js/src/gc/GC.cpp:1074
#5  0x0000000000a13433 in JS_SetGCZeal (cx=<optimized out>, zeal=<optimized out>, frequency=<optimized out>) at js/src/jsapi.cpp:7133
#6  0x000000000089d59c in GCZeal (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/TestingFunctions.cpp:920
#7  0x00000000005b2e4e in js::CallJSNative (cx=0x7ffff5f15000, native=0x89d4b0 <GCZeal(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/JSContext-inl.h:280
#8  0x00000000005a77af in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff5f15000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:467
#9  0x00000000005a7b8d in InternalCall (cx=0x7ffff5f15000, args=...) at js/src/vm/Interpreter.cpp:516
#10 0x0000000000599fc1 in js::CallFromStack (args=..., cx=<optimized out>) at js/src/vm/Interpreter.cpp:522
#11 Interpret (cx=0x7ffff5f15000, state=...) at js/src/vm/Interpreter.cpp:3084
#12 0x00000000005a726d in js::RunScript (cx=0x7ffff5f15000, state=...) at js/src/vm/Interpreter.cpp:417
#13 0x00000000005a7877 in js::InternalCallOrConstruct (cx=<optimized out>, cx@entry=0x7ffff5f15000, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:489
#14 0x00000000005a7b8d in InternalCall (cx=0x7ffff5f15000, args=...) at js/src/vm/Interpreter.cpp:516
#15 0x00000000005a7d10 in js::Call (cx=<optimized out>, fval=..., fval@entry=..., thisv=..., thisv@entry=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:535
#16 0x0000000000dde0ad in js::wasm::Instance::callImport (this=this@entry=0x7ffff49fb5f0, cx=<optimized out>, cx@entry=0x7ffff5f15000, funcImportIndex=funcImportIndex@entry=0, argc=argc@entry=0, argv=argv@entry=0x7fffffffc530, rval=..., rval@entry=...) at js/src/wasm/WasmInstance.cpp:156
#17 0x0000000000ddeba4 in js::wasm::Instance::callImport_i32 (instance=0x7ffff49fb5f0, funcImportIndex=0, argc=0, argv=0x7fffffffc530) at js/src/wasm/WasmInstance.cpp:252
#18 0x0000032ebe2830fc in ?? ()
[...]
#24 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff5f19000	140737319636992
rcx	0x7ffff6c282ad	140737333330605
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffb240	140737488335424
rsp	0x7fffffffb230	140737488335408
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4780	140737354024832
r10	0x58	88
r11	0x7ffff6b9e7a0	140737332766624
r12	0x7ffff5f15000	140737319620608
r13	0x7ffff5f1a688	140737319642760
r14	0x7ffff5f1a608	140737319642632
r15	0x39	57
rip	0xebf9d0 <js::gc::IsIncrementalGCUnsafe(JSRuntime*)+112>
=> 0xebf9d0 <js::gc::IsIncrementalGCUnsafe(JSRuntime*)+112>:	movl   $0x0,0x0
   0xebf9db <js::gc::IsIncrementalGCUnsafe(JSRuntime*)+123>:	ud2


Not s-s because it requires --wasm-gc which is disabled by default right now.
Reporter

Comment 1

a year ago
This can also crash [@ js::VerifyPreTracer::onChild].
Crash Signature: [@ js::VerifyPreTracer::onChild]
Summary: Assertion failure: !rt->mainContextFromOwnThread()->suppressGC, at js/src/gc/GC.cpp:7230 with wasm and GC → Assertion failure: !rt->mainContextFromOwnThread()->suppressGC, at js/src/gc/GC.cpp:7230 or Crash [@ js::VerifyPreTracer::onChild] with wasm and GC

Comment 2

a year ago
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:bisect]

Updated

a year ago
Whiteboard: [jsbugmon:bisect] → [jsbugmon:]
Assignee

Comment 3

a year ago
Jon, in js::gc::IsIncrementalGCUnsafe, does it make sense to return gc::AbortReason::IncrementalDisabled or gc::AbortReason::AbortRequested when --wasm-gc is on (and we explicitly want to disallow GCs)?
Flags: needinfo?(jcoppeard)
Assignee

Updated

a year ago
Blocks: 1444925
(In reply to Benjamin Bouvier [:bbouvier] from comment #3)
No, no GC is safe in this situation.

The correct fix is probably to make JS_SetZeal fail if we're running wasm with GC suppressed.
Flags: needinfo?(jcoppeard)
Assignee

Updated

a year ago
Duplicate of this bug: 1456676
Assignee

Comment 6

a year ago
Posted patch fix.patchSplinter Review
Jon, what do you think of something simple like this? This has GCRuntime::setZeal return early when --wasm-gc is enabled **and** there are wasm frames on the stack.

Probably we could also check --wasm-gc enabled and supressGC is not zero, but I think this is slightly less precise.
Assignee: nobody → bbouvier
Status: NEW → ASSIGNED
Attachment #8970842 - Flags: review?(jcoppeard)
Comment on attachment 8970842 [details] [diff] [review]
fix.patch

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

What happens if a test enables zeal with no wasm on the stack and then disables it when there is wasm on the stack, or vice versa?  I guess you ran the tests, but this is still pretty sketchy.

Grudging r+ since we've decided to go this route.

::: js/src/gc/GC.cpp
@@ +1070,5 @@
>  {
>      MOZ_ASSERT(zeal <= unsigned(ZealMode::Limit));
>  
> +#ifdef ENABLE_WASM_GC
> +    JSContext* cx = rt->mainContextFromOwnThread();

Please add a comment here saying what this is for and that it's a temporary fix.
Attachment #8970842 - Flags: review?(jcoppeard) → review+
(In reply to Jon Coppeard (:jonco) from comment #7)
bbouvier explained on IRC that --wasm-gc is only ever specified for the wasm GC tests, so it's not as bad as I thought in the previous comment.
Assignee

Updated

a year ago
Depends on: 1456824
Assignee

Updated

a year ago
No longer blocks: 1444925

Comment 9

a year ago
Pushed by bbouvier@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c74001d3808
Prevent calling setGcZeal with --wasm-gc and wasm frames on the stack; r=jonco

Comment 10

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8c74001d3808
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.