[compiler-rt] Fix interceptors with Solaris as #72973
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jakub Jelínek reports: As mentioned in https://gcc.gnu.org/PR112563, the new DECLARE_WRAPPER macro added in 37445e9 and ammended in 85d3873 doesn't work on SPARC/Solaris with Solaris as. While clang and GNU as when used from GCC seems to be forgiving on most architectures and allow both %function and @function (with the latter not being allowed on ARM/AArch64 I believe because @ is assembler comment start there), Solaris as doesn't allow the %function form. Fix it by using %function only for ARM. Co-developed-by: Jakub Jelínek <jakub@redhat.com> Reported-by: Jakub Jelínek <jakub@redhat.com> Closes: llvm#72970
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Marco Elver (melver) ChangesJakub Jelínek reports: As mentioned in https://gcc.gnu.org/PR112563, the new DECLARE_WRAPPER macro While clang and GNU as when used from GCC seems to be forgiving on most Fix it by using %function only for ARM. Co-developed-by: Jakub Jelínek <jakub@redhat.com> Full diff: https://github.com/llvm/llvm-project/pull/72973.diff 2 Files Affected:
diff --git a/compiler-rt/lib/interception/interception.h b/compiler-rt/lib/interception/interception.h
index 069f73d276f3c40..9d8b60b2eef58c2 100644
--- a/compiler-rt/lib/interception/interception.h
+++ b/compiler-rt/lib/interception/interception.h
@@ -185,6 +185,11 @@ const interpose_substitution substitution_##func_name[] \
# else
# define __ASM_WEAK_WRAPPER(func) ".weak " #func "\n"
# endif // SANITIZER_FREEBSD || SANITIZER_NETBSD
+# if defined(__arm__) || defined(__aarch64__)
+# define ASM_TYPE_FUNCTION_STR "%function"
+# else
+# define ASM_TYPE_FUNCTION_STR "@function"
+# endif
// Keep trampoline implementation in sync with sanitizer_common/sanitizer_asm.h
# define DECLARE_WRAPPER(ret_type, func, ...) \
extern "C" ret_type func(__VA_ARGS__); \
@@ -196,7 +201,8 @@ const interpose_substitution substitution_##func_name[] \
__ASM_WEAK_WRAPPER(func) \
".set " #func ", " SANITIZER_STRINGIFY(TRAMPOLINE(func)) "\n" \
".globl " SANITIZER_STRINGIFY(TRAMPOLINE(func)) "\n" \
- ".type " SANITIZER_STRINGIFY(TRAMPOLINE(func)) ", %function\n" \
+ ".type " SANITIZER_STRINGIFY(TRAMPOLINE(func)) ", " \
+ ASM_TYPE_FUNCTION_STR "\n" \
SANITIZER_STRINGIFY(TRAMPOLINE(func)) ":\n" \
SANITIZER_STRINGIFY(CFI_STARTPROC) "\n" \
SANITIZER_STRINGIFY(ASM_TAIL_CALL) " __interceptor_" \
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_asm.h b/compiler-rt/lib/sanitizer_common/sanitizer_asm.h
index 3c9bbdc9678b09a..bbb18cfbdf15feb 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_asm.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_asm.h
@@ -62,7 +62,11 @@
#if !defined(__APPLE__)
# define ASM_HIDDEN(symbol) .hidden symbol
-# define ASM_TYPE_FUNCTION(symbol) .type symbol, %function
+# if defined(__arm__) || defined(__aarch64__)
+# define ASM_TYPE_FUNCTION(symbol) .type symbol, %function
+# else
+# define ASM_TYPE_FUNCTION(symbol) .type symbol, @function
+# endif
# define ASM_SIZE(symbol) .size symbol, .-symbol
# define ASM_SYMBOL(symbol) symbol
# define ASM_SYMBOL_INTERCEPTOR(symbol) symbol
|
You can test this locally with the following command:git-clang-format --diff ef9bcace834e63f25bbbc5e8e2b615f89d85fb2f 0b0ce24a59f1c6f0a6144c27cb28002c77c252ad -- compiler-rt/lib/interception/interception.h compiler-rt/lib/sanitizer_common/sanitizer_asm.hView the diff from clang-format here.diff --git a/compiler-rt/lib/interception/interception.h b/compiler-rt/lib/interception/interception.h
index 9d8b60b2ee..ae849626db 100644
--- a/compiler-rt/lib/interception/interception.h
+++ b/compiler-rt/lib/interception/interception.h
@@ -185,18 +185,18 @@ const interpose_substitution substitution_##func_name[] \
# else
# define __ASM_WEAK_WRAPPER(func) ".weak " #func "\n"
# endif // SANITIZER_FREEBSD || SANITIZER_NETBSD
-# if defined(__arm__) || defined(__aarch64__)
-# define ASM_TYPE_FUNCTION_STR "%function"
-# else
-# define ASM_TYPE_FUNCTION_STR "@function"
-# endif
+# if defined(__arm__) || defined(__aarch64__)
+# define ASM_TYPE_FUNCTION_STR "%function"
+# else
+# define ASM_TYPE_FUNCTION_STR "@function"
+# endif
// Keep trampoline implementation in sync with sanitizer_common/sanitizer_asm.h
-# define DECLARE_WRAPPER(ret_type, func, ...) \
- extern "C" ret_type func(__VA_ARGS__); \
- extern "C" ret_type TRAMPOLINE(func)(__VA_ARGS__); \
- extern "C" ret_type __interceptor_##func(__VA_ARGS__) \
- INTERCEPTOR_ATTRIBUTE __attribute__((weak)) ALIAS(WRAP(func)); \
- asm( \
+# define DECLARE_WRAPPER(ret_type, func, ...) \
+ extern "C" ret_type func(__VA_ARGS__); \
+ extern "C" ret_type TRAMPOLINE(func)(__VA_ARGS__); \
+ extern "C" ret_type __interceptor_##func(__VA_ARGS__) \
+ INTERCEPTOR_ATTRIBUTE __attribute__((weak)) ALIAS(WRAP(func)); \
+ asm( \
".text\n" \
__ASM_WEAK_WRAPPER(func) \
".set " #func ", " SANITIZER_STRINGIFY(TRAMPOLINE(func)) "\n" \
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_asm.h b/compiler-rt/lib/sanitizer_common/sanitizer_asm.h
index bbb18cfbdf..56fea571ee 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_asm.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_asm.h
@@ -62,16 +62,16 @@
#if !defined(__APPLE__)
# define ASM_HIDDEN(symbol) .hidden symbol
-# if defined(__arm__) || defined(__aarch64__)
-# define ASM_TYPE_FUNCTION(symbol) .type symbol, %function
-# else
-# define ASM_TYPE_FUNCTION(symbol) .type symbol, @function
-# endif
-# define ASM_SIZE(symbol) .size symbol, .-symbol
-# define ASM_SYMBOL(symbol) symbol
-# define ASM_SYMBOL_INTERCEPTOR(symbol) symbol
-# if defined(__i386__) || defined(__powerpc__) || defined(__s390__) || \
- defined(__sparc__)
+# if defined(__arm__) || defined(__aarch64__)
+# define ASM_TYPE_FUNCTION(symbol) .type symbol, % function
+# else
+# define ASM_TYPE_FUNCTION(symbol) .type symbol, @function
+# endif
+# define ASM_SIZE(symbol) .size symbol, .- symbol
+# define ASM_SYMBOL(symbol) symbol
+# define ASM_SYMBOL_INTERCEPTOR(symbol) symbol
+# if defined(__i386__) || defined(__powerpc__) || defined(__s390__) || \
+ defined(__sparc__)
// For details, see interception.h
# define ASM_WRAPPER_NAME(symbol) __interceptor_##symbol
# define ASM_TRAMPOLINE_ALIAS(symbol, name) \
|
vitalybuka
approved these changes
Nov 21, 2023
|
There is clang format complains |
|
It follows the indentation of the surrounding code. |
This code is some old code that predates the new clang-format style changes, and the new code just follows that pre-existing formatting (same with my initial interceptor patches). We should either reformat everything so this doesn't happen again, or accept to ignore the clang-format warnings because as-is, incrementally applying formatting makes the whole file even more unreadable (I can barely stand looking at the macro-soup it already is). |
|
I usually reformat entire file in pre or followup separate commit |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Jakub Jelínek reports:
As mentioned in https://gcc.gnu.org/PR112563, the new DECLARE_WRAPPER macro
added in 37445e9 and ammended in 85d3873 doesn't work on SPARC/Solaris with
Solaris as.
While clang and GNU as when used from GCC seems to be forgiving on most
architectures and allow both %function and @function (with the latter not being
allowed on ARM/AArch64 I believe because @ is assembler comment start there),
Solaris as doesn't allow the %function form.
Fix it by using %function only for ARM.
Co-developed-by: Jakub Jelínek jakub@redhat.com
Reported-by: Jakub Jelínek jakub@redhat.com
Closes: #72970