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

Interceptors no longer build on SPARC/Solaris with Solaris as #72970

Closed
jakubjelinek opened this issue Nov 21, 2023 · 1 comment · Fixed by #72973
Closed

Interceptors no longer build on SPARC/Solaris with Solaris as #72970

jakubjelinek opened this issue Nov 21, 2023 · 1 comment · Fixed by #72973

Comments

@jakubjelinek
Copy link

jakubjelinek commented Nov 21, 2023

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.
builtins/functions.h has a proper

#if defined(__arm__) || defined(__aarch64__)
#define SYMBOL_IS_FUNC(name) .type name,%function
#else
#define SYMBOL_IS_FUNC(name) .type name,@function
#endif

but obviously that is for *.S files, not meant for inline assembly.

So, I'd like to propose:

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_asm.h b/compiler-rt/lib/sanitizer_common/sanitizer_asm.h
index 3c9bbdc9678b..bbb18cfbdf15 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
diff --git a/compiler-rt/lib/interception/interception.h b/compiler-rt/lib/interception/interception.h
index 069f73d276f3..9d8b60b2eef5 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_"                    \

(because unfortunately SANITIZER_STRINGIFY(ASM_TYPE_FUNCTION(TRAMPOLINE(func))) doesn't work).
@melver

@melver
Copy link
Contributor

melver commented Nov 21, 2023

Thanks, I've sent: #72973

melver added a commit that referenced this issue Nov 21, 2023
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
qihangkong pushed a commit to rvgpu/llvm that referenced this issue Apr 18, 2024
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/llvm-project#72970
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants