Library support for aarch64-unknown-linux-pauthtest target#156548
Library support for aarch64-unknown-linux-pauthtest target#156548jchlanda wants to merge 5 commits into
Conversation
|
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. |
|
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 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
|
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! |
|
@bors r+ rollup |
Library support for aarch64-unknown-linux-pauthtest target This is a follow up to rust-lang#155722, adding library support for the target.
|
💔 I suspect this PR failed tests as part of a rollup After fixing the problem, consider running a try job for the failed job before re-approving. Link to failure: #158858 (comment) |
|
This pull request was unapproved. This PR was contained in a rollup (#158858), which was unapproved. |
I addressed the failure in b749630. |
|
@bors try |
|
@jchlanda: 🔑 Insufficient privileges: not in try users |
View all comments
This is a follow up to #155722, adding library support for the target.