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.

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

Don't trap into the debugger on panics under Linux #130810

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

kromych
Copy link
Contributor

@kromych kromych commented Sep 25, 2024

This breaks rr, see #129019 (comment) for the discussion

CC @khuey @workingjubilee

@rustbot
Copy link
Collaborator

rustbot commented Sep 25, 2024

r? @thomcc

rustbot has assigned @thomcc.
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-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 25, 2024
@kromych
Copy link
Contributor Author

kromych commented Sep 25, 2024

r? workingjubilee

@rustbot rustbot assigned workingjubilee and unassigned thomcc Sep 25, 2024
@kromych
Copy link
Contributor Author

kromych commented Sep 25, 2024

@thomcc, I've r?'d @workingjubilee as they were on the PR where the code was introduced

@rustbot
Copy link
Collaborator

rustbot commented Sep 25, 2024

Failed to set assignee to 'd: 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.

@workingjubilee
Copy link
Member

Cool.

@bors r+

@bors
Copy link
Contributor

bors commented Sep 25, 2024

📌 Commit 49d1c3b 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 Sep 25, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 25, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#130549 (Add RISC-V vxworks targets)
 - rust-lang#130595 (Initial std library support for NuttX)
 - rust-lang#130734 (Fix: ices on virtual-function-elimination about principal trait)
 - rust-lang#130787 (Ban combination of GCE and new solver)
 - rust-lang#130809 (Update llvm triple for OpenHarmony targets)
 - rust-lang#130810 (Don't trap into the debugger on panics under Linux)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9d134cc into rust-lang:master Sep 25, 2024
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 25, 2024
Rollup merge of rust-lang#130810 - kromych:master, r=workingjubilee

Don't trap into the debugger on panics under Linux

This breaks `rr`, see rust-lang#129019 (comment) for the discussion

CC `@khuey` `@workingjubilee`
@rustbot rustbot added this to the 1.83.0 milestone Sep 25, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 25, 2024
Revert Break into the debugger on panic (129019)

This was talked about a bit at a recent libs meeting. While I think experimenting with this is worthwhile, I am nervous about this new behaviour reaching stable. We've already reverted on one tier 1 platform (Linux, rust-lang#130810) which means we have differing semantics on different tier 1 platforms. Also the fact it triggers even when `catch_unwind` is used to catch the panic means it can be very noisy in some projects.

At the very least I think it could use some more discussion before being instantly stable. I think this could maybe be re-landed with an environment variable to control/override the behaviour. But that part would likely need a libs-api decision.

cc `@workingjubilee` `@kromych`
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 26, 2024
Revert Break into the debugger on panic (129019)

This was talked about a bit at a recent libs meeting. While I think experimenting with this is worthwhile, I am nervous about this new behaviour reaching stable. We've already reverted on one tier 1 platform (Linux, rust-lang#130810) which means we have differing semantics on different tier 1 platforms. Also the fact it triggers even when `catch_unwind` is used to catch the panic means it can be very noisy in some projects.

At the very least I think it could use some more discussion before being instantly stable. I think this could maybe be re-landed with an environment variable to control/override the behaviour. But that part would likely need a libs-api decision.

cc `@workingjubilee` `@kromych`
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Sep 26, 2024
Revert Break into the debugger on panic (129019)

This was talked about a bit at a recent libs meeting. While I think experimenting with this is worthwhile, I am nervous about this new behaviour reaching stable. We've already reverted on one tier 1 platform (Linux, rust-lang#130810) which means we have differing semantics on different tier 1 platforms. Also the fact it triggers even when `catch_unwind` is used to catch the panic means it can be very noisy in some projects.

At the very least I think it could use some more discussion before being instantly stable. I think this could maybe be re-landed with an environment variable to control/override the behaviour. But that part would likely need a libs-api decision.

cc `@workingjubilee` `@kromych`
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 26, 2024
Revert Break into the debugger on panic (129019)

This was talked about a bit at a recent libs meeting. While I think experimenting with this is worthwhile, I am nervous about this new behaviour reaching stable. We've already reverted on one tier 1 platform (Linux, rust-lang#130810) which means we have differing semantics on different tier 1 platforms. Also the fact it triggers even when `catch_unwind` is used to catch the panic means it can be very noisy in some projects.

At the very least I think it could use some more discussion before being instantly stable. I think this could maybe be re-landed with an environment variable to control/override the behaviour. But that part would likely need a libs-api decision.

cc ``@workingjubilee`` ``@kromych``
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 27, 2024
Revert Break into the debugger on panic (129019)

This was talked about a bit at a recent libs meeting. While I think experimenting with this is worthwhile, I am nervous about this new behaviour reaching stable. We've already reverted on one tier 1 platform (Linux, rust-lang#130810) which means we have differing semantics on different tier 1 platforms. Also the fact it triggers even when `catch_unwind` is used to catch the panic means it can be very noisy in some projects.

At the very least I think it could use some more discussion before being instantly stable. I think this could maybe be re-landed with an environment variable to control/override the behaviour. But that part would likely need a libs-api decision.

cc ```@workingjubilee``` ```@kromych```
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Sep 27, 2024
Revert Break into the debugger on panic (129019)

This was talked about a bit at a recent libs meeting. While I think experimenting with this is worthwhile, I am nervous about this new behaviour reaching stable. We've already reverted on one tier 1 platform (Linux, rust-lang#130810) which means we have differing semantics on different tier 1 platforms. Also the fact it triggers even when `catch_unwind` is used to catch the panic means it can be very noisy in some projects.

At the very least I think it could use some more discussion before being instantly stable. I think this could maybe be re-landed with an environment variable to control/override the behaviour. But that part would likely need a libs-api decision.

cc ````@workingjubilee```` ````@kromych````
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 27, 2024
Rollup merge of rust-lang#130846 - ChrisDenton:revert-break, r=Noratrieb

Revert Break into the debugger on panic (129019)

This was talked about a bit at a recent libs meeting. While I think experimenting with this is worthwhile, I am nervous about this new behaviour reaching stable. We've already reverted on one tier 1 platform (Linux, rust-lang#130810) which means we have differing semantics on different tier 1 platforms. Also the fact it triggers even when `catch_unwind` is used to catch the panic means it can be very noisy in some projects.

At the very least I think it could use some more discussion before being instantly stable. I think this could maybe be re-landed with an environment variable to control/override the behaviour. But that part would likely need a libs-api decision.

cc ````@workingjubilee```` ````@kromych````
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-libs Relevant to the library 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