Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Install more signal stack trace handlers #137151

Merged
merged 1 commit into from
Feb 18, 2025
Merged

Install more signal stack trace handlers #137151

merged 1 commit into from
Feb 18, 2025

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Feb 16, 2025 •
edited
Loading

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
Copy link
Collaborator

rustbot commented Feb 16, 2025

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
Copy link
Collaborator

rustbot commented Feb 16, 2025

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

@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 Feb 16, 2025
@workingjubilee
Copy link
Member

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

workingjubilee
workingjubilee requested changes Feb 16, 2025
View reviewed changes
@Urgau Urgau force-pushed the register-more-signals branch from 9495da5 to 3bd0dc9 Compare February 16, 2025 22:41
@Urgau
Copy link
Member Author

Urgau commented Feb 16, 2025

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
Copy link
Member

I do not care what LLVM does, no.

@workingjubilee
Copy link
Member

workingjubilee commented Feb 16, 2025 •
edited
Loading

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.

@Urgau Urgau force-pushed the register-more-signals branch from 3bd0dc9 to a0a8e02 Compare February 17, 2025 18:00
@workingjubilee
Copy link
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
Copy link
Contributor

bors commented Feb 17, 2025

📌 Commit a0a8e02 has been approved by workingjubilee

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-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 of rust-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
@Zalathar Zalathar 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 of rust-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 of rust-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
@matthiaskrgr matthiaskrgr 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 of rust-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
@GuillaumeGomez GuillaumeGomez 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 of rust-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 of rust-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
@Urgau Urgau 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 of rust-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
@Urgau Urgau 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 of rust-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
@matthiaskrgr matthiaskrgr 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 of rust-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:master Feb 18, 2025
6 checks passed
@rustbot rustbot 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 for rust-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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk left review comments

@workingjubilee workingjubilee workingjubilee requested changes

Assignees
No one assigned
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
Milestone
1.87.0
Development

Successfully merging this pull request may close these issues.
5 participants
@Urgau @rustbot @workingjubilee @bors @oli-obk