Library support for aarch64-unknown-linux-pauthtest target#156548
Library support for aarch64-unknown-linux-pauthtest target#156548jchlanda wants to merge 6 commits into
Conversation
rustbot
added
A-compiletest
|
The relevant change is only in the two most recent commits, the rest is from the target introduction PR. |
|
@davidtwco, @folkertdev, @tgross35, @madsmtm FWI this is a follow up to #155722 |
|
This PR modifies If appropriate, please update Some changes occurred in src/tools/compiletest cc @jieyouxu The GCC codegen subtree was changed
Some changes occurred in src/doc/rustc/src/platform-support cc @Noratrieb This PR modifies cc @jieyouxu These commits modify compiler targets. |
rustbot
added
S-waiting-on-review
|
r? @folkertdev rustbot has assigned @folkertdev. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
jchlanda
force-pushed
the
jakub/pac_lib
branch
2 times, most recently
from
c54b875 to
26986cc
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
There was a problem hiding this comment.
@davidtwco I'd appreciate your review here too.
| // For pauthtest the only supported unwinding mechanism is provided by libunwind. | ||
| #[cfg(target_abi = "pauthtest")] | ||
| #[link(name = "unwind")] | ||
| unsafe extern "C" {} |
There was a problem hiding this comment.
this is making some assumptions about the target_os values that can occur in combination with target_abi = "pauthtest" right? Can you make those assumptions explicit in a comment here?
There was a problem hiding this comment.
I've re-worded it to mention that the only supported OS for pauthtest target is Linux.
| // `pacib` corresponds to `ptrauth_key_process_dependent_code` in <ptrauth.h>. | ||
| core::arch::asm!( | ||
| "pacib {addr}, {sp}", | ||
| addr = inout(reg) addr, | ||
| sp = in(reg) sp, | ||
| options(nostack, preserves_flags) | ||
| ); |
There was a problem hiding this comment.
should this be in stdarch? or does it really only make sense to use inline assembly here?
There was a problem hiding this comment.
I originally introduced an intrinsic for resigning. Then discussed it with @davidtwco, and decided that inline asm was the better approach. Providing an intrinsic could potentially introduce a security risk by making it easier to construct signing oracles. As for stdarch, my understanding is that there is ongoing work to provide more comprehensive pointer authentication support, but to my knowledge it has not been completed yet.
There was a problem hiding this comment.
Yeah, I'd like to propose eventually a handful of more complete pointer authentication intrinsics but making sure they can't be used as signing oracles is difficult so we've not proposed anything yet
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
|
Hi all (@davidtwco, @folkertdev, @tgross35, @madsmtm, @bjorn3), The PR introducing the pauthtest target has been merged into main. I'd be very grateful if we could move this one forward as well. Thank you! |
| #[cfg(any(target_arch = "loongarch32", target_arch = "loongarch64"))] | ||
| const UNWIND_DATA_REG: (i32, i32) = (4, 5); // a0, a1 | ||
|
|
||
| unsafe fn sign_lpad(context: *mut uw::_Unwind_Context, lpad: *const u8) -> *const u8 { |
There was a problem hiding this comment.
I'm not sure it's strictly necessary, but we could use #[rustc_force_inline] here which we added for PAuth-related use but haven't actually started using yet
There was a problem hiding this comment.
It does the trick, for a simple backtrace sample, when inspected with objdump, I can see the code inlined into rust_eh_personality:
000000000006a7b0 <rust_eh_personality>:
6a7b0: d503237f pacibsp
6a7b4: d102c3ff sub sp, sp, #0xb0
...
6ac7c: aa1303e0 mov x0, x19
6ac80: 528003e1 mov w1, #0x1f // =31
6ac84: 940053cf bl 0x7fbc0 <_Unwind_GetGR@plt>
6ac88: dac10416 pacib x22, x0
6ac8c: aa1303e0 mov x0, x19
6ac90: aa1603e1 mov x1, x22
6ac94: 940053cf bl 0x7fbd0 <_Unwind_SetIP@plt>
...no branching to sign_lpad, which got entirely optimised-out (inlined).
There was a problem hiding this comment.
In that case, given this fn is only used in std, we could use it here
5 participants
View all comments
This is a follow up to #155722, adding library support for the target.