Closed
Bug 1344334
Opened 8 years ago
Closed 8 years ago
Assertion failure: nslots > 0, at js/src/gc/Nursery.cpp:430 with OOM
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox51 | --- | unaffected |
firefox52 | --- | unaffected |
firefox-esr52 | --- | unaffected |
firefox53 | --- | unaffected |
firefox54 | + | fixed |
firefox55 | + | fixed |
People
(Reporter: decoder, Assigned: jandem)
References
(Blocks 1 open bug)
Details
(Keywords: assertion, bugmon, testcase, Whiteboard: [jsbugmon:update,bisect])
Attachments
(1 file)
(deleted),
patch
|
h4writer
:
review+
gchang
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 9732cd019a8b (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --baseline-eager):
var lfCodeBuffer = `
class TestClass {
constructor() {}
}
for (var fun of hasPrototype) {}
`;
if (lfCodeBuffer)
loadFile(lfCodeBuffer);
function loadFile(lfVarx) {
lfVarx + "x";
lfVarx.indexOf("y") === 0;
oomTest(new Function(lfVarx));
}
Backtrace:
received signal SIGSEGV, Segmentation fault.
0x0000000000dd0510 in js::Nursery::setSlotsForwardingPointer (nslots=<optimized out>, newSlots=<optimized out>, oldSlots=<optimized out>, this=<optimized out>) at js/src/gc/Nursery.cpp:430
#0 0x0000000000dd0510 in js::Nursery::setSlotsForwardingPointer (nslots=<optimized out>, newSlots=<optimized out>, oldSlots=<optimized out>, this=<optimized out>) at js/src/gc/Nursery.cpp:430
#1 js::TenuringTracer::moveSlotsToTenured (this=0x7fffffffd0b0, dst=0x7ffff468d600, src=0x7ffff47004a0, dstKind=<optimized out>) at js/src/gc/Marking.cpp:2981
#2 0x0000000000dd0c2b in js::TenuringTracer::moveObjectToTenured (this=this@entry=0x7fffffffd0b0, dst=dst@entry=0x7ffff468d600, src=src@entry=0x7ffff47004a0, dstKind=dstKind@entry=js::gc::AllocKind::OBJECT0_BACKGROUND) at js/src/gc/Marking.cpp:2925
#3 0x0000000000dd1491 in js::TenuringTracer::moveToTenured (this=this@entry=0x7fffffffd0b0, src=0x7ffff47004a0) at js/src/gc/Marking.cpp:2793
#4 0x0000000000dd16e7 in js::TenuringTracer::traverse<JSObject> (this=0x7fffffffd0b0, objp=0x7fffffffcee0) at js/src/gc/Marking.cpp:2612
#5 0x0000000000deb35c in js::TenuringTraversalFunctor<JS::Value>::operator()<JSObject> (this=<synthetic pointer>, trc=0x7fffffffd0b0, t=0x7ffff47004a0) at js/src/gc/Marking.cpp:2618
#6 js::DispatchTyped<js::TenuringTraversalFunctor<JS::Value>, js::TenuringTracer*>(js::TenuringTraversalFunctor<JS::Value>, JS::Value const&, js::TenuringTracer*&&) (f=..., val=...) at dist/include/js/Value.h:1440
#7 0x0000000000dd2951 in js::TenuringTracer::traverse<JS::Value> (thingp=0x7ffff694d040, this=0x7fffffffd0b0) at js/src/gc/Marking.cpp:2627
#8 js::TenuringTracer::traceSlots (end=<optimized out>, vp=0x7ffff694d040, this=0x7fffffffd0b0) at js/src/gc/Marking.cpp:2869
#9 js::TenuringTracer::traceObjectSlots (this=this@entry=0x7fffffffd0b0, nobj=nobj@entry=0x7ffff46b3830, start=start@entry=0, length=<optimized out>) at js/src/gc/Marking.cpp:2862
#10 0x0000000000dd2d9c in js::TenuringTracer::traceObject (this=this@entry=0x7fffffffd0b0, obj=obj@entry=0x7ffff46b3830) at js/src/gc/Marking.cpp:2848
#11 0x0000000000dd3271 in js::Nursery::collectToFixedPoint (this=this@entry=0x7ffff6961e18, mover=..., tenureCounts=...) at js/src/gc/Marking.cpp:2809
#12 0x0000000000dd3656 in js::Nursery::doCollection (this=this@entry=0x7ffff6961e18, reason=reason@entry=JS::gcreason::DESTROY_RUNTIME, tenureCounts=...) at js/src/gc/Nursery.cpp:724
#13 0x0000000000dd61a6 in js::Nursery::collect (this=0x7ffff6961e18, reason=reason@entry=JS::gcreason::DESTROY_RUNTIME) at js/src/gc/Nursery.cpp:590
#14 0x000000000097f45e in js::gc::GCRuntime::minorGC (this=0x7ffff695e620, reason=JS::gcreason::DESTROY_RUNTIME, phase=<optimized out>) at js/src/jsgc.cpp:6655
#15 0x00000000009a78d9 in js::gc::GCRuntime::evictNursery (reason=JS::gcreason::DESTROY_RUNTIME, this=<optimized out>) at js/src/gc/GCRuntime.h:1352
#16 js::EvictAllNurseries (reason=JS::gcreason::DESTROY_RUNTIME, rt=<optimized out>) at js/src/gc/Nursery-inl.h:81
#17 js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffff695e620, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DESTROY_RUNTIME) at js/src/jsgc.cpp:6232
#18 0x00000000009a8338 in js::gc::GCRuntime::collect (this=this@entry=0x7ffff695e620, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DESTROY_RUNTIME) at js/src/jsgc.cpp:6430
#19 0x00000000009a85b9 in js::gc::GCRuntime::gc (this=this@entry=0x7ffff695e620, gckind=gckind@entry=GC_NORMAL, reason=reason@entry=JS::gcreason::DESTROY_RUNTIME) at js/src/jsgc.cpp:6496
#20 0x0000000000b8bb45 in JSRuntime::destroyRuntime (this=this@entry=0x7ffff695e000) at js/src/vm/Runtime.cpp:308
#21 0x0000000000958fbc in js::DestroyContext (cx=0x7ffff6948000) at js/src/jscntxt.cpp:226
#22 0x00000000004389fc in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:8455
rax 0x0 0
rbx 0x7ffff47004a0 140737294369952
rcx 0x7ffff6c28a2d 140737333332525
rdx 0x0 0
rsi 0x7ffff6ef7770 140737336276848
rdi 0x7ffff6ef6540 140737336272192
rbp 0x7fffffffcd50 140737488342352
rsp 0x7fffffffcce0 140737488342240
r8 0x7ffff6ef7770 140737336276848
r9 0x7ffff7fe4740 140737354024768
r10 0x58 88
r11 0x7ffff6b9f750 140737332770640
r12 0x0 0
r13 0x7fffffffd0b0 140737488343216
r14 0x7ffff468d600 140737293899264
r15 0x7ffff6997000 140737330638848
rip 0xdd0510 <js::TenuringTracer::moveSlotsToTenured(js::NativeObject*, js::NativeObject*, js::gc::AllocKind)+640>
=> 0xdd0510 <js::TenuringTracer::moveSlotsToTenured(js::NativeObject*, js::NativeObject*, js::gc::AllocKind)+640>: movl $0x0,0x0
0xdd051b <js::TenuringTracer::moveSlotsToTenured(js::NativeObject*, js::NativeObject*, js::gc::AllocKind)+651>: ud2
Comment 1•8 years ago
|
||
I bisected this to:
changeset: 344947:712e84866cf5
user: Tooru Fujisawa <arai_a@mac.com>
date: Sun Feb 26 14:02:37 2017 +0900
summary: Bug 1342553 - Part 0.2: Support JSOP_CHECKISCALLABLE in JIT. r=shu
That seems plausible as the test case uses 'for of' and requires --baseline-eager to reproduce.
arai, any thoughts about what might have caused this?
Flags: needinfo?(arai.unmht)
Comment 2•8 years ago
|
||
apparently it's not from jitting that specific opcode, but enabling JIT for entire code that was blocked by the unsupported opcode at the point of part 0.1.
so, the entire structure of new FOR_OF (maybe the stack layout) seems to be problematic there.
I'll investigate it.
Comment 3•8 years ago
|
||
the crash disappears by commenting out emitBeginCodeNeedingIteratorClose/emitEndCodeNeedingIteratorClose.
so, the try-catch there could also be related.
Comment 4•8 years ago
|
||
the new stack layout seems to be unrelated.
adding try-catch to before-Bug 1342553 code causes the same issue
Comment 5•8 years ago
|
||
the underlying issue is pre-existing.
the crash happens with the following code, with parent revision of Bug 1342553.
var lfCodeBuffer = `
class TestClass {
constructor() {}
}
for (var fun of hasPrototype) {
try {} catch (e) {}
}
`;
if (lfCodeBuffer)
loadFile(lfCodeBuffer);
function loadFile(lfVarx) {
lfVarx + "x";
lfVarx.indexOf("y") === 0;
oomTest(new Function(lfVarx));
}
Flags: needinfo?(arai.unmht)
Comment 6•8 years ago
|
||
to be clear, Bug 1342553 patch adds implicit try-catch to for-of head and body, and that causes the issue with the testcase in comment #0,
and that's the same situation as the testcase in comment #5, without the patch for Bug 1342553
Comment 7•8 years ago
|
||
(In reply to Tooru Fujisawa [:arai] from comment #6)
Ah right, thanks for investigating.
Comment 8•8 years ago
|
||
bisected to this, with js shell debug binary on inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2caf5127bf42f6a1d1cee7a58b19ea8e234e5e2
Updated•8 years ago
|
Flags: needinfo?(jdemooij)
Assignee | ||
Comment 9•8 years ago
|
||
The AddSlot stub assumes DoTypeUpdateFallback is infallible, but it can currently return false if we failed to attach a stub. We should just ignore these OOMs so the IC code doesn't have to worry about this edge case.
I tried to come up with a better test case but it's not easy.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8844432 -
Flags: review?(hv1989)
Assignee | ||
Updated•8 years ago
|
status-firefox51:
--- → unaffected
status-firefox52:
--- → unaffected
status-firefox53:
--- → unaffected
status-firefox55:
--- → affected
tracking-firefox54:
--- → ?
tracking-firefox55:
--- → ?
Priority: -- → P1
Comment 10•8 years ago
|
||
Comment on attachment 8844432 [details] [diff] [review]
Patch
Review of attachment 8844432 [details] [diff] [review]:
-----------------------------------------------------------------
Good find!
Attachment #8844432 -
Flags: review?(hv1989) → review+
Updated•8 years ago
|
status-firefox-esr52:
--- → unaffected
Comment 11•8 years ago
|
||
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e6fc030c70b
Make DoTypeUpdateFallback infallible. r=h4writer
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8844432 [details] [diff] [review]
Patch
Approval Request Comment
[Feature/Bug causing the regression]: Bug 1091978.
[User impact if declined]: Crashes on OOM.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: No.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Very low risk.
[Why is the change risky/not risky?]: Only affects an OOM case that's unlikely to show up while browsing + the fix is simple.
[String changes made/needed]: None.
Attachment #8844432 -
Flags: approval-mozilla-aurora?
Comment 13•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 15•8 years ago
|
||
Comment on attachment 8844432 [details] [diff] [review]
Patch
Fix an assertion failure. Aurora54+.
Attachment #8844432 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 16•8 years ago
|
||
bugherder uplift |
You need to log in
before you can comment on or make changes to this bug.
Description
•