Skip to content
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

[asan][windows] use __builtin_function_address to avoid problematic codegen in weak function registration #108327

Merged

Conversation

barcharcraz
Copy link
Contributor

Previously we were relying on optnone for this, but that didn't seem to be sufficient.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 12, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Charlie Barto (barcharcraz)

Changes

Previously we were relying on optnone for this, but that didn't seem to be sufficient.


Full diff: https://github.com/llvm/llvm-project/pull/108327.diff

1 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_win_thunk_interception.h (+3-2)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_win_thunk_interception.h b/compiler-rt/lib/sanitizer_common/sanitizer_win_thunk_interception.h
index fa7b18fdc18f6d..80c56f8f3c9d0f 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_win_thunk_interception.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_win_thunk_interception.h
@@ -62,8 +62,10 @@ void initialize_thunks(const sanitizer_thunk *begin,
 // startup that will replace sanitizer_export with local_function
 #ifdef __clang__
 #  define REGISTER_WEAK_OPTNONE __attribute__((optnone))
+#  define REGISTER_WEAK_FUNCTION_ADDRESS(fn) __builtin_function_start(fn)
 #else
 #  define REGISTER_WEAK_OPTNONE
+#  define REGISTER_WEAK_FUNCTION_ADDRESS(fn) & fn
 #endif
 
 #define REGISTER_WEAK_FUNCTION(local_function)                          \
@@ -71,8 +73,7 @@ void initialize_thunks(const sanitizer_thunk *begin,
   extern "C" void WEAK_EXPORT_NAME(local_function)();                   \
   WIN_WEAK_IMPORT_DEF(local_function)                                   \
   REGISTER_WEAK_OPTNONE static int register_weak_##local_function() {   \
-    if ((uintptr_t) & local_function != (uintptr_t) &                   \
-        WEAK_EXPORT_NAME(local_function)) {                             \
+    if ((uintptr_t) REGISTER_WEAK_FUNCTION_ADDRESS(local_function) != (uintptr_t) REGISTER_WEAK_FUNCTION_ADDRESS(WEAK_EXPORT_NAME(local_function))) {                             \
       return __sanitizer::register_weak(                                \
           SANITIZER_STRINGIFY(WEAK_EXPORT_NAME(local_function)),        \
           reinterpret_cast<__sanitizer::uptr>(local_function));         \

Copy link

github-actions bot commented Sep 12, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@barcharcraz barcharcraz force-pushed the dev/chbarto/fix_clang_register_weak branch from 0df07f3 to 161445b Compare September 12, 2024 04:20
Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

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

LGTM, this fixes the issues in mingw configurations now!

@barcharcraz barcharcraz merged commit e31efd8 into llvm:main Sep 12, 2024
7 checks passed
VitaNuo pushed a commit to VitaNuo/llvm-project that referenced this pull request Sep 12, 2024
…odegen in weak function registration (llvm#108327)

Previously we were relying on optnone for this, but that didn't seem to
be sufficient.
@aeubanks
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants