-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Add -Zindirect-branch-cs-prefix
#140740
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
base: master
Are you sure you want to change the base?
Add -Zindirect-branch-cs-prefix
#140740
Conversation
4422b3c
to
0582d8f
Compare
0582d8f
to
b0392c5
Compare
@rustbot label F-target_modifiers A-rust-for-linux |
This comment has been minimized.
This comment has been minimized.
The error is related to the base PR -- I think this one should be clean (modulo the marked |
b0392c5
to
f395de3
Compare
This comment has been minimized.
This comment has been minimized.
f395de3
to
a79ff99
Compare
This comment has been minimized.
This comment has been minimized.
src/doc/unstable-book/src/compiler-flags/indirect-branch-cs-prefix.md
Outdated
Show resolved
Hide resolved
a79ff99
to
879a3bc
Compare
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
879a3bc
to
331c2b6
Compare
rustbot has assigned @compiler-errors. Use |
-Zindirect-branch-cs-prefix
on top of -Zretpoline*
-Zindirect-branch-cs-prefix
Assigning same reviewer as #135927. r? @davidtwco |
In the last RfL meeting we discussed this flag and clarified that this isn't useful w/out retpoline but you also might not always want it with retpoline, and with that context, it was left as a t-compiler decision whether or not to keep it a separate flag - for consistency with other language toolchains - or add it as a modifier to the retpoline flag - which is more consistent with our other flags. Given that, maybe it's best we don't re-use the MCP for retpoline and open a new one to put that question to the team. I think I lean towards a separate flag for something like this that's quite niche. |
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.
Implementation otherwise looks good to me
//@ only-x86_64 | ||
// FIXME: Are these required? | ||
//@ ignore-apple Symbol is called `___x86_indirect_thunk` (Darwin's extra underscore) | ||
//@ ignore-sgx Tests incompatible with LVI mitigations |
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.
Surprised that these would be required w/ only-x86_64
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.
I remember I needed those in the past for similar tests, but we can give it a try without them -- let me remove them.
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.
Since we still try to match the __x86_indirect_thunk
without the extra underscore, if an extra underscore is also added in Darwin for that case like it is for __x86_return_thunk
, then I suspect we may still need at least that one, but we will see.
(I guess we could modify the match expression below, but if so we should probably do it in all of them)
Sounds good, I will send it. |
This is intended to be used for Linux kernel RETPOLINE builds. Signed-off-by: Miguel Ojeda <[email protected]>
331c2b6
to
5d6ec1e
Compare
Cc: @azhogin @Darksonn
This goes on top of #135927, i.e. please skip the first commit here. Please feel free to inherit it there.
In fact, I am not sure if there is any use case for the flag without
-Zretpoline*
. GCC and Clang allow it, though.There is a
FIXME
for twoignore
s in the test that I took from another test I did in the past -- they may be needed or not here since I didn't run the full CI. Either way, it is not critical.Tracking issue: #116852.
MCP: rust-lang/compiler-team#868.