Sanitizer/MIPS: Use $t9 for preemptible function call #76894
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
llvmbot
added
clang
|
@llvm/pr-subscribers-compiler-rt-sanitizer @llvm/pr-subscribers-clang Author: YunQiang Su (wzssyqa) ChangesOn MIPS pre-R6, instruction b can only work within 64KiB, We need the help of GOT. lui $t8, %hi(%neg(%gp_rel(SANITIZER_STRINGIFY(TRAMPOLINE(func))))) And then get the address of __interceptor_func, and jump to it ld $t9, %got_disp(_interceptor" SANITIZER_STRINGIFY(func) ")($t8) MIPS/O32 has .cpload, which can help to generate 3 instructions to get GOT. MIPSr6 has instruction bc, which can jump long enough. Full diff: https://github.com/llvm/llvm-project/pull/76894.diff 4 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Arch/Mips.cpp b/clang/lib/Driver/ToolChains/Arch/Mips.cpp
index f9f14c01b2b9f0..fe9d112b8800b1 100644
--- a/clang/lib/Driver/ToolChains/Arch/Mips.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/Mips.cpp
@@ -221,6 +221,7 @@ void mips::getMIPSTargetFeatures(const Driver &D, const llvm::Triple &Triple,
bool IsN64 = ABIName == "64";
bool IsPIC = false;
bool NonPIC = false;
+ bool HasNaN2008Opt = false;
Arg *LastPICArg = Args.getLastArg(options::OPT_fPIC, options::OPT_fno_PIC,
options::OPT_fpic, options::OPT_fno_pic,
@@ -285,9 +286,10 @@ void mips::getMIPSTargetFeatures(const Driver &D, const llvm::Triple &Triple,
if (Arg *A = Args.getLastArg(options::OPT_mnan_EQ)) {
StringRef Val = StringRef(A->getValue());
if (Val == "2008") {
- if (mips::getIEEE754Standard(CPUName) & mips::Std2008)
+ if (mips::getIEEE754Standard(CPUName) & mips::Std2008) {
Features.push_back("+nan2008");
- else {
+ HasNaN2008Opt = true;
+ } else {
Features.push_back("-nan2008");
D.Diag(diag::warn_target_unsupported_nan2008) << CPUName;
}
@@ -323,6 +325,8 @@ void mips::getMIPSTargetFeatures(const Driver &D, const llvm::Triple &Triple,
D.Diag(diag::err_drv_unsupported_option_argument)
<< A->getSpelling() << Val;
}
+ } else if (HasNaN2008Opt) {
+ Features.push_back("+abs2008");
}
AddTargetFeature(Args, Features, options::OPT_msingle_float,
diff --git a/clang/test/Driver/mips-features.c b/clang/test/Driver/mips-features.c
index 5ae566774959f1..fad6009ffb89ba 100644
--- a/clang/test/Driver/mips-features.c
+++ b/clang/test/Driver/mips-features.c
@@ -213,7 +213,13 @@
// RUN: %clang -target mips-linux-gnu -march=mips32r3 -### -c %s \
// RUN: -mnan=legacy -mnan=2008 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-NAN2008 %s
-// CHECK-NAN2008: "-target-feature" "+nan2008"
+// CHECK-NAN2008: "-target-feature" "+nan2008" "-target-feature" "+abs2008"
+//
+// -mnan=2008 -mabs=legacy
+// RUN: %clang -target mips-linux-gnu -march=mips32r3 -### -c %s \
+// RUN: -mabs=legacy -mnan=2008 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-ABSLEGACYNAN2008 %s
+// CHECK-ABSLEGACYNAN2008: "-target-feature" "+nan2008" "-target-feature" "-abs2008"
//
// -mnan=legacy
// RUN: %clang -target mips-linux-gnu -march=mips32r3 -### -c %s \
diff --git a/compiler-rt/lib/interception/interception.h b/compiler-rt/lib/interception/interception.h
index 9d8b60b2eef58c..58e969378a9082 100644
--- a/compiler-rt/lib/interception/interception.h
+++ b/compiler-rt/lib/interception/interception.h
@@ -205,8 +205,9 @@ const interpose_substitution substitution_##func_name[] \
ASM_TYPE_FUNCTION_STR "\n" \
SANITIZER_STRINGIFY(TRAMPOLINE(func)) ":\n" \
SANITIZER_STRINGIFY(CFI_STARTPROC) "\n" \
- SANITIZER_STRINGIFY(ASM_TAIL_CALL) " __interceptor_" \
- SANITIZER_STRINGIFY(ASM_PREEMPTIBLE_SYM(func)) "\n" \
+ C_ASM_TAIL_CALL(SANITIZER_STRINGIFY(TRAMPOLINE(func)), \
+ "__interceptor_" \
+ SANITIZER_STRINGIFY(ASM_PREEMPTIBLE_SYM(func))) "\n" \
SANITIZER_STRINGIFY(CFI_ENDPROC) "\n" \
".size " SANITIZER_STRINGIFY(TRAMPOLINE(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 bbb18cfbdf15fe..37fa77b2d6759e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_asm.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_asm.h
@@ -44,6 +44,8 @@
#if defined(__x86_64__) || defined(__i386__) || defined(__sparc__)
# define ASM_TAIL_CALL jmp
+#elif defined(__mips__) && __mips_isa_rev >= 6
+# define ASM_TAIL_CALL bc
#elif defined(__arm__) || defined(__aarch64__) || defined(__mips__) || \
defined(__powerpc__) || defined(__loongarch_lp64)
# define ASM_TAIL_CALL b
@@ -53,6 +55,25 @@
# define ASM_TAIL_CALL tail
#endif
+#if defined(__mips64) && __mips_isa_rev < 6
+# define C_ASM_TAIL_CALL(tfunc, ifunc) \
+ "lui $t8, %hi(%neg(%gp_rel(" tfunc ")))\n" \
+ "daddu $t8, $t8, $t9\n" \
+ "daddu $t8, $t8, %lo(%neg(%gp_rel(" tfunc ")))\n" \
+ "ld $t9, %got_disp(" ifunc ")($t8)\n" \
+ "jr $t9\n"
+#elif defined(__mips__) && __mips_isa_rev < 6
+# define C_ASM_TAIL_CALL(tfunc, ifunc) \
+ ".set noreorder\n" \
+ ".cpload $t9\n" \
+ ".set reorder\n" \
+ "lw $t9, %got(" ifunc ")($gp)\n" \
+ "jr $t9\n"
+#elif defined(ASM_TAIL_CALL)
+# define C_ASM_TAIL_CALL(tfunc, ifunc) \
+ SANITIZER_STRINGIFY(ASM_TAIL_CALL) " " ifunc
+#endif
+
#if defined(__ELF__) && defined(__x86_64__) || defined(__i386__) || \
defined(__riscv)
# define ASM_PREEMPTIBLE_SYM(sym) sym@plt
|
You can test this locally with the following command:git-clang-format --diff 49138d97c0d8a6d1c6935da414a1f3fea839263b eff3c340eb154d5acc9bf99ec5e544fe4697a228 -- 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 58e969378a..deb5a811fc 100644
--- a/compiler-rt/lib/interception/interception.h
+++ b/compiler-rt/lib/interception/interception.h
@@ -191,12 +191,12 @@ const interpose_substitution substitution_##func_name[] \
# 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 3af66a4e44..6be30c5d46 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_asm.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_asm.h
@@ -58,21 +58,25 @@
// on MIPSr6. To be compatiable with them, we have to set $t9 properly.
// MIPS uses GOT to get the address of preemptible functions.
#if defined(__mips64)
-# define C_ASM_TAIL_CALL(t_func, i_func) \
- "lui $t8, %hi(%neg(%gp_rel(" t_func ")))\n" \
- "daddu $t8, $t8, $t9\n" \
- "daddiu $t8, $t8, %lo(%neg(%gp_rel(" t_func ")))\n" \
- "ld $t9, %got_disp(" i_func ")($t8)\n" \
+# define C_ASM_TAIL_CALL(t_func, i_func) \
+ "lui $t8, %hi(%neg(%gp_rel(" t_func \
+ ")))\n" \
+ "daddu $t8, $t8, $t9\n" \
+ "daddiu $t8, $t8, %lo(%neg(%gp_rel(" t_func \
+ ")))\n" \
+ "ld $t9, %got_disp(" i_func \
+ ")($t8)\n" \
"jr $t9\n"
#elif defined(__mips__)
-# define C_ASM_TAIL_CALL(t_func, i_func) \
- ".set noreorder\n" \
- ".cpload $t9\n" \
- ".set reorder\n" \
- "lw $t9, %got(" i_func ")($gp)\n" \
+# define C_ASM_TAIL_CALL(t_func, i_func) \
+ ".set noreorder\n" \
+ ".cpload $t9\n" \
+ ".set reorder\n" \
+ "lw $t9, %got(" i_func \
+ ")($gp)\n" \
"jr $t9\n"
#elif defined(ASM_TAIL_CALL)
-# define C_ASM_TAIL_CALL(t_func, i_func) \
+# define C_ASM_TAIL_CALL(t_func, i_func) \
SANITIZER_STRINGIFY(ASM_TAIL_CALL) " " i_func
#endif
@@ -96,30 +100,29 @@
# 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) \
- .weak symbol; \
- .set symbol, ASM_WRAPPER_NAME(name)
-# define ASM_INTERCEPTOR_TRAMPOLINE(name)
-# define ASM_INTERCEPTOR_TRAMPOLINE_SUPPORT 0
-# else // Architecture supports interceptor trampoline
+# define ASM_WRAPPER_NAME(symbol) __interceptor_##symbol
+# define ASM_TRAMPOLINE_ALIAS(symbol, name) \
+ .weak symbol; \
+ .set symbol, ASM_WRAPPER_NAME(name)
+# define ASM_INTERCEPTOR_TRAMPOLINE(name)
+# define ASM_INTERCEPTOR_TRAMPOLINE_SUPPORT 0
+# else // Architecture supports interceptor trampoline
// Keep trampoline implementation in sync with interception/interception.h
-# define ASM_WRAPPER_NAME(symbol) ___interceptor_##symbol
-# define ASM_TRAMPOLINE_ALIAS(symbol, name) \
- .weak symbol; \
- .set symbol, __interceptor_trampoline_##name
-# define ASM_INTERCEPTOR_TRAMPOLINE(name) \
- .weak __interceptor_##name; \
- .set __interceptor_##name, ASM_WRAPPER_NAME(name); \
- .globl __interceptor_trampoline_##name; \
- ASM_TYPE_FUNCTION(__interceptor_trampoline_##name); \
- __interceptor_trampoline_##name: \
- CFI_STARTPROC; \
- ASM_TAIL_CALL ASM_PREEMPTIBLE_SYM(__interceptor_##name); \
- CFI_ENDPROC; \
- ASM_SIZE(__interceptor_trampoline_##name)
-# define ASM_INTERCEPTOR_TRAMPOLINE_SUPPORT 1
-# endif // Architecture supports interceptor trampoline
+# define ASM_WRAPPER_NAME(symbol) ___interceptor_##symbol
+# define ASM_TRAMPOLINE_ALIAS(symbol, name) \
+ .weak symbol; \
+ .set symbol, __interceptor_trampoline_##name
+# define ASM_INTERCEPTOR_TRAMPOLINE(name) \
+ .weak __interceptor_##name; \
+ .set __interceptor_##name, ASM_WRAPPER_NAME(name); \
+ .globl __interceptor_trampoline_##name; \
+ ASM_TYPE_FUNCTION(__interceptor_trampoline_##name); \
+ __interceptor_trampoline_##name : CFI_STARTPROC; \
+ ASM_TAIL_CALL ASM_PREEMPTIBLE_SYM(__interceptor_##name); \
+ CFI_ENDPROC; \
+ ASM_SIZE(__interceptor_trampoline_##name)
+# define ASM_INTERCEPTOR_TRAMPOLINE_SUPPORT 1
+# endif // Architecture supports interceptor trampoline
#else
# define ASM_HIDDEN(symbol)
# define ASM_TYPE_FUNCTION(symbol)
|
|
This also has a Clang patch mixed in. |
wzssyqa
force-pushed
the
sanitizer-prer6
branch
from
9893b37
to
c74541c
Compare
wzssyqa
force-pushed
the
sanitizer-prer6
branch
from
c74541c
to
dbbe6c5
Compare
brad0
added
backend:MIPS
and removed
clang
vitalybuka
reviewed
Jan 4, 2024
wzssyqa
force-pushed
the
sanitizer-prer6
branch
from
dbbe6c5
to
d264460
Compare
|
Change daddu to daddiu, since gas cannot use daddu for const value. |
MaskRay
reviewed
Jan 7, 2024
| extern "C" ret_type __interceptor_##func(__VA_ARGS__) \ | ||
| INTERCEPTOR_ATTRIBUTE __attribute__((weak)) ALIAS(WRAP(func)); \ | ||
| asm( \ | ||
| # define DECLARE_WRAPPER(ret_type, func, ...) \ |
There was a problem hiding this comment.
You can ignore clang-format error to reduce diff.
Does this patch address all comments I left on https://reviews.llvm.org/D158491 ?
I left a comment on the Phabricator instance (which I cannot access now) that this is 128KiB. |
|
I remember that I left a detailed alternative description on Phabricator but read-only mirror does not contain the latest comment... The description should mention it's a mips fixup for https://reviews.llvm.org/D151085 and include some information about how the interceptor symbols work on mips. For example, on x86 it's: https://maskray.me/blog/2023-01-08-all-about-sanitizer-interceptors contains some notes about the interceptor mechanism. |
Currently, almost all of the shared libraries of MIPS, rely on $t9 to get the address of current function, instead of PCREL instructions, even on MIPSr6. So we have to set $t9 properly. To get the address of preemptible function, we need the help of GOT. MIPS/O32 has .cpload, which can help to generate 3 instructions to get GOT. For __mips64, we can get GOT by: lui $t8, %hi(%neg(%gp_rel(SANITIZER_STRINGIFY(TRAMPOLINE(func))))) daddu $t8, $t8, $t9 daddiu $t8, $t8, %hi(%neg(%gp_rel(SANITIZER_STRINGIFY(TRAMPOLINE(func))))) And then get the address of __interceptor_func, and jump to it ld $t9, %got_disp(__interceptor_" SANITIZER_STRINGIFY(func) ")($t8) jr $t9
wzssyqa
force-pushed
the
sanitizer-prer6
branch
from
d264460
to
eff3c34
Compare
wzssyqa
changed the title
|
@MaskRay Thanks for your note. I find a new problem: R6 also need t9 due to that the existing libraries depends on $t9. |
|
The old comment is: |
MaskRay
approved these changes
Jan 15, 2024
|
@MaskRay From your last set of comments were you going to make some changes to this? |
This is the history in review.llvm.org. |
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.
Currently, almost all of the shared libraries of MIPS, rely on $t9
to get the address of current function, instead of PCREL instructions,
even on MIPSr6. So we have to set $t9 properly.
To get the address of preemptible function, we need the help of GOT.
MIPS/O32 has .cpload, which can help to generate 3 instructions to get GOT.
For __mips64, we can get GOT by:
lui $t8, %hi(%neg(%gp_rel(SANITIZER_STRINGIFY(TRAMPOLINE(func)))))
daddu $t8, $t8, $t9
daddiu $t8, $t8, %hi(%neg(%gp_rel(SANITIZER_STRINGIFY(TRAMPOLINE(func)))))
And then get the address of __interceptor_func, and jump to it
ld $t9, %got_disp(_interceptor" SANITIZER_STRINGIFY(func) ")($t8)
jr $t9
Fixes #74047