Report non-standard compile flags on ICE #48266
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
I feel like it is generally useful for our ICE reports to contain as much information as possible; for example, I would see it reasonable for us to enable backtrace generation as well as all of the arguments passed to rustc being in the message (some may be useless, but can't really hurt). We can always refine this, removing information seems easier/better than not having it. |
|
@Mark-Simulacrum I'm not sure including all the CLI arguments by default is a wise choice: cargo can emit a lot of them, and the user would just receive a big wall of text, which can be difficult both to analize and to copy/paste correctly. I feel the same about dumping the whole backtrace in the output: for most users is just noise, and copying large amounts of stuff from the terminal can be tricky. Showing only the relevant stuff (like only the arguments that can change the behavior of rustc) instead can help us reproduce the issue more easily, and if we can't reproduce we can just ask the user "please run that with RUST_BACKTRACE=1". Another idea that crossed my mind is, we could show the minimal debug message on std{out,err}, and then generate a full report with everything we need (backtraces, full CLI arguments, etc) in About what arguments should be displayed, maybe the PR is a bit too restrictive, but some filtering has to be done. For example the arguments for a medium-size project of mine are: Even after removing the Now, Should I change the PR to allow them to show up? |
|
The generation of a report in |
pietroalbini
added
S-waiting-on-author
pietroalbini
force-pushed
the
report-compiler-flags-on-ice
branch
from
054dcbf to
d94546c
Compare
|
Ok, I relaxed the filtering a bit. |
pietroalbini
added
S-waiting-on-review
src/librustc_driver/lib.rs
Outdated
| if let Ok(string) = arg.into_string() { | ||
| args.push(string); | ||
| } else { | ||
| return None; |
There was a problem hiding this comment.
Hm, I feel like we shouldn't just return None but maybe just use into_string_lossy() here? That seems more reasonable...
|
Leaving off some arguments while printing others could easily turn out to be misleading. My suggestion is to blacklist Another concern is privacy. Including arguments would reveal the directory structure of users, something that not everyone might be comfortable with. Not printing arguments would solve this problem. |
So you suggest keeping arguments like
At the moment, I think all the arguments with paths are stripped out. Am I missing any of them? Edit: I'm a bit busy with exams at the moment, I hope I can finish this PR sometimes later this week. |
Yeah, there are case where these make a difference. But you are right, most of the time they won't make a difference. It's fine to leave them off if we list them in the list of a omitted arguments.
That was more a general comment of mine. If we go with stripping arguments (which I think we should) then I consider the privacy issue solved. |
pietroalbini
added
S-waiting-on-author
pietroalbini
force-pushed
the
report-compiler-flags-on-ice
branch
from
d94546c to
2985a1a
Compare
|
@michaelwoerister is this what you mean? Now when the flags provided by cargo by default are hidden the "some of the compiler flags provided by cargo are hidden" note will also appear in the output. |
pietroalbini
added
S-waiting-on-review
|
Thanks, @pietroalbini! Yeah, let's try it like that for now. We can always change it if it turns out to be misleading (that is, if people overlook that the list is not exhaustive). @bors r+ |
|
📌 Commit 2985a1a has been approved by |
bors
added
S-waiting-on-bors
…on-ice, r=michaelwoerister Report non-standard compile flags on ICE Some ICEs (such as the recent rust-lang#48248) only happens when a non-standard compiler flag is provided to rustc, but users don't always report the used flags. This can slow down reproducing the issue, so this PR shows all the non-standard compiler flags in the ICE error message. For example, the output of rust-lang#48248 with this PR is: ``` error: internal compiler error: [...] thread 'rustc' panicked at [...] note: Run with `RUST_BACKTRACE=1` for a backtrace. note: the compiler unexpectedly panicked. this is a bug. note: we would appreciate a bug report: [...] note: rustc 1.25.0-dev running on x86_64-unknown-linux-gnu note: compiler flags: -C link-dead-code ``` ### Open questions * At the moment, only `-C` and `-Z` flags are shown by default, and all the ones provided by cargo in a standard build are ignored: I did this to only show the flags that probably caused the ICE, and to remove some noise from the message. This removed flags like `opt-level` and `debuginfo` though, could those be useful for reproducing ICEs?
…on-ice, r=michaelwoerister Report non-standard compile flags on ICE Some ICEs (such as the recent rust-lang#48248) only happens when a non-standard compiler flag is provided to rustc, but users don't always report the used flags. This can slow down reproducing the issue, so this PR shows all the non-standard compiler flags in the ICE error message. For example, the output of rust-lang#48248 with this PR is: ``` error: internal compiler error: [...] thread 'rustc' panicked at [...] note: Run with `RUST_BACKTRACE=1` for a backtrace. note: the compiler unexpectedly panicked. this is a bug. note: we would appreciate a bug report: [...] note: rustc 1.25.0-dev running on x86_64-unknown-linux-gnu note: compiler flags: -C link-dead-code ``` ### Open questions * At the moment, only `-C` and `-Z` flags are shown by default, and all the ones provided by cargo in a standard build are ignored: I did this to only show the flags that probably caused the ICE, and to remove some noise from the message. This removed flags like `opt-level` and `debuginfo` though, could those be useful for reproducing ICEs?
Some ICEs (such as the recent #48248) only happens when a non-standard compiler flag is provided to rustc, but users don't always report the used flags. This can slow down reproducing the issue, so this PR shows all the non-standard compiler flags in the ICE error message.
For example, the output of #48248 with this PR is:
Open questions
-Cand-Zflags are shown by default, and all the ones provided by cargo in a standard build are ignored: I did this to only show the flags that probably caused the ICE, and to remove some noise from the message. This removed flags likeopt-levelanddebuginfothough, could those be useful for reproducing ICEs?