I'm not aware of any platforms where this will work, but the code should at least compile.
HWASAN_WITH_INTERCEPTORS=OFF means there is magic in libc that would call hwasan_thread_enter /
hwasan_thread_exit as appropriate.
Details
- Reviewers
pcc winksaville - Commits
- rZORG4245a161dd69: [hwasan] Fix HWASAN_WITH_INTERCEPTORS=OFF build on not-android.
rZORG77436c2bef67: [hwasan] Fix HWASAN_WITH_INTERCEPTORS=OFF build on not-android.
rG4245a161dd69: [hwasan] Fix HWASAN_WITH_INTERCEPTORS=OFF build on not-android.
rG77436c2bef67: [hwasan] Fix HWASAN_WITH_INTERCEPTORS=OFF build on not-android.
rGc4bfa0d662ff: [hwasan] Fix HWASAN_WITH_INTERCEPTORS=OFF build on not-android.
rCRT359914: [hwasan] Fix HWASAN_WITH_INTERCEPTORS=OFF build on not-android.
rL359914: [hwasan] Fix HWASAN_WITH_INTERCEPTORS=OFF build on not-android.
Diff Detail
- Repository
- rG LLVM Github Monorepo
- Build Status
Buildable 31194 Build 31193: arc lint + arc unit
Event Timeline
compiler-rt/lib/hwasan/hwasan_linux.cpp | ||
---|---|---|
250 | This seems weird, what happens if HAWSAN_WITH_INTERCEPTORS is true and SANITIZER_ANDROID is true. We would get this code but the GetCurrentThreadLongPtr() would be the version that calls get_android_tls_ptr() is that valid? | |
310 | Since __hwasan_thread_enter() is always defined why is this code conditional? The other possibility is that the condition should be !SANITIZER_ANDROID so that it matches the value returned by GetCurrentThreadLongPtr(). |
HWASAN_WITH_INTERCEPTORS=OFF is a mode where we rely on libc to call hwasan_thread_enter and hwasan_thread_exit as appropriate. This is particularly important when libc is built with hwasan, because then instrumented code may run on a thread before GetCurrentThread is called. At this moment, only bionic supports this, as far as I know.
These two conditional blocks are independent. The one with HwasanTSDThreadInit implements calling __hwasan_thread_exit in the HWASAN_WITH_INTERCEPTORS=OFF mode. The one with GetCurrentThreadLongPtr defines access to the thread local storage (which is also part of the instrumentation ABI). Android uses the fast fixed slot in either mode.
@eugenis, which of the following conditions have tests?
HWASAN_WITH_INTERCEPTORS=OFF, SANITIZER_ANDROID=OFF
HWASAN_WITH_INTERCEPTORS=OFF, SANITIZER_ANDROID=ON
HWASAN_WITH_INTERCEPTORS=ON, SANITIZER_ANDROID=OFF
HWASAN_WITH_INTERCEPTORS=ON, SANITIZER_ANDROID=ON
This configuration does not exist.
HWASAN_WITH_INTERCEPTORS=OFF, SANITIZER_ANDROID=ON
This needs custom-built android to test. We've got some integration tests downstream.
Now that I think of it, the upstream test harness could provide something to emulate that on stock Android, but that would still be quite far from the real thing.
HWASAN_WITH_INTERCEPTORS=ON, SANITIZER_ANDROID=OFF
This is check-hwasan on x86_64-linux.
HWASAN_WITH_INTERCEPTORS=ON, SANITIZER_ANDROID=ON
This is check-hwasan on aarch64-linux-android.
@eugenis, which of the following conditions have tests?
HWASAN_WITH_INTERCEPTORS=OFF, SANITIZER_ANDROID=OFF
This configuration does not exist.
Should this be an error and cause an "unsupported configuration"?
But we don't really know that, because a change that makes this configuration supported needs to be in libc or in platform build files, not in compiler-rt. I'd rather not block anyone from experimenting.
What about makeing it a warning, "untested configuration"? Obviously your call, just seems prudent to me.
Should this be an error and cause an "unsupported configuration"?
But we don't really know that, because a change that makes this configuration supported needs to be in libc or in platform build files, not in compiler-rt. I'd rather not block anyone from experimenting.
What about makeing it a warning, "untested configuration"? Obviously your call, just seems prudent to me.
Sorry, but I don't like it for the same reason.
How about a comment something like:
// Confiturations of HWASAN_WITH_INTERCEPTORS and SANITIZER_ANDROID // // HWASAN_WITH_INTERCEPTORS=OFF, SANITIZER_ANDROID=OFF // // Not currently tested // // HWASAN_WITH_INTERCEPTORS=OFF, SANITIZER_ANDROID=ON // // Integration tests downstream exit // // HWASAN_WITH_INTERCEPTORS=ON, SANITIZER_ANDROID=OFF // // Tested with check-hwasan on x86_64-linux. // // HWASAN_WITH_INTERCEPTORS=ON, SANITIZER_ANDROID=ON // // Tested with check-hwasan on aarch64-linux-android.
Adding action "Accept Revision", I'm not sure this is the processes so correct me if this is inapprporpriate and let me know what I should do.
This seems weird, what happens if HAWSAN_WITH_INTERCEPTORS is true and SANITIZER_ANDROID is true. We would get this code but the GetCurrentThreadLongPtr() would be the version that calls get_android_tls_ptr() is that valid?