-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Check AArch64 branch-protection earlier in the pipeline. #105421
Conversation
The branch-protection codegen option has always been treated as an error for non-AArch64 targets.
cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice. One nit inline, but otherwise r=me.
if sess.target.arch == "aarch64" { | ||
if let Some(BranchProtection { bti, pac_ret }) = sess.opts.unstable_opts.branch_protection { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than making this a no-op, we should probably bug!
this, so that any issues with checking in rustc_session
don’t get silently ignored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Done in 73d374f.
@bors r+ |
⌛ Testing commit 73d374f with merge e2bf6962bd0d2bd9a3710dc9fb074eb049d2d957... |
💔 Test failed - checks-actions |
The test fails on an off-by-one error on some documentation layout. It doesn't appear at all relevant to this PR. Note, however, that I can reproduce the error reliably on my branch, but only when run as part of the full sequence. For example, I can't reproduce it by isolating the failing test as follows:
Note that dropping Attempting to work out what the failure actually was:
Manually rendering the test file in Firefox shows a At this point, I am somewhat at a loss. @willcrichton, it seems that you did some work on this test recently. Have you any ideas? |
The job Click to see the possible cause of the failure (guessed by this bot)
|
@bors retry unrelated rustdoc-gui failure |
I will check why this test is flaky. cc @GuillaumeGomez have you seen any issues like this w/ browser-UI-test before? |
⌛ Testing commit 73d374f with merge 6ea11f0a9cc8b2b2792e1d101bddad3b4978b7b2... |
💔 Test failed - checks-actions |
Oh no, a different GUI test failed... @GuillaumeGomez this looks like a broader flakiness problem. |
For the GUI failure, linked to #93784. |
Since it's not related to this PR, let's retry. @bors retry |
☀️ Test successful - checks-actions |
Finished benchmarking commit (aef17b7): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. |
The job Click to see the possible cause of the failure (guessed by this bot)
|
…=nagisa Check AArch64 branch-protection earlier in the pipeline. As suggested in rust-lang#93516. r? `@nagisa`
As suggested in #93516.
r? @nagisa