Skip to content

Affirm -Cforce-frame-pointers=off does not override #140774

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

Conversation

workingjubilee
Copy link
Member

@workingjubilee workingjubilee commented May 7, 2025

This PR exists to document that we (that is, the compiler reviewer) implicitly made a decision in #86652 that defies the expectations of some programmers. Some programmers believe -Cforce-frame-pointers=false should obey the programmer in all cases, forcing the compiler to avoid generating frame pointers, even if the target specification would indicate they must be generated. However, many targets rely on frame pointers for fast or sound unwinding.

T-compiler had a weekly triage meeting on 2025-05-22. This topic was put to discussion because some programmers may expect the target-overriding behavior. In that meeting we decided removing frame pointers, at least with regards to the contract of the -Cforce-frame-pointers option, is not required, even if =off is passed, and that we will not do so if the target would expect them. This follows from the documentation here: https://doc.rust-lang.org/rustc/codegen-options/index.html#force-frame-pointers

We may separately pursue trying to clarify the situation more emphatically in our documentation, or warn when people pass the option when it doesn't do anything.

@workingjubilee workingjubilee added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-compiler-nominated Nominated for discussion during a compiler team meeting. labels May 7, 2025
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 7, 2025
@workingjubilee workingjubilee added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 7, 2025
@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member

bjorn3 commented May 7, 2025

The current behavior of being a ratchet makes most sense to me. Enabling force-frame-pointers forces frame pointers to be used, disabling it is equivalent to not passing it at all and causes the default to be used. This matches the behavior of force-unwind-tables (for which the current behavior is required. disabling unwind tables when they are enabled by default is unsound and picking the target default even when force-unwind-tables is enabled makes the cli option useless)

@Noratrieb
Copy link
Member

I agree and think the naming conveys this as well. The absence of force should not force the opposite, it just leaves it up to the compiler to choose.

In fact, this is already clearly documented: https://doc.rust-lang.org/rustc/codegen-options/index.html?highlight=force-frame#force-frame-pointers

n, no, off or false: do not force-enable frame pointers. This does not necessarily mean frame pointers will be removed.

And this documentation has been there basically forever: #65136

@workingjubilee
Copy link
Member Author

Yes. This is essentially me looking for a clarification that yes, we intended to head in this direction, to make sure I have something authoritative to point to if it comes up again and help inform any near-future decisions about frame-pointer-related things.

@apiraino
Copy link
Contributor

apiraino commented May 22, 2025

Discussed durint T-compiler triage on Zulip.

We agree the situation with this multiple-value boolean flag is awkward. Maybe updating the documentation (comment) could alleviate this confusion.

We are in favor of accepting this patch! Thanks for raising the topic.

@rustbot label -I-compiler-nominated

@rustbot rustbot removed the I-compiler-nominated Nominated for discussion during a compiler team meeting. label May 22, 2025
@apiraino apiraino added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels May 29, 2025
@apiraino
Copy link
Contributor

apiraino commented May 29, 2025

I guess the next step is getting someone review this - right?

@workingjubilee please proceed with this work (if you need to) of feel free to ask for a review

@rustbot author

@rustbot rustbot 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 May 29, 2025
This test only makes sense if you send it back in time and run it with
a now-old Rust commit, e.g. 50e0cc5
However, if you do go back that far in time, you will see it pass.
@workingjubilee workingjubilee force-pushed the should-force-frame-pointers-favor-the-target-or-cli branch from aaa10fe to e05c680 Compare June 4, 2025 21:51
@rust-log-analyzer

This comment has been minimized.

@workingjubilee workingjubilee force-pushed the should-force-frame-pointers-favor-the-target-or-cli branch 2 times, most recently from 59c6fb8 to 4d62cab Compare June 4, 2025 22:48
@workingjubilee workingjubilee changed the title Should -Cforce-frame-pointers favor the target or CLI? Decide -Cforce-frame-pointers favors the target Jun 4, 2025
@workingjubilee workingjubilee changed the title Decide -Cforce-frame-pointers favors the target Affirm -Cforce-frame-pointers=off does not override Jun 4, 2025
@workingjubilee workingjubilee changed the title Affirm -Cforce-frame-pointers=off does not override compiler: affirm -Cforce-frame-pointers=off does not override Jun 4, 2025
@workingjubilee workingjubilee changed the title compiler: affirm -Cforce-frame-pointers=off does not override Affirm -Cforce-frame-pointers=off does not override Jun 4, 2025
@workingjubilee
Copy link
Member Author

Yes, I'm indecisive sometimes.

r? compiler

@workingjubilee workingjubilee 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 Jun 4, 2025
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 8, 2025
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 8, 2025
…pointers-favor-the-target-or-cli, r=jieyouxu

Affirm `-Cforce-frame-pointers=off` does not override

This PR exists to document that we (that is, the compiler reviewer) implicitly made a decision in rust-lang#86652 that defies the expectations of some programmers. Some programmers believe `-Cforce-frame-pointers=false` should obey the programmer in all cases, forcing the compiler to avoid generating frame pointers, even if the target specification would indicate they must be generated. However, many targets rely on frame pointers for fast or sound unwinding.

T-compiler had a weekly triage meeting on 2025-05-22. This topic was put to discussion because some programmers may expect the target-overriding behavior. In that meeting we decided removing frame pointers, at least with regards to the contract of the `-Cforce-frame-pointers` option, is not required, even if `=off` is passed, and that we will not do so if the target would expect them. This follows from the documentation here: https://doc.rust-lang.org/rustc/codegen-options/index.html#force-frame-pointers

We may separately pursue trying to clarify the situation more emphatically in our documentation, or warn when people pass the option when it doesn't do anything.
bors added a commit that referenced this pull request Jun 8, 2025
Rollup of 11 pull requests

Successful merges:

 - #140774 (Affirm `-Cforce-frame-pointers=off` does not override)
 - #141001 (Make NonZero<char> possible)
 - #141700 (Atomic intrinsics : use const generic ordering, part 2)
 - #142008 (const-eval error: always say in which item the error occurred)
 - #142053 (Add new Tier-3 targets: `loongarch32-unknown-none*`)
 - #142089 (Replace all uses of sysroot_candidates with get_or_default_sysroot)
 - #142108 (compiler: Add track_caller to AbiMapping::unwrap)
 - #142132 (`tests/ui`: A New Order [6/N])
 - #142162 (UnsafePinned: update get() docs and signature to allow shared mutation)
 - #142171 (`tests/ui`: A New Order [7/N])
 - #142179 (store `target.min_global_align` as an `Align`)

r? `@ghost`
`@rustbot` modify labels: rollup
@workingjubilee
Copy link
Member Author

hurr
@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 8, 2025
@workingjubilee
Copy link
Member Author

@bors2 try jobs=test-various

@rust-bors
Copy link

rust-bors bot commented Jun 17, 2025

⌛ Trying commit 5a449fb with merge 9ca9e59

To cancel the try build, run the command @bors2 try cancel.

rust-bors bot added a commit that referenced this pull request Jun 17, 2025
…vor-the-target-or-cli, r=<try>

Affirm `-Cforce-frame-pointers=off` does not override

This PR exists to document that we (that is, the compiler reviewer) implicitly made a decision in #86652 that defies the expectations of some programmers. Some programmers believe `-Cforce-frame-pointers=false` should obey the programmer in all cases, forcing the compiler to avoid generating frame pointers, even if the target specification would indicate they must be generated. However, many targets rely on frame pointers for fast or sound unwinding.

T-compiler had a weekly triage meeting on 2025-05-22. This topic was put to discussion because some programmers may expect the target-overriding behavior. In that meeting we decided removing frame pointers, at least with regards to the contract of the `-Cforce-frame-pointers` option, is not required, even if `=off` is passed, and that we will not do so if the target would expect them. This follows from the documentation here: https://doc.rust-lang.org/rustc/codegen-options/index.html#force-frame-pointers

We may separately pursue trying to clarify the situation more emphatically in our documentation, or warn when people pass the option when it doesn't do anything.
try-job: test-various
@rust-bors
Copy link

rust-bors bot commented Jun 17, 2025

☀️ Try build successful (CI)
Build commit: 9ca9e59 (9ca9e593391be5f0d36a3713e358f418d16ba1fe, parent: 55d436467c351b56253deeba209ae2553d1c243f)

@workingjubilee
Copy link
Member Author

workingjubilee commented Jun 17, 2025

Effectively a minor spelling error in the codegen test (some platforms can add dso_local).

@bors r=jieyouxu

@bors
Copy link
Collaborator

bors commented Jun 17, 2025

📌 Commit 5a449fb has been approved by jieyouxu

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 17, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Jun 18, 2025
…pointers-favor-the-target-or-cli, r=jieyouxu

Affirm `-Cforce-frame-pointers=off` does not override

This PR exists to document that we (that is, the compiler reviewer) implicitly made a decision in rust-lang#86652 that defies the expectations of some programmers. Some programmers believe `-Cforce-frame-pointers=false` should obey the programmer in all cases, forcing the compiler to avoid generating frame pointers, even if the target specification would indicate they must be generated. However, many targets rely on frame pointers for fast or sound unwinding.

T-compiler had a weekly triage meeting on 2025-05-22. This topic was put to discussion because some programmers may expect the target-overriding behavior. In that meeting we decided removing frame pointers, at least with regards to the contract of the `-Cforce-frame-pointers` option, is not required, even if `=off` is passed, and that we will not do so if the target would expect them. This follows from the documentation here: https://doc.rust-lang.org/rustc/codegen-options/index.html#force-frame-pointers

We may separately pursue trying to clarify the situation more emphatically in our documentation, or warn when people pass the option when it doesn't do anything.
bors added a commit that referenced this pull request Jun 18, 2025
Rollup of 10 pull requests

Successful merges:

 - #135656 (Add `-Z hint-mostly-unused` to tell rustc that most of a crate will go unused)
 - #138237 (Get rid of `EscapeDebugInner`.)
 - #140772 ({aarch64,x86_64}-pc-windows-gnullvm: build host tools)
 - #140774 (Affirm `-Cforce-frame-pointers=off` does not override)
 - #141610 (Stabilize `feature(generic_arg_infer)`)
 - #141864 (Handle win32 separator for cygwin paths)
 - #142384 (Bringing `rustc_rayon_core` in tree as `rustc_thread_pool`)
 - #142502 (rustdoc_json: improve handling of generic args)
 - #142571 (Reason about borrowed classes in CopyProp.)
 - #142591 (Add spawn APIs for BootstrapCommand to support deferred command execution)

r? `@ghost`
`@rustbot` modify labels: rollup
Kobzol added a commit to Kobzol/rust that referenced this pull request Jun 18, 2025
…pointers-favor-the-target-or-cli, r=jieyouxu

Affirm `-Cforce-frame-pointers=off` does not override

This PR exists to document that we (that is, the compiler reviewer) implicitly made a decision in rust-lang#86652 that defies the expectations of some programmers. Some programmers believe `-Cforce-frame-pointers=false` should obey the programmer in all cases, forcing the compiler to avoid generating frame pointers, even if the target specification would indicate they must be generated. However, many targets rely on frame pointers for fast or sound unwinding.

T-compiler had a weekly triage meeting on 2025-05-22. This topic was put to discussion because some programmers may expect the target-overriding behavior. In that meeting we decided removing frame pointers, at least with regards to the contract of the `-Cforce-frame-pointers` option, is not required, even if `=off` is passed, and that we will not do so if the target would expect them. This follows from the documentation here: https://doc.rust-lang.org/rustc/codegen-options/index.html#force-frame-pointers

We may separately pursue trying to clarify the situation more emphatically in our documentation, or warn when people pass the option when it doesn't do anything.
bors added a commit that referenced this pull request Jun 18, 2025
Rollup of 12 pull requests

Successful merges:

 - #135656 (Add `-Z hint-mostly-unused` to tell rustc that most of a crate will go unused)
 - #140774 (Affirm `-Cforce-frame-pointers=off` does not override)
 - #141610 (Stabilize `feature(generic_arg_infer)`)
 - #142123 (Implement initial support for timing sections (`--json=timings`))
 - #142383 (CodeGen: rework Aggregate implemention for rvalue_creates_operand cases)
 - #142502 (rustdoc_json: improve handling of generic args)
 - #142591 (Add spawn APIs for BootstrapCommand to support deferred command execution)
 - #142606 (AsyncDrop trait without sync Drop generates an error)
 - #142619 (apply clippy::or_fun_call)
 - #142624 (Actually take `--build` into account in bootstrap)
 - #142627 (Add `StepMetadata` to describe steps)
 - #142660 (remove joboet from review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
Kobzol added a commit to Kobzol/rust that referenced this pull request Jun 18, 2025
…pointers-favor-the-target-or-cli, r=jieyouxu

Affirm `-Cforce-frame-pointers=off` does not override

This PR exists to document that we (that is, the compiler reviewer) implicitly made a decision in rust-lang#86652 that defies the expectations of some programmers. Some programmers believe `-Cforce-frame-pointers=false` should obey the programmer in all cases, forcing the compiler to avoid generating frame pointers, even if the target specification would indicate they must be generated. However, many targets rely on frame pointers for fast or sound unwinding.

T-compiler had a weekly triage meeting on 2025-05-22. This topic was put to discussion because some programmers may expect the target-overriding behavior. In that meeting we decided removing frame pointers, at least with regards to the contract of the `-Cforce-frame-pointers` option, is not required, even if `=off` is passed, and that we will not do so if the target would expect them. This follows from the documentation here: https://doc.rust-lang.org/rustc/codegen-options/index.html#force-frame-pointers

We may separately pursue trying to clarify the situation more emphatically in our documentation, or warn when people pass the option when it doesn't do anything.
bors added a commit that referenced this pull request Jun 18, 2025
Rollup of 11 pull requests

Successful merges:

 - #140774 (Affirm `-Cforce-frame-pointers=off` does not override)
 - #141610 (Stabilize `feature(generic_arg_infer)`)
 - #142383 (CodeGen: rework Aggregate implemention for rvalue_creates_operand cases)
 - #142591 (Add spawn APIs for BootstrapCommand to support deferred command execution)
 - #142619 (apply clippy::or_fun_call)
 - #142624 (Actually take `--build` into account in bootstrap)
 - #142627 (Add `StepMetadata` to describe steps)
 - #142660 (remove joboet from review rotation)
 - #142666 (Skip tidy triagebot linkcheck if `triagebot.toml` doesn't exist)
 - #142672 (Clarify bootstrap tools description)
 - #142674 (remove duplicate crash test)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ff14611 into rust-lang:master Jun 18, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 18, 2025
rust-timer added a commit that referenced this pull request Jun 18, 2025
Rollup merge of #140774 - workingjubilee:should-force-frame-pointers-favor-the-target-or-cli, r=jieyouxu

Affirm `-Cforce-frame-pointers=off` does not override

This PR exists to document that we (that is, the compiler reviewer) implicitly made a decision in #86652 that defies the expectations of some programmers. Some programmers believe `-Cforce-frame-pointers=false` should obey the programmer in all cases, forcing the compiler to avoid generating frame pointers, even if the target specification would indicate they must be generated. However, many targets rely on frame pointers for fast or sound unwinding.

T-compiler had a weekly triage meeting on 2025-05-22. This topic was put to discussion because some programmers may expect the target-overriding behavior. In that meeting we decided removing frame pointers, at least with regards to the contract of the `-Cforce-frame-pointers` option, is not required, even if `=off` is passed, and that we will not do so if the target would expect them. This follows from the documentation here: https://doc.rust-lang.org/rustc/codegen-options/index.html#force-frame-pointers

We may separately pursue trying to clarify the situation more emphatically in our documentation, or warn when people pass the option when it doesn't do anything.
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Jun 19, 2025
Rollup of 11 pull requests

Successful merges:

 - rust-lang/rust#140774 (Affirm `-Cforce-frame-pointers=off` does not override)
 - rust-lang/rust#141610 (Stabilize `feature(generic_arg_infer)`)
 - rust-lang/rust#142383 (CodeGen: rework Aggregate implemention for rvalue_creates_operand cases)
 - rust-lang/rust#142591 (Add spawn APIs for BootstrapCommand to support deferred command execution)
 - rust-lang/rust#142619 (apply clippy::or_fun_call)
 - rust-lang/rust#142624 (Actually take `--build` into account in bootstrap)
 - rust-lang/rust#142627 (Add `StepMetadata` to describe steps)
 - rust-lang/rust#142660 (remove joboet from review rotation)
 - rust-lang/rust#142666 (Skip tidy triagebot linkcheck if `triagebot.toml` doesn't exist)
 - rust-lang/rust#142672 (Clarify bootstrap tools description)
 - rust-lang/rust#142674 (remove duplicate crash test)

r? `@ghost`
`@rustbot` modify labels: rollup
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.

8 participants