Skip to content
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
2 changes: 2 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ See docs/process.md for how version tagging works.

Current Trunk
-------------
- ASAN_SHADOW_SIZE is deprecated. When using AddressSanitizer, the correct
amount of shadow memory will now be calculated automatically.

2.0.5: 09/28/2020
-----------------
Expand Down
29 changes: 20 additions & 9 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1498,12 +1498,6 @@ def filter_out_duplicate_dynamic_libs(inputs):
# so setErrNo JS library function can report errno back to C
shared.Settings.EXPORTED_FUNCTIONS += ['___errno_location']

if shared.Settings.GLOBAL_BASE < 0:
# default if nothing else sets it
# a higher global base is useful for optimizing load/store offsets, as it
# enables the --post-emscripten pass
shared.Settings.GLOBAL_BASE = 1024

if shared.Settings.SAFE_HEAP:
# SAFE_HEAP check includes calling emscripten_get_sbrk_ptr() from wasm
shared.Settings.EXPORTED_FUNCTIONS += ['_emscripten_get_sbrk_ptr']
Expand Down Expand Up @@ -1777,9 +1771,20 @@ def include_and_export(name):
'_asan_c_store_f', '_asan_c_store_d',
]

shared.Settings.GLOBAL_BASE = shared.Settings.ASAN_SHADOW_SIZE
shared.Settings.INITIAL_MEMORY += shared.Settings.ASAN_SHADOW_SIZE
assert shared.Settings.INITIAL_MEMORY < 2**32
if shared.Settings.ASAN_SHADOW_SIZE != -1:
diagnostics.warning('emcc', 'ASAN_SHADOW_SIZE is ignored and will be removed in a future release')

if shared.Settings.GLOBAL_BASE != -1:
exit_with_error("ASan does not support custom GLOBAL_BASE")

max_mem = shared.Settings.INITIAL_MEMORY
if shared.Settings.ALLOW_MEMORY_GROWTH:
max_mem = shared.Settings.MAXIMUM_MEMORY
if max_mem == -1:
exit_with_error('ASan requires a finite MAXIMUM_MEMORY')

shadow_size = max_mem // 8
shared.Settings.GLOBAL_BASE = shadow_size
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the user sets GLOBAL_BASE? Perhaps we should not allow that in general?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that makes sense.


if shared.Settings.SAFE_HEAP:
# SAFE_HEAP instruments ASan's shadow memory accesses.
Expand All @@ -1793,6 +1798,12 @@ def include_and_export(name):
if sanitize and '-g4' in args:
shared.Settings.LOAD_SOURCE_MAP = 1

if shared.Settings.GLOBAL_BASE == -1:
# default if nothing else sets it
# a higher global base is useful for optimizing load/store offsets, as it
# enables the --post-emscripten pass
shared.Settings.GLOBAL_BASE = 1024

# various settings require malloc/free support from JS
if shared.Settings.RELOCATABLE or \
shared.Settings.BUILD_AS_WORKER or \
Expand Down
15 changes: 0 additions & 15 deletions src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -551,21 +551,6 @@ LibraryManager.library = {
return false;
#endif
}
#if USE_ASAN
// One byte of ASan's shadow memory shadows 8 bytes of real memory. Shadow memory area has a fixed size,
// so do not allow resizing past that limit.
maxHeapSize = Math.min(maxHeapSize, {{{ 8 * ASAN_SHADOW_SIZE }}});
if (requestedSize > maxHeapSize) {
#if ASSERTIONS
err('Failed to grow the heap from ' + oldSize + ', as we reached the limit of our shadow memory. Increase ASAN_SHADOW_SIZE.');
#endif
#if ABORTING_MALLOC
abortOnCannotGrowMemory(requestedSize);
#else
return false;
#endif
}
#endif

var minHeapSize = 16777216;

Expand Down
7 changes: 4 additions & 3 deletions src/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -1530,9 +1530,10 @@ var MINIFY_HTML = 1;
// bisecting.
var MAYBE_WASM2JS = 0;

// The size of our shadow memory.
// By default, we have 32 MiB. This supports 256 MiB of real memory.
var ASAN_SHADOW_SIZE = 33554432;
// This option is no longer used. The appropriate shadow memory size is now
// calculated from INITIAL_MEMORY and MAXIMUM_MEMORY. Will be removed in a
// future release.
var ASAN_SHADOW_SIZE = -1

// Internal: Tracks whether Emscripten should link in exception throwing (C++
// 'throw') support library. This does not need to be set directly, but pass
Expand Down
6 changes: 5 additions & 1 deletion tests/asan-no-leak.js
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
Module = {ASAN_OPTIONS: 'detect_leaks=0'};
if (Module != undefined) {
Module.ASAN_OPTIONS = 'detect_leaks=0';
} else {
Module = {ASAN_OPTIONS: 'detect_leaks=0'};
}
34 changes: 19 additions & 15 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -5774,7 +5774,10 @@ def test_fakestat(self):
self.do_run_in_out_file_test('tests', 'core', 'test_fakestat.c')

def test_mmap(self):
self.set_setting('INITIAL_MEMORY', 128 * 1024 * 1024)
# ASan needs more memory, but that is set up separately
if '-fsanitize=address' not in self.emcc_args:
self.set_setting('INITIAL_MEMORY', 128 * 1024 * 1024)

# needs to flush stdio streams
self.set_setting('EXIT_RUNTIME', 1)
self.do_run_in_out_file_test('tests', 'core', 'test_mmap.c')
Expand Down Expand Up @@ -6278,12 +6281,6 @@ def run_all(x):
@no_asan('autodebug logging interferes with asan')
@with_env_modify({'EMCC_AUTODEBUG': '1'})
def test_autodebug_wasm(self):
# Autodebug does not work with too much shadow memory.
# Memory consumed by autodebug depends on the size of the WASM linear memory.
# With a large shadow memory, the JS engine runs out of memory.
if '-fsanitize=address' in self.emcc_args:
self.set_setting('ASAN_SHADOW_SIZE', 16 * 1024 * 1024)

# test that the program both works and also emits some of the logging
# (but without the specific output, as it is logging the actual locals
# used and so forth, which will change between opt modes and updates of
Expand Down Expand Up @@ -8015,7 +8012,9 @@ def test_template_class_deduction(self):
'cpp': ['test_asan_no_error.cpp'],
})
def test_asan_no_error(self, name):
self.emcc_args += ['-fsanitize=address', '-s', 'ALLOW_MEMORY_GROWTH=1']
self.emcc_args.append('-fsanitize=address')
self.emcc_args.append('-sALLOW_MEMORY_GROWTH=1')
self.emcc_args.append('-sINITIAL_MEMORY=314572800')
self.do_runf(path_from_root('tests', 'core', name), '', assert_returncode=NON_ZERO)

# note: these tests have things like -fno-builtin-memset in order to avoid
Expand Down Expand Up @@ -8082,7 +8081,9 @@ def test_asan(self, name, expected_output, cflags=None):
if not self.get_setting('WASM'):
self.skipTest('wasm2js has no ASan support')

self.emcc_args += ['-fsanitize=address', '-s', 'ALLOW_MEMORY_GROWTH=1']
self.emcc_args.append('-fsanitize=address')
self.emcc_args.append('-sALLOW_MEMORY_GROWTH=1')
self.emcc_args.append('-sINITIAL_MEMORY=314572800')
if cflags:
self.emcc_args += cflags
self.do_runf(path_from_root('tests', 'core', name),
Expand All @@ -8091,14 +8092,16 @@ def test_asan(self, name, expected_output, cflags=None):

@no_wasm2js('TODO: ASAN in wasm2js')
def test_asan_js_stack_op(self):
self.emcc_args += ['-fsanitize=address', '-s', 'ALLOW_MEMORY_GROWTH=1']
self.emcc_args.append('-fsanitize=address')
self.emcc_args.append('-sALLOW_MEMORY_GROWTH=1')
self.emcc_args.append('-sINITIAL_MEMORY=314572800')
self.do_runf(path_from_root('tests', 'core', 'test_asan_js_stack_op.c'),
expected_output='Hello, World!')

@no_wasm2js('TODO: ASAN in wasm2js')
def test_asan_api(self):
self.emcc_args.append('-fsanitize=address')
self.set_setting('ALLOW_MEMORY_GROWTH')
self.set_setting('INITIAL_MEMORY', 314572800)
self.do_run_in_out_file_test('tests', 'core', 'test_asan_api.c')

@no_wasm2js('TODO: ASAN in wasm2js')
Expand All @@ -8108,6 +8111,7 @@ def test_asan_modularized_with_closure(self):
self.emcc_args.append('-sUSE_CLOSURE_COMPILER=1')
self.emcc_args.append('-fsanitize=address')
self.emcc_args.append('-sALLOW_MEMORY_GROWTH=1')
self.emcc_args.append('-sINITIAL_MEMORY=314572800')

def post(filename):
with open(filename, 'a') as f:
Expand Down Expand Up @@ -8358,10 +8362,10 @@ def setUp(self):
# Add DEFAULT_TO_CXX=0
strict = make_run('strict', emcc_args=[], settings={'STRICT': 1})

lsan = make_run('lsan', emcc_args=['-fsanitize=leak'], settings={'ALLOW_MEMORY_GROWTH': 1})
asan = make_run('asan', emcc_args=['-fsanitize=address'], settings={'ALLOW_MEMORY_GROWTH': 1, 'ASAN_SHADOW_SIZE': 128 * 1024 * 1024})
asani = make_run('asani', emcc_args=['-fsanitize=address', '--pre-js', os.path.join(os.path.dirname(__file__), 'asan-no-leak.js')],
settings={'ALLOW_MEMORY_GROWTH': 1})
lsan = make_run('lsan', emcc_args=['-fsanitize=leak', '-O2'], settings={'ALLOW_MEMORY_GROWTH': 1})
asan = make_run('asan', emcc_args=['-fsanitize=address', '-O2'], settings={'ALLOW_MEMORY_GROWTH': 1, 'INITIAL_MEMORY': 314572800})
asani = make_run('asani', emcc_args=['-fsanitize=address', '-O2', '--pre-js', os.path.join(os.path.dirname(__file__), 'asan-no-leak.js')],
settings={'ALLOW_MEMORY_GROWTH': 1, 'INITIAL_MEMORY': 314572800})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean that without setting MAXIMUM_MEMORY we use the default 2GB, which means shadow memory is 256MB?

if so that may not be best for CI, as each test will use that much memory immediately. maybe worth keeping the maximum?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this goes back to using the default MAXIMUM_MEMORY, but we were already doing that before this PR. The MAXIMUM_MEMORY that we used to have in this PR was just 7/8 of the default, so not a whole lot better.

I could believe that this might cause CI issues, but it hasn't so far, so I think that would be better addressed in a separate PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I was reading the diff wrong. sgtm.

Last question: why add -O2 here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SIMD tests were failing because -O0 combined with ASan was leading to functions with too many locals. Those failures aren't related to this change, so I could remove the -O2 from this PR, but adding it didn't seem to have any downsides, either.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, sounds fine.


# Experimental modes (not tested by CI)
lld = make_run('lld', emcc_args=[], settings={'LLD_REPORT_UNDEFINED': 1})
Expand Down
8 changes: 4 additions & 4 deletions tests/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -8486,21 +8486,21 @@ def test_lsan_no_stack_trace(self):

def test_asan_null_deref(self):
self.do_smart_test(path_from_root('tests', 'other', 'test_asan_null_deref.c'),
emcc_args=['-fsanitize=address', '-s', 'ALLOW_MEMORY_GROWTH=1'],
emcc_args=['-fsanitize=address', '-sALLOW_MEMORY_GROWTH=1', '-sINITIAL_MEMORY=314572800'],
assert_returncode=NON_ZERO, literals=[
'AddressSanitizer: null-pointer-dereference on address',
])

def test_asan_no_stack_trace(self):
self.do_smart_test(path_from_root('tests', 'other', 'test_lsan_leaks.c'),
emcc_args=['-fsanitize=address', '-s', 'ALLOW_MEMORY_GROWTH=1', '-DDISABLE_CONTEXT', '-s', 'EXIT_RUNTIME'],
emcc_args=['-fsanitize=address', '-sALLOW_MEMORY_GROWTH=1', '-sINITIAL_MEMORY=314572800', '-DDISABLE_CONTEXT', '-s', 'EXIT_RUNTIME'],
assert_returncode=NON_ZERO, literals=[
'Direct leak of 3427 byte(s) in 3 object(s) allocated from:',
'SUMMARY: AddressSanitizer: 3427 byte(s) leaked in 3 allocation(s).',
])

def test_asan_pthread_stubs(self):
self.do_smart_test(path_from_root('tests', 'other', 'test_asan_pthread_stubs.c'), emcc_args=['-fsanitize=address', '-s', 'ALLOW_MEMORY_GROWTH=1'])
self.do_smart_test(path_from_root('tests', 'other', 'test_asan_pthread_stubs.c'), emcc_args=['-fsanitize=address', '-sALLOW_MEMORY_GROWTH=1', '-sINITIAL_MEMORY=314572800'])

def test_proxy_to_pthread_stack(self):
with js_engines_modify([NODE_JS + ['--experimental-wasm-threads', '--experimental-wasm-bulk-memory']]):
Expand Down Expand Up @@ -8601,7 +8601,7 @@ def test_mmap_and_munmap_anonymous(self):
self.do_other_test('mmap_and_munmap_anonymous', emcc_args=['-s', 'NO_FILESYSTEM'])

def test_mmap_and_munmap_anonymous_asan(self):
self.do_other_test('mmap_and_munmap_anonymous', emcc_args=['-s', 'NO_FILESYSTEM', '-fsanitize=address', '-s', 'ALLOW_MEMORY_GROWTH=1'])
self.do_other_test('mmap_and_munmap_anonymous', emcc_args=['-s', 'NO_FILESYSTEM', '-fsanitize=address', '-s', 'ALLOW_MEMORY_GROWTH=1', '-sINITIAL_MEMORY=314572800'])

def test_mmap_memorygrowth(self):
self.do_other_test('mmap_memorygrowth', ['-s', 'ALLOW_MEMORY_GROWTH=1'])
Expand Down