Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rustc_codegen_llvm: Add a new 'pc' option to branch-protection #132259

Merged
merged 1 commit into from
Nov 6, 2024
Merged

rustc_codegen_llvm: Add a new 'pc' option to branch-protection #132259

merged 1 commit into from
Nov 6, 2024

Conversation

mrkajetanp
Copy link
Contributor

Add a new 'pc' option to -Z branch-protection for aarch64 that enables the use of PC as a diversifier in PAC branch protection code.

When the pauth-lr target feature is enabled in combination with -Z branch-protection=pac-ret,pc, the new 9.5-a instructions (pacibsppc, retaasppc, etc) will be generated.

@rustbot
Copy link
Collaborator

rustbot commented Oct 28, 2024

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 28, 2024
@rustbot
Copy link
Collaborator

rustbot commented Oct 28, 2024

Some changes occurred in src/doc/unstable-book/src/compiler-flags/branch-protection.md

cc @rust-lang/project-exploit-mitigations, @rcvalle

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@mrkajetanp mrkajetanp force-pushed the branch-protection-pauth-lr branch from e107afd to 45c2aa0 Compare October 28, 2024 11:29
@rcvalle
Copy link
Member

rcvalle commented Oct 29, 2024

Thank you for your time and for working on this, @mrkajetanp! Much appreciated.

I've a few questions:

  • Is the new pc option a no op for targets that don't support the pauth-lr feature in armclang/clang as well or it causes an error?
  • Does the pauth-lr feature have to be explicitly enabled as an option for using the pc option in armclang/clang as well or is it implicitly enabled for some targets?

@Nadrieril
Copy link
Member

I know nothing of codegen

r? compiler

@rustbot rustbot assigned compiler-errors and unassigned Nadrieril Oct 29, 2024
@bors

This comment was marked as resolved.

Sign in to view
@mrkajetanp
Copy link
Contributor Author

  • Is the new pc option a no op for targets that don't support the pauth-lr feature in armclang/clang as well or it causes an error?

The way it's currently implemented in LLVM is that adding the pc option on its own will generate hint-space instructions that are NOPs on targets without the feature.
Adding the pc option alongside -Ctarget-feature=+pauth-lr will generate non-hint instructions that will SIGILL on targets that don't support them.

  • Does the pauth-lr feature have to be explicitly enabled as an option for using the pc option in armclang/clang as well or is it implicitly enabled for some targets?

As explained above, the pc option can be used without enabling the feature thanks to the hint instructions. To actually use the new instructions, the feature needs to be explicitly enabled. It won't be implicitly enabled on any targets because it's very new, there is currently no silicon with that feature anywhere.

The merge conflicts are due to some refactoring from what I can see, I'll rebase once this is approved or once some changes are requested?

@rcvalle
Copy link
Member

rcvalle commented Oct 30, 2024

Thank you for all the information, @mrkajetanp! LGTM (FYI, @cuviper).

@mrkajetanp
rustc_codegen_llvm: Add a new 'pc' option to branch-protection
10edeea 
Add a new 'pc' option to -Z branch-protection for aarch64 that
enables the use of PC as a diversifier in PAC branch protection code.

When the pauth-lr target feature is enabled in combination
with -Z branch-protection=pac-ret,pc, the new 9.5-a instructions
(pacibsppc, retaasppc, etc) will be generated.
@mrkajetanp mrkajetanp force-pushed the branch-protection-pauth-lr branch from 45c2aa0 to 10edeea Compare October 31, 2024 13:51
@compiler-errors
Copy link
Member

r? compiler

@davidtwco
Copy link
Member

@mrkajetanp is a colleague of mine so hadn't assigned myself to review and approve previously, but given that @rcvalle has also reviewed, I'm happy to approve it.

r? @davidtwco
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 5, 2024

📌 Commit 10edeea has been approved by davidtwco

It is now in the queue for this repository.

@rustbot rustbot assigned davidtwco and unassigned petrochenkov Nov 5, 2024
@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 5, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 5, 2024
@matthiaskrgr
Rollup merge of rust-lang#132259 - mrkajetanp:branch-protection-pauth…
75352b9 
…-lr, r=davidtwco

rustc_codegen_llvm: Add a new 'pc' option to branch-protection

Add a new 'pc' option to -Z branch-protection for aarch64 that enables the use of PC as a diversifier in PAC branch protection code.

When the pauth-lr target feature is enabled in combination with -Z branch-protection=pac-ret,pc, the new 9.5-a instructions (pacibsppc, retaasppc, etc) will be generated.
This was referenced Nov 5, 2024
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 5, 2024
@bors
Auto merge of rust-lang#132661 - matthiaskrgr:rollup-npytbl6, r=matth…
4a91ff6 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#132259 (rustc_codegen_llvm: Add a new 'pc' option to branch-protection)
 - rust-lang#132409 (CI: switch 7 linux jobs to free runners)
 - rust-lang#132498 (Suggest fixing typos and let bindings at the same time)
 - rust-lang#132524 (chore(style): sync submodule exclusion list between tidy and rustfmt)
 - rust-lang#132567 (Properly suggest `E::assoc` when we encounter `E::Variant::assoc`)
 - rust-lang#132571 (add const_eval_select macro to reduce redundancy)
 - rust-lang#132637 (Do not filter empty lint passes & re-do CTFE pass)
 - rust-lang#132642 (Add documentation on `ast::Attribute`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c8247c0 into rust-lang:master Nov 6, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 6, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 6, 2024
@rust-timer
Unrolled build for rust-lang#132259
dff0407 
Rollup merge of rust-lang#132259 - mrkajetanp:branch-protection-pauth-lr, r=davidtwco

rustc_codegen_llvm: Add a new 'pc' option to branch-protection

Add a new 'pc' option to -Z branch-protection for aarch64 that enables the use of PC as a diversifier in PAC branch protection code.

When the pauth-lr target feature is enabled in combination with -Z branch-protection=pac-ret,pc, the new 9.5-a instructions (pacibsppc, retaasppc, etc) will be generated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@davidtwco davidtwco

Labels
A-run-make Area: port run-make Makefiles to rmake.rs PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.84.0
Development

Successfully merging this pull request may close these issues.
9 participants
@mrkajetanp @rustbot @rust-log-analyzer @rcvalle @Nadrieril @bors @compiler-errors @davidtwco @petrochenkov