1100080 - Assertion failure: !used(), at jit/Label.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

// Random chosen test: js/src/jit-test/tests/ion/bug861439.js
try {
    evalcx('')
} catch (e) {}
// Random chosen test: js/src/jit-test/tests/jaeger/bug606662-2.js
// -- Reduced away --
// Random chosen test: js/src/jit-test/tests/auto-regress/bug790921.js
gcparam("maxBytes", gcparam("gcBytes"));
// jsfunfuzz
function f01(){}
function f02(){}
function f03(){}
function f04(){}
function f05(){}
function f06(){}
function f07(){}
function f08(){}
function f09(){}
function f10(){}
function f11(){}
function f12(){}
function f13(){}
function f14(){}
function f15(){}
function f16(){}
function f17(){}
function f18(){}
function f19(){}
function f20(){}
function f21(){}
function f22(){}
function f23(){}
function f24(){}
function f25(){}
function f26(){}
function f27(){}
function f28(){}
function f29(){}
function f30(){}
function f31(){}
function f32(){}
function f33(){}
function f34(){}
function f35(){}
function f36(){}
function f37(){}
function f38(){}
function f39(){}
function f40(){}
function f41(){}
function f42(){}
function f43(){}
function f44(){}
function f45(){}
function f46(){}
function f47(){}
function f48(){}
function f49(){}
function f50(){}
function f51(){}
function f52(){}
function f53(){}
function f54(){}
function f55(){}
function f56(){}
function f57(){}
function f58(){}
function f59(){}
function f60(){}
function f61(){}
function f62(){}
function f63(){}
function f64(){}
function f65(){}
function f66(){}
function f67(){}
function f68(){}
function f69(){}
function f70(){}
function f71(){}
function f72(){}
function f73(){}
function f74(){}
function f75(){}
function f76(){}
function f77(){}
function f78(){}
function f79(){}
function f80(){}
function f81(){}
function f82(){}
function f83(){}
function f84(){}
function f85(){}
function f86(){}
function f87(){}
function f88(){}
function f89(){}
function f90(){}
function f91(){}
function f92(){}
function f93(){}
function f94(){}
function f95(){}
function f96(){}
function f97(){}
function f98(){}
function f99(){}
gc()
var x = Object.getOwnPropertyNames(this)
for (var i = 0; i < 302; ++i) {
    y = x[i]
    if (4 < y.charCodeAt() & y.n != "") {
        var g = this[y]
        if (typeof g == "function" && g.toString().indexOf("")) {}
    }
}
"".replace(/\s/, "")

asserts js debug shell on m-c changeset ab137ddd3746 with --fuzzing-safe --ion-eager --no-threads --no-baseline at Assertion failure: !used(), at jit/Label.h.

Debug configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

This was found by combining random jit-tests together with jsfunfuzz, the specific files are:

http://hg.mozilla.org/mozilla-central/file/ab137ddd3746/js/src/jit-test/tests/ion/bug861439.js
http://hg.mozilla.org/mozilla-central/file/ab137ddd3746/js/src/jit-test/tests/jaeger/bug606662-2.js
http://hg.mozilla.org/mozilla-central/file/ab137ddd3746/js/src/jit-test/tests/auto-regress/bug790921.js

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/ad0fdfc44d48
user:        Lars T Hansen
date:        Thu Oct 23 14:23:16 2014 +0200
summary:     Bug 979594 - Atomics object, atomic operations on SharedArrayBuffer (no jit support). r=luke

Lars, is bug 979594 a possible regressor?

Setting s-s and assuming sec-critical because this seems to involve gc.

Comment 1

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

Attachment: stack

(lldb) bt 5
* thread #1: tid = 0x3c9850, 0x00000001001cc0d7 js-dbg-opt-64-dm-nsprBuild-darwin-ab137ddd3746`js::irregexp::NativeRegExpMacroAssembler::~NativeRegExpMacroAssembler() [inlined] js::jit::Label::~Label() + 67 at Label.h:91, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001001cc0d7 js-dbg-opt-64-dm-nsprBuild-darwin-ab137ddd3746`js::irregexp::NativeRegExpMacroAssembler::~NativeRegExpMacroAssembler() [inlined] js::jit::Label::~Label() + 67 at Label.h:91
    frame #1: 0x00000001001cc094 js-dbg-opt-64-dm-nsprBuild-darwin-ab137ddd3746`js::irregexp::NativeRegExpMacroAssembler::~NativeRegExpMacroAssembler() [inlined] js::Vector<js::irregexp::NativeRegExpMacroAssembler::LabelPatch, 4ul, js::SystemAllocPolicy>::~Vector(this=<unavailable>) + 18 at Label.h:81
    frame #2: 0x00000001001cc082 js-dbg-opt-64-dm-nsprBuild-darwin-ab137ddd3746`js::irregexp::NativeRegExpMacroAssembler::~NativeRegExpMacroAssembler(this=<unavailable>) + 370 at NativeRegExpMacroAssembler.h:82
    frame #3: 0x000000010019dad5 js-dbg-opt-64-dm-nsprBuild-darwin-ab137ddd3746`js::irregexp::CompilePattern(JSContext*, js::RegExpShared*, js::irregexp::RegExpCompileData*, JS::Handle<JSLinearString*>, bool, bool, bool, bool, bool) [inlined] mozilla::Maybe<js::irregexp::NativeRegExpMacroAssembler>::ref(this=<unavailable>) + 2069 at NativeRegExpMacroAssembler.h:82
    frame #4: 0x000000010019dac9 js-dbg-opt-64-dm-nsprBuild-darwin-ab137ddd3746`js::irregexp::CompilePattern(JSContext*, js::RegExpShared*, js::irregexp::RegExpCompileData*, JS::Handle<JSLinearString*>, bool, bool, bool, bool, bool) [inlined] mozilla::Maybe<js::irregexp::NativeRegExpMacroAssembler>::reset(this=0xfffa800101f1ad01) + 16 at Maybe.h:416
(lldb)

Comment 2

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

(In reply to Gary Kwong [:gkw] [:nth10sd] GMT+8 after Oct 9 from comment #0)
> Lars, is bug 979594 a possible regressor?

However, irregexp is all over the stack, so now I'm confused as to what the cause of this may be.

This issue seems intermittent on different revisions (gc-related?) but it seems fairly stable on the specified m-c rev. I've also emailed Lars and Jan the testcase and binary to hopefully move this forward.

Comment 3

[Christian Holler (:decoder)]

(In reply to Gary Kwong [:gkw] [:nth10sd] GMT+8 after Oct 9 from comment #2)

> This issue seems intermittent on different revisions (gc-related?) but it
> seems fairly stable on the specified m-c rev. 

It's likely not intermittent due to gc, but because you use gcparam, which causes an OOM (using gcparam with 'maxBytes' just limits the amount of memory the GC can allocate). It's also very unlikely to be s-s. Here's my rough guess what's wrong:

The assertion !used generally does not hold on OOM, that's why it's guarded:


> // The assertion below doesn't hold if an error occurred.
> if (OOM_counter > OOM_maxAllocations)
>     return;
> if (IonContext *context = MaybeGetIonContext()) {
>     if (context->runtime->hadOutOfMemory())
> > return;
> }
> 
> MOZ_ASSERT(!used());

However, that requires that an OOM encountered is actually marked as such in the context. Historically, different subsystems (e.g. Yarr, Type Inference, etc) had their own OOM handling and failed to work properly with the engine's OOM handling in various ways. It might just be that irregexp also has this problem somewhere.

Comment 4

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

Unclear if this is sec-critical, leaving someone else to set the rating.

Comment 5

[Lars T Hansen [:lth]]

(In reply to Gary Kwong [:gkw] [:nth10sd] GMT+8 after Oct 9 from comment #0)

> autoBisect shows this is probably related to the following changeset:
> 
> The first bad revision is:
> changeset:   https://hg.mozilla.org/mozilla-central/rev/ad0fdfc44d48
> user:        Lars T Hansen
> date:        Thu Oct 23 14:23:16 2014 +0200
> summary:     Bug 979594 - Atomics object, atomic operations on
> SharedArrayBuffer (no jit support). r=luke
> 
> Lars, is bug 979594 a possible regressor?

It is fairly unlikely that 979594 is the regressor per se.  The patches on that bug introduced a new type hierarchy around TypedArray and generalized some JIT functionality around those types but should not have affected any code run in your test case.

However those patches did introduce new engine types so will have perturbed some internal tables, with the chance that some cases in the engine were not updated where needed and/or preexisting bugs elsewhere in the code became exposed.

On the whole, decoder's explanation in comment #3 seems more likely.

Comment 6

[Lars T Hansen [:lth]]

Also: bug 979594 introduced 10 new global constructors, thus affecting the contents of the heap at startup.  I would expect that to affect (some) GC tests in a size-limited heap.

Comment 7

[Lars T Hansen [:lth]]

It appears that the OOM is triggered by NativeRegExpMacroAssembler::GenerateCode in its call to ensureJitCompartmentExists, but no OOM flag is set by that call (so far as I can tell).  Hence the destruction of the NativeRegExpMacroAssembler may at least in principle hit the assert in the JIT following decoder's analysis.  (This is all with Gary's build.)

ISTR bhackett originated this code, redirecting question to him.

Comment 8

[Brian Hackett [Laid off!]]

(In reply to Lars T Hansen [:lth] from comment #7)
> It appears that the OOM is triggered by
> NativeRegExpMacroAssembler::GenerateCode in its call to
> ensureJitCompartmentExists, but no OOM flag is set by that call (so far as I
> can tell).  Hence the destruction of the NativeRegExpMacroAssembler may at
> least in principle hit the assert in the JIT following decoder's analysis. 
> (This is all with Gary's build.)
> 
> ISTR bhackett originated this code, redirecting question to him.

ensureJitCompartmentExists should call js_ReportOutOfMemory if it returns false.  NativeRegExpMacroAssembler correctly returns an error value (an empty RegExpCode) in this case, and maybe it is failing rather than other components because it has the misfortune of creating a Label before calling ensureJitCompartmentExists.  The general strength of our assertions vis a vis OOM handling is pretty awful and I think our OOM handling mechanism needs to be rewritten, which is something I'd like to talk about at the workweek.  So I'd rather not look at this bug (which I can't reproduce anyways) until after that talk.  Also, this assertion is only meaningful when we didn't have an OOM, so it isn't s-s.

Comment 9

[Brian Hackett [Laid off!]]

Attachment: patch

JitRuntime::initialize doesn't call ReportOutOfMemory if it fails, since it is called under the exclusive access lock and can't GC.  This patch adds a call when it finishes, though longer term we should have either static analysis support to detect this sort of thing, or better yet remove ReportOutOfMemory and treat failed-with-no-exception as an OOM.

Comment 10

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/11d3848296da

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/11d3848296da