Closed Bug 1404306 Opened 3 years ago Closed 3 years ago

Assertion failure: !ion || ion == ((js::jit::IonScript*)0x1), at js/src/jsscript.h:1584

Categories

(Core :: JavaScript Engine, defect, P1, critical)

ARM
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox-esr52 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: decoder, Assigned: jandem)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, bugmon, testcase, Whiteboard: [jsbugmon:update,bisect][fuzzblocker])

Attachments

(2 files)

The following testcase crashes on mozilla-central revision 8db0c4ecd94c (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu --enable-simulator=arm, run with --fuzzing-safe --cpu-count=2):

Testcase attached (but probably non-functional).

Backtrace:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x086637f6 in JSScript::hasBaselineScript (this=<optimized out>, this=<optimized out>) at js/src/jsscript.h:1584
#1  0x08669505 in JSScript::hasBaselineScript (this=0xf4109438, this=0xf4109438) at js/src/jsscript.cpp:4520
#2  JSScript::updateBaselineOrIonRaw (this=0xf4109438, maybeRuntime=0x0) at js/src/jsscript.cpp:4505
#3  0x0826931b in JSScript::setBaselineScript (this=0xf4109438, maybeRuntime=0x0, baselineScript=0x0) at js/src/jsscriptinlines.h:188
#4  0x0825ca0a in js::jit::FinishDiscardBaselineScript (fop=0xf4f7abbc, script=0xf4109438) at js/src/jit/BaselineJIT.cpp:1181
#5  0x08a72a5a in JS::Zone::discardJitCode (this=0xf71b7000, fop=0xf4f7abbc, discardBaselineCode=true) at js/src/gc/Zone.cpp:215
#6  0x085d93dd in js::gc::GCRuntime::sweepJitDataOnMainThread (this=0xf4921440, fop=0xf4f7abbc) at js/src/jsgc.cpp:5397
#7  0x085ffe7e in js::gc::GCRuntime::beginSweepingSweepGroup (this=0xf4921440) at js/src/jsgc.cpp:5554
#8  0x08600c28 in js::gc::GCRuntime::beginSweepPhase (this=0xf4921440, reason=JS::gcreason::DESTROY_RUNTIME, lock=...) at js/src/jsgc.cpp:5647
#9  0x08604f45 in js::gc::GCRuntime::incrementalCollectSlice (this=0xf4921440, budget=..., reason=JS::gcreason::DESTROY_RUNTIME, lock=...) at js/src/jsgc.cpp:6857
#10 0x086064f0 in js::gc::GCRuntime::gcCycle (this=0xf4921440, nonincrementalByAPI=true, budget=..., reason=JS::gcreason::DESTROY_RUNTIME) at js/src/jsgc.cpp:7140
#11 0x08606bed in js::gc::GCRuntime::collect (this=0xf4921440, nonincrementalByAPI=true, budget=..., reason=JS::gcreason::DESTROY_RUNTIME) at js/src/jsgc.cpp:7289
#12 0x08606ed4 in js::gc::GCRuntime::gc (this=0xf4921440, gckind=GC_NORMAL, reason=JS::gcreason::DESTROY_RUNTIME) at js/src/jsgc.cpp:7356
#13 0x0880629f in JSRuntime::destroyRuntime (this=0xf4921000) at js/src/vm/Runtime.cpp:320
#14 0x08598317 in js::DestroyContext (cx=0xf71b2800) at js/src/jscntxt.cpp:249
#15 0x080a7c65 in <lambda()>::operator() (__closure=0xf4f7b248) at js/src/shell/js.cpp:3476
#16 mozilla::ScopeExit<WorkerMain(void*)::<lambda()> >::~ScopeExit (this=0xf4f7b248, __in_chrg=<optimized out>) at dist/include/mozilla/ScopeExit.h:112
#17 WorkerMain (arg=0xf54a4e60) at js/src/shell/js.cpp:3482
#18 0x080afebb in js::detail::ThreadTrampoline<void (&)(void*), WorkerInput*&>::callMain<0u> (this=0xf711e190) at js/src/threading/Thread.h:234
#19 js::detail::ThreadTrampoline<void (&)(void*), WorkerInput*&>::Start (aPack=0xf711e190) at js/src/threading/Thread.h:227
#20 0xf778228a in start_thread (arg=0xf4f7bb40) at pthread_create.c:333
#21 0xf74a94ce in clone () from /lib32/libc.so.6
eax	0x0	0
ebx	0x8d93ff4	148455412
ecx	0xf7574864	-145274780
edx	0x0	0
esi	0x0	0
edi	0xf544fb50	-180028592
ebp	0xf4f7a9e8	4109871592
esp	0xf4f7a9e0	4109871584
eip	0x86637f6 <JSScript::hasBaselineScript() const+38>
=> 0x86637f6 <JSScript::hasBaselineScript() const+38>:	movl   $0x0,0x0
   0x8663800 <JSScript::hasBaselineScript() const+48>:	ud2    



The testcase unfortunately never reproduced for me, neither did any others. However, this is coming in at a high frequency so I'm filing this bug to get it resolved maybe even without a working test. I've been seeing these crashes across the board on all architectures, but x86/ARM seem to be more frequent.
Attached file Testcase
Flags: needinfo?(jdemooij)
It's pretty weird, no idea what could have introduced this. I can't repro but I can try more next week.

Let's lock this bug for now; I don't really like this assertion failure.
Group: javascript-core-security
Attached patch PatchSplinter Review
OK I could repro this under rr chaos mode (after hundreds of runs!).

In IonCompile, we have code like this:

    if (!StartOffThreadIonCompile(cx, builder)) {
        ... snip ...
    }
    if (!recompile)
        builderScript->setIonScript(cx->runtime(), ION_COMPILING_SCRIPT);

Now, *between* these two if-statements, a different worker thread can call oomTest/stackTest/interruptTest -> GlobalHelperThreadState::waitForAllThreads and cancel all compilations.

FinishOffThreadBuilder will then be confused, because we didn't execute the second if-statement yet.

I think this is shell-only because waitForAllThreads and the AllCompilations selector are only used by the OOM testing functions. It's a bit of a footgun though because it allows certain races that aren't possible without the AllCompilations selector.

The patch just adds an AutoLockHelperThreadState around both if-statements. I also made an assert in setBaselineScript a bit stronger and removed an unused function.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8914699 - Flags: review?(jcoppeard)
Not an OOM bug.
Summary: Assertion failure: !ion || ion == ((js::jit::IonScript*)0x1), at js/src/jsscript.h:1584 with OOM → Assertion failure: !ion || ion == ((js::jit::IonScript*)0x1), at js/src/jsscript.h:1584
Comment on attachment 8914699 [details] [diff] [review]
Patch

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

LGTM.
Attachment #8914699 - Flags: review?(jcoppeard) → review+
(In reply to Jan de Mooij [:jandem] from comment #3)
> OK I could repro this under rr chaos mode (after hundreds of runs!).

I tested this again with the patch yesterday, then I forgot I left it running in the evening - didn't fail after 120,000 runs ;)
calling it sec-other if it's just a js shell race?
Priority: -- → P1
Yes let's open this up, shell-only.
Group: javascript-core-security
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c42f60eda845
Set ION_COMPILING_SCRIPT in AutoLockHelperThreadState scope to avoid a shell-only race. r=jonco
https://hg.mozilla.org/mozilla-central/rev/c42f60eda845
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.