[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: I think polluting space with `__has_feature` is the wrong approach, will gate on `__PTRAUTH__ || __arm64e__` if you can verify that `__PTRAUTH__` is defined for you folk when building the .S files https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: Will fix momentarily. I guess gcc does not have `__has_feature`, so I'll look at how libunwind usually does this. We may just gate on the AARCH_PTRAUTH flag instead. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/ojhunt closed https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 approved this pull request. LGTM, thanks! https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -22,7 +22,6 @@ #include "dwarf2.h" #include "libunwind_ext.h" - kovdan01 wrote: Nit: unintended formatting change https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -118,22 +118,62 @@ _LIBUNWIND_HIDDEN int __unw_set_reg(unw_cursor_t *cursor,
unw_regnum_t regNum,
typedef LocalAddressSpace::pint_t pint_t;
AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
if (co->validReg(regNum)) {
-co->setReg(regNum, (pint_t)value);
// special case altering IP to re-find info (being called by personality
// function)
if (regNum == UNW_REG_IP) {
unw_proc_info_t info;
// First, get the FDE for the old location and then update it.
co->getInfo(&info);
- co->setInfoBasedOnIPRegister(false);
+
+ pint_t sp = (pint_t)co->getReg(UNW_REG_SP);
+
+#if defined(_LIBUNWIND_TARGET_AARCH64_AUTHENTICATED_UNWINDING)
+ {
+// It is only valid to set the IP within the current function.
+// This is important for ptrauth, otherwise the IP cannot be correctly
+// signed.
+// We re-sign to a more usable form and then use it directly.
+union {
kovdan01 wrote:
Please avoid using unions here as well since reading from `authenticated_value`
while `opaque_value` was the last assigned member is UB in C++.
I've prepared a fix which works on my side. You are welcome to just apply that
if you are happy with the fix implementation. See commit
a29af825c71d70e83445cd4214f7145642201506 from my branch
[ptrauth-unwinding-2025-10-19](https://github.com/kovdan01/llvm-project/commits/ptrauth-unwinding-2025-10-19/)
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 edited https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 requested changes to this pull request. @ojhunt Thanks for tons of fixes! We have one more usage of unions in libunwind.cpp which is technically UB. I've proposed a fix for this. See commit a29af825c71d70e83445cd4214f7145642201506 from my branch [ptrauth-unwinding-2025-10-19](https://github.com/kovdan01/llvm-project/commits/ptrauth-unwinding-2025-10-19/) As soon as this is addressed, I have no other objections from my side regarding this PR. Thanks for a great work! https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -597,6 +606,18 @@ _LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct
_Unwind_Context *context) {
unw_cursor_t *cursor = (unw_cursor_t *)context;
unw_word_t result;
__unw_get_reg(cursor, UNW_REG_IP, &result);
+
+#if defined(__ARM64E__)
ojhunt wrote:
Righto, I thought this was one of the arm64e specific behaviors
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 edited https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 edited https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 requested changes to this pull request. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -90,6 +90,18 @@
} while (0)
#endif
+// There is not currently a clean way to cast between an authenticated
+// integer and an authenticated function pointer, so we need this helper
+// function to keep things clean.
+static _Unwind_Personality_Fn get_handler_function(unw_proc_info_t *frameInfo)
{
+ union {
+void *opaque_handler;
+_Unwind_Personality_Fn __ptrauth_unwind_upi_handler *handler;
+ } u;
+ u.opaque_handler = (void *)&frameInfo->handler;
+ return *u.handler;
ojhunt wrote:
The alternative is to force a copy + re-sign
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -30,6 +30,45 @@ EXCEPTION_DISPOSITION _GCC_specific_handler(PEXCEPTION_RECORD, void *, PCONTEXT, _Unwind_Personality_Fn); #endif +#if __has_feature(ptrauth_qualifier) +#include +#if __has_feature(ptrauth_restricted_intptr_qualifier) ojhunt wrote: > 1. (To be fixed in scope of this PR) Comment in this PR about > `ptrauth_restricted_intptr_qualifier`: TODO No, that renders the library unbuildable in shipping apple clang. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -47,10 +47,10 @@ struct _LIBCXXABI_HIDDEN __cxa_exception {
// In Wasm, a destructor returns its argument
void *(_LIBCXXABI_DTOR_FUNC *exceptionDestructor)(void *);
#else
-void (_LIBCXXABI_DTOR_FUNC *exceptionDestructor)(void *);
+void(_LIBCXXABI_DTOR_FUNC *__ptrauth_cxxabi_exception_destructor
exceptionDestructor)(void*);
ojhunt wrote:
sigh I thought clang-format + checking/adjustment had actually got these ones
right
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: > @ojhunt Thanks for addressing the issues! > > I've run tests on our side and I only see 2 issues at this point, and both of > them seem to be fixed trivially. See my branch > https://github.com/kovdan01/llvm-project/commits/pointer-authenticated-unwinding-fix-2025-10-14/ > > 1. You use `ptrauth_function_pointer_type_discriminator` in UnwindLevel1.c, > but it looks like this is not defined anywhere, which causes compile error. > Please add a definition or just use `ptrauth_type_discriminator`. For my > local tests, I've switched to `ptrauth_type_discriminator` to avoid compiler > error - see commit > [kovdan01@7a43f48](https://github.com/kovdan01/llvm-project/commit/7a43f48bdac2bba68fc9f392361b44d586ca06d9) Indeed you're correct - I've created https://github.com/llvm/llvm-project/pull/163456 > 2. In `get_handler_function`, you assume that function pointer is signed with > type discrimination enabled. This is not necessarily true, for example, this > is not included in pauthtest ABI. Please add a corresponding check against > `#if __has_feature(ptrauth_function_pointer_type_discrimination)` - see > commit > [kovdan01@57c4673](https://github.com/kovdan01/llvm-project/commit/57c46737ba26b28010443b66ea24e1a3dc6e9890) > `ptrauth_function_pointer_type_discriminator` does the correct thing here - that's why it exists :D > When these issues are resolved, this PR LGTM. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -79,16 +79,18 @@ struct _LIBCXXABI_HIDDEN __cxa_exception {
// http://sourcery.mentor.com/archives/cxx-abi-dev/msg01924.html
// The layout of this structure MUST match the layout of __cxa_exception, with
// primaryException instead of referenceCount.
+// The tags used in the pointer authentication qualifiers also need to match
+// those of the corresponding members in __cxa_exception.
struct _LIBCXXABI_HIDDEN __cxa_dependent_exception {
#if defined(__LP64__) || defined(_WIN64) || defined(_LIBCXXABI_ARM_EHABI)
void* reserve; // padding.
void* primaryException;
#endif
std::type_info *exceptionType;
-void (_LIBCXXABI_DTOR_FUNC *exceptionDestructor)(void *);
-std::unexpected_handler unexpectedHandler;
-std::terminate_handler terminateHandler;
+void(_LIBCXXABI_DTOR_FUNC *__ptrauth_cxxabi_exception_destructor
exceptionDestructor)(void*);
kovdan01 wrote:
Nit: revert unintended formatting change
```suggestion
void (_LIBCXXABI_DTOR_FUNC *__ptrauth_cxxabi_exception_destructor
exceptionDestructor)(void *);
```
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -597,6 +606,18 @@ _LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct
_Unwind_Context *context) {
unw_cursor_t *cursor = (unw_cursor_t *)context;
unw_word_t result;
__unw_get_reg(cursor, UNW_REG_IP, &result);
+
+#if defined(__ARM64E__)
kovdan01 wrote:
It looks like you've accidentally changes this from a generic check against
ptrauth being enabled to a check against arm64e. To make tests passing on
Linux, we need to use smth like
`_LIBUNWIND_TARGET_AARCH64_AUTHENTICATED_UNWINDING`.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -557,7 +596,19 @@ set_registers(_Unwind_Exception* unwind_exception, _Unwind_Context* context, reinterpret_cast(unwind_exception)); _Unwind_SetGR(context, __builtin_eh_return_data_regno(1), static_cast(results.ttypeIndex)); +#if defined(__APPLE__) && __has_feature(ptrauth_qualifier) + auto stack_pointer = _Unwind_GetGR(context, UNW_REG_SP); + // We manually re-sign the IP as the __ptrauth qualifiers cannot + // express the required relationship with the destination address + const auto existingDiscriminator = ptrauth_blend_discriminator( + &results.landingPad, ptrauth_string_discriminator(_LIBCXXABI_PTRAUTH_SCANRESULT_LANDINGPAD_DISC)); + unw_word_t newIP = + (unw_word_t)ptrauth_auth_and_resign(*(void**)&results.landingPad, _LIBCXXABI_PTRAUTH_KEY, existingDiscriminator, + ptrauth_key_return_address, stack_pointer); + _Unwind_SetIP(context, newIP); +#else atrosinenko wrote: Sorry, it was a bad idea to refer to a *generic comment* on the particular file - the comment I referred to is https://github.com/llvm/llvm-project/pull/143230#discussion_r2155220556 and it is not currently shown by Github in the "Files changed" view :) https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
kovdan01 wrote: As I mentioned in our prior discussion, just putting old signed RA handling logic under `!defined(LIBUNWIND_PTRAUTH_CALLS_AND_RETURNS)` is fine - see my [explanation](https://github.com/llvm/llvm-project/commit/e2f8b9d3859eff96442ce04662aefb40debbef3f#r165673683) (I kindly ask you to continue discussion here since it's easy to lose comments in individual commits). Also, I've ensured that pac-ret remains working properly after applying this patch and my proposed fix ced8b99373c9b0756f171896f44a74bdf46d. Particularly: 1. Mainline llvm, **mainline** runtime libraries built **with** pac-ret, user code built with pac-ret - unwinding works fine (`RA_SIGN_STATE` detected in all stack frames) 2. Mainline llvm, **mainline** runtime libraries built **without** pac-ret, user code built with pac-ret - unwinding works fine (`RA_SIGN_STATE` detected in user stack frames and not detected in stack frames of runtime libraries) 3. Mainline llvm, **mainline + this PR + my fix** runtime libraries built **with** pac-ret, user code built with pac-ret - unwinding works fine (`RA_SIGN_STATE` detected in all stack frames) 4. Mainline llvm, **mainline + this PR + my fix** runtime libraries built **without** pac-ret, user code built with pac-ret - unwinding works fine (`RA_SIGN_STATE` detected in user stack frames and not detected in stack frames of runtime libraries) So, the proposed change ced8b99373c9b0756f171896f44a74bdf46d does not break anything. Those using pac-ret instead of full arm64e/pauthtest would not even notice something changed. P.S. In future, we want to unify libunwind logic for handling signed RA. I've created a task for this: #160110. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
kovdan01 wrote: @ojhunt Thanks for an update! I've resolved several threads, but some things should still be fixed. Please let me know if you have any comments/questions or any help is needed from my side. ### Trivial style-related issues These require almost zero effort to apply. Please consider fixing these first so we do not need to worry about them anymore. 1. https://github.com/llvm/llvm-project/pull/143230#discussion_r2388284017 2. https://github.com/llvm/llvm-project/pull/143230#discussion_r2388324262 3. https://github.com/llvm/llvm-project/pull/143230#discussion_r2388325718 ### Non-trivial styling-related issues Refactor preprocessor checks against ptrauth. See thread https://github.com/llvm/llvm-project/pull/143230#discussion_r2369029642 and proposed fix 644405b56cfa59dd3787119182df087dff6e756c ### Breaking change to be reverted > > ### An issue causing a runtime crash > > Do not mix pac-ret and ptrauth_returns. See thread > > https://github.com/llvm/llvm-project/pull/143230/files#r2369419226 and > > commit > > [ced8b99](https://github.com/llvm/llvm-project/commit/ced8b99373c9b0756f171896f44a74bdf46d). > > Fixed with an arm64e guard. This does not seem to fix the originally described issue, moreover, it looks like this breaks previously working behavior. See https://github.com/llvm/llvm-project/pull/143230#discussion_r2388356981. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -207,7 +208,8 @@ bool DwarfInstructions::isReturnAddressSignedWithPC(A &addressSpace, #endif template -int DwarfInstructions::stepWithDwarf(A &addressSpace, pint_t pc, +int DwarfInstructions::stepWithDwarf(A &addressSpace, + typename R::link_reg_t &pc, kovdan01 wrote: ```suggestion typename const R::link_reg_t &pc, ``` https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -300,12 +302,12 @@ int DwarfInstructions::stepWithDwarf(A &addressSpace, pint_t pc, isSignalFrame = cieInfo.isSignalFrame; -#if defined(_LIBUNWIND_TARGET_AARCH64) - // If the target is aarch64 then the return address may have been signed - // using the v8.3 pointer authentication extensions. The original - // return address needs to be authenticated before the return address is - // restored. autia1716 is used instead of autia as autia1716 assembles - // to a NOP on pre-v8.3a architectures. +#if defined(__ARM64E__) kovdan01 wrote: I've not done local testing with the latest version of the PR, but it looks like that this changes implements *exactly* the opposite behavior of what we are trying to achieve :) The code under this conditional serves for handling LR signed with pac-ret. And we actually need this to be guarded with smth like `#ifndef LIBUNWIND_PTRAUTH_CALLS_AND_RETURNS` to avoid executing this code when using Apple's arm64e/Linux's pauthtest (because this way, LR is signed as part of corresponding ABI and not by pac-ret). If we put this under `#if defined(__ARM64E__)`, we entirely disable pac-ret handling for non-arm64e. This would break things which were working as intended before this change. See my comment https://github.com/llvm/llvm-project/pull/143230#discussion_r2369419226 and commit with proposed fix ced8b99373c9b0756f171896f44a74bdf46d Please let me know if any help is needed with fixing the issue. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -313,6 +314,18 @@ bool CFI_Parser::findFDE(A &addressSpace, pint_t pc,
pint_t ehSectionStart,
}
return false;
}
+namespace {
+// This helper function handles setting the manually signed personality on
+// CIE_Info without attempt to authenticate and/or re-sign
+template
+[[maybe_unused]] void set_cie_info_personality(CIE_Info *info,
+ T signed_personality) {
+ static_assert(sizeof(info->personality) == sizeof(signed_personality),
+"Signed personality is the wrong size");
+ memmove((void *)&info->personality, (void *)&signed_personality,
kovdan01 wrote:
> Hmmm, other platforms haven't simply adopted the "handle overlap correctly
> everywhere" approach? :-/
@ojhunt I honestly can't say for all the platforms, but I suppose that `memcpy`
expresses the intention better than `memmove` here
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: > The patch looks fine to me but you should finish addressing comments by > @kovdan01 . > > I wonder, however, what kind of test coverage we have for your changes. Do we > have any? In the current CI, are we even exercising these changes? These changes do not effect behavior if there is no pointer authentication - if there is pointer authentication then these have to be consistent with the host platform so I'm not sure how to test conformance without adding what are in essence "expected fail" tests where we construct intentionally incorrect frames and attempt to unwind through them. Having this upstreamed would permitted arm64e apple silicon test bots to run which might be useful? https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
kovdan01 wrote: @ojhunt I closed all the threads which are no longer relevant or are duplicates. Now we have only 5 non-trivial and 3 trivial issues which really **need** to be resolved before merging the PR. All other enhancements could be done as follow-up patches. I've created [pointer-authenticated-unwinding-2025-09-22-with-fixes](https://github.com/kovdan01/llvm-project/commits/pointer-authenticated-unwinding-2025-09-22-with-fixes/) in my fork containing fixes for all the non-trivial issues listed below. Commit links are provided in the list below and in corresponding threads. Please let me know if you have any questions or any help is needed regarding this. ## To be done in scope of this PR ### Trivial formatting issues These require almost zero effort to apply. Please consider fixing these first so we do not need to worry about them anymore. 1. https://github.com/llvm/llvm-project/pull/143230/files#r2369166326 2. https://github.com/llvm/llvm-project/pull/143230/files#r2369411804 3. https://github.com/llvm/llvm-project/pull/143230/files#r2369413223 ### Non-trivial styling-related issues 1. Refactor preprocessor checks against ptrauth. See thread https://github.com/llvm/llvm-project/pull/143230/files#r2369029642 and commit 644405b56cfa59dd3787119182df087dff6e756c 2. Avoid use of `__ptrauth_restricted_intptr` which is not present in mainline. See thread https://github.com/llvm/llvm-project/pull/143230/files#r2369036612 and commit a2390e1e285023af78d27d768540bb8f30efea76. I do get the point that it's needed in downstream, but I suppose that it's better to avoid exposing downstream-specific stuff to mainline. I suppose that you can use a simple downstream patch over mainline libunwind which would resolve the build issue for you. 3. Do smth with `[[maybe_unused]]` attribute which only serves as a workaround for a spurious compiler bug. See thread https://github.com/llvm/llvm-project/pull/143230/files#r2368805350 and commit 4eaa8c7dff48bfc96b071ab0e219369839e3758c ### An issue causing a runtime crash Do not mix pac-ret and ptrauth_returns. See thread https://github.com/llvm/llvm-project/pull/143230/files#r2369419226 and commit ced8b99373c9b0756f171896f44a74bdf46d. ### Other non-trivial issues Verify FP is handled correctly https://github.com/llvm/llvm-project/pull/143230/files#r2369428305 ## To be done as a follow-up 1. https://github.com/llvm/llvm-project/issues/96528#issuecomment-3320192622 2. https://github.com/llvm/llvm-project/issues/160101 3. https://github.com/llvm/llvm-project/issues/160110 4. https://github.com/llvm/llvm-project/issues/160114 5. https://github.com/llvm/llvm-project/issues/160117 6. https://github.com/llvm/llvm-project/issues/160119 7. https://github.com/llvm/llvm-project/issues/160120 https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
kovdan01 wrote: @ojhunt Thanks for an update! With the latest fixes, we only have one trivial issue which prevents Linux tests from passing (and I believe that it was introduced accidentally and it looks like that we need literally zero effort to fix that). With that being fixed, tests are passing on Linux in the following configurations: 1. Pauthtest for both runtime libraries and executables 2. Pac-ret for both runtime libraries and executables 3. Pac-ret for executables but not runtime libraries 4. No pauth at all (on aarch64) Besides that, there are several other small issues here and there. See the list below. Please let me know if any help with fixing these issues is needed - I would be glad to provide any support. 1. Use a check against `_LIBUNWIND_TARGET_AARCH64_AUTHENTICATED_UNWINDING` against a check against arm64e: https://github.com/llvm/llvm-project/pull/143230#discussion_r2407141767 This is the only thing which prevents linux tests from passing. 2. Avoid UB in `get_handler_function`: https://github.com/llvm/llvm-project/pull/143230#discussion_r2406229405 Similar issue should also be addressed in https://github.com/llvm/llvm-project/pull/161027#discussion_r2388230955 3. It looks like that while `_LIBUNWIND_TARGET_AARCH64_AUTHENTICATED_UNWINDING` macro was added (and it's used in some places), in most cases we still have plain checks against `__has_feature(ptrauth_calls)` and `__has_feature(ptrauth_returns)`. As discussed earlier, it's better to use this macro for such checks. See thread https://github.com/llvm/llvm-project/pull/143230#discussion_r2369029642 and proposed fix [644405b](https://github.com/llvm/llvm-project/commit/644405b56cfa59dd3787119182df087dff6e756c) 4. Add a comment explaining conditionals regarding `__ptrauth_restricted_intptr` qualifier to make it clearer for thouse not familiar with Apple's clang specific. I've provided an example comments, see https://github.com/llvm/llvm-project/pull/143230#discussion_r2407114846 5. `__ptrauth_unwind_pacret_personality_disc` should probably be renamed to `__ptrauth_unwind_pauthtest_personality_disc`. It's clearly not related to pac-ret which is about LR signing only. See https://github.com/llvm/llvm-project/pull/143230#discussion_r2406175800 Looking forward for further updates and hope that this could be finally merged relatively soon :) https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: Remaining formatting changes differ significantly from the surrounding code. I don't want to intermingle significant changes with formatting changes. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -90,6 +90,18 @@
} while (0)
#endif
+// There is not currently a clean way to cast between an authenticated
+// integer and an authenticated function pointer, so we need this helper
+// function to keep things clean.
+static _Unwind_Personality_Fn get_handler_function(unw_proc_info_t *frameInfo)
{
+ union {
+void *opaque_handler;
+_Unwind_Personality_Fn __ptrauth_unwind_upi_handler *handler;
+ } u;
+ u.opaque_handler = (void *)&frameInfo->handler;
+ return *u.handler;
kovdan01 wrote:
While this should be valid in C, in C++ reading from the member of the union
that wasn't most recently written is technically UB:
https://en.cppreference.com/w/cpp/language/union.html.
I think we need to conform to C++ standard and use smth like `memcpy` for doing
such a trick.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -541,7 +583,38 @@ struct scan_results
};
} // unnamed namespace
+} // extern "C"
+
+#if !defined(_LIBCXXABI_ARM_EHABI)
+namespace {
+// The logical model for casting authenticated function pointers makes
+// it impossible to directly cast them without breaking the authentication,
+// as a result we need this pair of helpers.
+//
+// __ptrauth_nop_cast cannot be used here as the authentication schemas include
+// address diversification.
+template
+void set_landing_pad_as_ptr(scan_results& results, const PtrType& out) {
kovdan01 wrote:
I'll open a "placeholder" thread just as a reminder that we need to get #161027
with additional macros merged first, and make use of those macros instead of
these functions.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: gnahh why is [a9d9113](https://github.com/llvm/llvm-project/pull/143230/commits/a9d911322fe3020fd098ff0d6cbf6745b66b93e6) apparently fixing the armv8 build? That implies something it assuming Arm64Registers is a POD type, and I can't work out where :-/ https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -90,6 +90,18 @@
} while (0)
#endif
+// There is not currently a clean way to cast between an authenticated
+// integer and an authenticated function pointer, so we need this helper
+// function to keep things clean.
+static _Unwind_Personality_Fn get_handler_function(unw_proc_info_t *frameInfo)
{
+ union {
+void *opaque_handler;
+_Unwind_Personality_Fn __ptrauth_unwind_upi_handler *handler;
+ } u;
+ u.opaque_handler = (void *)&frameInfo->handler;
+ return *u.handler;
ojhunt wrote:
memcpy cannot be used -> these are address discriminated values, the entire
reason for these hoops is to deal with this problem.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -541,7 +583,38 @@ struct scan_results
};
} // unnamed namespace
+} // extern "C"
+
+#if !defined(_LIBCXXABI_ARM_EHABI)
+namespace {
+// The logical model for casting authenticated function pointers makes
+// it impossible to directly cast them without breaking the authentication,
+// as a result we need this pair of helpers.
+//
+// __ptrauth_nop_cast cannot be used here as the authentication schemas include
+// address diversification.
+template
+void set_landing_pad_as_ptr(scan_results& results, const PtrType& out) {
ojhunt wrote:
none of those macros work -> these are address discriminated values. That's
also why memcpy does not work.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -43,6 +43,104 @@
#define LIBUNWIND_AVAIL
#endif
+#if __has_feature(ptrauth_calls)
+
+ #include
+
+ #if __has_extension(ptrauth_restricted_intptr_qualifier)
+#define __unwind_ptrauth_restricted_intptr(...) \
+ __ptrauth_restricted_intptr(__VA_ARGS__)
+ #else
+#define __unwind_ptrauth_restricted_intptr(...) \
+ __ptrauth(__VA_ARGS__)
+ #endif
+
+// ptrauth_string_discriminator("unw_proc_info_t::handler") == 0x7405
+ #define __ptrauth_unwind_upi_handler_disc 0x7405
+
+ #define __ptrauth_unwind_upi_handler \
+__ptrauth(ptrauth_key_function_pointer, 1,
__ptrauth_unwind_upi_handler_disc)
+
+ #define __ptrauth_unwind_upi_handler_intptr \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_function_pointer, 1,\
+ __ptrauth_unwind_upi_handler_disc)
+
+// ptrauth_string_discriminator("unw_proc_info_t::start_ip") == 0xCA2C
+ #define __ptrauth_unwind_upi_startip \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_process_independent_code,
1, 0xCA2C)
+
+// ptrauth_string_discriminator("unw_proc_info_t::end_ip") == 0xE183
+ #define __ptrauth_unwind_upi_endip \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_process_independent_code,
1, 0xE183)
+
+// ptrauth_string_discriminator("unw_proc_info_t::lsda") == 0x83DE
+ #define __ptrauth_unwind_upi_lsda \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_process_dependent_data, 1,
0x83DE)
+
+// ptrauth_string_discriminator("unw_proc_info_t::flags") == 0x79A1
+ #define __ptrauth_unwind_upi_flags \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_process_dependent_data, 1,
0x79A1)
+
+// ptrauth_string_discriminator("unw_proc_info_t::unwind_info") == 0xC20C
+ #define __ptrauth_unwind_upi_info \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_process_dependent_data, 1,
0xC20C)
+
+// ptrauth_string_discriminator("unw_proc_info_t::extra") == 0x03DF
+ #define __ptrauth_unwind_upi_extra \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_process_dependent_data, 1,
0x03DF)
+
+// ptrauth_string_discriminator("Registers_arm64::link_reg_t") == 0x8301
+ #define __ptrauth_unwind_registers_arm64_link_reg \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_process_dependent_code, 1,
0x8301)
+
+// ptrauth_string_discriminator("UnwindInfoSections::dso_base") == 0x4FF5
+ #define __ptrauth_unwind_uis_dso_base \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_process_dependent_data, 1,
0x4FF5)
+
+// ptrauth_string_discriminator("UnwindInfoSections::dwarf_section") == 0x4974
+ #define __ptrauth_unwind_uis_dwarf_section \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_process_dependent_data, 1,
0x4974)
+
+// ptrauth_string_discriminator("UnwindInfoSections::dwarf_section_length") ==
0x2A9A
+ #define __ptrauth_unwind_uis_dwarf_section_length \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_process_dependent_data, 1,
0x2A9A)
+
+// ptrauth_string_discriminator("UnwindInfoSections::compact_unwind_section")
== 0xA27B
+ #define __ptrauth_unwind_uis_compact_unwind_section \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_process_dependent_data, 1,
0xA27B)
+
+//
ptrauth_string_discriminator("UnwindInfoSections::compact_unwind_section_length")
== 0x5D0A
+ #define __ptrauth_unwind_uis_compact_unwind_section_length \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_process_dependent_data, 1,
0x5D0A)
+
+// ptrauth_string_discriminator("CIE_Info::personality") == 0x6A40
+ #define __ptrauth_unwind_cie_info_personality_disc 0x6A40
+ #define __ptrauth_unwind_cie_info_personality \
+__unwind_ptrauth_restricted_intptr(ptrauth_key_function_pointer, 1, \
+
__ptrauth_unwind_cie_info_personality_disc)
+
+// ptrauth_string_discriminator("personality") == 0x7EAD)
+ #define __ptrauth_unwind_pacret_personality_disc 0x7EAD
kovdan01 wrote:
I suppose the name is a bit misleading. Let's use smth like the following:
```suggestion
#define __ptrauth_unwind_pauthtest_personality_disc 0x7EAD
```
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -43,6 +43,104 @@ #define LIBUNWIND_AVAIL #endif +#if __has_feature(ptrauth_calls) + + #include + + #if __has_extension(ptrauth_restricted_intptr_qualifier) kovdan01 wrote: Let's add a comment explaining that we need that because of Apple's fork so people unaware of the problem would know why such checks are needed. The comment might look like this: ``` // In Apple's fork of clang, `__ptrauth` qualifier can be only applied to // pointer types, and `__ptrauth_restricted_intptr` is used instead for // integer types. In mainline clang, we only have `__ptrauth` which could // be applied both to pointer and integer types. // These helper macros allow seemless build of runtime libraries with // Apple's fork of clang. // TODO: If/when Apple's clang switches to mainline way of using // `__ptrauth` qualifier, get rid of these conditionals. ``` https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -313,6 +314,18 @@ bool CFI_Parser::findFDE(A &addressSpace, pint_t pc,
pint_t ehSectionStart,
}
return false;
}
+namespace {
+// This helper function handles setting the manually signed personality on
+// CIE_Info without attempt to authenticate and/or re-sign
+template
+[[maybe_unused]] void set_cie_info_personality(CIE_Info *info,
+ T signed_personality) {
+ static_assert(sizeof(info->personality) == sizeof(signed_personality),
+"Signed personality is the wrong size");
+ memmove((void *)&info->personality, (void *)&signed_personality,
kovdan01 wrote:
Nit: I guess we can just use `memcpy` since `memmove` has additional overhead
of handling overlapping memory regions. Here, `signed_personality` is a local
variable, so we can guarantee that there is no overlapping.
Also, it's probably worth explicitly including `` (well, ``
should be better for C++ headers, but we already have includes for C-style
headers, so let's stick with existing conventions)
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: > @ojhunt Thanks for an update! I've resolved several threads, but some things > should still be fixed. > > Please let me know if you have any comments/questions or any help is needed > from my side. > > ### Trivial style-related issues > These require almost zero effort to apply. Please consider fixing these first > so we do not need to worry about them anymore. > > 1. [[runtimes][PAC] Harden unwinding when possible #143230 > (comment)](https://github.com/llvm/llvm-project/pull/143230#discussion_r2388284017) > 2. [[runtimes][PAC] Harden unwinding when possible #143230 > (comment)](https://github.com/llvm/llvm-project/pull/143230#discussion_r2388324262) > 3. [[runtimes][PAC] Harden unwinding when possible #143230 > (comment)](https://github.com/llvm/llvm-project/pull/143230#discussion_r2388325718) I think fixed? > > ### Non-trivial styling-related issues > Refactor preprocessor checks against ptrauth. See thread [#143230 > (comment)](https://github.com/llvm/llvm-project/pull/143230#discussion_r2369029642) > and proposed fix > [644405b](https://github.com/llvm/llvm-project/commit/644405b56cfa59dd3787119182df087dff6e756c) That sounds ok > > ### Breaking change to be reverted > > > ### An issue causing a runtime crash > > > Do not mix pac-ret and ptrauth_returns. See thread > > > https://github.com/llvm/llvm-project/pull/143230/files#r2369419226 and > > > commit > > > [ced8b99](https://github.com/llvm/llvm-project/commit/ced8b99373c9b0756f171896f44a74bdf46d). > > > > > > Fixed with an arm64e guard. > > This does not seem to fix the originally described issue, moreover, it looks > like this breaks previously working behavior. See [#143230 > (comment)](https://github.com/llvm/llvm-project/pull/143230#discussion_r2388356981). Minor detail :D I think fixed correctly now (based on what you posted) but I need to verify the correct thing happens on arm64e (this will take a few days). There's still the ifdef'd definition of the helper function in the current version, but I'll move that around in a bit. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -47,10 +47,10 @@ struct _LIBCXXABI_HIDDEN __cxa_exception {
// In Wasm, a destructor returns its argument
void *(_LIBCXXABI_DTOR_FUNC *exceptionDestructor)(void *);
#else
-void (_LIBCXXABI_DTOR_FUNC *exceptionDestructor)(void *);
+void(_LIBCXXABI_DTOR_FUNC *__ptrauth_cxxabi_exception_destructor
exceptionDestructor)(void*);
kovdan01 wrote:
Nit: revert unintended formatting change
```suggestion
void (_LIBCXXABI_DTOR_FUNC *__ptrauth_cxxabi_exception_destructor
exceptionDestructor)(void *);
```
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 requested changes to this pull request. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: > @ojhunt I closed all the threads which are no longer relevant or are > duplicates. 👍 > ## To be done in scope of this PR > ### Trivial formatting issues > These require almost zero effort to apply. Please consider fixing these first > so we do not need to worry about them anymore. Done > > ### Non-trivial styling-related issues > 1. Refactor preprocessor checks against ptrauth. See thread > https://github.com/llvm/llvm-project/pull/143230/files#r2369029642 and commit > [644405b](https://github.com/llvm/llvm-project/commit/644405b56cfa59dd3787119182df087dff6e756c) > 2. Avoid use of `__ptrauth_restricted_intptr` which is not present in > mainline. See thread > https://github.com/llvm/llvm-project/pull/143230/files#r2369036612 and commit > [a2390e1](https://github.com/llvm/llvm-project/commit/a2390e1e285023af78d27d768540bb8f30efea76). > I do get the point that it's needed in downstream, but I suppose that it's > better to avoid exposing downstream-specific stuff to mainline. I suppose > that you can use a simple downstream patch over mainline libunwind which > would resolve the build issue for you. That would mean that apple clang will not be able to build the runtimes. That's not a real option. > 3. Do smth with `[[maybe_unused]]` attribute which only serves as a > workaround for a spurious compiler bug. See thread > https://github.com/llvm/llvm-project/pull/143230/files#r2368805350 and commit > [4eaa8c7](https://github.com/llvm/llvm-project/commit/4eaa8c7dff48bfc96b071ab0e219369839e3758c) I've moved this behind a matching #define of the code that calls them, I'm trying to work out if there is a real reason for the functions being separated from their usage as much as they are. > ### An issue causing a runtime crash > Do not mix pac-ret and ptrauth_returns. See thread > https://github.com/llvm/llvm-project/pull/143230/files#r2369419226 and commit > [ced8b99](https://github.com/llvm/llvm-project/commit/ced8b99373c9b0756f171896f44a74bdf46d). Fixed with an arm64e guard. > ### Other non-trivial issues > Verify FP is handled correctly > https://github.com/llvm/llvm-project/pull/143230/files#r2369428305 It is, I've fixed the misleading - we were experimenting with signed frame pointers for a while. > ## To be done as a follow-up > 1. [[PAC] Update pointer authentication docs #96528 > (comment)](https://github.com/llvm/llvm-project/issues/96528#issuecomment-3320192622) > 2. [[PAC][libunwind] Refactor way of describing internal signing schemas > #160101](https://github.com/llvm/llvm-project/issues/160101) > 3. [[PAC][libunwind] Unify signed return address handling logic > #160110](https://github.com/llvm/llvm-project/issues/160110) > 4. [[PAC][libunwind] Investigate if compatibility with v8.2-a could be > enhanced #160114](https://github.com/llvm/llvm-project/issues/160114) Pinged Ahmed on this one, I don't know the full impact of that change > 5. [[PAC][libunwind] Enhance comments and error messages related to libunwind > hardening #160117](https://github.com/llvm/llvm-project/issues/160117) My preference would be to remove those from release builds entirely and replace LIBUNWIND_ABORT() with explicit immediate traps. > 6. [[PAC][libunwind] Move signing schemas which are part of public ABI to > ptrauth.h header #160119](https://github.com/llvm/llvm-project/issues/160119) This is possibly difficult. The functional ABI implication of most of these is purely between libcxx and libcxxabi, and our platform model does not support updating/changing those separately, which means we think we might be able to change/improve some of the aspects in future. Moving them to `ptrauth.h` functionally removes that option even if there was no technical reason to. > 7. [[PAC][ibunwind] Support signed personality pointer on Linux targets > #160120](https://github.com/llvm/llvm-project/issues/160120) this is the "personality" discriminated one right? I think I already incorporated it into the latest revision. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: Sigh, built wrong repo, basic errors which I'm fixing now https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -680,11 +682,19 @@ int CompactUnwinder_arm64::stepWithCompactEncodingFrame( savedRegisterLoc -= 8; } - uint64_t fp = registers.getFP(); + Registers_arm64::reg_t fp = registers.getFP(); // fp points to old fp registers.setFP(addressSpace.get64(fp)); - // old sp is fp less saved fp and lr + + // old sp is fp less saved fp and lr. Set this before FP & LR because in + // arm64e it's the discriminator used for those registers. registers.setSP(fp + 16); + + Registers_arm64::reg_t oldfp = addressSpace.get64(fp); + + // fp points to old fp + registers.setFP(oldfp); ojhunt wrote: There was a period we experimented with frame signing (it was gated on a ptrauth_frame feature test), and while prepping these I removed the defunct code to avoid the appearance of it existing, or providing a no longer verified to be working feature, it's possible some of the weird ordering isn't needed for this anymore. I'll update the comments to remove the reference to needing FP stuff. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -680,11 +682,19 @@ int CompactUnwinder_arm64::stepWithCompactEncodingFrame( savedRegisterLoc -= 8; } - uint64_t fp = registers.getFP(); + Registers_arm64::reg_t fp = registers.getFP(); // fp points to old fp registers.setFP(addressSpace.get64(fp)); - // old sp is fp less saved fp and lr + + // old sp is fp less saved fp and lr. Set this before FP & LR because in + // arm64e it's the discriminator used for those registers. registers.setSP(fp + 16); + + Registers_arm64::reg_t oldfp = addressSpace.get64(fp); + + // fp points to old fp + registers.setFP(oldfp); ojhunt wrote: Cleaned this up https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -118,22 +119,51 @@ _LIBUNWIND_HIDDEN int __unw_set_reg(unw_cursor_t *cursor,
unw_regnum_t regNum,
typedef LocalAddressSpace::pint_t pint_t;
AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
if (co->validReg(regNum)) {
-co->setReg(regNum, (pint_t)value);
// special case altering IP to re-find info (being called by personality
// function)
if (regNum == UNW_REG_IP) {
unw_proc_info_t info;
// First, get the FDE for the old location and then update it.
co->getInfo(&info);
- co->setInfoBasedOnIPRegister(false);
+
+ pint_t sp = (pint_t)co->getReg(UNW_REG_SP);
+
+#if __has_feature(ptrauth_calls)
+ // It is only valid to set the IP within the current function.
+ // This is important for ptrauth, otherwise the IP cannot be correctly
+ // signed.
+ [[maybe_unused]]unw_word_t stripped_value =
ojhunt wrote:
I may move the helpers - currently I've put a new ifdef, while I work out if
there was a real reason for where I placed the helpers.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: Had to rebase as there was a divergence between my tree and this one - possibly I accidentally merged main to the PR https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -238,7 +282,20 @@ COMPILER_RT_ABI _Unwind_Reason_Code __gcc_personality_v0( _Unwind_SetGR(context, __builtin_eh_return_data_regno(0), (uintptr_t)exceptionObject); _Unwind_SetGR(context, __builtin_eh_return_data_regno(1), 0); - _Unwind_SetIP(context, (funcStart + landingPad)); +#define LANDING_PAD_DISCRIMINATOR "__gcc_personality_v0'landingPad" + size_t PERSONALITY_PTRAUTH_RI_RA(LANDING_PAD_DISCRIMINATOR) landingPad = + funcStart + landingPadOffset; +#if defined(__APPLE__) && __has_feature(ptrauth_qualifier) + uintptr_t stack_pointer = _Unwind_GetGR(context, -2); + const uintptr_t existingDiscriminator = ptrauth_blend_discriminator( + &landingPad, ptrauth_string_discriminator(LANDING_PAD_DISCRIMINATOR)); + uintptr_t newIP = (uintptr_t)ptrauth_auth_and_resign( + *(void **)&landingPad, ptrauth_key_function_pointer, + existingDiscriminator, ptrauth_key_return_address, stack_pointer); + _Unwind_SetIP(context, newIP); +#else + _Unwind_SetIP(context, landingPad); +#endif kovdan01 wrote: Closing in favor of https://github.com/llvm/llvm-project/pull/143230/files#r2369029642. To be fixed in scope of this PR. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 edited https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -30,6 +30,45 @@ EXCEPTION_DISPOSITION _GCC_specific_handler(PEXCEPTION_RECORD, void *, PCONTEXT, _Unwind_Personality_Fn); #endif +#if __has_feature(ptrauth_qualifier) +#include +#if __has_feature(ptrauth_restricted_intptr_qualifier) +#define __ptrauth_gcc_personality_intptr(key, addressDiscriminated, \ kovdan01 wrote: Closing in favor of https://github.com/llvm/llvm-project/pull/143230/files#r2369029642 To be done in scope of this PR https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -30,6 +30,51 @@ EXCEPTION_DISPOSITION _GCC_specific_handler(PEXCEPTION_RECORD, void *, PCONTEXT, _Unwind_Personality_Fn); #endif +#if __has_feature(ptrauth_calls) kovdan01 wrote: As discussed previously, let's use the new macro `LIBUNWIND_PTRAUTH_CALLS_AND_RETURNS` defined as follows everywhere instead of `__has_include()`, `__has_feature(ptrauth_calls)`, `__has_feature(ptrauth_returns)`, `__has_extension(ptrauth_qualifier)`, etc: ``` #if __has_feature(ptrauth_calls) && __has_feature(ptrauth_returns) #define LIBUNWIND_PTRAUTH_CALLS_AND_RETURNS #elif __has_feature(ptrauth_calls) || __has_feature(ptrauth_returns) #error "Either both or none of ptrauth_calls and ptrauth_returns is allowed to be enabled" #endif ``` See proposed fix in commit 644405b56cfa59dd3787119182df087dff6e756c in my branch https://github.com/kovdan01/llvm-project/commits/pointer-authenticated-unwinding-2025-09-22-with-fixes/. I think this should be done in scope of this PR. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -377,13 +391,31 @@ const char *CFI_Parser::parseCIE(A &addressSpace,
pint_t cie,
case 'z':
cieInfo->fdesHaveAugmentationData = true;
break;
- case 'P':
+ case 'P': {
cieInfo->personalityEncoding = addressSpace.get8(p);
++p;
cieInfo->personalityOffsetInCIE = (uint8_t)(p - cie);
-cieInfo->personality = addressSpace
-.getEncodedP(p, cieContentEnd, cieInfo->personalityEncoding);
+pint_t personality = addressSpace.getEncodedP(
+p, cieContentEnd, cieInfo->personalityEncoding,
+/*datarelBase=*/0, &resultAddr);
+#if __libunwind_has_ptrauth
kovdan01 wrote:
Closing this in favor of
https://github.com/llvm/llvm-project/pull/143230/files#r2369029642
To be done in scope of this PR
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -47,10 +47,10 @@ struct _LIBCXXABI_HIDDEN __cxa_exception {
// In Wasm, a destructor returns its argument
void *(_LIBCXXABI_DTOR_FUNC *exceptionDestructor)(void *);
#else
-void (_LIBCXXABI_DTOR_FUNC *exceptionDestructor)(void *);
+void(_LIBCXXABI_DTOR_FUNC* __ptrauth_cxxabi_exception_destructor
exceptionDestructor)(void*);
kovdan01 wrote:
Nit: revert unintended formatting change
```suggestion
void (_LIBCXXABI_DTOR_FUNC* __ptrauth_cxxabi_exception_destructor
exceptionDestructor)(void *);
```
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -1047,18 +1051,26 @@ class UnwindCursor : public AbstractUnwindCursor{
bool getInfoFromFdeCie(const typename CFI_Parser::FDE_Info &fdeInfo,
const typename CFI_Parser::CIE_Info &cieInfo,
pint_t pc, uintptr_t dso_base);
- bool getInfoFromDwarfSection(pint_t pc, const UnwindInfoSections §s,
-uint32_t fdeSectionOffsetHint=0);
+ bool getInfoFromDwarfSection(const typename R::link_reg_t &pc,
+ const UnwindInfoSections §s,
+ uint32_t fdeSectionOffsetHint = 0);
int stepWithDwarfFDE(bool stage2) {
+#if __has_extension(ptrauth_calls)
kovdan01 wrote:
Closing in favor of
https://github.com/llvm/llvm-project/pull/143230/files#r2369029642
To be done in scope of this PR
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -47,10 +47,11 @@ struct _LIBCXXABI_HIDDEN __cxa_exception {
// In Wasm, a destructor returns its argument
void *(_LIBCXXABI_DTOR_FUNC *exceptionDestructor)(void *);
#else
-void (_LIBCXXABI_DTOR_FUNC *exceptionDestructor)(void *);
+void(_LIBCXXABI_DTOR_FUNC*
_LIBCXXABI_PTRAUTH_FN("__cxa_exception::exceptionDestructor")
+ exceptionDestructor)(void*);
#endif
-std::unexpected_handler unexpectedHandler;
-std::terminate_handler terminateHandler;
+std::unexpected_handler
_LIBCXXABI_PTRAUTH_FN("__cxa_exception::unexpectedHandler") unexpectedHandler;
+std::terminate_handler
_LIBCXXABI_PTRAUTH_FN("__cxa_exception::terminateHandler") terminateHandler;
kovdan01 wrote:
Closing in favor of issue #160119
To be done as a follow-up
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -79,16 +79,18 @@ struct _LIBCXXABI_HIDDEN __cxa_exception {
// http://sourcery.mentor.com/archives/cxx-abi-dev/msg01924.html
// The layout of this structure MUST match the layout of __cxa_exception, with
// primaryException instead of referenceCount.
+// The tags used in the pointer authentication qualifiers also need to match
+// those of the corresponding members in __cxa_exception.
struct _LIBCXXABI_HIDDEN __cxa_dependent_exception {
#if defined(__LP64__) || defined(_WIN64) || defined(_LIBCXXABI_ARM_EHABI)
void* reserve; // padding.
void* primaryException;
#endif
std::type_info *exceptionType;
-void (_LIBCXXABI_DTOR_FUNC *exceptionDestructor)(void *);
-std::unexpected_handler unexpectedHandler;
-std::terminate_handler terminateHandler;
+void(_LIBCXXABI_DTOR_FUNC* __ptrauth_cxxabi_exception_destructor
exceptionDestructor)(void*);
kovdan01 wrote:
Nit: revert unintended formatting change
```suggestion
void (_LIBCXXABI_DTOR_FUNC* __ptrauth_cxxabi_exception_destructor
exceptionDestructor)(void *);
```
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 requested changes to this pull request. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -681,7 +681,18 @@ DEFINE_LIBUNWIND_FUNCTION(__libunwind_Registers_arm64_jumpto) // context struct, because it is allocated on the stack, and an exception // could clobber the de-allocated portion of the stack after sp has been // restored. - ldrx16, [x0, #0x0F8] + + ldrx16, [x0, #0x0F8] // load sp into scratch + ldrlr, [x0, #0x100] // restore pc into lr + +#if __has_feature(ptrauth_calls) + // The LR is signed with its address inside the register state. Time + // to resign to be a regular ROP signed pointer + addx1, x0, #0x100 + autib lr, x1 + pacib lr, x16 // signed the scratch register for sp +#endif kovdan01 wrote: Closing in favor of issue #160114 To be done as a follow-up https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -61,10 +62,10 @@ struct _LIBCXXABI_HIDDEN __cxa_exception {
int propagationCount;
#else
int handlerSwitchValue;
-const unsigned char *actionRecord;
-const unsigned char *languageSpecificData;
-void *catchTemp;
-void *adjustedPtr;
+const unsigned char*
_LIBCXXABI_PTRAUTH_PDD("__cxa_exception::actionRecord") actionRecord;
+const unsigned char*
_LIBCXXABI_PTRAUTH_PDD("__cxa_exception::languageSpecificData")
languageSpecificData;
+void* _LIBCXXABI_PTRAUTH_PDD("__cxa_exception::catchTemp") catchTemp;
+void* _LIBCXXABI_PTRAUTH_PDD("__cxa_exception::adjustedPtr") adjustedPtr;
kovdan01 wrote:
Closing in favor of issue #160119
To be done as a follow-up
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -1845,10 +1871,53 @@ class _LIBUNWIND_HIDDEN Registers_arm64 {
uint64_t getSP() const { return _registers.__sp; }
void setSP(uint64_t value) { _registers.__sp = value; }
- uint64_t getIP() const { return _registers.__pc; }
- void setIP(uint64_t value) { _registers.__pc = value; }
- uint64_t getFP() const { return _registers.__fp; }
- void setFP(uint64_t value) { _registers.__fp = value; }
+ uint64_t getIP() const {
+uint64_t value = _registers.__pc;
+#if __has_feature(ptrauth_calls)
+// Note the value of the PC was signed to its address in the register state
+// but everyone else expects it to be sign by the SP, so convert on return.
+value = (uint64_t)ptrauth_auth_and_resign(
+(void *)_registers.__pc, ptrauth_key_return_address, &_registers.__pc,
+ptrauth_key_return_address, getSP());
+#endif
+return value;
+ }
+ void setIP(uint64_t value) {
+#if __has_feature(ptrauth_calls)
+// Note the value which was set should have been signed with the SP.
+// We then resign with the slot we are being stored in to so that both SP
+// and LR can't be spoofed at the same time.
+value = (uint64_t)ptrauth_auth_and_resign(
+(void *)value, ptrauth_key_return_address, getSP(),
+ptrauth_key_return_address, &_registers.__pc);
+#endif
+_registers.__pc = value;
+ }
+ uint64_t getFP() const { return _registers.__fp; }
+ void setFP(uint64_t value) { _registers.__fp = value; }
+
+ typedef uint64_t reg_t;
+ typedef uint64_t
+ __LIBUNWIND_PTRAUTH_RI_PDC("Registers_arm64::link_reg_t") link_reg_t;
+ void
+ loadAndAuthenticateLinkRegister(reg_t inplaceAuthedLinkRegister,
+ link_reg_t *referenceAuthedLinkRegister) {
+#if __has_feature(ptrauth_calls)
kovdan01 wrote:
Closing in favor of issue #160110
To be done as a follow up.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -118,22 +119,51 @@ _LIBUNWIND_HIDDEN int __unw_set_reg(unw_cursor_t *cursor,
unw_regnum_t regNum,
typedef LocalAddressSpace::pint_t pint_t;
AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
if (co->validReg(regNum)) {
-co->setReg(regNum, (pint_t)value);
// special case altering IP to re-find info (being called by personality
// function)
if (regNum == UNW_REG_IP) {
unw_proc_info_t info;
// First, get the FDE for the old location and then update it.
co->getInfo(&info);
- co->setInfoBasedOnIPRegister(false);
+
+ pint_t sp = (pint_t)co->getReg(UNW_REG_SP);
+
+#if __has_feature(ptrauth_calls)
+ // It is only valid to set the IP within the current function.
+ // This is important for ptrauth, otherwise the IP cannot be correctly
+ // signed.
+ [[maybe_unused]]unw_word_t stripped_value =
ojhunt wrote:
it may not even be needed following a bunch of the refactoring
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 requested changes to this pull request. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 edited https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 edited https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -694,7 +705,12 @@ DEFINE_LIBUNWIND_FUNCTION(__libunwind_Registers_arm64_jumpto) gcspushm x30 Lnogcs: #endif + +#if __has_feature(ptrauth_calls) + retab +#else kovdan01 wrote: Closing the thread in favor of: 1. (To be fixed in scope of this PR) Comment in this PR about use of feature checks: TODO 2. (To be done as a follow up) Investigation issue about v8.2 compatibility: #160114 https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -30,6 +30,51 @@ EXCEPTION_DISPOSITION
_GCC_specific_handler(PEXCEPTION_RECORD, void *, PCONTEXT,
_Unwind_Personality_Fn);
#endif
+#if __has_feature(ptrauth_calls)
+#include
+
+#if __has_feature(ptrauth_restricted_intptr_qualifier)
+#define __ptrauth_gcc_personality_intptr(key, addressDiscriminated,
\
+ discriminator)
\
+ __ptrauth_restricted_intptr(key, addressDiscriminated, discriminator)
+#else
+#define __ptrauth_gcc_personality_intptr(key, addressDiscriminated,
\
+ discriminator)
\
+ __ptrauth(key, addressDiscriminated, discriminator)
+#endif
+#else
+#define __ptrauth_gcc_personality_intptr(...)
+#endif
+
+#define __ptrauth_gcc_personality_func_key ptrauth_key_function_pointer
+
+// ptrauth_string_discriminator("__gcc_personality_v0'funcStart") == 0xDFEB
+#define __ptrauth_gcc_personality_func_start
\
+ __ptrauth_gcc_personality_intptr(__ptrauth_gcc_personality_func_key, 1,
\
+ 0xDFEB)
kovdan01 wrote:
Resolving this in favor of #160101. To be done as a follow-up.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -680,11 +682,19 @@ int CompactUnwinder_arm64::stepWithCompactEncodingFrame( savedRegisterLoc -= 8; } - uint64_t fp = registers.getFP(); + Registers_arm64::reg_t fp = registers.getFP(); // fp points to old fp registers.setFP(addressSpace.get64(fp)); - // old sp is fp less saved fp and lr + + // old sp is fp less saved fp and lr. Set this before FP & LR because in + // arm64e it's the discriminator used for those registers. registers.setSP(fp + 16); + + Registers_arm64::reg_t oldfp = addressSpace.get64(fp); + + // fp points to old fp + registers.setFP(oldfp); kovdan01 wrote: It's unclear whether FP is signed or not. `setFP` and `getFP` functions just trivially return or assign desired values without any auth or sign logic (while `setIP` and `getIP` contain such logic). If FP is signed, I suppose `setFP` and `getFP` should have similar logic as well. Otherwise, if FP is not signed, this piece of code seems redundant, just as mentioned by @atrosinenko https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -30,6 +30,45 @@ EXCEPTION_DISPOSITION _GCC_specific_handler(PEXCEPTION_RECORD, void *, PCONTEXT, _Unwind_Personality_Fn); #endif +#if __has_feature(ptrauth_qualifier) +#include +#if __has_feature(ptrauth_restricted_intptr_qualifier) kovdan01 wrote: Resolving the thread in favor of: 1. (To be fixed in scope of this PR) Comment in this PR about `ptrauth_restricted_intptr_qualifier`: TODO 2. (To be done as a follow-up) Comment in the "update docs" issue: https://github.com/llvm/llvm-project/issues/96528#issuecomment-3320192622 https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -807,7 +812,12 @@ DEFINE_LIBUNWIND_FUNCTION(__unw_getcontext) strd31, [x0, #0x208] #endif movx0, #0 // return UNW_ESUCCESS + +#if __has_feature(ptrauth_calls) + retab +#else kovdan01 wrote: Closing the thread in favor of: 1. (To be fixed in scope of this PR) Comment in this PR about use of feature checks: TODO 2. (To be done as a follow up) Investigation issue about v8.2 compatibility: #160114 https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -126,6 +130,36 @@ _LIBUNWIND_HIDDEN int __unw_set_reg(unw_cursor_t *cursor,
unw_regnum_t regNum,
// First, get the FDE for the old location and then update it.
co->getInfo(&info);
co->setInfoBasedOnIPRegister(false);
+
+#if __has_feature(ptrauth_calls)
+ // It is only valid to set the IP within the current function.
+ // This is important for ptrauth, otherwise the IP cannot be correctly
+ // signed.
+ unw_word_t stripped_value =
+ (unw_word_t)ptrauth_strip((void *)value, ptrauth_key_return_address);
+ (void)stripped_value;
+ assert(stripped_value >= info.start_ip && stripped_value <= info.end_ip);
+#endif
+
+ pint_t sp = (pint_t)co->getReg(UNW_REG_SP);
+
+#if __has_feature(ptrauth_calls)
+ {
+// PC should have been signed with the sp, so we verify that
+// roundtripping does not fail.
+pint_t pc = (pint_t)co->getReg(UNW_REG_IP);
+if (ptrauth_auth_and_resign((void *)pc, ptrauth_key_return_address, sp,
+ptrauth_key_return_address,
+sp) != (void *)pc) {
+ _LIBUNWIND_LOG("Bad unwind through arm64e (0x%llX,
0x%llX)->0x%llX\n",
kovdan01 wrote:
Closing the thread in favor of issue #160117
To be done as a follow-up.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -103,10 +104,68 @@
#define _LIBCXXABI_DTOR_FUNC
#endif
-#if __cplusplus < 201103L
-# define _LIBCXXABI_NOEXCEPT throw()
-#else
-# define _LIBCXXABI_NOEXCEPT noexcept
+#if __has_include()
+# include
#endif
+#if __has_extension(ptrauth_qualifier)
+
+// The actual value of the discriminators listed below is not important
+// beyond their impact on ABI. The derivation of the constants is only
+// being included for the purpose of maintaining a record of how they
+// were originally produced.
+
+// ptrauth_string_discriminator("__cxa_exception::actionRecord") == 0xFC91
kovdan01 wrote:
Closing this in favor of issue #160119.
To be done as a follow-up.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -541,7 +588,33 @@ struct scan_results
};
} // unnamed namespace
+} // extern "C"
+
+namespace {
+// The logical model for casting authenticated function pointers makes
+// it impossible to directly cast them without breaking the authentication,
+// as a result we need this pair of helpers.
+template
+[[maybe_unused]] void set_landing_pad_as_ptr(scan_results& results, const
PtrType& out) {
kovdan01 wrote:
I've compiled toolchain with three different sysroots:
1. AArch64 with pauthtest enabled
2. AArch64 with no pauth hardening
3. x86_64
No warnings were present when building these.
I suggest to do the following. Let's revert these `[[maybe_unused]]` changes
and look at buildbot status in pre-merge checks. Depending on status of the
builds, we can do one of the following.
1. If there are no issues related to this, we are fine.
2. Otherwise, we need to investigate why I'm not facing the issue locally and
find the mainline commit which fixed that. And, when adding `[[maybe_unused]]`
workaround, we should describe the bug and put corresponding references to
commits/issues in comments.
To make things easier, you are welcome to use the commit
80cfa58a28a15d55938f4732792747722988a521 from my
[branch](https://github.com/kovdan01/llvm-project/commits/pointer-authenticated-unwinding-2025-09-22-with-fixes/)
- the commit reverts this workaround.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -118,22 +119,51 @@ _LIBUNWIND_HIDDEN int __unw_set_reg(unw_cursor_t *cursor,
unw_regnum_t regNum,
typedef LocalAddressSpace::pint_t pint_t;
AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
if (co->validReg(regNum)) {
-co->setReg(regNum, (pint_t)value);
// special case altering IP to re-find info (being called by personality
// function)
if (regNum == UNW_REG_IP) {
unw_proc_info_t info;
// First, get the FDE for the old location and then update it.
co->getInfo(&info);
- co->setInfoBasedOnIPRegister(false);
+
+ pint_t sp = (pint_t)co->getReg(UNW_REG_SP);
+
+#if __has_feature(ptrauth_calls)
+ // It is only valid to set the IP within the current function.
+ // This is important for ptrauth, otherwise the IP cannot be correctly
+ // signed.
+ [[maybe_unused]]unw_word_t stripped_value =
kovdan01 wrote:
I guess this would break builds with older C++ versions.
@ldionne Since this code is not in a public header, can we assume that
libunwind is built with at least C++17, or do we need to use old trick with
dummy cast to void to silence the warning?
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -63,10 +63,11 @@ class DwarfInstructions {
pint_t cfa, const RegisterLocation
&savedReg);
static pint_t getCFA(A &addressSpace, const PrologInfo &prolog,
- const R ®isters) {
-if (prolog.cfaRegister != 0)
- return (pint_t)((sint_t)registers.getRegister((int)prolog.cfaRegister) +
- prolog.cfaRegisterOffset);
+ R ®isters) {
kovdan01 wrote:
```suggestion
const R ®isters) {
```
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -43,6 +43,102 @@ #define LIBUNWIND_AVAIL #endif +#if __has_feature(ptrauth_calls) + + #include + + #if __has_extension(ptrauth_restricted_intptr_qualifier) kovdan01 wrote: We do not have `__ptrauth_restricted_intptr`, so let's avoid including these changes in mainline runtime libraries. I do get the point that such a qualifier is used in downstream Apple code, but it's better to keep macro definitions dependent on `__ptrauth_restricted_intptr` in Apple code and do not expose them to mainline since this literally makes no sense for anyone else. See proposed fix a2390e1e285023af78d27d768540bb8f30efea76 in my branch https://github.com/kovdan01/llvm-project/commits/pointer-authenticated-unwinding-2025-09-22-with-fixes/ https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -30,6 +30,45 @@ EXCEPTION_DISPOSITION _GCC_specific_handler(PEXCEPTION_RECORD, void *, PCONTEXT, _Unwind_Personality_Fn); #endif +#if __has_feature(ptrauth_qualifier) +#include +#if __has_feature(ptrauth_restricted_intptr_qualifier) atrosinenko wrote: I wonder if the expected versions of compilers are explicitly documented somewhere. I glanced through llvm's libunwind docs and found nothing about the expected compilers - whether it should be buildable by the very same version of Clang built from the same commit of the monorepo? any Clang not older `v.`? recent GCC versions as well? Furthermore, this PR additionally spans compiler-rt builtins, sanitizers and libc++ libraries. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/atrosinenko edited https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -30,6 +30,51 @@ EXCEPTION_DISPOSITION
_GCC_specific_handler(PEXCEPTION_RECORD, void *, PCONTEXT,
_Unwind_Personality_Fn);
#endif
+#if __has_feature(ptrauth_calls)
+#include
+
+#if __has_feature(ptrauth_restricted_intptr_qualifier)
+#define __ptrauth_gcc_personality_intptr(key, addressDiscriminated,
\
+ discriminator)
\
+ __ptrauth_restricted_intptr(key, addressDiscriminated, discriminator)
+#else
+#define __ptrauth_gcc_personality_intptr(key, addressDiscriminated,
\
+ discriminator)
\
+ __ptrauth(key, addressDiscriminated, discriminator)
+#endif
+#else
+#define __ptrauth_gcc_personality_intptr(...)
+#endif
+
+#define __ptrauth_gcc_personality_func_key ptrauth_key_function_pointer
+
+// ptrauth_string_discriminator("__gcc_personality_v0'funcStart") == 0xDFEB
+#define __ptrauth_gcc_personality_func_start
\
+ __ptrauth_gcc_personality_intptr(__ptrauth_gcc_personality_func_key, 1,
\
+ 0xDFEB)
atrosinenko wrote:
(As an illustration for the "cover letter" of this review) I assume many such
macroses define purely internal signing schemas. In such cases it should be
possible to compute the discriminator automatically - it could probably improve
readability to use something like
```cpp
PTRAUTH_INTPTR(uintptr_t, start) = readEncodedPointer(&p, callSiteEncoding);
```
instead of
```cpp
uintptr_t __ptrauth_gcc_personality_start start =
readEncodedPointer(&p, callSiteEncoding);
```
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/atrosinenko commented:
It would be probably useful to explicitly list the configurations that we would
like to support at the end of the day. Some combinations are probably
unsupported or not planned for the first version of the patch. I have already
left similar comments before, but let me summarize them in a more structured
fashion. Corrections are welcome :)
There seems to be three different aspects independent from each other. The
first aspect is pac-ret hardening mode the calling code was built with. If my
understanding is correct, pac-ret is largely considered as not affecting the
ABI (because signing and authentication code belongs to the same function) and
thus the mode can be arbitrarily chosen per shared object, per-library or
technically even per-function. Thus, this mode should probably be inferred not
from the preprocessor macroses observed at libunwind build time, but from the
DWARF information (which raises the question of when we can or cannot trust
this information under Pointer Authentication threat model). See [`aadwarf64`
document](https://github.com/ARM-software/abi-aa/releases/tag/2025Q1) for the
description of CFA operations controlling `RA_SIGN_STATE` pseudo register.
Furthermore, the *key* being used by the particular stack frame is communicated
via optional character `B` in the CIE augmentation string (described in the
same document). There already exist significant amount of code in
[`DwarfInstructions::stepWithDwarf`](https://github.com/llvm/llvm-project/blob/47c1b650626043f0a8f8e32851617201751f9439/libunwind/src/DwarfInstructions.hpp#L210)
- I wonder if it could be reused in a secure manner.
The second aspect is ABI-affecting variations of unwind information format. One
thing I do know about is signing of the pointer to the personality function,
but I don't have the complete list of structs and fields in my mind. It would
probably be useful to explicitly enumerate the minimal set of "public" fields
whose signing schemas we should choose in accordance with the ABI. After all,
these details should probably end up being documented in some specification
sooner or later.
The third aspect is which internal fields are signed and when. This is more a
matter of the targeted ISA version and best-effort guess of the desired
"protection mode" of the affected libraries. According to
https://github.com/llvm/llvm-project/pull/143230#discussion_r2173023794, this
could probably be as simple as "if `__has_feature(ptrauth_calls)` is true, then
assume we target at least Armv8.3-a and enable signing of all the affected
internal fields". (I don't remember, but maybe some rudimentary v8.2-compatible
hardening of PC exists in DwarfInstructions as well.)
By the way, the separation of purely internal fields should also help to
simplify the code: while ABI-defined signing schemas have to be described
explicitly, for the internal struct fields and local variables it should be
possible to define macroses like
```cpp
#define PTRAUTH_INTPTR(type, name) \
type __ptrauth_gcc_personality_intptr(__ptrauth_gcc_personality_func_key, \
1, ptrauth_string_discriminator("__gcc_personality_v0'" #name)) name
```
or maybe embed the type into the macro name like `PTRAUTH_UINTPTR_T(name)`.
Then it can be used along these lines:
```cpp
PTRAUTH_INTPTR(uintptr_t, start) = readEncodedPointer(&p, callSiteEncoding);
```
This would additionally clarify the distinction between ABI-defined and
arbitrarily chosen schemas. I wonder if this approach has its own important
downsides that I don't see.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
atrosinenko wrote: I tried to figure out what is the difference between `-mbranch-protection=pac-ret` and `-fptrauth-returns` in the mainline Clang/LLVM. Though, I'm not very familiar with all that option marshalling machinery in Clang, so this included significant amount of grepping. ## Clang There are several options that can be considered entry points on the Clang driver and/or `clang -cc1` sides. There is a pair of `f`-options (`-fptrauth-returns` and `-fno-ptrauth-returns`) as well as three `m`-options: `-msign-return-address=none|all|non-leaf`, `-msign-return-address-key=a_key|b_key`, `-mbranch-protection=` (there is also `-mbranch-protection-pauth-lr` option, but let's consider it out of scope for now). As far as I know, there is significant amount of duplication between `-f...` and `-m...` options w.r.t. pac-ret hardening for hystorical reasons. ### `-fptrauth-returns` Considering the `f`-options, I started with searching for `f(no[_-])?ptrauth[_-]returns` under `/clang/`, excluding `/clang/test/`. This shown me testing for conflicts with `m`-options in `CollectARMPACBTIOptions()`, some amount of hand-written marshalling code and the related field of the `LangOptions` structure: `PointerAuthReturns`. By the way, the tablegen definition itself was not found because it has the form `defm ptrauth_returns : OptInCC1FFlag<"ptrauth-returns", "...">`). Then, by searching for "PointerAuthReturns", one can find `ptrauth_returns` feature (to be exposed via `__has_feature`), `PAuthABIVersion` module flag, more cross-checking with `m`-options in `AArch64TargetInfo::validateBranchProtection()` and `PointerAuthOptions::ReturnAddresses` field. Finally, searching for `ReturnAddresses` reveals setting `"ptrauth-returns"` function attribute. Searching for `ptrauth-returns` under `/clang/` does not add anything new except for the original `defm ptrauth_returns : ...` definition. ### `m`-options Searching for "msign[_-]return[_-]address|mbranch[_-]protection", again, shows testing against `f`-options and some amount of marshalling. Unlike `-fptrauth-returns`, `m`-options are used to choose the multilib variant to use. Finally, `SignReturnAddressScope` and `SignReturnAddressKey` fields of `LangOptions` and corresponding accessor functions can be found. Searching for "SignReturnAddress" (this includes `get*()` and `set*()` accessors, as well as functions like `hasSignReturnAddress()`) yields defining `__ARM_FEATURE_PAC_DEFAULT` macro, more parsing of string representations of branch-protection modes, and two other related symbols: `SignReturnAddr` and `SignKey` fields of `BranchProtectionInfo`. Furthermore, "sign-return-address", "sign-return-address-all" and "sign-return-address-with-bkey" module attributes are set as well as "sign-return-address" and "sign-return-address-key" function attributes. ## LLVM Considering the codegen performed by LLVM backend, grepping for "sign-return-address|ptrauth-returns" suggests that `-fptrauth-returns` can be considered as `-msign-return-address=non-leaf -msign-return-address-key=b_key` (see `GetSignReturnAddress` and `ShouldSignWithBKey` functions in AArch64MachineFunctionInfo.cpp), though there can be minor differences such as different default checking mode for authenticated LR when performing tail calls and emission of some file-level meta-information by AArch64AsmPrinter. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
kovdan01 wrote:
> > * Fix personality pointer authentication on non-Apple targets:
> > [[runtimes][PAC] Harden unwinding when possible #143230
> > (comment)](https://github.com/llvm/llvm-project/pull/143230#discussion_r2318636296)
>
> I'm not sure what's going on in this diff as it does not appear related to
> changes in this PR? Is it just a general issue that already exists?
@ojhunt Well, I'd rather say that it's not an issue that already exists, it's
just missing functionality - libunwind does not support ptrauth-hardened
personality pointer, and this is true for both signed personality pointer on
Apple and Linux. We just have different signing schemes, and the signing scheme
on Linux is defined in this PR: #119361
If you prefer not to include support for signed personality on Linux, I would
be happy to submit support for that as a follow-up PR as soon as this one is
merged (since logic for Linux is similar to logic for Apple, we just have
different discriminators). But, if we take this approach, let's at least add a
TODO note mentioning that signed personality now only works for Apple's arm64e,
and support for Linux will be implemented in future.
Probably, we can use smth like this:
```
#if __has_feature(ptrauth_calls) // as discussed in discord, let's replace that
with some macro for calls+returns
if (personality) {
#if __APPLE__
const auto discriminator = ptrauth_blend_discriminator(
&cieInfo->personality,
__ptrauth_unwind_cie_info_personality_disc);
void *signedPtr = ptrauth_auth_and_resign(
(void *)personality, ptrauth_key_function_pointer, resultAddr,
ptrauth_key_function_pointer, discriminator);
personality = (pint_t)signedPtr;
#else
#error "Signed personality function pointer is not supported for non-Apple
targets"
// TODO: Implement support for Linux. The signing scheme of personality pointer
// differ for Linux and Apple targets. For Linux targets, the signing scheme is
// defined in https://github.com/llvm/llvm-project/pull/119361
#endif
}
#endif
```
Which approach do you prefer? Is it applying the suggestion from commit
https://github.com/llvm/llvm-project/commit/e2f8b9d3859eff96442ce04662aefb40debbef3f
or postponing the Linux support and using the code snippet like above?
I'm OK with both, just please let me know which is more comfortable for you.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -557,7 +596,19 @@ set_registers(_Unwind_Exception* unwind_exception, _Unwind_Context* context, reinterpret_cast(unwind_exception)); _Unwind_SetGR(context, __builtin_eh_return_data_regno(1), static_cast(results.ttypeIndex)); +#if defined(__APPLE__) && __has_feature(ptrauth_qualifier) + auto stack_pointer = _Unwind_GetGR(context, UNW_REG_SP); + // We manually re-sign the IP as the __ptrauth qualifiers cannot + // express the required relationship with the destination address + const auto existingDiscriminator = ptrauth_blend_discriminator( + &results.landingPad, ptrauth_string_discriminator(_LIBCXXABI_PTRAUTH_SCANRESULT_LANDINGPAD_DISC)); + unw_word_t newIP = + (unw_word_t)ptrauth_auth_and_resign(*(void**)&results.landingPad, _LIBCXXABI_PTRAUTH_KEY, existingDiscriminator, + ptrauth_key_return_address, stack_pointer); + _Unwind_SetIP(context, newIP); +#else kovdan01 wrote: @atrosinenko Could you please add a little bit of context for your question? I tried to dive into your comments related to gcc_personality_v0.c (as you are referring to that here), but I'm not sure I get your question. I just want to resolve all the threads which are no longer relevant, and I'm not sure if we have an issue here. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -126,6 +130,36 @@ _LIBUNWIND_HIDDEN int __unw_set_reg(unw_cursor_t *cursor, unw_regnum_t regNum, // First, get the FDE for the old location and then update it. co->getInfo(&info); co->setInfoBasedOnIPRegister(false); + +#if __has_feature(ptrauth_calls) + // It is only valid to set the IP within the current function. + // This is important for ptrauth, otherwise the IP cannot be correctly + // signed. + unw_word_t stripped_value = + (unw_word_t)ptrauth_strip((void *)value, ptrauth_key_return_address); + (void)stripped_value; + assert(stripped_value >= info.start_ip && stripped_value <= info.end_ip); +#endif + + pint_t sp = (pint_t)co->getReg(UNW_REG_SP); kovdan01 wrote: This looks outdated and seems to be fixed, so I'm resolving the thread. @ldionne Feel free to re-open the thread if I've misinterpreted smth and this is not actually fixed. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 edited https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
https://github.com/kovdan01 edited https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
kovdan01 wrote: > In principle I could set up this configuration right? (Obviously not actually > test it, but in principle at least be able to verify it builds before pushing > the update) @ojhunt It would be very nice if you could do it and ensure things are at least building w/o compile errors. I'll anyway set up pac-ret-enabled toolchain build locally and ensure that llvm-test-suite runs w/o issues. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: > > I _think_ doing it as a follow up makes sense: it's not a matter of me not > > wanting to do this for linux, it's that I cannot test it at all. Doing it > > as a follow up would also give you the constants block to put the > > definition rather than having it just slotted in the code. I've found > > moving to the constant blocks to be incredibly useful for being able to see > > where/how they cross ABI boundaries. > > @ojhunt Thanks! So could you please adopt the code snippet from my previous > comment to make things explicitly unsupported for Linux? > > And also I kindly ask you to fix p.4 from [#143230 > (comment)](https://github.com/llvm/llvm-project/pull/143230#issuecomment-3249013139) > (we've already discussed that in Discord). Please let me know if you need > any help. > > A couple of things are on my side: > > 1. Compile a local pac-ret-enabled toolchain. This would allow you to fix p.3 > from the same comment. I'll let you know when it's ready. In _principle_ I could set up this configuration right? (Obviously not actually test it, but in principle at least be able to verify it builds before pushing the update) > 2. Look through other comments in this PR (they have lower priority since > they are not related to any runtime/compile-time failures). I'll highlight > the ones which I think are worth fixing in scope of this PR (if any). > > Please let me know if I've missed smth or if you have any objections on this > plan. GH makes finding these things so utterly miserable. I hate it so very much. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote:
> > > * Fix personality pointer authentication on non-Apple targets:
> > > [[runtimes][PAC] Harden unwinding when possible #143230
> > > (comment)](https://github.com/llvm/llvm-project/pull/143230#discussion_r2318636296)
> >
> >
> > I'm not sure what's going on in this diff as it does not appear related to
> > changes in this PR? Is it just a general issue that already exists?
>
> @ojhunt Well, I'd rather say that it's not an issue that already exists, it's
> just missing functionality - libunwind does not support ptrauth-hardened
> personality pointer, and this is true for both signed personality pointer on
> Apple and Linux. We just have different signing schemes, and the signing
> scheme on Linux is defined in this PR: #119361
>
> If you prefer not to include support for signed personality on Linux, I would
> be happy to submit support for that as a follow-up PR as soon as this one is
> merged (since logic for Linux is similar to logic for Apple, we just have
> different discriminators). But, if we take this approach, let's at least add
> a TODO note mentioning that signed personality now only works for Apple's
> arm64e, and support for Linux will be implemented in future.
>
> Probably, we can use smth like this:
>
> ```
> #if __has_feature(ptrauth_calls) // as discussed in discord, let's replace
> that with some macro for calls+returns
> if (personality) {
> #if __APPLE__
> const auto discriminator = ptrauth_blend_discriminator(
> &cieInfo->personality,
> __ptrauth_unwind_cie_info_personality_disc);
> void *signedPtr = ptrauth_auth_and_resign(
> (void *)personality, ptrauth_key_function_pointer, resultAddr,
> ptrauth_key_function_pointer, discriminator);
> personality = (pint_t)signedPtr;
> #else
> #error "Signed personality function pointer is not supported for non-Apple
> targets"
> // TODO: Implement support for Linux. The signing scheme of personality
> pointer
> // differ for Linux and Apple targets. For Linux targets, the signing scheme
> is
> // defined in https://github.com/llvm/llvm-project/pull/119361
> #endif
> }
> #endif
> ```
>
> Which approach do you prefer? Is it applying the suggestion from commit
> [e2f8b9d](https://github.com/llvm/llvm-project/commit/e2f8b9d3859eff96442ce04662aefb40debbef3f)
> or postponing the Linux support and using the code snippet like above?
>
> I'm OK with both, just please let me know which is more comfortable for you.
I _think_ doing it as a follow up makes sense: it's not a matter of me not
wanting to do this for linux, it's that I cannot test it at all. Doing it as a
follow up would also give you the constants block to put the definition rather
than having it just slotted in the code. I've found moving to the constant
blocks to be incredibly useful for being able to see where/how they cross ABI
boundaries.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
kovdan01 wrote: @ojhunt Thanks! So could you please adopt the code snippet from my previous comment to make things explicitly unsupported for Linux? And also I kindly ask you to fix p.4 from https://github.com/llvm/llvm-project/pull/143230#issuecomment-3249013139 (we've already discussed that in Discord). Please let me know if you need any help. A couple of things are on my side: 1. Compile a local pac-ret-enabled toolchain. This would allow you to fix p.3 from the same comment. I'll let you know when it's ready. 2. Look through other comments in this PR (they have lower priority since they are not related to any runtime/compile-time failures). I'll highlight the ones which I think are worth fixing in scope of this PR (if any). Please let me know if I've missed smth or if you have any objections on this plan. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: > * Fix personality pointer authentication on non-Apple targets: > [[runtimes][PAC] Harden unwinding when possible #143230 > (comment)](https://github.com/llvm/llvm-project/pull/143230#discussion_r2318636296) > * I'm not sure what's going on in this diff as it does not appear related to changes in this PR? Is it just a general issue that already exists? https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: Updating cxxabi and compiler-rt formatting as they seem to be set up such that clang-format matches the existing style https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: (Updating to throw at build bots) https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
ojhunt wrote: hmmm, I thought `ptrauth_calls` was one of the checks guarded by a darwin target so I might revert the `|| defined(__PTRAUTH__)` changes https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -16,6 +16,12 @@ #include #endif +#if __has_feature(ptrauth_calls) || defined(__PTRAUTH__) ojhunt wrote: I found this while updating the __PTRAUTH__ guards -- I would be ok leaving this as a task for you folk as I think this is a linux specific file? https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -103,10 +104,68 @@
#define _LIBCXXABI_DTOR_FUNC
#endif
-#if __cplusplus < 201103L
-# define _LIBCXXABI_NOEXCEPT throw()
-#else
-# define _LIBCXXABI_NOEXCEPT noexcept
+#if __has_include()
+# include
#endif
+#if __has_extension(ptrauth_qualifier)
+
+// The actual value of the discriminators listed below is not important
+// beyond their impact on ABI. The derivation of the constants is only
+// being included for the purpose of maintaining a record of how they
+// were originally produced.
+
+// ptrauth_string_discriminator("__cxa_exception::actionRecord") == 0xFC91
ojhunt wrote:
The only actual reason for the "this was the origin" is to clarify that these
weren't arbitrary - essentially providing provenance if anyone is _really_
curious. It's really not important (these aren't used for cryptography, where
the origin of any constant is important, but we started doing this out of habit
a while back for all those we could still find the original derivation on
principle)
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -103,6 +103,52 @@ #define _LIBCXXABI_DTOR_FUNC #endif +#if __has_include() +# include +#endif ojhunt wrote: Once I get my local build setup again I will be doing a bunch of the ptrauth.h cleanup - there are many places that were obviously transitively included, and I'd rather minimize the different includes and guards https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -1047,18 +1051,26 @@ class UnwindCursor : public AbstractUnwindCursor{
bool getInfoFromFdeCie(const typename CFI_Parser::FDE_Info &fdeInfo,
const typename CFI_Parser::CIE_Info &cieInfo,
pint_t pc, uintptr_t dso_base);
- bool getInfoFromDwarfSection(pint_t pc, const UnwindInfoSections §s,
-uint32_t fdeSectionOffsetHint=0);
+ bool getInfoFromDwarfSection(const typename R::link_reg_t &pc,
+ const UnwindInfoSections §s,
+ uint32_t fdeSectionOffsetHint = 0);
int stepWithDwarfFDE(bool stage2) {
+#if __has_extension(ptrauth_calls)
ojhunt wrote:
sigh, I thought I had found all of the has_extension and has_feature checks
everywhere (except for those places that define the macros we use -- the few
places it seemed unnecessary should have been updated with `__PTRAUTH__` checks.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -541,7 +588,33 @@ struct scan_results
};
} // unnamed namespace
+} // extern "C"
+
+namespace {
+// The logical model for casting authenticated function pointers makes
+// it impossible to directly cast them without breaking the authentication,
+// as a result we need this pair of helpers.
+template
+[[maybe_unused]] void set_landing_pad_as_ptr(scan_results& results, const
PtrType& out) {
ojhunt wrote:
you did not get a warning when not compiling arm64e support? It's possible
trunk clang has fixed this?
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -30,6 +30,45 @@ EXCEPTION_DISPOSITION _GCC_specific_handler(PEXCEPTION_RECORD, void *, PCONTEXT, _Unwind_Personality_Fn); #endif +#if __has_feature(ptrauth_qualifier) +#include +#if __has_feature(ptrauth_restricted_intptr_qualifier) ojhunt wrote: Backwards compatibility, always backwards compatibility. The blind use of restricted intptr should just fail the build, so I assume this ongoing local build issues. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -557,7 +630,21 @@ set_registers(_Unwind_Exception* unwind_exception, _Unwind_Context* context, reinterpret_cast(unwind_exception)); _Unwind_SetGR(context, __builtin_eh_return_data_regno(1), static_cast(results.ttypeIndex)); +#if __has_feature(ptrauth_qualifier) + auto stackPointer = _Unwind_GetGR(context, UNW_REG_SP); ojhunt wrote: Sigh, i've been fighting some local build issues and was wanting to get bot feedback while getting my local build working again (see https://github.com/llvm/llvm-project/pull/156607 that I ran into because I was accidentally building clang fat). I should have commented that this was not ready for re-review (though the feedback is needed, it would hopefully mitigate manually finding failures) Including libunwind.h is necessary -- pointer auth makes these ABI linked, and while we _could_ redeclare these in cxxabi that would obscure the linkage _and_ mean changes are not automatic and silently break. https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
kovdan01 wrote: Thanks @ojhunt for updates! These resolve some of the previously found issues, but many problems are still present (both previously reported and newly found). I tried to resolve all the threads which are now no longer relevant, so please treat all the opened threads as issues which need your attention. I would be happy to provide any help - just let me know if it's needed. Here are the issues I'd want to highlight - these should probably be resolved first. 1. Build issues due to missing `unw_word_t` and `UNW_REG_SP` declarations in libcxxabi/src/cxa_personality.cpp: https://github.com/llvm/llvm-project/pull/143230#discussion_r2316986679 2. Fix personality pointer authentication on non-Apple targets: https://github.com/llvm/llvm-project/pull/143230#discussion_r2318636296 3. Fix handling signed RA in `DwarfInstructions::stepWithDwarf`: https://github.com/llvm/llvm-project/pull/143230#discussion_r2318709982 4. Look through ptrauth_calls/ptrauth_returns/other ptrauth checks and ensure that they are correct. On arm64e, there is no difference, while correct checks might be important for other ptrauth-enabled ABIs when having feature X does not necessarily imply having feature Y https://github.com/llvm/llvm-project/pull/143230 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [libcxx] [libcxxabi] [libunwind] [runtimes][PAC] Harden unwinding when possible (PR #143230)
@@ -126,6 +130,36 @@ _LIBUNWIND_HIDDEN int __unw_set_reg(unw_cursor_t *cursor,
unw_regnum_t regNum,
// First, get the FDE for the old location and then update it.
co->getInfo(&info);
co->setInfoBasedOnIPRegister(false);
+
+#if __has_feature(ptrauth_calls)
+ // It is only valid to set the IP within the current function.
+ // This is important for ptrauth, otherwise the IP cannot be correctly
+ // signed.
+ unw_word_t stripped_value =
+ (unw_word_t)ptrauth_strip((void *)value, ptrauth_key_return_address);
+ (void)stripped_value;
+ assert(stripped_value >= info.start_ip && stripped_value <= info.end_ip);
+#endif
+
+ pint_t sp = (pint_t)co->getReg(UNW_REG_SP);
+
+#if __has_feature(ptrauth_calls)
+ {
+// PC should have been signed with the sp, so we verify that
+// roundtripping does not fail.
+pint_t pc = (pint_t)co->getReg(UNW_REG_IP);
+if (ptrauth_auth_and_resign((void *)pc, ptrauth_key_return_address, sp,
+ptrauth_key_return_address,
+sp) != (void *)pc) {
+ _LIBUNWIND_LOG("Bad unwind through arm64e (0x%llX,
0x%llX)->0x%llX\n",
kovdan01 wrote:
While FPAC does the job and with that being enabled, we do not reach this
logging, it's not true when we do not have FPAC :) We cannot guarantee that
it's present. And anyway, it's better to use more generic messages in places
which are not specific for arm64e.
I suggest to look through the PR and think of replacing mentions of arm64e with
smth describing arbitrary ptrauth-enabled platforms (where applicable).
I can help with that when all the functional issues with this PR are resolved.
https://github.com/llvm/llvm-project/pull/143230
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
