1570173 - Crash [@ js::AutoUnsafeCallWithABI::AutoUnsafeCallWithABI] or Assertion failure: cx, at js/src/threading/ProtectedData.cpp:52

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

The following testcase crashes on mozilla-central revision 06cd837c3943 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --ion-offthread-compile=off --ion-eager 42.wrapper 42.wasm):

See attachment.

Backtrace:

#0  js::CheckContextLocal::check (this=0x230) at js/src/threading/ProtectedData.cpp:52
#1  0x0000561f4f9dcc1e in js::ProtectedData<js::CheckContextLocal, bool>::ref (this=0x228) at js/src/threading/ProtectedData.h:145
#2  js::ProtectedData<js::CheckContextLocal, bool>::operator bool const& (this=0x228) at js/src/threading/ProtectedData.h:89
#3  js::AutoUnsafeCallWithABI::AutoUnsafeCallWithABI (this=0x7f89ff2fd3c0, strictness=js::AllowPendingExceptions) at js/src/vm/JSContext.cpp:1506
#4  0x0000561f50503302 in js::NumberMod (a=10, b=69) at js/src/jslibmath.h:35
/snip

For detailed crash information, see attachment.

I also see crashes [@ js::AutoUnsafeCallWithABI::AutoUnsafeCallWithABI] with variants of the testcase.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Detailed Crash Information

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Testcase

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: out.wast

.wast form of the .wasm file, as disassembled by wasm-dis from binaryen.

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Due to skipped revisions, the first bad revision could be any of:
changeset: https://hg.mozilla.org/mozilla-central/rev/a56b3fe80336
user: Kristen Wright
date: Mon Jul 29 19:08:45 2019 +0000
summary: Bug 1559659 - 2. Create, initialize, destroy vector of JSContext* with GlobalHelperThreadState r=jandem

changeset: https://hg.mozilla.org/mozilla-central/rev/0eff8ea5dbf1
user: Kristen Wright
date: Mon Jul 29 19:09:00 2019 +0000
summary: Bug 1559659 - 3. Get rid of per-thread jscontext instantiation and use the global pool instead r=jandem

Kris, is bug 1559659 a likely regressor?

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Sample stack involving the crash only:

backtrace

#0  js::AutoUnsafeCallWithABI::AutoUnsafeCallWithABI (this=0x7f3ec08fe390, strictness=js::AllowPendingExceptions) at /home/ubuntu/trees/mozilla-central/js/src/vm/JSContext.cpp:1506
#1  0x0000556776d3f1e2 in js::NumberDiv (a=0, b=0) at /home/ubuntu/trees/mozilla-central/js/src/jslibmath.h:20
#2  EvaluateConstantOperands (alloc=..., ins=0x7f3ebf1becf0, ptypeChange=0x0) at /home/ubuntu/trees/mozilla-central/js/src/jit/MIR.cpp:144
#3  0x0000556776d423a6 in js::jit::MDiv::foldsTo (this=0x7f3ebf1becf0, alloc=...) at /home/ubuntu/trees/mozilla-central/js/src/jit/MIR.cpp:3134
#4  0x0000556776a51b0e in js::jit::ValueNumberer::simplified (this=<optimized out>, def=<optimized out>) at /home/ubuntu/trees/mozilla-central/js/src/jit/ValueNumbering.cpp:614
#5  js::jit::ValueNumberer::visitDefinition (this=0x7f3ec08fe718, def=0x7f3ebf1becf0) at /home/ubuntu/trees/mozilla-central/js/src/jit/ValueNumbering.cpp:771
#6  0x0000556776a52e35 in js::jit::ValueNumberer::visitBlock (this=0x7f3ec08fe718, block=<optimized out>) at /home/ubuntu/trees/mozilla-central/js/src/jit/ValueNumbering.cpp:1016
#7  0x0000556776a532e8 in js::jit::ValueNumberer::visitDominatorTree (this=0x7f3ec08fe718, dominatorRoot=<optimized out>) at /home/ubuntu/trees/mozilla-central/js/src/jit/ValueNumbering.cpp:1072
#8  0x0000556776a534ab in js::jit::ValueNumberer::visitGraph (this=0x7f3ec08fe718) at /home/ubuntu/trees/mozilla-central/js/src/jit/ValueNumbering.cpp:1111
#9  0x0000556776a53cb4 in js::jit::ValueNumberer::run (this=0x7f3ec08fe718, updateAliasAnalysis=<optimized out>) at /home/ubuntu/trees/mozilla-central/js/src/jit/ValueNumbering.cpp:1279
#10 0x0000556776c068d9 in js::jit::OptimizeMIR (mir=0x7f3ec08fe980) at /home/ubuntu/trees/mozilla-central/js/src/jit/Ion.cpp:1265
#11 0x0000556776eb0f68 in js::wasm::IonCompileFunctions (env=..., lifo=..., inputs=..., code=0x7f3ebf29b638, error=0x7f3ec09007f8) at /home/ubuntu/trees/mozilla-central/js/src/wasm/WasmIonCompile.cpp:4211
#12 0x0000556776ea1333 in ExecuteCompileTask (task=0x7f3ebf29b290, error=0x7f3ec09007f8) at /home/ubuntu/trees/mozilla-central/js/src/wasm/WasmGenerator.cpp:736
#13 0x0000556776ea1002 in js::wasm::ExecuteCompileTaskFromHelperThread (task=0x7f3ebf29b290) at /home/ubuntu/trees/mozilla-central/js/src/wasm/WasmGenerator.cpp:760
#14 0x00005567761cdeb7 in js::HelperThread::handleWasmWorkload (this=0x7f3ec0d07d00, locked=..., mode=js::wasm::CompileMode::Tier2) at /home/ubuntu/trees/mozilla-central/js/src/vm/HelperThreads.cpp:2106
#15 0x00005567761cdb6f in js::HelperThread::threadLoop (this=0x7f3ec0d07d00) at /home/ubuntu/trees/mozilla-central/js/src/vm/HelperThreads.cpp:2563
#16 0x00005567761c934d in js::HelperThread::ThreadMain (arg=0x7f3ec08fe390) at /home/ubuntu/trees/mozilla-central/js/src/vm/HelperThreads.cpp:2083
#17 0x000055677620a6ff in js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::callMain<0ul> (this=0x7f3ec0d25090) at /home/ubuntu/trees/mozilla-central/js/src/threading/Thread.h:239
#18 js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::Start (aPack=0x7f3ec0d25090) at /home/ubuntu/trees/mozilla-central/js/src/threading/Thread.h:232
#19 0x00007f3ec2b656db in start_thread (arg=0x7f3ec0901700) at pthread_create.c:463
#20 0x00007f3ec1b2e88f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Since files involving MIR are near the top of the stack, I'm locking this s-s to be safe. All the testcases reduce to the assertion failure though.

Comment 6

[Kris Wright :KrisWright]

Attachment: Bug 1570173 - Add AutoSetHelperThreadContext to js::wasm::CompileTask::runTask(). r=jandem

CompileTask can call js::AutoUnsafeCallWtihABI::AutoUnsafeCallWithABI which uses a thread-local context, so we've got to set one to the thread in CompileTask::runTask().

Comment 7

[Kris Wright :KrisWright]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #4)

Due to skipped revisions, the first bad revision could be any of:
changeset: https://hg.mozilla.org/mozilla-central/rev/a56b3fe80336
user: Kristen Wright
date: Mon Jul 29 19:08:45 2019 +0000
summary: Bug 1559659 - 2. Create, initialize, destroy vector of JSContext* with GlobalHelperThreadState r=jandem

changeset: https://hg.mozilla.org/mozilla-central/rev/0eff8ea5dbf1
user: Kristen Wright
date: Mon Jul 29 19:09:00 2019 +0000
summary: Bug 1559659 - 3. Get rid of per-thread jscontext instantiation and use the global pool instead r=jandem

Kris, is bug 1559659 a likely regressor?

Looks like wasm::CompileTask::runTask does not set a context to the thread, so the protected data checks are getting a null TlsContext. All that should need to be done is adding AutoSetHelperThreadContext to runTask(). I've put up a patch that should fix this.

Comment 8

[Jan de Mooij [:jandem]]

Opening this up as it's just a nullptr context.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2861d1f5db97
Add AutoSetHelperThreadContext to js::wasm::CompileTask::runTask(). r=jandem

Comment 10

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/2861d1f5db97