1620213 - Assertion failure: addr == p, at jit/ProcessExecutableMemory.cpp:428 or Assertion failure: makeWritable(), at jit/JitRealm.h:750 with OOM

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect, P2)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20200305-2e1a978b09d7 (build with --enable-debug, run with --fuzzing-safe --no-threads --ion-warmup-threshold=1 --baseline-warmup-threshold=0 --disable-oom-functions):

try {
evaluate(`
  for (var i40 = 0; i40 < 1536; {}) {
    let target  = new ArrayBuffer(1024*1024);
  }
`);
} catch(exc) {}

Backtrace:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x000055bf37c3967e in ProcessExecutableMemory::deallocate(void*, unsigned long, bool) ()
#0  0x000055bf37c3967e in ProcessExecutableMemory::deallocate(void*, unsigned long, bool) ()
#1  0x000055bf37aa5e0a in js::jit::ExecutableAllocator::releasePoolPages(js::jit::ExecutablePool*) ()
#2  0x000055bf37aa70cf in js::jit::ExecutableAllocator::purge() ()
#3  0x000055bf3777f313 in js::gc::GCRuntime::endSweepingSweepGroup(JSFreeOp*, js::SliceBudget&) ()
#4  0x000055bf377bcc41 in sweepaction::SweepActionSequence::run(js::gc::SweepAction::Args&) ()
#5  0x000055bf377acba7 in sweepaction::SweepActionForEach<js::gc::SweepGroupsIter, JSRuntime*>::run(js::gc::SweepAction::Args&) ()
#6  0x000055bf37781fc5 in js::gc::GCRuntime::performSweepActions(js::SliceBudget&) ()
#7  0x000055bf377867b9 in js::gc::GCRuntime::incrementalSlice(js::SliceBudget&, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason, js::gc::AutoGCSession&) ()
#8  0x000055bf377895dc in js::gc::GCRuntime::gcCycle(bool, js::SliceBudget, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason) ()
#9  0x000055bf3778b22e in js::gc::GCRuntime::collect(bool, js::SliceBudget, mozilla::Maybe<JSGCInvocationKind> const&, JS::GCReason) ()
#10 0x000055bf37745637 in js::gc::GCRuntime::gc(JSGCInvocationKind, JS::GCReason) ()
#11 0x000055bf371fdfb5 in JSRuntime::destroyRuntime() ()
#12 0x000055bf37104012 in js::DestroyContext(JSContext*) ()
#13 0x000055bf36c6fe72 in main ()
rax	0x55bf384acdde	94279771540958
rbx	0x10000	65536
rcx	0x55bf393fd850	94279787599952
rdx	0x0	0
rsi	0x7fd4a04c3770	140551199143792
rdi	0x7fd4a04c2540	140551199139136
rbp	0x7ffd7699d9c0	140726593247680
rsp	0x7ffd7699d990	140726593247632
r8	0x7fd4a04c3770	140551199143792
r9	0x7fd4a1566d00	140551216590080
r10	0x58	88
r11	0x7fd4a016a7a0	140551195633568
r12	0x212390cc8000	36436636893184
r13	0x212390c78000	36436636565504
r14	0x55bf393ff368	94279787606888
r15	0x1	1
rip	0x55bf37c3967e <ProcessExecutableMemory::deallocate(void*, unsigned long, bool)+718>
=> 0x55bf37c3967e <_ZN23ProcessExecutableMemory10deallocateEPvmb+718>:	movl   $0x1ac,0x0
   0x55bf37c39689 <_ZN23ProcessExecutableMemory10deallocateEPvmb+729>:	callq  0x55bf36cf805e <abort>

This bug is intermittently happening and varying in outcome between the two assertions in the summary and an "unhandlable oom" message:

[unhandlable oom] Failed to mmap, likely no more mappings available memory/build/mozjemalloc.cpp : 1297

The question here is, are we okay with these assertions firing intermittently in OOM situations? If it can be fixed easily, then it might be worth doing so, because we see this happening in fuzzing intermittently.

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Nicolas B. Pierron [:nbp]]

I am not sure what to think about this issue at first glance, the 3 failures reported here seems to all happen in different cases, and the stack trace is the unexpected as it is actually discarding memory.

Jan, can you look at this issue?

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Nicolas B. Pierron [:nbp] from comment #2)

and the stack trace is the unexpected as it is actually discarding memory.

What's happening is that there's a single memory mapping and we try to free pages in the middle, so the kernel has to split the mapping into three separate ones.

This test is allocating large ArrayBuffers in an infinite loop and the max number of mappings on Linux isn't too big:

$ sysctl vm.max_map_count
vm.max_map_count = 65530

Because this fails the same way in jemalloc, it's not JIT-specific and is more of a GC issue.

I added some logging and on my Linux system we fail after creating a bit more than 32700 ArrayBuffers. At that point we have a ton of mappings like this:

7f6f43200000-7f6f43300000 rw-p 00000000 00:00 0 
7f6f43300000-7f6f43400000 ---p 00000000 00:00 0 
7f6f43400000-7f6f43500000 rw-p 00000000 00:00 0 
7f6f43500000-7f6f43600000 ---p 00000000 00:00 0 
7f6f43600000-7f6f43700000 rw-p 00000000 00:00 0 
7f6f43700000-7f6f43800000 ---p 00000000 00:00 0 
7f6f43800000-7f6f43900000 rw-p 00000000 00:00 0 

So basically for every 1 MB chunk, jemalloc adds a guard page (its own 1 MB region because of chunk alignment), resulting in at least 32,700 * 2 = 65400 mappings - very close to the 65530 max_mmap_count.

Maybe we should be GC'ing a bit more aggressively after allocating this much memory.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1620213 - Check malloc thresholds more often r?sfink

We currently account for this memory but we don't always trigger a GC based on it.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dcd22a0c3618
Check malloc thresholds more often r=sfink

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/dcd22a0c3618

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jonco, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 8

[Jon Coppeard (:jonco)]

Comment on attachment 9132979 [details]
Bug 1620213 - Check malloc thresholds more often r?sfink

Beta/Release Uplift Approval Request

• User impact if declined: Possible OOM or crashes.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a simple change to check malloc memory pressure more often and trigger GC based on that. It won't have any effect in most cases.
• String changes made/needed: None

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9132979 [details]
Bug 1620213 - Check malloc thresholds more often r?sfink

Simple patch to check for memory pressure more often to avoid possible OOM crashes. Approved for 75.0b6.

Comment 10

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e7d6a90db3dc