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.

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

Split pauth target feature #93782

Merged
merged 1 commit into from
Feb 12, 2022
Merged

Conversation

adamgemmell
Copy link
Contributor

Per discussion on #86941 we'd like to split pauth into paca and pacg in order to better support possible future environments that only have the keys available for address or generic authentication. At the moment LLVM has the one pauth target_feature while Linux presents separate paca and pacg flags for feature detection.

Because the use of target_feature will "allow the compiler to generate code under the assumption that this code will only be reached in hosts that support the feature", it does not make sense to simply translate paca into the LLVM feature pauth, as it will generate code as if pacg is available.

To accommodate this we error if only one of the two features is present. If LLVM splits them in the future we can remove this restriction without making a breaking change.

r? @Amanieu

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 8, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 8, 2022
Copy link
Member

@Amanieu Amanieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM apart from a minor nit with the error message.

compiler/rustc_codegen_llvm/src/llvm_util.rs Outdated Show resolved Hide resolved
@Amanieu
Copy link
Member

Amanieu commented Feb 10, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Feb 10, 2022

📌 Commit 9885b5457fc81ecb258393ef9ad53fc74cae14a6 has been approved by Amanieu

@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 Feb 10, 2022
@rust-log-analyzer

This comment has been minimized.

@Amanieu
Copy link
Member

Amanieu commented Feb 10, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Feb 10, 2022

📌 Commit d39a637 has been approved by Amanieu

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 11, 2022
…h, r=Amanieu

Split `pauth` target feature

Per discussion on rust-lang#86941 we'd like to split `pauth` into `paca` and `pacg` in order to better support possible future environments that only have the keys available for address or generic authentication. At the moment LLVM has the one `pauth` target_feature while Linux presents separate `paca` and `pacg` flags for feature detection.

Because the use of [target_feature](https://rust-lang.github.io/rfcs/2045-target-feature.html) will "allow the compiler to generate code under the assumption that this code will only be reached in hosts that support the feature", it does not make sense to simply translate `paca` into the LLVM feature `pauth`, as it will generate code as if `pacg` is available.

To accommodate this we error if only one of the two features is present. If LLVM splits them in the future we can remove this restriction without making a breaking change.

r? `@Amanieu`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 11, 2022
…h, r=Amanieu

Split `pauth` target feature

Per discussion on rust-lang#86941 we'd like to split `pauth` into `paca` and `pacg` in order to better support possible future environments that only have the keys available for address or generic authentication. At the moment LLVM has the one `pauth` target_feature while Linux presents separate `paca` and `pacg` flags for feature detection.

Because the use of [target_feature](https://rust-lang.github.io/rfcs/2045-target-feature.html) will "allow the compiler to generate code under the assumption that this code will only be reached in hosts that support the feature", it does not make sense to simply translate `paca` into the LLVM feature `pauth`, as it will generate code as if `pacg` is available.

To accommodate this we error if only one of the two features is present. If LLVM splits them in the future we can remove this restriction without making a breaking change.

r? ``@Amanieu``
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 11, 2022
…askrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#90955 (Rename `FilenameTooLong` to `InvalidFilename` and also use it for Windows' `ERROR_INVALID_NAME`)
 - rust-lang#91607 (Make `span_extend_to_prev_str()` more robust)
 - rust-lang#92895 (Remove some unused functionality)
 - rust-lang#93635 (Add missing platform-specific information on current_dir and set_current_dir)
 - rust-lang#93660 (rustdoc-json: Add some tests for typealias item)
 - rust-lang#93782 (Split `pauth` target feature)
 - rust-lang#93868 (Fix incorrect register conflict detection in asm!)
 - rust-lang#93888 (Implement `AsFd` for `&T` and `&mut T`.)
 - rust-lang#93909 (Fix typo: explicitely -> explicitly)
 - rust-lang#93910 (fix mention of moved function in `rustc_hir` docs)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 13d636d into rust-lang:master Feb 12, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 12, 2022
@adamgemmell adamgemmell deleted the dev/adagem01/split-pauth branch February 13, 2022 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

6 participants