-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[mimalloc] Error on mimalloc + ASan #23314
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
Conversation
tools/system_libs.py
Outdated
@@ -2369,6 +2369,9 @@ def add_sanitizer_libs(): | |||
add_library('libc') | |||
if settings.MALLOC == 'mimalloc': | |||
add_library('libmimalloc') | |||
if settings.USE_ASAN: | |||
# See https://github.com/emscripten-core/emscripten/issues/23288#issuecomment-2571648258 | |||
shared.exit_with_error('mimalloc should not be used with asan') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mimalloc is not compatible with with asan
?
Or "not currently" maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounded from the mimalloc people's responses there that this was by design (ASan uses its own allocator), IIUC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, then 'mimalloc is not compatible with with asan' maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm either way
@@ -15209,6 +15209,11 @@ def test_wasm64_no_asan(self): | |||
err = self.expect_fail([EMCC, test_file('hello_world.c'), '-sMEMORY64', '-fsanitize=address']) | |||
self.assertContained('error: MEMORY64 does not yet work with ASAN', err) | |||
|
|||
def test_mimalloc_no_asan(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there other mimalloc tests that this can be grouped with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I grouped this with the other asan test right above, which it is parallel to?
test/test_other.py
Outdated
def test_mimalloc_no_asan(self): | ||
# See https://github.com/emscripten-core/emscripten/issues/23288#issuecomment-2571648258 | ||
err = self.expect_fail([EMCC, test_file('hello_world.c'), '-sMALLOC=mimalloc', '-fsanitize=address']) | ||
self.assertContained('error: mimalloc is not compatible with with -fsanitize=address', err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this?
self.assertContained('error: mimalloc is not compatible with with -fsanitize=address', err) | |
self.assertContained('error: mimalloc is not compatible with -fsanitize=address', err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mighty1231 !
mimalloc is not meant to be used with ASan in general, see
#23288 (comment)
Fixes #23288