[WIP] Introduce aarch64-unknown-linux-pauthtest target#154759
[WIP] Introduce aarch64-unknown-linux-pauthtest target#154759jchlanda wants to merge 39 commits intorust-lang:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
88b623e to
3b3fcce
Compare
This comment has been minimized.
This comment has been minimized.
3b3fcce to
e33dbf3
Compare
This comment has been minimized.
This comment has been minimized.
e33dbf3 to
9e48aaa
Compare
This comment has been minimized.
This comment has been minimized.
9e48aaa to
4468c36
Compare
This comment has been minimized.
This comment has been minimized.
4468c36 to
c9fe7d6
Compare
This comment has been minimized.
This comment has been minimized.
c9fe7d6 to
7ecdaa6
Compare
This comment has been minimized.
This comment has been minimized.
7ecdaa6 to
65007e0
Compare
This comment has been minimized.
This comment has been minimized.
65007e0 to
566b1b6
Compare
This comment has been minimized.
This comment has been minimized.
0db30a1 to
5bc3e48
Compare
63e45fd to
0606d27
Compare
This comment has been minimized.
This comment has been minimized.
| if (!C) | ||
| return Ptr; | ||
| if (!C->getType()->isPointerTy()) | ||
| return Ptr; | ||
| if (isa<UndefValue>(C) || isa<ConstantPointerNull>(C)) | ||
| return Ptr; |
There was a problem hiding this comment.
Do we expect values non-conforming to these conditions being passed to this function? Locally, I've commented out these lines, and nothing seems to be broken.
So, can we safely convert these to assertions? Or, maybe, some checks which would be present in release mode as well (and would panic when mismatch is detected)? Please just explain which contract do we have, who is responsible for these checks and whether the checks need to be just assertions or if we need to make them panicking or smth.
If there's a reason why we need to keep the current behavior, it's totally fine. But if so, can we somehow rename the function? Now it's name might make one think that we always wrap the underlying constant pointer value to ptrauth constant. But we also have this chunk of logic returning the exact input value w/o any change, and this is not clear from the function name.
There was a problem hiding this comment.
Fair.
With it now being wrapped in const_ptr_auth and only two call sites we can't violate the contract.
However it is still a symbol that can be accessed freely. I'm going to change it to asserts.
This comment has been minimized.
This comment has been minimized.
Looks like there is no bug in there, it's an expected behaviour. I'm guessing that when you compiled for You can verify that by either changing the arch to Will disable the test and add a comment. |
What I'm worried about is that in compiler/rustc_target/src/spec/targets/aarch64_unknown_linux_pauthtest.rs we have And my understanding was that if we add |
|
|
||
| options: TargetOptions { | ||
| env: Env::Pauthtest, | ||
| features: "+v8.3a,+outline-atomics,+pauth".into(), |
There was a problem hiding this comment.
I've just found this in llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:
// Generate outline atomics library calls only if LSE was not specified for
// subtarget
if (Subtarget->outlineAtomics() && !Subtarget->hasLSE()) {
And I've also inspected test/CodeGen/AArch64/atomic-ops-lse.ll and it looks like that yes, having both +lse and +outline-atomics makes the latter ineffective.
So, I suppose it's worth deleting +outline-atomics from here because it's just misleading. And might be also worth adding a comment explaining that pauthtest target is different to other aarch64-linux targets (all including +outline-atomics), because v8.3a includes lse, and lse makes outline-attomics ineffective.
There was a problem hiding this comment.
Sorry, posted It in a wrong thread:
Targeting musl with: "target-features"="+lse,+outline-atomics", results:
.file "aarch64_outline_atomics.bca081c37e0149a7-cgu.0"
.text
.globl _RNvCsgc3x3YG0xfv_23aarch64_outline_atomics16compare_exchange // -- Begin function _RNvCsgc3x3YG0xfv_23aarch64_outline_atomics16compare_exchange
.p2align 2
.type _RNvCsgc3x3YG0xfv_23aarch64_outline_atomics16compare_exchange,@function
_RNvCsgc3x3YG0xfv_23aarch64_outline_atomics16compare_exchange: // @_RNvCsgc3x3YG0xfv_23aarch64_outline_atomics16compare_exchange
.cfi_startproc
// %bb.0: // %start
mov w9, #10 // =0xa
mov w8, wzr
cas w8, w9, [x0]
ret
.Lfunc_end0:
.size _RNvCsgc3x3YG0xfv_23aarch64_outline_atomics16compare_exchange, .Lfunc_end0-_RNvCsgc3x3YG0xfv_23aarch64_outline_atomics16compare_exchange
.cfi_endproc
// -- End function
.ident "rustc version 1.97.0-dev"
.section ".note.GNU-stack","",@progbits
.addrsigI guess that means, that we should not be requesting outline-atomics, as the compiler will never be able to provide it.
This comment has been minimized.
This comment has been minimized.
6ee8cab to
9f2ac73
Compare
| pauthtest-enabled sysroot with custom musl, serving as a reference libc | ||
| implementation. | ||
|
|
||
| Supported features include: |
| in Rust can be found at | ||
| [#148640](https://github.com/rust-lang/rust/issues/148640). | ||
|
|
||
| Existing compiler options such as `-mbranch-protection` provide limited pointer |
There was a problem hiding this comment.
Is it -mbranch-protection for Rust? My understanding was that it's -Z branch-protection (while its indeed -mbranch-protection for clang). See also src/doc/unstable-book/src/compiler-flags/branch-protection.md.
Also, I'm not sure if it's worth talking about BTI - I doubt that anyone would mess BTI with pauthtest. But for pac-ret and pauthtest - it's non-obvious for new-comers because both these are based on the same PAC extension for aarch64 CPUs
| Clang-based toolchain. In this case, no wrapper script is required, | ||
| `<toolchain_root>/bin/aarch64-linux-pauthtest-clang` can be used directly. | ||
|
|
||
| ## Building the target |
There was a problem hiding this comment.
Removing the second one, both paragraphs should belong to the same header.
Is this update already present in this PR? Like, I'm still seeing the same header at lines 94 and 173.
| `aarch64-unknown-linux-pauthtest` target enabled. | ||
|
|
||
| For a comprehensive example of how to interact between C and Rust programs | ||
| withing the testing framework please consult |
There was a problem hiding this comment.
Typo: within
| * non-ABI-affecting indirect control flow hardening features included in | ||
| pauthtest ABI (corresponding to `-faarch64-jump-table-hardening`, | ||
| `-fptrauth-indirect-gotos`) | ||
| * signed ELF GOT entries (gated behind `-Z ptrauth-elf-got` off by default) |
There was a problem hiding this comment.
Nit: probably a comma missed before 'off'?
View all comments
This PR introduces
aarch64-unknown-linux-pauthtesttarget, that enablesPointer Authentication Code (PAC) support in Rust on AArch64 ELF based Linux
systems using a pauthtest ABI (provided by LLVM) and pauthtest-enabled sysroot
with custom musl, serving as a reference libc implementation.
Supported features include:
(corresponds to
-fptrauth-callsincluded in pauthtest ABI as defined inLLVM)
address after restoring from stack for non-leaf functions (corresponds to
-fptrauth-returns)(corresponds to
-fptrauth-auth-traps)pauthtest ABI (corresponding to
-fptrauth-init-fini,-fptrauth-init-fini-address-discrimination)pauthtest ABI (corresponding to
-faarch64-jump-table-hardening,-fptrauth-indirect-gotos)-Z pauth_enable_elf_got, off bydefault)
Please note that efforts were made to split the work into individual commits
that encapsulate different areas of the code; however, the commits are not
atomic and cannot be built or tested in isolation.
Useful links: