fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia - #159873
fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia#159873PiJoules wants to merge 3 commits into
Conversation
|
These commits modify compiler targets. |
|
r? @adwinwhite rustbot has assigned @adwinwhite. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
r? @erickt |
|
Failed to set assignee to
|
This comment has been minimized.
This comment has been minimized.
|
huh. that's weird. |
|
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
Make it also enabled by default just like it is for clang.
tests/assembly-llvm/stack-protector/stack-protector-target-support.rs. Since safestack is enabled by default for fuchsia, it will not emit __stack_chk_fail calls.
2d56acc to
e717aa8
Compare
|
These commits modify compiler targets. |
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@rustbot ready |
There was a problem hiding this comment.
So __stack_chk_fail is optimized away if we have safe stack on?
Rather than remove testing for stack protector support, would it be better to mark the function with #[sanitize(safestack = "off")]?
There was a problem hiding this comment.
So __stack_chk_fail is optimized away if we have safe stack on?
Specifically it's never emitted since the safestack machinery would move all stack references to the unsafe stack so the current safe stack wouldn't need the normal stack protector instrumentation.
Rather than remove testing for stack protector support, would it be better to mark the function with #[sanitize(safestack = "off")]?
Yeah I think that also works
There was a problem hiding this comment.
Apparently safestack wasn't a valid option for that macro, but #161888 should make it one
There was a problem hiding this comment.
Could you also add a comment on the sanitize attribute that we only do this because we can't disable default sanitizers via compile-flags?
the function with safestack.
…ble, r=adwinwhite compiler: Allow safestack to be togglable via #[sanitize(safestack = "...")] Since safestack is a sanitizer, it should be possible to allow enabling/disabling it on functions like we do with the other sanitizers. It's likely that when adding the `#[sanitize(...)]` support for the other ones that safestack was just missed, so this makes safestack follow suite. It was also suggested in rust-lang#159873 that we just disable safestack this way for a specific test where we should check stack protector instrumentation.
Rollup merge of #161888 - PiJoules:sanitize-safestack-togglable, r=adwinwhite compiler: Allow safestack to be togglable via #[sanitize(safestack = "...")] Since safestack is a sanitizer, it should be possible to allow enabling/disabling it on functions like we do with the other sanitizers. It's likely that when adding the `#[sanitize(...)]` support for the other ones that safestack was just missed, so this makes safestack follow suite. It was also suggested in #159873 that we just disable safestack this way for a specific test where we should check stack protector instrumentation.
Make it also enabled by default just like it is for clang.