Skip to content

Don't modify libstd to dump rustc ICEs #115627

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 2 commits into from
Sep 19, 2023
Merged

Don't modify libstd to dump rustc ICEs #115627

merged 2 commits into from
Sep 19, 2023

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Sep 7, 2023 •
edited
Loading

Do a much simpler thing and just dump a std::backtrace::Backtrace to file.

r? @estebank @oli-obk

Fixes #115610

@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 7, 2023
@rust-log-analyzer

This comment has been minimized.

# to view
@oli-obk
Copy link
Contributor

oli-obk commented Sep 7, 2023

@bors rollup=never (perf may show a binary size reduction)

@m-ou-se
Copy link
Member

m-ou-se commented Sep 19, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 19, 2023

📌 Commit b594807 has been approved by m-ou-se

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 19, 2023
@bors
Copy link
Collaborator

bors commented Sep 19, 2023

⌛ Testing commit b594807 with merge 42f5828...

@estebank
Copy link
Contributor

Is the previous change in beta? Should we consider backporting this?

@compiler-errors compiler-errors added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 19, 2023
@compiler-errors
Copy link
Member Author

Not clear who'll be responsible for beta-accepting this -- guess we'll see who gets to it first, libs or compiler 😅

@ChrisDenton
Copy link
Member

From a libs point of view this is simply a revert, right?

@bors
Copy link
Collaborator

bors commented Sep 19, 2023

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing 42f5828 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 19, 2023
@bors bors merged commit 42f5828 into rust-lang:master Sep 19, 2023
@rustbot rustbot added this to the 1.74.0 milestone Sep 19, 2023
@bors bors mentioned this pull request Sep 19, 2023
Open
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (42f5828): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -0.2% [-0.2%, -0.2%] 1
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) -0.2% [-0.2%, -0.2%] 1

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 4.5% [4.5%, 4.5%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 631.446s -> 631.582s (0.02%)
Artifact size: 317.83 MiB -> 317.76 MiB (-0.02%)

@cuviper
Copy link
Member

cuviper commented Sep 21, 2023

Not clear who'll be responsible for beta-accepting this -- guess we'll see who gets to it first, libs or compiler 😅

Let's go for consensus -- libs approved today on zulip, pending compiler approval.

@compiler-errors compiler-errors deleted the icedump-no-std branch September 21, 2023 01:14
@apiraino
Copy link
Contributor

Thumbs up also from compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Sep 21, 2023
@cuviper cuviper mentioned this pull request Sep 22, 2023
Merged
@cuviper cuviper modified the milestones: 1.74.0, 1.73.0 Sep 22, 2023
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 22, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 22, 2023
@bors
Auto merge of rust-lang#116044 - cuviper:beta-next, r=cuviper
7c76587 
[beta] backports

-  Fix a pthread_t handle leak rust-lang#114696
-  MCP661: Move wasm32-wasi-preview1-threads target to Tier 2 rust-lang#115345
-  Don't modify libstd to dump rustc ICEs rust-lang#115627
-  Paper over an accidental regression rust-lang#115844
-  Update to LLVM 17.0.0 rust-lang#115959

r? cuviper
@jieyouxu jieyouxu mentioned this pull request Nov 19, 2024
Closed
3 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@estebank estebank

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. 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-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.73.0
Development

Successfully merging this pull request may close these issues.

default binary size increase due to ice dumps logic