Closed
Bug 1472638
Opened 6 years ago
Closed 6 years ago
Crash [@ __memset_sse2] or Hit MOZ_CRASH(Tried to access a protected region!) with OOM
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox-esr60 | --- | unaffected |
firefox61 | --- | unaffected |
firefox62 | --- | fixed |
firefox63 | --- | fixed |
People
(Reporter: decoder, Assigned: nbp)
References
(Blocks 1 open bug)
Details
(5 keywords, Whiteboard: [jsbugmon:update])
Crash Data
Attachments
(1 file)
(deleted),
patch
|
tcampbell
:
review+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 23885c14f025 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe):
oomTest(function() { eval(`
var BIG_INDEX = 4294967290;
var arr = Array(BIG_INDEX);
arr[BIG_INDEX - 1] = 'a';
arr.length = BIG_INDEX - 5000;
status = inSection(1);
`); });
Backtrace:
received signal SIGSEGV, Segmentation fault.
__memset_sse2 () at ../sysdeps/x86_64/multiarch/../memset.S:65
#0 __memset_sse2 () at ../sysdeps/x86_64/multiarch/../memset.S:65
#1 0x0000000000536f7b in memset (__len=40, __ch=206, __dest=<optimized out>) at /usr/include/x86_64-linux-gnu/bits/string3.h:90
#2 js::detail::BumpChunk::setBump (this=this@entry=0x7fffec3e2000, newBump=newBump@entry=0x7fffec3e3018 '\315' <repeats 199 times>, <incomplete sequence \315>...) at js/src/ds/LifoAlloc.h:335
#3 0x00000000005591d9 in js::detail::BumpChunk::tryAlloc (n=40, this=0x7fffec3e2000) at js/src/ds/LifoAlloc.h:461
#4 js::LifoAlloc::allocImpl (n=40, this=0x7ffff49fd578) at js/src/ds/LifoAlloc.h:590
#5 js::LifoAlloc::alloc (this=0x7ffff49fd578, n=40) at js/src/ds/LifoAlloc.h:663
#6 0x0000000000d3a6ed in js::LifoAlloc::new_<js::ObjectGroup::Property, jsid&> (this=<optimized out>) at js/src/ds/LifoAlloc.h:887
#7 js::ObjectGroup::getProperty (this=this@entry=0x7ffff2b60a90, sweep=..., cx=cx@entry=0x7ffff5f17000, obj=obj@entry=0x7ffff4d36a30, id=id@entry=...) at js/src/vm/TypeInference-inl.h:1142
#8 0x0000000000d04fd3 in js::ObjectGroup::markPropertyNonData (this=0x7ffff2b60a90, cx=cx@entry=0x7ffff5f17000, obj=obj@entry=0x7ffff4d36a30, id=..., id@entry=...) at js/src/vm/TypeInference.cpp:2885
#9 0x00000000004db4ef in js::MarkTypePropertyNonData (id=..., obj=0x7ffff4d36a30, cx=0x7ffff5f17000) at js/src/vm/TypeInference-inl.h:471
#10 js::DeleteProperty (result=..., id=..., obj=..., cx=0x7ffff5f17000) at js/src/vm/JSObject-inl.h:256
#11 js::DeleteElement (cx=0x7ffff5f17000, obj=..., obj@entry=..., index=index@entry=4294967237, result=...) at js/src/vm/JSObject-inl.h:268
#12 0x00000000004c812e in js::ArraySetLength (cx=<optimized out>, cx@entry=0x7ffff5f17000, arr=arr@entry=..., id=..., id@entry=..., attrs=attrs@entry=4, value=..., value@entry=..., result=...) at js/src/builtin/Array.cpp:834
#13 0x00000000004c906b in array_length_setter (cx=cx@entry=0x7ffff5f17000, obj=..., id=..., v=..., result=...) at js/src/builtin/Array.cpp:645
#14 0x0000000000c168c9 in js::CallJSSetterOp (result=..., v=..., id=..., obj=..., op=<optimized out>, cx=0x7ffff5f17000) at js/src/vm/JSContext-inl.h:301
#15 NativeSetExistingDataProperty (result=..., v=..., shape=..., obj=..., cx=0x7ffff5f17000) at js/src/vm/NativeObject.cpp:2535
#16 SetExistingProperty (cx=<optimized out>, id=..., id@entry=..., v=v@entry=..., receiver=..., receiver@entry=..., pobj=..., pobj@entry=..., prop=..., prop@entry=..., result=...) at js/src/vm/NativeObject.cpp:2745
#17 0x0000000000c3a5d5 in js::NativeSetProperty<(js::QualifiedBool)1> (cx=<optimized out>, cx@entry=0x7ffff5f17000, obj=..., id=id@entry=..., v=..., v@entry=..., receiver=..., receiver@entry=..., result=...) at js/src/vm/NativeObject.cpp:2788
#18 0x00000000005b786c in js::SetProperty (cx=0x7ffff5f17000, obj=..., id=..., v=..., receiver=..., result=...) at js/src/vm/NativeObject.h:1705
#19 0x00000000005a345e in SetPropertyOperation (rval=..., id=..., lval=..., op=<optimized out>, cx=0x7ffff5f17000) at js/src/vm/Interpreter.cpp:268
#20 Interpret (cx=0x7ffff5f17000, state=...) at js/src/vm/Interpreter.cpp:2990
#21 0x00000000005ae5f6 in js::RunScript (cx=0x7ffff5f17000, state=...) at js/src/vm/Interpreter.cpp:423
#22 0x00000000005b195d in js::ExecuteKernel (cx=<optimized out>, cx@entry=0x7ffff5f17000, script=..., script@entry=..., envChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=<optimized out>) at js/src/vm/Interpreter.cpp:771
#23 0x00000000005ea3b0 in EvalKernel (cx=0x7ffff5f17000, v=..., v@entry=..., evalType=evalType@entry=DIRECT_EVAL, caller=..., env=env@entry=..., pc=<optimized out>, vp=...) at js/src/builtin/Eval.cpp:319
#24 0x00000000005eabbe in js::DirectEval (cx=<optimized out>, v=..., vp=...) at js/src/builtin/Eval.cpp:427
#25 0x000000000069a683 in js::jit::DoCallFallback (cx=<optimized out>, frame=0x7fffffffbf28, stub_=<optimized out>, argc=<optimized out>, vp=0x7fffffffbed8, res=...) at js/src/jit/BaselineIC.cpp:2641
#26 0x00002ade6844028c in ?? ()
#27 0x00007fffffffbea0 in ?? ()
[...]
#58 0x0000000000000000 in ?? ()
rax 0x7fffec3e2ff0 140737156886512
rbx 0x7fffec3e2000 140737156882432
rcx 0x7fffec3e2ff0 140737156886512
rdx 0x28 40
rsi 0xce 206
rdi 0x7fffec3e2ff0 140737156886512
rbp 0x7fffffffa240 140737488331328
rsp 0x7fffffffa158 140737488331096
r8 0x5dee488d 1575897229
r9 0x9e3779b8 2654435768
r10 0x0 0
r11 0x7ffff487b400 140737295922176
r12 0x0 0
r13 0x7fffec3e3018 140737156886552
r14 0x28 40
r15 0x7fffec3e3018 140737156886552
rip 0x7ffff6bc026c <__memset_sse2+44>
=> 0x7ffff6bc026c <__memset_sse2+44>: movdqu %xmm0,-0x10(%rdi,%rdx,1)
0x7ffff6bc0272 <__memset_sse2+50>: ja 0x7ffff6bc0280 <__memset_sse2+64>
The crash looks like a stack space exhaustion but outside of GDB I also get the MOZ_CRASH, so I suspect this might be taking a different control flow route depending on how much stack space is available. Marking s-s until investigated further.
Reporter | ||
Updated•6 years ago
|
Flags: needinfo?(nicolas.b.pierron)
Assignee | ||
Comment 1•6 years ago
|
||
This definitely looks like what is being reported to us by crash-stat, i-e some write on the bottom of protected buffers during a LifoAlloc allocation. (see also Bug 1263794 comment 34)
Assignee: nobody → nicolas.b.pierron
Status: NEW → ASSIGNED
Flags: needinfo?(nicolas.b.pierron)
Assignee | ||
Comment 2•6 years ago
|
||
This particular problem happens on OOM under getOrCreateChunk, when we cannot allocate a new chunk. The first step of getOrCreateChunk is to flag the last chunk as reserved, but in case of OOM it will remain in the last chunk position.
In this particular case we attempt to allocate a 64k hash set, fail, and resume under ObjectGroup::markPropertyNonData as if nothing happend. Thus the last chunk is being protected while remaining the last chunk.
This is an instrumentation bug and might not be the issue we are looking for. I will make a quick fix to handle this false-positive.
Thread 1 hit Breakpoint 3, js::detail::BumpChunk::setRWUntil (this=0xb90ce20,
loc=js::detail::BumpChunk::Loc::Reserved) at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.cpp:76
76 if (!protect_)
(rr) bt
#0 js::detail::BumpChunk::setRWUntil (this=0xb90ce20, loc=js::detail::BumpChunk::Loc::Reserved)
at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.cpp:76
#1 0x000000000148c254 in js::LifoAlloc::getOrCreateChunk (this=0x2fbe8c8, n=65544)
at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.cpp:210
#2 0x00000000006120d2 in js::LifoAlloc::allocImpl (this=0x2fbe8c8, n=65544)
at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.h:593
#3 0x000000000061204d in js::LifoAlloc::alloc (this=0x2fbe8c8, n=65544)
at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.h:663
#4 0x000000000120feb8 in js::LifoAlloc::newArrayUninitialized<js::ObjectGroup::Property*> (this=0x2fbe8c8,
count=8193) at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.h:763
#5 0x000000000120fabd in js::LifoAlloc::newArray<js::ObjectGroup::Property*> (this=0x2fbe8c8, count=8193)
at /home/nicolas/mozilla/wksp-9/js/src/ds/LifoAlloc.h:753
#6 0x000000000120fced in js::TypeHashSet::InsertTry<jsid, js::ObjectGroup::Property, js::ObjectGroup::Property> (
alloc=..., values=@0x7f61a0c68f08: 0xb8fae28, count=@0x7ffd252e571c: 2048, key=...)
at /home/nicolas/mozilla/wksp-9/js/src/vm/TypeInference-inl.h:778
#7 0x0000000001201db5 in js::TypeHashSet::Insert<jsid, js::ObjectGroup::Property, js::ObjectGroup::Property> (
alloc=..., values=@0x7f61a0c68f08: 0xb8fae28, count=@0x7ffd252e571c: 2048, key=...)
at /home/nicolas/mozilla/wksp-9/js/src/vm/TypeInference-inl.h:852
#8 0x00000000011ff04a in js::ObjectGroup::getProperty (this=0x7f61a0c68ee0, sweep=..., cx=0x2f8a160,
obj=0x7f61a0e38930, id=...) at /home/nicolas/mozilla/wksp-9/js/src/vm/TypeInference-inl.h:1149
#9 0x00000000011bad48 in js::ObjectGroup::markPropertyNonData (this=0x7f61a0c68ee0, cx=0x2f8a160,
obj=0x7f61a0e38930, id=...) at /home/nicolas/mozilla/wksp-9/js/src/vm/TypeInference.cpp:2885
Updated•6 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Comment 3•6 years ago
|
||
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/44a69a4ebc09
user: Nicolas B. Pierron
date: Wed Feb 14 17:12:00 2018 +0000
summary: Bug 1437600 - Use mprotect to prevent mutations of inaccessible regions. r=luke
This iteration took 318.517 seconds to run.
Assignee | ||
Comment 4•6 years ago
|
||
The problem is that we were protecting the last chunk, even in case of some allocation failures that we recover from. Which should explain the cases reported on TI.
This patch adds a lambda to protect the last chunk. I did not used a ScopeExit because last() returns an iterator, and even if it would be safe in the current implementation, it would be unwise to alias it and use it after doing
some append() to the same container.
Attachment #8989156 -
Flags: review?(tcampbell)
Comment 5•6 years ago
|
||
Calling it sec-other because it sounds like a false-positive (comment 2). If I'm interpreting that correctly please unhide the bug and remove sec-other. Otherwise please rate appropriately
Comment 6•6 years ago
|
||
Comment on attachment 8989156 [details] [diff] [review]
Only protect the last chunk when we append a new chunk.
Review of attachment 8989156 [details] [diff] [review]:
-----------------------------------------------------------------
Seems reasonable. Other approach would be to add your anonymous function to the BumpChunk class instead, but what you have may be simpler for now.
Attachment #8989156 -
Flags: review?(tcampbell) → review+
Assignee | ||
Updated•6 years ago
|
Pushed by npierron@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b0328b70458a
Only protect the last chunk when we append a new chunk. r=tcampbell
Comment 8•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Updated•6 years ago
|
status-firefox61:
--- → unaffected
status-firefox62:
--- → disabled
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → unaffected
Assignee | ||
Comment 9•6 years ago
|
||
Comment on attachment 8989156 [details] [diff] [review]
Only protect the last chunk when we append a new chunk.
Approval Request Comment
[Feature/Bug causing the regression]: Bug 1437600
[User impact if declined]: false-positive crash reports
[Is this code covered by automated tests?]: tested by fuzzers.
[Has the fix been verified in Nightly?]: yes. (removed ~1 crash per day from nightly)
[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?]: no
[Why is the change risky/not risky?]: It removes wrong & extra memory protection from pages in case of OOM during a large allocation.
[String changes made/needed]: N/A
Attachment #8989156 -
Flags: approval-mozilla-beta?
This is marked disabled for 62 but looks like it would still affect dev edition.
Comment on attachment 8989156 [details] [diff] [review]
Only protect the last chunk when we append a new chunk.
Fix for false positive asserts, had good results in nightly, so let's uplift for beta 8.
Attachment #8989156 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 12•6 years ago
|
||
bugherder uplift |
You need to log in
before you can comment on or make changes to this bug.
Description
•