Skip to content

Install more signal stack trace handlers - #137151

Merged
bors merged 1 commit into
rust-lang:masterfrom
Urgau:register-more-signals
Feb 18, 2025
Merged

Install more signal stack trace handlers#137151
bors merged 1 commit into
rust-lang:masterfrom
Urgau:register-more-signals

Conversation

@Urgau

@UrgauUrgau commented Feb 16, 2025•
edited
Loading

Copy link
Copy Markdown
Member

This PR install the signal stack handler to more signals (SIGILL, SIGTRAP, SIGABRT, SIGFPE, SIGBUS, SIGQUIT).

Noticed in #137138 that we didn't print anything for SIGILL, so I though we could just handle more signals.

r? @workingjubilee since you last touched it

@rustbot

rustbot commented Feb 16, 2025

Copy link
Copy Markdown
Collaborator

Could not assign reviewer from: workingjubilee.
User(s) workingjubilee are either the PR author, already assigned, or on vacation. Please use r? to specify someone else to assign.

@rustbot

rustbot commented Feb 16, 2025

Copy link
Copy Markdown
Collaborator

r? @oli-obk

rustbot has assigned @oli-obk.
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

@rustbotrustbot 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 Feb 16, 2025
@workingjubilee

workingjubilee commented Feb 16, 2025

Copy link
Copy Markdown
Member

No, we should not try to handle every signal in this way.

workingjubilee
workingjubilee requested changes Feb 16, 2025
View reviewed changes
Comment threadcompiler/rustc_driver_impl/src/signal_handler.rs Outdated
Comment threadcompiler/rustc_driver_impl/src/signal_handler.rs Outdated
Comment threadcompiler/rustc_driver_impl/src/signal_handler.rs
Comment threadcompiler/rustc_driver_impl/src/signal_handler.rs Outdated
Comment threadcompiler/rustc_driver_impl/src/signal_handler.rs
Comment threadcompiler/rustc_driver_impl/src/signal_handler.rs Outdated
@Urgau
Urgauforce-pushed the register-more-signals branch from 9495da5 to 3bd0dc9CompareFebruary 16, 2025 22:41
@Urgau

Urgau commented Feb 16, 2025

Copy link
Copy Markdown
MemberAuthor

No, we should not try to handle every signal in this way.

For the record, I took the list from LLVM own signal handling, https://github.com/llvm/llvm-project/blob/6812fc02fbb81d679f95d5c3e15768ae11e1bad8/llvm/lib/Support/Unix/Signals.inc#L218-L224.

Removed the ones you didn't wanted.

@workingjubilee

workingjubilee commented Feb 16, 2025

Copy link
Copy Markdown
Member

I do not care what LLVM does, no.

@workingjubilee

workingjubilee commented Feb 16, 2025•
edited
Loading

Copy link
Copy Markdown
Member

Half of the entire point of why I worked on this handler is to make it easier to diagnose when the problem is in LLVM and then file an issue with them. The entire file constitutes a sort of disagreement with LLVM about how errors should be handled.

I did intend to add a SIGILL handler but originally I intended to make it slightly more clever when doing so, because I wanted to make it sharper and the diagnostics significantly different if it detected when the error was specifically in LLVM's code. Unfortunately then I discovered doing so would require a more extensive rewrite of a number of things, so I didn't get around to it.

oli-obk
oli-obk reviewed Feb 17, 2025
View reviewed changes
Comment threadcompiler/rustc_driver_impl/src/signal_handler.rs
@Urgau
Urgauforce-pushed the register-more-signals branch from 3bd0dc9 to a0a8e02CompareFebruary 17, 2025 18:00
@workingjubilee

workingjubilee commented Feb 17, 2025

Copy link
Copy Markdown
Member

Thank you for doing this. Quibbles about what signals we should be intercepting aside, I ought to have done this long ago, so I'm glad that someone who could do it without immediately getting sidetracked by advanced backtrace formatting nonsense did it.

@bors r+ rollup

@bors

bors commented Feb 17, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit a0a8e02 has been approved by workingjubilee

It is now in the queue for this repository.

@borsbors 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 Feb 17, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Feb 18, 2025
@Zalathar
Rollup merge ofrust-lang#137151- Urgau:register-more-signals, r=wor…
6ceaf47
…kingjubilee
Install more signal stack trace handlers
This PR install the signal stack handler to more signals (`SIGILL`, ~~`SIGTRAP`~~, ~~`SIGABRT`~~, ~~`SIGFPE`~~, `SIGBUS`, ~~`SIGQUIT`~~).
Noticed in rust-lang#137138 that we didn't print anything for `SIGILL`, so I though we could just handle more signals.
r? `@workingjubilee` since you last touched it
@ZalatharZalathar mentioned this pull request Feb 18, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 18, 2025
@bors
Auto merge ofrust-lang#137199- Zalathar:rollup-18ux22s, r=Zalathar
54f47f8
Rollup of 8 pull requests
Successful merges:
- rust-lang#135767 (Future incompatibility warning `unsupported_fn_ptr_calling_conventions`: Also warn in dependencies)
- rust-lang#136457 (Expose algebraic floating point intrinsics)
- rust-lang#136985 (Do not ignore uninhabited types for function-call ABI purposes. (Remove BackendRepr::Uninhabited))
- rust-lang#137000 (Deeply normalize item bounds in new solver)
- rust-lang#137151 (Install more signal stack trace handlers)
- rust-lang#137155 (Organize `OsString`/`OsStr` shims)
- rust-lang#137161 (Pattern Migration 2024: fix incorrect messages/suggestions when errors arise in macro expansions)
- rust-lang#137162 (Move methods from `Map` to `TyCtxt`, part 2.)
r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 18, 2025
@matthiaskrgr
Rollup merge ofrust-lang#137151- Urgau:register-more-signals, r=wor…
d9fcda0
…kingjubilee
Install more signal stack trace handlers
This PR install the signal stack handler to more signals (`SIGILL`, ~~`SIGTRAP`~~, ~~`SIGABRT`~~, ~~`SIGFPE`~~, `SIGBUS`, ~~`SIGQUIT`~~).
Noticed in rust-lang#137138 that we didn't print anything for `SIGILL`, so I though we could just handle more signals.
r? ``@workingjubilee`` since you last touched it
@matthiaskrgrmatthiaskrgr mentioned this pull request Feb 18, 2025
Closed
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 18, 2025
@GuillaumeGomez
Rollup merge ofrust-lang#137151- Urgau:register-more-signals, r=wor…
1f517f1
…kingjubilee
Install more signal stack trace handlers
This PR install the signal stack handler to more signals (`SIGILL`, ~~`SIGTRAP`~~, ~~`SIGABRT`~~, ~~`SIGFPE`~~, `SIGBUS`, ~~`SIGQUIT`~~).
Noticed in rust-lang#137138 that we didn't print anything for `SIGILL`, so I though we could just handle more signals.
r? ```@workingjubilee``` since you last touched it
@GuillaumeGomezGuillaumeGomez mentioned this pull request Feb 18, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 18, 2025
@bors
Auto merge ofrust-lang#137212- GuillaumeGomez:rollup-fc4rqfu, r=Gui…
a584f94
…llaumeGomez
Rollup of 11 pull requests
Successful merges:
- rust-lang#127793 (Added project-specific Zed IDE settings)
- rust-lang#134995 (Stabilize const_slice_flatten)
- rust-lang#135767 (Future incompatibility warning `unsupported_fn_ptr_calling_conventions`: Also warn in dependencies)
- rust-lang#136599 (librustdoc: more usages of `Joined::joined`)
- rust-lang#136750 (Make ub_check message clear that it's not an assert)
- rust-lang#137000 (Deeply normalize item bounds in new solver)
- rust-lang#137126 (fix docs for inherent str constructors)
- rust-lang#137151 (Install more signal stack trace handlers)
- rust-lang#137161 (Pattern Migration 2024: fix incorrect messages/suggestions when errors arise in macro expansions)
- rust-lang#137167 (tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg)
- rust-lang#137177 (Update `minifier-rs` version to `0.3.5`)
r? `@ghost`
`@rustbot` modify labels: rollup
Urgau added a commit to Urgau/rust that referenced this pull request Feb 18, 2025
@Urgau
Rollup merge ofrust-lang#137151- Urgau:register-more-signals, r=wor…
a3d9cb0
…kingjubilee
Install more signal stack trace handlers
This PR install the signal stack handler to more signals (`SIGILL`, ~~`SIGTRAP`~~, ~~`SIGABRT`~~, ~~`SIGFPE`~~, `SIGBUS`, ~~`SIGQUIT`~~).
Noticed in rust-lang#137138 that we didn't print anything for `SIGILL`, so I though we could just handle more signals.
r? ````@workingjubilee```` since you last touched it
@UrgauUrgau mentioned this pull request Feb 18, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 18, 2025
@bors
Auto merge ofrust-lang#137226- Urgau:rollup-8htn7j0, r=Urgau
4e243bb
Rollup of 9 pull requests
Successful merges:
- rust-lang#137151 (Install more signal stack trace handlers)
- rust-lang#137167 (tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg)
- rust-lang#137195 (cg_clif: use exclusively ABI alignment)
- rust-lang#137202 (Enforce T: Hash for Interned<...>)
- rust-lang#137205 (Remove `std::os::wasi::fs::FileExt::tell`)
- rust-lang#137211 (don't ICE for alias-relate goals with error term)
- rust-lang#137213 (Remove `rustc_middle::mir::tcx` module.)
- rust-lang#137214 (add last std diagnostic items for clippy)
- rust-lang#137221 (Remove scrutinee_hir_id from ExprKind::Match)
r? `@ghost`
`@rustbot` modify labels: rollup
@UrgauUrgau mentioned this pull request Feb 18, 2025
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 18, 2025
@matthiaskrgr
Rollup merge ofrust-lang#137151- Urgau:register-more-signals, r=wor…
a2b6256
…kingjubilee
Install more signal stack trace handlers
This PR install the signal stack handler to more signals (`SIGILL`, ~~`SIGTRAP`~~, ~~`SIGABRT`~~, ~~`SIGFPE`~~, `SIGBUS`, ~~`SIGQUIT`~~).
Noticed in rust-lang#137138 that we didn't print anything for `SIGILL`, so I though we could just handle more signals.
r? ``````@workingjubilee`````` since you last touched it
@matthiaskrgrmatthiaskrgr mentioned this pull request Feb 18, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 18, 2025
@bors
Auto merge ofrust-lang#137231- Urgau:rollup-heiq934, r=Urgau
827a0d6
Rollup of 9 pull requests
Successful merges:
- rust-lang#136750 (Make ub_check message clear that it's not an assert)
- rust-lang#137151 (Install more signal stack trace handlers)
- rust-lang#137167 (tests: Also gate `f16::erfc()` doctest with `reliable_f16_math` cfg)
- rust-lang#137195 (cg_clif: use exclusively ABI alignment)
- rust-lang#137202 (Enforce T: Hash for Interned<...>)
- rust-lang#137205 (Remove `std::os::wasi::fs::FileExt::tell`)
- rust-lang#137211 (don't ICE for alias-relate goals with error term)
- rust-lang#137214 (add last std diagnostic items for clippy)
- rust-lang#137221 (Remove scrutinee_hir_id from ExprKind::Match)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 1391f75 into rust-lang:masterFeb 18, 2025
@rustbotrustbot added this to the 1.87.0 milestone Feb 18, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 18, 2025
@rust-timer
Unrolled build forrust-lang#137151
f012a56
Rollup merge of rust-lang#137151 - Urgau:register-more-signals, r=workingjubilee
Install more signal stack trace handlers
This PR install the signal stack handler to more signals (`SIGILL`, ~~`SIGTRAP`~~, ~~`SIGABRT`~~, ~~`SIGFPE`~~, `SIGBUS`, ~~`SIGQUIT`~~).
Noticed in rust-lang#137138 that we didn't print anything for `SIGILL`, so I though we could just handle more signals.
r? `````@workingjubilee````` since you last touched it
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obkoli-obkoli-obk left review comments
@workingjubileeworkingjubileeworkingjubilee requested changes

Assignees

No one assigned

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.87.0

Development

Successfully merging this pull request may close these issues.

5 participants

@Urgau@rustbot@workingjubilee@bors@oli-obk