+++ This bug was initially created as a clone of Bug #1052573 +++ We would like to have a separate heap for strings, buffers, and other data which can be controlled by content. This will separate it from "normal" C++ allocations and make a variety of heap-spray attacks much more difficult. I think that at a minimum, we just need a single data heap which is separate from the main malloc heap. But if it's possible to have an arbitrary number of heaps by creating a heap object and pointing to it, that might provide more flexibility later. ----- Bug 1052573 adds the API. This is about actually making it do something.

Comment on attachment 8911014 [details] Bug 1402174 - Move AlignedAllocator around, so that calloc, realloc and free and grouped with malloc and memalign. https://reviewboard.mozilla.org/r/182478/#review187810

Comment on attachment 8911015 [details] Bug 1402174 - Merge imalloc and icalloc into a single function. https://reviewboard.mozilla.org/r/182480/#review187812 ::: memory/build/mozjemalloc.cpp:3345 (Diff revision 1) > - if (size <= arena_maxclass) > - return choose_arena(size)->Malloc(size, false); > - else > - return (huge_malloc(size, false)); > -} > + } > - > + return huge_malloc(aSize, aZero); I'd use ?: here. ::: memory/build/mozjemalloc.cpp:4684 (Diff revision 1) > > if (aSize == 0) { > aSize = 1; > } > > - ret = imalloc(aSize); > + ret = imalloc(aSize, false); `/* zero = */ false` would be nice. ::: memory/build/mozjemalloc.cpp:4742 (Diff revision 1) > /* size_t overflow. */ > ret = nullptr; > goto RETURN; > } > > - ret = icalloc(num_size); > + ret = imalloc(num_size, true); ditto ::: memory/build/mozjemalloc.cpp:4773 (Diff revision 1) > } > } else { > if (malloc_init()) { > ret = nullptr; > } else { > - ret = imalloc(aSize); > + ret = imalloc(aSize, false); ditto

Comment on attachment 8911016 [details] Bug 1402174 - Add an arena argument to imalloc, ipalloc and iralloc. https://reviewboard.mozilla.org/r/182482/#review187814 ::: memory/build/mozjemalloc.cpp:3963 (Diff revision 1) > > - /* Try to avoid moving the allocation. */ > + /* Try to avoid moving the allocation. */ > - if (size < small_min) { > - if (oldsize < small_min && > - ffs((int)(pow2_ceil(size) >> (TINY_MIN_2POW + 1))) > - == ffs((int)(pow2_ceil(oldsize) >> (TINY_MIN_2POW + 1)))) > + if (aSize < small_min) { > + if (aOldSize < small_min && > + ffs((int)(pow2_ceil(aSize) >> (TINY_MIN_2POW + 1))) > + == ffs((int)(pow2_ceil(aOldSize) >> (TINY_MIN_2POW + 1)))) { == on previous line ::: memory/build/mozjemalloc.cpp:3969 (Diff revision 1) > - goto IN_PLACE; /* Same size class. */ > + goto IN_PLACE; /* Same size class. */ > - } else if (size <= small_max) { > - if (oldsize >= small_min && oldsize <= small_max && > - (QUANTUM_CEILING(size) >> opt_quantum_2pow) > - == (QUANTUM_CEILING(oldsize) >> opt_quantum_2pow)) > + } > + } else if (aSize <= small_max) { > + if (aOldSize >= small_min && aOldSize <= small_max && > + (QUANTUM_CEILING(aSize) >> opt_quantum_2pow) > + == (QUANTUM_CEILING(aOldSize) >> opt_quantum_2pow)) { ditto ::: memory/build/mozjemalloc.cpp:4029 (Diff revision 1) > - oldsize = isalloc(ptr); > + oldsize = isalloc(aPtr); > > - if (size <= arena_maxclass) > - return (arena_ralloc(ptr, size, oldsize)); > - else > - return (huge_ralloc(ptr, size, oldsize)); > + if (aSize <= arena_maxclass) { > + return arena_ralloc(aPtr, aSize, oldsize, aArena); > + } > + return huge_ralloc(aPtr, aSize, oldsize); I'd use ?: here too.

Comment on attachment 8911017 [details] Bug 1402174 - Add a helper class implementing the base allocator functions for a given arena. https://reviewboard.mozilla.org/r/182484/#review187816 ::: memory/build/mozjemalloc.cpp:4693 (Diff revision 1) > + > +#define MALLOC_FUNCS MALLOC_FUNCS_MALLOC_BASE > +#include "malloc_decls.h" > + > + BaseAllocator(arena_t* aArena) > + : mArena(aArena) Indent the ':' by 2 spaces. Or put the whole function on a single line.

Comment on attachment 8911018 [details] Bug 1402174 - Initial actual implementation of the moz_arena_* API. https://reviewboard.mozilla.org/r/182486/#review187818 The TODOs in this patch are disconcerting. I trust you will address them soon. ::: commit-message-a5185:8 (Diff revision 2) > +Things left for followups: > +- Full cleanup of disposed arenas: bug 1364359. > +- Random arena Ids: bug #######. > +- Forcing the arena to match on realloc and free: bug #######. > +- Make it impossible to use arenas not created with moz_create_arena > + with moz_arena_* functions: bug #######. Please fill these bug numbers in before landing!

(In reply to Nicholas Nethercote [:njn] from comment #8) > Comment on attachment 8911015 [details] > Bug 1402174 - Merge imalloc and icalloc into a single function. > > https://reviewboard.mozilla.org/r/182480/#review187812 > > ::: memory/build/mozjemalloc.cpp:3345 > (Diff revision 1) > > - if (size <= arena_maxclass) > > - return choose_arena(size)->Malloc(size, false); > > - else > > - return (huge_malloc(size, false)); > > -} > > + } > > - > > + return huge_malloc(aSize, aZero); > > I'd use ?: here. Note this part changes in the next patch, making it a bad choice to do this in this one.

Comment on attachment 8911018 [details] Bug 1402174 - Initial actual implementation of the moz_arena_* API. https://reviewboard.mozilla.org/r/182486/#review187818 The goal here is to have something out the door early in the cycle. The followups are why the code is NIGHTLY_BUILD only for now, but I do intend to address everything before the end of this cycle.

Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/14dac365b5f6 Move AlignedAllocator around, so that calloc, realloc and free and grouped with malloc and memalign. r=njn https://hg.mozilla.org/integration/autoland/rev/5bba0584a7d8 Merge imalloc and icalloc into a single function. r=njn https://hg.mozilla.org/integration/autoland/rev/e356ac32e297 Add an arena argument to imalloc, ipalloc and iralloc. r=njn https://hg.mozilla.org/integration/autoland/rev/c589ad71d9ca Add a helper class implementing the base allocator functions for a given arena. r=njn https://hg.mozilla.org/integration/autoland/rev/b23c870c7e45 Initial actual implementation of the moz_arena_* API. r=njn

Backed out for frequently asserting rbp_i_cmp > 0, at mozjemalloc.cpp:2381: https://hg.mozilla.org/integration/autoland/rev/c298db9e14fb60576b38398db72b4f77ed2a5307 https://hg.mozilla.org/integration/autoland/rev/59fe562e84ff70658a0627866240923ae990fd96 https://hg.mozilla.org/integration/autoland/rev/60d2aa745bb275ad705f98e667fe450d8b0788f8 https://hg.mozilla.org/integration/autoland/rev/cc3be123aa1ff51ba5c6af8ab3484cedc3e467b1 https://hg.mozilla.org/integration/autoland/rev/1ca53af21d0d46808dd8c892ad1d54e8a0ef300f Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b23c870c7e45e3b9cc3f519eba1d9e58389a3c3d&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log example: https://treeherder.mozilla.org/logviewer.html#?job_id=132681336&repo=autoland [task 2017-09-22T13:02:06.691Z] 13:02:06 INFO - TEST-START | docshell/test/browser/browser_click_link_within_view_source.js [task 2017-09-22T13:02:07.479Z] 13:02:07 INFO - GECKO(2711) | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to /tmp/tmpRs80hH.mozrunner/runtests_leaks_tab_pid2922.log [task 2017-09-22T13:02:07.762Z] 13:02:07 INFO - GECKO(2711) | --DOCSHELL 0xe5a09c00 == 0 [pid = 2895] [id = {ade3fc54-7d47-4f25-b6df-584fda505e9b}] [task 2017-09-22T13:02:09.634Z] 13:02:09 INFO - GECKO(2711) | --DOMWINDOW == 3 (0xda702800) [pid = 2895] [serial = 1] [outer = (nil)] [url = file:///builds/worker/workspace/build/tests/mochitest/browser/docshell/test/browser/test-form_sjis.html] [task 2017-09-22T13:02:09.654Z] 13:02:09 INFO - GECKO(2711) | [Child 2895, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 403 [task 2017-09-22T13:02:09.657Z] 13:02:09 INFO - GECKO(2711) | [Child 2895, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 403 [task 2017-09-22T13:02:09.659Z] 13:02:09 INFO - GECKO(2711) | [Child 2895, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 403 [task 2017-09-22T13:02:09.662Z] 13:02:09 INFO - GECKO(2711) | [Child 2895, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 403 [task 2017-09-22T13:02:09.663Z] 13:02:09 INFO - GECKO(2711) | [Child 2895, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 403 [task 2017-09-22T13:02:09.664Z] 13:02:09 INFO - GECKO(2711) | [Child 2895, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 403 [task 2017-09-22T13:02:09.670Z] 13:02:09 INFO - GECKO(2711) | [Child 2895, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 403 [task 2017-09-22T13:02:09.675Z] 13:02:09 INFO - GECKO(2711) | [Child 2895, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 403 [task 2017-09-22T13:02:09.682Z] 13:02:09 INFO - GECKO(2711) | --DOMWINDOW == 2 (0xda70a400) [pid = 2895] [serial = 2] [outer = (nil)] [url = about:blank] [task 2017-09-22T13:02:09.684Z] 13:02:09 INFO - GECKO(2711) | --DOMWINDOW == 1 (0xda138000) [pid = 2895] [serial = 3] [outer = (nil)] [url = file:///builds/worker/workspace/build/tests/mochitest/browser/docshell/test/browser/test-form_sjis.html] [task 2017-09-22T13:02:09.692Z] 13:02:09 INFO - GECKO(2711) | --DOMWINDOW == 0 (0xd7b16400) [pid = 2895] [serial = 4] [outer = (nil)] [url = file:///builds/worker/workspace/build/tests/mochitest/browser/docshell/test/browser/test-form_sjis.html] [task 2017-09-22T13:02:09.919Z] 13:02:09 INFO - GECKO(2711) | nsStringStats [task 2017-09-22T13:02:09.926Z] 13:02:09 INFO - GECKO(2711) | => mAllocCount: 18618 [task 2017-09-22T13:02:09.927Z] 13:02:09 INFO - GECKO(2711) | => mReallocCount: 111 [task 2017-09-22T13:02:09.929Z] 13:02:09 INFO - GECKO(2711) | => mFreeCount: 18618 [task 2017-09-22T13:02:09.931Z] 13:02:09 INFO - GECKO(2711) | => mShareCount: 11923 [task 2017-09-22T13:02:09.937Z] 13:02:09 INFO - GECKO(2711) | => mAdoptCount: 1878 [task 2017-09-22T13:02:09.941Z] 13:02:09 INFO - GECKO(2711) | => mAdoptFreeCount: 1878 [task 2017-09-22T13:02:09.943Z] 13:02:09 INFO - GECKO(2711) | => Process ID: 2895, Thread ID: 4147058432 [task 2017-09-22T13:02:10.258Z] 13:02:10 INFO - GECKO(2711) | ++DOCSHELL 0xe5b09c00 == 1 [pid = 2922] [id = {e6b7dbbc-e5c2-4eeb-a1ed-14fa5ade012f}] [task 2017-09-22T13:02:10.455Z] 13:02:10 INFO - GECKO(2711) | ++DOMWINDOW == 1 (0xda802800) [pid = 2922] [serial = 1] [outer = (nil)] [task 2017-09-22T13:02:10.598Z] 13:02:10 INFO - GECKO(2711) | --DOMWINDOW == 2 (0xdaf0b800) [pid = 2798] [serial = 37] [outer = (nil)] [url = about:blank] [task 2017-09-22T13:02:10.659Z] 13:02:10 INFO - GECKO(2711) | ++DOMWINDOW == 2 (0xda80a400) [pid = 2922] [serial = 2] [outer = 0xda802800] [task 2017-09-22T13:02:12.052Z] 13:02:12 INFO - GECKO(2711) | --DOMWINDOW == 10 (0xe4484000) [pid = 2711] [serial = 29] [outer = (nil)] [url = about:blank] [task 2017-09-22T13:02:12.292Z] 13:02:12 INFO - GECKO(2711) | --DOMWINDOW == 0 (0xd979a000) [pid = 2821] [serial = 51] [outer = (nil)] [url = http://mochi.test:8888/browser/docshell/test/browser/file_bug852909.pdf] [task 2017-09-22T13:02:12.311Z] 13:02:12 INFO - GECKO(2711) | ++DOMWINDOW == 3 (0xda23c400) [pid = 2922] [serial = 3] [outer = 0xda802800] [task 2017-09-22T13:02:12.494Z] 13:02:12 INFO - GECKO(2711) | [Child 2922, Main Thread] WARNING: stylo: Web Components not supported yet: file /builds/worker/workspace/build/src/dom/base/nsDocument.cpp, line 6391 [task 2017-09-22T13:02:12.498Z] 13:02:12 INFO - GECKO(2711) | [Child 2922, Main Thread] WARNING: stylo: Web Components not supported yet: file /builds/worker/workspace/build/src/dom/base/nsDocument.cpp, line 6391 [task 2017-09-22T13:02:12.532Z] 13:02:12 INFO - GECKO(2711) | Assertion failure: rbp_i_cmp > 0, at /builds/worker/workspace/build/src/memory/build/mozjemalloc.cpp:2381 [task 2017-09-22T13:02:14.776Z] 13:02:14 INFO - GECKO(2711) | --DOCSHELL 0xd7f60000 == 0 [pid = 2845] [id = {dc2f52fa-1636-4001-88c8-1370ccc24046}] [task 2017-09-22T13:02:15.300Z] 13:02:15 INFO - GECKO(2711) | --DOMWINDOW == 1 (0xd7f61000) [pid = 2845] [serial = 140] [outer = (nil)] [url = about:blank] [task 2017-09-22T13:02:16.209Z] 13:02:16 INFO - GECKO(2711) | ###!!! [Parent][MessageChannel] Error: (msgtype=0x150081,name=PBrowser::Msg_Destroy) Channel error: cannot send/recv [task 2017-09-22T13:02:16.347Z] 13:02:16 INFO - GECKO(2711) | ++DOCSHELL 0xcb7c4c00 == 5 [pid = 2711] [id = {8d72137a-cd92-4523-96a1-90d7a8faaf8a}] [task 2017-09-22T13:02:16.350Z] 13:02:16 INFO - GECKO(2711) | ++DOMWINDOW == 11 (0xcb7c7c00) [pid = 2711] [serial = 47] [outer = (nil)] [task 2017-09-22T13:02:16.445Z] 13:02:16 INFO - GECKO(2711) | ++DOMWINDOW == 12 (0xcc0cf400) [pid = 2711] [serial = 48] [outer = 0xcb7c7c00] [task 2017-09-22T13:02:16.745Z] 13:02:16 INFO - GECKO(2711) | ++DOMWINDOW == 13 (0xcd533c00) [pid = 2711] [serial = 49] [outer = 0xcb7c7c00] [task 2017-09-22T13:02:16.828Z] 13:02:16 INFO - GECKO(2711) | [Parent 2711, Gecko_IOThread] WARNING: waitpid failed pid:2922 errno:10: file /builds/worker/workspace/build/src/ipc/chromium/src/base/process_util_posix.cc, line 276 [task 2017-09-22T13:02:19.261Z] 13:02:19 INFO - GECKO(2711) | --DOMWINDOW == 0 (0xd8679c00) [pid = 2845] [serial = 141] [outer = (nil)] [url = about:blank] [task 2017-09-22T13:03:07.493Z] 13:03:07 INFO - GECKO(2711) | --DOMWINDOW == 12 (0xd7315000) [pid = 2711] [serial = 37] [outer = (nil)] [url = chrome://browser/content/browser.xul] [task 2017-09-22T13:03:15.597Z] 13:03:15 INFO - GECKO(2711) | --DOMWINDOW == 11 (0xcc0cf400) [pid = 2711] [serial = 48] [outer = (nil)] [url = about:blank] [task 2017-09-22T13:03:15.599Z] 13:03:15 INFO - GECKO(2711) | --DOMWINDOW == 10 (0xd7318c00) [pid = 2711] [serial = 38] [outer = (nil)] [url = about:blank] [task 2017-09-22T13:03:37.769Z] 13:03:37 INFO - TEST-INFO | started process screentopng [task 2017-09-22T13:03:40.655Z] 13:03:40 INFO - TEST-INFO | screentopng: exit 0 [task 2017-09-22T13:03:40.660Z] 13:03:40 INFO - Buffered messages logged at 13:02:06 [task 2017-09-22T13:03:40.663Z] 13:03:40 INFO - Entering test bound test_click_link_within_view_source [task 2017-09-22T13:03:40.666Z] 13:03:40 INFO - Buffered messages logged at 13:02:12 [task 2017-09-22T13:03:40.668Z] 13:03:40 INFO - Console message: [JavaScript Warning: "Use of nsIFile in content process is deprecated." {file: "resource://gre/modules/FileUtils.jsm" line: 174}] [task 2017-09-22T13:03:40.677Z] 13:03:40 INFO - Buffered messages logged at 13:02:16 [task 2017-09-22T13:03:40.680Z] 13:03:40 INFO - Console message: [JavaScript Error: "remote browser crashed while on file:///builds/worker/workspace/build/tests/mochitest/browser/docshell/test/browser/file_click_link_within_view_source.html [task 2017-09-22T13:03:40.682Z] 13:03:40 INFO - " {file: "chrome://mochikit/content/mochitest-e10s-utils.js" line: 8}] [task 2017-09-22T13:03:40.684Z] 13:03:40 INFO - e10s_init/<@chrome://mochikit/content/mochitest-e10s-utils.js:8:5 [task 2017-09-22T13:03:40.688Z] 13:03:40 INFO - EventListener.handleEvent*EventTargetInterposition.methods.addEventListener@resource://gre/modules/RemoteAddonsParent.jsm:668:5 [task 2017-09-22T13:03:40.701Z] 13:03:40 INFO - interposeProperty/desc.value@jar:file:///builds/worker/workspace/build/application/firefox/omni.ja!/components/multiprocessShims.js:157:52 [task 2017-09-22T13:03:40.706Z] 13:03:40 INFO - e10s_init@chrome://mochikit/content/mochitest-e10s-utils.js:6:3 [task 2017-09-22T13:03:40.712Z] 13:03:40 INFO - testInit@chrome://mochikit/content/browser-test.js:101:5 [task 2017-09-22T13:03:40.714Z] 13:03:40 INFO - setTimeout handler*@chrome://mochikit/content/browser-test.js:25:3 [task 2017-09-22T13:03:40.715Z] 13:03:40 INFO - [task 2017-09-22T13:03:40.721Z] 13:03:40 INFO - Buffered messages finished [task 2017-09-22T13:03:40.729Z] 13:03:40 INFO - TEST-UNEXPECTED-FAIL | docshell/test/browser/browser_click_link_within_view_source.js | Test timed out -

Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/2cee088f5e73 Move AlignedAllocator around, so that calloc, realloc and free and grouped with malloc and memalign. r=njn https://hg.mozilla.org/integration/autoland/rev/f1994fc758be Merge imalloc and icalloc into a single function. r=njn https://hg.mozilla.org/integration/autoland/rev/ce9d9f4124d6 Add an arena argument to imalloc, ipalloc and iralloc. r=njn https://hg.mozilla.org/integration/autoland/rev/6c8cc804f12d Add a helper class implementing the base allocator functions for a given arena. r=njn https://hg.mozilla.org/integration/autoland/rev/9966b8c1720c Initial actual implementation of the moz_arena_* API. r=njn

https://hg.mozilla.org/mozilla-central/rev/2cee088f5e73 https://hg.mozilla.org/mozilla-central/rev/f1994fc758be https://hg.mozilla.org/mozilla-central/rev/ce9d9f4124d6 https://hg.mozilla.org/mozilla-central/rev/6c8cc804f12d https://hg.mozilla.org/mozilla-central/rev/9966b8c1720c