-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[PAC][compiler-rt] Fix init/fini array signing schema #150691
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
Conversation
9a8bf68 to
a049b76
Compare
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions c -- compiler-rt/lib/builtins/crtbegin.cView the diff from clang-format here.diff --git a/compiler-rt/lib/builtins/crtbegin.c b/compiler-rt/lib/builtins/crtbegin.c
index 447474bd0..44cd9b614 100644
--- a/compiler-rt/lib/builtins/crtbegin.c
+++ b/compiler-rt/lib/builtins/crtbegin.c
@@ -54,33 +54,33 @@ static void __attribute__((used)) __do_init(void) {
}
#ifdef CRT_HAS_INITFINI_ARRAY
-# if __has_feature(ptrauth_init_fini)
+#if __has_feature(ptrauth_init_fini)
// TODO: use __ptrauth-qualified pointers when they are supported on clang side
-# if __has_feature(ptrauth_init_fini_address_discrimination)
+#if __has_feature(ptrauth_init_fini_address_discrimination)
__attribute__((section(".init_array"), used)) static void *__init =
ptrauth_sign_constant(&__do_init, ptrauth_key_init_fini_pointer,
ptrauth_blend_discriminator(
&__init, __ptrauth_init_fini_discriminator));
-# else
+#else
__attribute__((section(".init_array"), used)) static void *__init =
ptrauth_sign_constant(&__do_init, ptrauth_key_init_fini_pointer,
__ptrauth_init_fini_discriminator);
-# endif
-# elif __has_feature(ptrauth_calls)
-# ifdef __aarch64__
+#endif
+#elif __has_feature(ptrauth_calls)
+#ifdef __aarch64__
// If ptrauth_init_fini feature is not present, compiler emits raw unsigned
// pointers in .init_array. Use inline assembly to avoid implicit signing of
// __do_init function pointer with ptrauth_calls enabled.
__asm__(".pushsection .init_array,\"aw\",@init_array\n\t"
".xword __do_init\n\t"
".popsection");
-# else
-# error "ptrauth_calls is only supported for AArch64"
-# endif
-# else
+#else
+#error "ptrauth_calls is only supported for AArch64"
+#endif
+#else
__attribute__((section(".init_array"),
used)) static void (*__init)(void) = __do_init;
-# endif
+#endif
#elif defined(__i386__) || defined(__x86_64__)
__asm__(".pushsection .init,\"ax\",@progbits\n\t"
"call __do_init\n\t"
@@ -136,33 +136,33 @@ static void __attribute__((used)) __do_fini(void) {
}
#ifdef CRT_HAS_INITFINI_ARRAY
-# if __has_feature(ptrauth_init_fini)
+#if __has_feature(ptrauth_init_fini)
// TODO: use __ptrauth-qualified pointers when they are supported on clang side
-# if __has_feature(ptrauth_init_fini_address_discrimination)
+#if __has_feature(ptrauth_init_fini_address_discrimination)
__attribute__((section(".fini_array"), used)) static void *__fini =
ptrauth_sign_constant(&__do_fini, ptrauth_key_init_fini_pointer,
ptrauth_blend_discriminator(
&__fini, __ptrauth_init_fini_discriminator));
-# else
+#else
__attribute__((section(".fini_array"), used)) static void *__fini =
ptrauth_sign_constant(&__do_fini, ptrauth_key_init_fini_pointer,
__ptrauth_init_fini_discriminator);
-# endif
-# elif __has_feature(ptrauth_calls)
-# ifdef __aarch64__
+#endif
+#elif __has_feature(ptrauth_calls)
+#ifdef __aarch64__
// If ptrauth_init_fini feature is not present, compiler emits raw unsigned
// pointers in .fini_array. Use inline assembly to avoid implicit signing of
// __do_fini function pointer with ptrauth_calls enabled.
__asm__(".pushsection .fini_array,\"aw\",@fini_array\n\t"
".xword __do_fini\n\t"
".popsection");
-# else
-# error "ptrauth_calls is only supported for AArch64"
-# endif
-# else
+#else
+#error "ptrauth_calls is only supported for AArch64"
+#endif
+#else
__attribute__((section(".fini_array"),
used)) static void (*__fini)(void) = __do_fini;
-# endif
+#endif
#elif defined(__i386__) || defined(__x86_64__)
__asm__(".pushsection .fini,\"ax\",@progbits\n\t"
"call __do_fini\n\t"
|
|
While clang-format complains about custom macro indents, I suppose it's worth keep them to enhance readability in nested ifdefs. |
When `ptrauth_calls` is present but `ptrauth_init_fini` is not, compiler emits raw unsigned pointers in `.init_array`/`.fini_array` sections. Previously, `__do_init`/`__do_fini` pointers in these sections were implicitly signed (due to the presense of `ptrauth_calls`). As a result, the sections contained a mix of unsigned function pointers and function pointers signed with default signing schema. This patch introduces use of inline assembly for this particular case, so we can manually specify that we do not want to sign the pointers. Note that we cannot use `__builtin_ptrauth_strip` for this purpose since its result is not a constant expression.
a049b76 to
919c7df
Compare
atrosinenko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure whether fixing clang-format violations is strictly required in the builtins library (by the way, even in this file I can see an indented define, though only a single line), but in other respects this patch looks good to me.
When
ptrauth_callsis present butptrauth_init_finiis not, compiler emits raw unsigned pointers in.init_array/.fini_arraysections. Previously,__do_init/__do_finipointers in these sections were implicitly signed (due to the presense ofptrauth_calls).
Considering the description, it feels like it could be made a bit easier to understand by mentioning that __do_init and __do_fini pointers are explicitly added to these sections - that's why they ended up being implicitly signed while every other pointer was non-signed.
|
/cherry-pick 19ba224 |
|
/pull-request #151096 |
When `ptrauth_calls` is present but `ptrauth_init_fini` is not, compiler emits raw unsigned pointers in `.init_array`/`.fini_array` sections. Previously, `__do_init`/`__do_fini` pointers, which are explicitly added to the sections, were implicitly signed (due to the presense of `ptrauth_calls`), while all the other pointers in the sections were implicitly added by the compiler and thus non-signed.. As a result, the sections contained a mix of unsigned function pointers and function pointers signed with default signing schema. This patch introduces use of inline assembly for this particular case, so we can manually specify that we do not want to sign the pointers. Note that we cannot use `__builtin_ptrauth_strip` for this purpose since its result is not a constant expression. (cherry picked from commit 19ba224)
When
ptrauth_callsis present butptrauth_init_finiis not, compiler emits raw unsigned pointers in.init_array/.fini_arraysections. Previously,__do_init/__do_finipointers, which are explicitly added to the sections, were implicitly signed (due to the presense ofptrauth_calls), while all the other pointers in the sections were implicitly added by the compiler and thus non-signed.. As a result, the sections contained a mix of unsigned function pointers and function pointers signed with default signing schema.This patch introduces use of inline assembly for this particular case, so we can manually specify that we do not want to sign the pointers.
Note that we cannot use
__builtin_ptrauth_stripfor this purpose since its result is not a constant expression.