Closed Bug 1236473 Opened 9 years ago Closed 9 years ago

Crash [@ js::gc::MergeCompartments] with OOM

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: decoder, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])

Crash Data

Attachments

(1 file)

The following testcase crashes on mozilla-central revision d7a0ad85d9fb (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe): oomTest(() => { offThreadCompileScript(`try {} catch (NaN) {}`); runOffThreadScript(); }); Backtrace: Program received signal SIGSEGV, Segmentation fault. js::gc::MergeCompartments (source=<optimized out>, target=target@entry=0x7ffff6949000) at js/src/jsobj.h:122 #0 js::gc::MergeCompartments (source=<optimized out>, target=target@entry=0x7ffff6949000) at js/src/jsobj.h:122 #1 0x0000000000a2a9b7 in js::GlobalHelperThreadState::mergeParseTaskCompartment (this=<optimized out>, rt=rt@entry=0x7ffff695d000, parseTask=0x7ffff28b1480, global=global@entry=..., dest=0x7ffff6949000) at js/src/vm/HelperThreads.cpp:1151 #2 0x0000000000a368bd in js::GlobalHelperThreadState::finishParseTask (this=<optimized out>, maybecx=maybecx@entry=0x7ffff6907800, rt=rt@entry=0x7ffff695d000, token=token@entry=0x7ffff28b1480) at js/src/vm/HelperThreads.cpp:1053 #3 0x00000000008c4cde in JS::FinishOffThreadScript (maybecx=maybecx@entry=0x7ffff6907800, rt=rt@entry=0x7ffff695d000, token=token@entry=0x7ffff28b1480) at js/src/jsapi.cpp:4111 #4 0x0000000000488773 in runOffThreadScript (cx=0x7ffff6907800, argc=<optimized out>, vp=0x7fffffffc338) at js/src/shell/js.cpp:3670 #5 0x00007ffff7ff7d28 in ?? () #6 0x0000000000000000 in ?? () rax 0x7ffff2529b30 140737258887984 rbx 0x7ffff2529b20 140737258887968 rcx 0x1 1 rdx 0x0 0 rsi 0x7ffff2600000 140737259765760 rdi 0x7ffff26911c8 140737260360136 rbp 0x7fffffffc060 140737488339040 rsp 0x7fffffffbf10 140737488338704 r8 0x210 528 r9 0x1 1 r10 0x568a6942 1451911490 r11 0x6 6 r12 0x7ffff2691160 140737260360032 r13 0x0 0 r14 0x7ffff6949000 140737330319360 r15 0x0 0 rip 0x9274bd <js::gc::MergeCompartments(JSCompartment*, JSCompartment*)+701> => 0x9274bd <js::gc::MergeCompartments(JSCompartment*, JSCompartment*)+701>: mov 0x0(%r13),%rax 0x9274c1 <js::gc::MergeCompartments(JSCompartment*, JSCompartment*)+705>: lea 0x1269058(%rip),%rsi # 0x1b90520 <_ZN2js10CallObject6class_E>
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/de72e2291ae8 user: Jan de Mooij date: Wed Dec 09 22:55:50 2015 -0500 summary: Bug 1225396 part 3 - Make %GeneratorPrototype% inherit from %IteratorPrototype%. r=jorendorff This iteration took 283.490 seconds to run.
Jan, is bug 1225396 a likely regressor?
Blocks: 1225396
Flags: needinfo?(jdemooij)
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #2) > Jan, is bug 1225396 a likely regressor? No it's a regression from bug 1213574. The problem is that we OOM while allocating the script's shared data, *after* we call JSScript::partiallyInit. So we've set the length of the JSScript arrays for block scopes, constants, etc. but we haven't filled these arrays yet. Then in gc::MergeCompartments, we do: if (script->hasBlockScopes()) { BlockScopeArray* scopes = script->blockScopes(); for (uint32_t i = 0; i < scopes->length; i++) { uint32_t scopeIndex = scopes->vector[i].index; if (scopeIndex != BlockScopeNote::NoBlockScopeIndex) { ScopeObject* scope = &script->getObject(scopeIndex)->as<ScopeObject>(); We use calloc, so scopeIndex is 0 and script->getObject(scopeIndex) is nullptr, so we crash. Not sure how to fix it. The easiest and safest fix may be to initialize the block scope list with NoBlockScopeIndex instead of 0, in JSScript::partiallyInit.
Blocks: 1213574
No longer blocks: 1225396
Flags: needinfo?(jdemooij) → needinfo?(shu)
Flags: needinfo?(shu)
Comment on attachment 8709758 [details] [diff] [review] Do not merge scripts that didn't successfully compile. Review of attachment 8709758 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, that's simpler than my suggestion. ::: js/src/jit-test/tests/gc/bug-1236473.js @@ +2,5 @@ > + quit(); > + > +oomTest(() => { > + offThreadCompileScript(`try {} catch (NaN) {}`); > + runOffThreadScript(); Nit: make sure the test doesn't fail with --no-threads (offThreadCompileScript will throw in that case). ::: js/src/jsgc.cpp @@ +6857,5 @@ > script->setTypesGeneration(target->zone()->types.generation); > > + // If the script failed to compile, no need to fix up. > + if (!script->code()) > + continue; It seems a little safer to do this before we modify the script's compartment.
Attachment #8709758 - Flags: review?(jdemooij) → review+
(In reply to Jan de Mooij [:jandem] from comment #5) > ::: js/src/jsgc.cpp > @@ +6857,5 @@ > > script->setTypesGeneration(target->zone()->types.generation); > > > > + // If the script failed to compile, no need to fix up. > > + if (!script->code()) > > + continue; > > It seems a little safer to do this before we modify the script's compartment. The reason I did it after was that that was the old mergeCompartment code. I don't know enough about the invariants to say if the source compartment expects all of its things to be moved. If you know for certain there is no such invariant, then I agree it'd be a little safer.
(In reply to Shu-yu Guo [:shu] from comment #6) > The reason I did it after was that that was the old mergeCompartment code. I > don't know enough about the invariants to say if the source compartment > expects all of its things to be moved. If you know for certain there is no > such invariant, then I agree it'd be a little safer. OK I'm not sure, and I'm fine with the patch either way. I'm just a bit worried about cross-compartment script -> object edges causing problems elsewhere.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: