Skip to content

fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia - #159873

Open
PiJoules wants to merge 1 commit into
rust-lang:mainfrom
PiJoules:fuchsia-safestack
Open

fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia#159873
PiJoules wants to merge 1 commit into
rust-lang:mainfrom
PiJoules:fuchsia-safestack

Conversation

@PiJoules

@PiJoules PiJoules commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

View all comments

Make it also enabled by default just like it is for clang.

@rustbot

rustbot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

These commits modify compiler targets.
(See the Target Tier Policy.)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 24, 2026
@rustbot

rustbot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

r? @adwinwhite

rustbot has assigned @adwinwhite.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 74 candidates
  • Random selection from 18 candidates

@PiJoules

Copy link
Copy Markdown
Contributor Author

r? @erickt

@rustbot

rustbot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Failed to set assignee to erickt: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@rust-log-analyzer

This comment has been minimized.

@workingjubilee

Copy link
Copy Markdown
Member

huh. that's weird.

@adwinwhite

Copy link
Copy Markdown
Contributor

@rustbot author

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 13, 2026
@rustbot

rustbot commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 13, 2026
@rustbot

rustbot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

These commits modify compiler targets.
(See the Target Tier Policy.)

@rustbot

This comment has been minimized.

@PiJoules

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 26, 2026

@adwinwhite adwinwhite Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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")]?

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Apparently safestack wasn't a valid option for that macro, but #161888 should make it one

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@adwinwhite adwinwhite added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 27, 2026
jhpratt added a commit to jhpratt/rust that referenced this pull request Aug 29, 2026
…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.
rust-bors Bot pushed a commit that referenced this pull request Aug 29, 2026
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.
@rustbot

rustbot commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

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.

@PiJoules

PiJoules commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Sorry I accidentally did a force push but all the comments should be resolved

@adwinwhite

Copy link
Copy Markdown
Contributor

No worries. It's good to squash commits for small changes.
@bors r+ rollup

@rust-bors

rust-bors Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 7a44484 has been approved by adwinwhite

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 2, 2026
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 2, 2026
…nwhite

fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia

Make it also enabled by default just like it is for clang.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 2, 2026
…nwhite

fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia

Make it also enabled by default just like it is for clang.
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 2, 2026
…nwhite

fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia

Make it also enabled by default just like it is for clang.
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 2, 2026
…nwhite

fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia

Make it also enabled by default just like it is for clang.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 2, 2026
…nwhite

fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia

Make it also enabled by default just like it is for clang.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 2, 2026
…nwhite

fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia

Make it also enabled by default just like it is for clang.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 2, 2026
…nwhite

fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia

Make it also enabled by default just like it is for clang.
rust-bors Bot pushed a commit that referenced this pull request Sep 2, 2026
…uwer

Rollup of 23 pull requests

Successful merges:

 - #162164 (Revert "Implement Debug for C-like enums with a concatenated string")
 - #160564 (volatile: allow accesses to non-AM memory to trap)
 - #161579 (suggest calling a fn item used as the iterator of a `for` loop)
 - #162044 (coverage: Resolve spans to file-coordinates in a separate step)
 - #162120 (Introduce `PerOwnerLoweringState`)
 - #162151 (Test itanium mangling of `f16` and `f128`)
 - #162162 (Don't special-case `!` in stability checks anymore)
 - #162181 (Remove wrong UnusedBraces lint for iterator loop in edition 2024 )
 - #162187 (Rename `thir::ExprKind::Use` to `ValueExpr`)
 - #158401 (mgca: Don't ICE when evaluating ValTrees that contain error constants)
 - #159873 (fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia)
 - #161847 (Preserve visibility in nested macro import suggestions)
 - #161972 (Improve tests for `#[track_caller]` in async)
 - #162008 (Render the `box` pattern removal diagnostic more actionable & remove `box` expression recovery)
 - #162065 (std: don't reference `libc::O_NOFOLLOW` on VxWorks in `set_perm_nofollow`)
 - #162076 (docs(num): clarify conditions under which error occurs in `impl TryFrom<int> for int`)
 - #162152 (Revert "retrieve supported GCC targets from the sysroot")
 - #162153 (Prefer `LLVMGetVersion` for runtime info)
 - #162168 (fix ICE in project_goals/inherent)
 - #162171 (Explain LoongArch f16 NaN-boxing in inline asm)
 - #162173 (fix supposedly unreachable `bug!` being reachable)
 - #162191 (core: mark float `ClampBounds` methods as `#[inline]`)
 - #162199 (docs(time): clarify exact seconds for hour and minute)
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 2, 2026
…nwhite

fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia

Make it also enabled by default just like it is for clang.
rust-bors Bot pushed a commit that referenced this pull request Sep 3, 2026
Rollup of 23 pull requests

Successful merges:

 - #157808 (sanitizers: Implement support for the sanitize ignorelist)
 - #160564 (volatile: allow accesses to non-AM memory to trap)
 - #161579 (suggest calling a fn item used as the iterator of a `for` loop)
 - #162044 (coverage: Resolve spans to file-coordinates in a separate step)
 - #162120 (Introduce `PerOwnerLoweringState`)
 - #162151 (Test itanium mangling of `f16` and `f128`)
 - #162162 (Don't special-case `!` in stability checks anymore)
 - #162181 (Remove wrong UnusedBraces lint for iterator loop in edition 2024 )
 - #162187 (Rename `thir::ExprKind::Use` to `ValueExpr`)
 - #158401 (mgca: Don't ICE when evaluating ValTrees that contain error constants)
 - #159873 (fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia)
 - #161847 (Preserve visibility in nested macro import suggestions)
 - #161972 (Improve tests for `#[track_caller]` in async)
 - #162008 (Render the `box` pattern removal diagnostic more actionable & remove `box` expression recovery)
 - #162065 (std: don't reference `libc::O_NOFOLLOW` on VxWorks in `set_perm_nofollow`)
 - #162076 (docs(num): clarify conditions under which error occurs in `impl TryFrom<int> for int`)
 - #162152 (Revert "retrieve supported GCC targets from the sysroot")
 - #162153 (Prefer `LLVMGetVersion` for runtime info)
 - #162168 (fix ICE in project_goals/inherent)
 - #162171 (Explain LoongArch f16 NaN-boxing in inline asm)
 - #162173 (fix supposedly unreachable `bug!` being reachable)
 - #162191 (core: mark float `ClampBounds` methods as `#[inline]`)
 - #162199 (docs(time): clarify exact seconds for hour and minute)
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 3, 2026
…nwhite

fuchsia: Add safestack as a supported sanitizer for x86_64 fuchsia

Make it also enabled by default just like it is for clang.
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.

5 participants