Skip to content

FCW for #[panic_handler] on unsafe fn. - #162974

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
theemathas:fcw-unsafe-panic-handler
Oct 4, 2026
Merged

rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
theemathas:fcw-unsafe-panic-handler

Conversation

@theemathas

@theemathas theemathas commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

View all comments

Mitigates #162967.

A function annotated with #[panic_handler] can be called by the compiler from anywhere. So, such functions must not have any safety preconditions.

A github search shows many crates that would run into this, so a hard error seems infeasible. If needed, I can modify the code in order to run crater to check how much code would be flagged by this FCW.

I have not yet created a proper tracking issue for the FCW. If this PR seems like the right direction, I will do so before merging. I've created a tracking issue for this FCW at #163263

An LLM pointed me towards check_panic_info_fn and emit_node_span_lint (which I verified to be right). However, this PR is otherwise written manually.

@theemathas theemathas added T-lang Relevant to the language team T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. labels Sep 18, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 18, 2026
@rustbot

rustbot commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

r? @wesleywiser

rustbot has assigned @wesleywiser.
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 77 candidates
  • Random selection from 21 candidates

@theemathas
theemathas force-pushed the fcw-unsafe-panic-handler branch from 6e2584d to bdd65bd Compare September 18, 2026 15:49
@rust-log-analyzer

This comment has been minimized.

@asquared31415

This comment was marked as outdated.

@theemathas
theemathas force-pushed the fcw-unsafe-panic-handler branch from bdd65bd to bebf730 Compare September 18, 2026 17:04
@rust-log-analyzer

This comment has been minimized.

@theemathas
theemathas force-pushed the fcw-unsafe-panic-handler branch from bebf730 to 141c52b Compare September 18, 2026 17:52
@rust-log-analyzer

This comment has been minimized.

@theemathas
theemathas force-pushed the fcw-unsafe-panic-handler branch 2 times, most recently from 9998bc2 to 67ceffa Compare September 19, 2026 06:38
@rust-log-analyzer

This comment has been minimized.

@theemathas theemathas added the I-lang-nominated Nominated for discussion during a lang team meeting. label Sep 20, 2026
@traviscross traviscross added the P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang label Sep 23, 2026
@traviscross

Copy link
Copy Markdown
Contributor

Makes sense to me. Thanks @theemathas.

@rfcbot fcp merge lang

@rust-rfcbot

rust-rfcbot commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

@traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rust-rfcbot rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. and removed needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. labels Sep 23, 2026
@traviscross traviscross added the I-lang-radar Items that are on lang's radar and will need eventual work or consideration. label Sep 23, 2026
@tmandry

tmandry commented Sep 23, 2026

Copy link
Copy Markdown
Member

@rfcbot reviewed

@rust-rfcbot rust-rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Sep 23, 2026
@rust-rfcbot

Copy link
Copy Markdown
Collaborator

🔔 This is now entering its final comment period, as per the review above. 🔔

@traviscross traviscross removed I-lang-nominated Nominated for discussion during a lang team meeting. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang labels Sep 23, 2026
@rust-log-analyzer

This comment has been minimized.

@theemathas
theemathas force-pushed the fcw-unsafe-panic-handler branch from 16a575e to 69759e7 Compare September 24, 2026 14:48
@mejrs

mejrs commented Sep 24, 2026

Copy link
Copy Markdown
Member

@RalfJung That's at

fn finalize_check(cx: &FinalizeCheckContext<'_, '_>, attr_span: Span) {

. I considered putting the check somewhere around there, but it seems like it might be a mess? Not sure.

There's not a way to check that in attribute parsing, you don't get to look at the item the attribute is on, only what it is. You could wait for #162530 and implement it by checking AstTarget.

Does that make sense? I'm not sure, it seems there are a lot of these checks scattered around and I'm not sure what the best place is.

@RalfJung

Copy link
Copy Markdown
Member

I thought we had a separate check was entirely independent of attribute parsing. But it seems things got moved and my knowledge is outdated. Never mind :)

@mejrs

mejrs commented Sep 25, 2026

Copy link
Copy Markdown
Member

There is also the check_attr pass where such things usually are, especially if the checks need to use the tcx. Otherwise we'd like them to be in the attribute parser.

@RalfJung

RalfJung commented Sep 25, 2026 via email

Copy link
Copy Markdown
Member

@rust-rfcbot rust-rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Oct 3, 2026
@rust-rfcbot

Copy link
Copy Markdown
Collaborator

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

@rust-rfcbot rust-rfcbot added the to-announce Announce this issue on triage meeting label Oct 3, 2026
@traviscross traviscross added the waived-reference-pr This language change does not need a Reference PR. label Oct 3, 2026
@rust-bors

This comment has been minimized.

@mejrs mejrs left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

r? me

r=me after rebase

View changes since this review

@rustbot rustbot assigned mejrs and unassigned wesleywiser Oct 4, 2026
@theemathas
theemathas force-pushed the fcw-unsafe-panic-handler branch from 69759e7 to 0ed52fa Compare October 4, 2026 09:07
@rustbot

rustbot commented Oct 4, 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.

@theemathas

Copy link
Copy Markdown
Contributor Author

@bors r=mejrs rollup

@rust-bors

rust-bors Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 0ed52fa has been tentatively approved by mejrs

It will be put into the queue for this repository once PR CI succeeds.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 4, 2026
rust-bors Bot pushed a commit that referenced this pull request Oct 4, 2026
…uwer

Rollup of 6 pull requests

Successful merges:

 - #162974 (FCW for `#[panic_handler]` on `unsafe fn`.)
 - #163627 (Add documentation for the `no_main` and `repr` attributes)
 - #163671 (Add `--frontend-threads` option to `./x perf`)
 - #163694 (c_str_alloc_error test: mention why this is mostly Miri-only)
 - #163711 (Stabilize `CStr::display`)
 - #163723 ([tiny] Remove useless `.into()` calls)
@rust-bors
rust-bors Bot merged commit dff61b8 into rust-lang:main Oct 4, 2026
14 checks passed
rust-bors Bot pushed a commit that referenced this pull request Oct 4, 2026
Rollup merge of #162974 - theemathas:fcw-unsafe-panic-handler, r=mejrs

FCW for `#[panic_handler]` on `unsafe fn`.

Mitigates #162967.

A function annotated with `#[panic_handler]` can be called by the compiler from anywhere. So, such functions must not have any safety preconditions.

A [github search](https://github.com/search?q=language%3Arust+%2F%23%5C%5Bpanic_handler%5C%5D%5Cnunsafe%2F&type=code) shows many crates that would run into this, so a hard error seems infeasible. If needed, I can modify the code in order to run crater to check how much code would be flagged by this FCW.

~~I have not yet created a proper tracking issue for the FCW. If this PR seems like the right direction, I will do so before merging.~~ I've created a tracking issue for this FCW at #163263

An LLM pointed me towards `check_panic_info_fn` and `emit_node_span_lint` (which I verified to be right). However, this PR is otherwise written manually.
@rustbot rustbot added this to the 1.101.0 milestone Oct 4, 2026
@theemathas
theemathas deleted the fcw-unsafe-panic-handler branch October 4, 2026 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. 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. T-lang Relevant to the language team to-announce Announce this issue on triage meeting waived-reference-pr This language change does not need a Reference PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.