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

Use error-on-mismatch policy for PAuth module flags. #93269

Merged
merged 1 commit into from
Jan 25, 2022

Conversation

jacobbramley
Copy link
Contributor

@jacobbramley jacobbramley commented Jan 24, 2022

This agrees with Clang, and avoids an error when using LTO with mixed
C/Rust. LLVM considers different behaviour flags to be a mismatch,
even when the flag value itself is the same.

This also makes the flag setting explicit for all uses of
LLVMRustAddModuleFlag.


I believe that this fixes #92885, but have only reproduced it locally on Linux hosts so cannot confirm that it fixes the issue as reported.

I have not included a test for this because it is covered by an existing test (src/test/run-make-fulldeps/cross-lang-lto-clang). It is not without its problems, though:

This agrees with Clang, and avoids an error when using LTO with mixed
C/Rust. LLVM considers different behaviour flags to be a mismatch,
even when the flag value itself is the same.

This also makes the flag setting explicit for all uses of
LLVMRustAddModuleFlag.
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 24, 2022
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @petrochenkov (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 24, 2022
@petrochenkov
Copy link
Contributor

I'm not familiar with this code, but the changes all look reasonable, the existing LLVMModFlagBehavior::Warning behavior is preserved in most cases, except for branch-target-enforcement and sign-return-address* which are now changed to errors.

@bors r+
cc @bjorn3 @michaelwoerister @nikic

@bors
Copy link
Contributor

bors commented Jan 25, 2022

📌 Commit e02e958 has been approved by petrochenkov

@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 Jan 25, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 25, 2022
…petrochenkov

Use error-on-mismatch policy for PAuth module flags.

This agrees with Clang, and avoids an error when using LTO with mixed
C/Rust. LLVM considers different behaviour flags to be a mismatch,
even when the flag value itself is the same.

This also makes the flag setting explicit for all uses of
LLVMRustAddModuleFlag.

----

I believe that this fixes rust-lang#92885, but have only reproduced it locally on Linux hosts so cannot confirm that it fixes the issue as reported.

I have not included a test for this because it is covered by an existing test (`src/test/run-make-fulldeps/cross-lang-lto-clang`). It is not without its problems, though:
* The test requires Clang and `--run-clang-based-tests-with=...` to run, and this is not the case on the CI.
   * Any test I add would have a similar requirement.
* With this patch applied, the test gets further, but it still fails (for other reasons). I don't think that affects rust-lang#92885.
This was referenced Jan 25, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 25, 2022
…askrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#88794 (Add a `try_clone()` function to `OwnedFd`.)
 - rust-lang#93064 (Properly track `DepNode`s in trait evaluation provisional cache)
 - rust-lang#93118 (Move param count error emission to end of `check_argument_types`)
 - rust-lang#93144 (Work around missing code coverage data causing llvm-cov failures)
 - rust-lang#93169 (Fix inconsistency of local blanket impls)
 - rust-lang#93175 (Implement stable overlap check considering negative traits)
 - rust-lang#93251 (rustdoc settings: use radio buttons for theme)
 - rust-lang#93269 (Use error-on-mismatch policy for PAuth module flags.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 13b87d8 into rust-lang:master Jan 25, 2022
@rustbot rustbot added this to the 1.60.0 milestone Jan 25, 2022
@pnkfelix pnkfelix added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 26, 2022
@pnkfelix
Copy link
Member

nominating for beta-backport since #92885 is regression from stable-to-beta.

@jacobbramley jacobbramley deleted the dev/pauth-option-1 branch February 1, 2022 09:28
@apiraino
Copy link
Contributor

apiraino commented Feb 1, 2022

Beta backport declined as per compiler team on Zulip, in favor of #93523 (revert of #88354)

@rustbot label -beta-nominated

@rustbot rustbot removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 1, 2022
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.

Some mixes of Rust with C/C++ are broken for arm64 mac and windows
7 participants