Skip to content

Check for a 32-bit overflow in sbrk, to error properly on trying to malloc >4GB #11047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
Apr 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,8 @@ commands:
CHROME_FLAGS_NOCACHE: "--disk-cache-dir=/dev/null --disk-cache-size=1 --media-cache-size=1 --disable-application-cache --incognito"
command: |
export EMTEST_BROWSER="/usr/bin/google-chrome $CHROME_FLAGS_BASE $CHROME_FLAGS_HEADLESS $CHROME_FLAGS_WASM $CHROME_FLAGS_NOCACHE"
python3 tests/runner.py browser
# skip test_zzz_zzz_4GB_fail as it OOMs on the current bot
python3 tests/runner.py browser skip:browser.test_zzz_zzz_4GB_fail
test-upstream-sockets-chrome:
description: "Runs emscripten sockets tests under chrome"
steps:
Expand Down
25 changes: 15 additions & 10 deletions system/lib/sbrk.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,16 @@
#define SET_ERRNO()
#endif

#define RETURN_ERROR() { \
SET_ERRNO() \
return (void*)-1; \
}

void *sbrk(intptr_t increment) {
uintptr_t old_size;
// Enforce preserving a minimal 4-byte alignment for sbrk.
increment = (increment + 3) & ~3;
#if __EMSCRIPTEN_PTHREADS__
// Our default dlmalloc uses locks around each malloc/free, so no additional
// work is necessary to keep things threadsafe, but we also make sure sbrk
// itself is threadsafe so alternative allocators work. We do that by looping
// and retrying if we hit interference with another thread.
intptr_t expected;
while (1) {
#endif // __EMSCRIPTEN_PTHREADS__

Expand All @@ -52,23 +49,27 @@ void *sbrk(intptr_t increment) {
intptr_t old_brk = *sbrk_ptr;
#endif
intptr_t new_brk = old_brk + increment;
// Check for a 32-bit overflow, which would indicate that we are trying to
// allocate over 4GB, which is never possible in wasm32.
if (increment > 0 && (uint32_t)new_brk <= (uint32_t)old_brk) {
goto Error;
}
#ifdef __wasm__
uintptr_t old_size = __builtin_wasm_memory_size(0) * WASM_PAGE_SIZE;
old_size = __builtin_wasm_memory_size(0) * WASM_PAGE_SIZE;
#else
uintptr_t old_size = emscripten_get_heap_size();
old_size = emscripten_get_heap_size();
#endif
if (new_brk > old_size) {
// Try to grow memory.
intptr_t diff = new_brk - old_size;
if (!emscripten_resize_heap(new_brk)) {
RETURN_ERROR();
goto Error;
}
}
#if __EMSCRIPTEN_PTHREADS__
// Attempt to update the dynamic top to new value. Another thread may have
// beat this one to the update, in which case we will need to start over
// by iterating the loop body again.
intptr_t expected = old_brk;
expected = old_brk;
__c11_atomic_compare_exchange_strong(
(_Atomic(intptr_t)*)sbrk_ptr,
&expected, new_brk,
Expand All @@ -88,6 +89,10 @@ void *sbrk(intptr_t increment) {
#if __EMSCRIPTEN_PTHREADS__
}
#endif // __EMSCRIPTEN_PTHREADS__

Error:
SET_ERRNO();
return (void*)-1;
}

int brk(intptr_t ptr) {
Expand Down
30 changes: 30 additions & 0 deletions tests/browser/test_4GB_fail.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void* allocation;

int main() {
const int MB = 1024 * 1024;
const int CHUNK = 512 * MB;
long num_allocations;
for (num_allocations = 0;; num_allocations++) {
allocation = malloc(CHUNK);
if (!allocation) {
puts("failed to allocate any more");
break;
}
printf("%ld: Allocated %d MB, total so far: %ld MB\n",
num_allocations, CHUNK / MB,
(num_allocations * CHUNK) / MB);
printf(" (writing to make sure, to %p)\n", allocation);
memset(allocation, 42, CHUNK);
}
// We should have allocated at least 2GB.
assert(num_allocations >= 4);
// We should have allocated less than 4GB (we can't get to exactly 4GB
// since we started with some small amount, and then add 512MB chunks).
assert(num_allocations < 8);
puts("success");
}
2 changes: 2 additions & 0 deletions tests/browser/test_4GB_fail.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
failed to allocate any more
success
12 changes: 6 additions & 6 deletions tests/code_size/hello_webgl2_fastcomp_asmjs.json
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
{
"a.html": 580,
"a.html.gz": 382,
"a.js": 5322,
"a.js.gz": 2505,
"a.js": 5326,
"a.js.gz": 2510,
"a.mem": 321,
"a.mem.gz": 219,
"a.asm.js": 9778,
"a.asm.js.gz": 3528,
"total": 16001,
"total_gz": 6634
"a.asm.js": 9814,
"a.asm.js.gz": 3544,
"total": 16041,
"total_gz": 6655
}
12 changes: 6 additions & 6 deletions tests/code_size/hello_webgl2_fastcomp_wasm.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.html": 563,
"a.html.gz": 377,
"a.js": 5288,
"a.js.gz": 2533,
"a.wasm": 7982,
"a.wasm.gz": 4368,
"total": 13833,
"total_gz": 7278
"a.js": 5293,
"a.js.gz": 2537,
"a.wasm": 8004,
"a.wasm.gz": 4376,
"total": 13860,
"total_gz": 7290
}
12 changes: 6 additions & 6 deletions tests/code_size/hello_webgl2_wasm.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.html": 563,
"a.html.gz": 377,
"a.js": 5069,
"a.js.gz": 2418,
"a.wasm": 10895,
"a.wasm.gz": 6927,
"total": 16527,
"total_gz": 9722
"a.js": 5073,
"a.js.gz": 2423,
"a.wasm": 10912,
"a.wasm.gz": 6940,
"total": 16548,
"total_gz": 9740
}
8 changes: 4 additions & 4 deletions tests/code_size/hello_webgl2_wasm2js.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.html": 588,
"a.html.gz": 386,
"a.js": 23664,
"a.js.gz": 8885,
"a.js": 23705,
"a.js.gz": 8899,
"a.mem": 3168,
"a.mem.gz": 2711,
"total": 27420,
"total_gz": 11982
"total": 27461,
"total_gz": 11996
}
12 changes: 6 additions & 6 deletions tests/code_size/hello_webgl_fastcomp_asmjs.json
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
{
"a.html": 580,
"a.html.gz": 382,
"a.js": 4868,
"a.js.gz": 2350,
"a.js": 4872,
"a.js.gz": 2355,
"a.mem": 321,
"a.mem.gz": 219,
"a.asm.js": 9778,
"a.asm.js.gz": 3528,
"total": 15547,
"total_gz": 6479
"a.asm.js": 9814,
"a.asm.js.gz": 3544,
"total": 15587,
"total_gz": 6500
}
12 changes: 6 additions & 6 deletions tests/code_size/hello_webgl_fastcomp_wasm.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.html": 563,
"a.html.gz": 377,
"a.js": 4776,
"a.js.gz": 2355,
"a.wasm": 7982,
"a.wasm.gz": 4368,
"total": 13321,
"total_gz": 7100
"a.js": 4779,
"a.js.gz": 2358,
"a.wasm": 8004,
"a.wasm.gz": 4376,
"total": 13346,
"total_gz": 7111
}
12 changes: 6 additions & 6 deletions tests/code_size/hello_webgl_wasm.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.html": 563,
"a.html.gz": 377,
"a.js": 4554,
"a.js.gz": 2242,
"a.wasm": 10895,
"a.wasm.gz": 6927,
"total": 16012,
"total_gz": 9546
"a.js": 4558,
"a.js.gz": 2247,
"a.wasm": 10912,
"a.wasm.gz": 6940,
"total": 16033,
"total_gz": 9564
}
8 changes: 4 additions & 4 deletions tests/code_size/hello_webgl_wasm2js.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"a.html": 588,
"a.html.gz": 386,
"a.js": 23154,
"a.js.gz": 8722,
"a.js": 23195,
"a.js.gz": 8737,
"a.mem": 3168,
"a.mem.gz": 2711,
"total": 26910,
"total_gz": 11819
"total": 26951,
"total_gz": 11834
}
8 changes: 4 additions & 4 deletions tests/code_size/random_printf_wasm.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"a.html": 13732,
"a.html.gz": 7335,
"total": 13732,
"total_gz": 7335
"a.html": 13724,
"a.html.gz": 7323,
"total": 13724,
"total_gz": 7323
}
8 changes: 4 additions & 4 deletions tests/code_size/random_printf_wasm2js.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"a.html": 19933,
"a.html.gz": 8343,
"total": 19933,
"total_gz": 8343
"a.html": 19929,
"a.html.gz": 8345,
"total": 19929,
"total_gz": 8345
}
19 changes: 17 additions & 2 deletions tests/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -5014,10 +5014,11 @@ def test_zzz_zzz_4GB(self):

# test that we can allocate in the 2-4GB range, if we enable growth and
# set the max appropriately
self.emcc_args += ['-O2', '-s', 'ALLOW_MEMORY_GROWTH', '-s', 'MAXIMUM_MEMORY=4GB', '-s', 'ASSERTIONS']
self.emcc_args += ['-O2', '-s', 'ALLOW_MEMORY_GROWTH', '-s', 'MAXIMUM_MEMORY=4GB']
self.do_run_in_out_file_test('tests', 'browser', 'test_4GB', js_engines=[V8_ENGINE])

@no_fastcomp('only upstream supports 4GB')
@no_firefox('no 4GB support yet')
def test_zzz_zzz_2GB_fail(self):
# TODO Convert to an actual browser test when it reaches stable.
# For now, keep this in browser as this suite runs serially, which
Expand All @@ -5027,5 +5028,19 @@ def test_zzz_zzz_2GB_fail(self):

# test that growth doesn't go beyond 2GB without the max being set for that,
# and that we can catch an allocation failure exception for that
self.emcc_args += ['-O2', '-fexceptions', '-s', 'ALLOW_MEMORY_GROWTH', '-s', 'MAXIMUM_MEMORY=2GB', '-s', 'ASSERTIONS']
self.emcc_args += ['-O2', '-s', 'ALLOW_MEMORY_GROWTH', '-s', 'MAXIMUM_MEMORY=2GB']
self.do_run_in_out_file_test('tests', 'browser', 'test_2GB_fail', js_engines=[V8_ENGINE])

@no_fastcomp('only upstream supports 4GB')
@no_firefox('no 4GB support yet')
def test_zzz_zzz_4GB_fail(self):
# TODO Convert to an actual browser test when it reaches stable.
# For now, keep this in browser as this suite runs serially, which
# means we don't compete for memory with anything else (and run it
# at the very very end, to reduce the risk of it OOM-killing the
# browser).

# test that we properly report an allocation error that would overflow over
# 4GB.
self.emcc_args += ['-O2', '-s', 'ALLOW_MEMORY_GROWTH', '-s', 'MAXIMUM_MEMORY=4GB', '-s', 'ABORTING_MALLOC=0']
self.do_run_in_out_file_test('tests', 'browser', 'test_4GB_fail', js_engines=[V8_ENGINE])