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

Stable compilers don't emit ICE files? #132245

Open
saethlin opened this issue Oct 28, 2024 · 5 comments
Open

Stable compilers don't emit ICE files? #132245

saethlin opened this issue Oct 28, 2024 · 5 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. D-diagnostic-infra Diagnostics: Issues that affect all diagnostics, or relate to the diagnostic machinery itself. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@saethlin
Copy link
Member

I don't know for sure if this a bug or an unexpected intentional decision.

It looks like, based on some experimentation and recent ICE reports on stable such as #132240, that the stable compiler does not write and mention ICE files. I don't want that behavior, because some fraction of the time people try to be helpful and abbreviate the compiler's output, and instead omit all the important information.

@saethlin saethlin added the C-bug Category: This is a bug. label Oct 28, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 28, 2024
@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. D-diagnostic-infra Diagnostics: Issues that affect all diagnostics, or relate to the diagnostic machinery itself. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 28, 2024
@jieyouxu
Copy link
Member

ICE disk dump MCP: rust-lang/compiler-team#578
Previous compiler discussion: https://rust-lang.zulipchat.com/#narrow/stream/233931-xxx/topic/Store.20ICE.20backtraces.20to.20disk.20and.20point.20en.E2.80.A6.20compiler-team.23578
Original PR: #108714

Haven't figured out the rationale for why the ICE dumps are nightly or dev only. cc @estebank maybe you know?

@jieyouxu jieyouxu added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed C-bug Category: This is a bug. labels Oct 28, 2024
@estebank estebank pinned this issue Oct 29, 2024
@estebank
Copy link
Contributor

It was explicitly made nightly-only so that it would have time to "bake" and minimize the likelihood of this infrastructure breaking stable users due to some hypothetical obscure bug that only affected ICE dumping. I would be in favor of enabling it in stable now, for the same reasons @saethlin wants it enabled. I was initially thinking that we should wait until the cargo gc work was finished before moving this to stable, but at the same time I feel it should be fine to change the path we write to after the fact.

CC @yaahc, as she's working on this space and might have thoughts. I know that we were looking at tweaking the setting and override for these.

@compiler-errors
Copy link
Member

compiler-errors commented Oct 29, 2024

Why is this pinned

edit: I'm gonna unpin it. It's not particularly user-facing or anything.

@compiler-errors compiler-errors unpinned this issue Oct 29, 2024
@yaahc
Copy link
Member

yaahc commented Oct 30, 2024

I have no objections to us enabling the ice dumps on stable. It still wouldn't be considered a part of our stable interface so even if we do run into problems down the line with disk usage (which I don't find likely) we can always disable it again while we get that sorted.

@jieyouxu jieyouxu self-assigned this Nov 13, 2024
jieyouxu added a commit to jieyouxu/rust that referenced this issue Nov 19, 2024
cf. <rust-lang#132245> where originally
we made it nightly-only in case there was some problems with the ICE
dumping infra breaking stable users. However, a good amount of time has
since passed and such bugs have not materialized. It was also waiting
for cargo gc work, but the timeline of that is unclear.

Also note that ICE dumping is already perma-unstable behavior, so if
this somehow is problematic it is very easy to revert.
@jieyouxu
Copy link
Member

jieyouxu commented Nov 20, 2024

Blockers:

  • ICE file dumps can be spammy

    I would prefer not to put this on stable until we stop writing these files into the directory where the crate is located. If rustc is going to ICE then it's probably going to ICE multiple times, if you have check-on-save enabled then every save is another ICE file so you can easily wind up with dozens of ICE dumps written into your crate's directory which is a terrible experience

  • ICE dumps leak user info: ICE dumps leak user names etc #128594

@jieyouxu jieyouxu removed their assignment Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. D-diagnostic-infra Diagnostics: Issues that affect all diagnostics, or relate to the diagnostic machinery itself. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants