-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
Use delay_span_bug in validate-mir. #147712
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -329,6 +329,13 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls { | |
| } | ||
|
|
||
| fn exit(exit_code: i32) -> ! { | ||
| // MIR validation uses delayed bugs. Flush them as we won't return for `run_compiler` to do it. | ||
| ty::tls::with_opt(|opt_tcx| { | ||
| if let Some(tcx) = opt_tcx { | ||
| tcx.dcx().flush_delayed() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems a bit odd to only do this at the end... I assume this is fairly cheap? Maybe we should just do it in |
||
| } | ||
| }); | ||
|
|
||
| // Drop the tracing guard before exiting, so tracing calls are flushed correctly. | ||
| deinit_loggers(); | ||
| // Make sure the supervisor knows about the exit code. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,28 +1,23 @@ | ||
| error: internal compiler error: compiler/rustc_mir_transform/src/validate.rs:LL:CC: broken MIR in Item(DefId) (after phase change to runtime-optimized) at bb0[1]: | ||
| note: no errors encountered even though delayed bugs were created | ||
|
|
||
|
|
||
| error: internal compiler error: broken MIR in Item(DefId) (after phase change to runtime-optimized) at bb0[1]: | ||
| place (*(_2.0: *mut i32)) has deref as a later projection (it is only permitted as the first projection) | ||
| --> tests/panic/mir-validation.rs:LL:CC | ||
| | | ||
| LL | *(tuple.0) = 1; | ||
| | ^^^^^^^^^^^^^^ | ||
|
|
||
|
|
||
| thread 'rustc' ($TID) panicked at compiler/rustc_mir_transform/src/validate.rs:LL:CC: | ||
| Box<dyn Any> | ||
| stack backtrace: | ||
| | | ||
|
|
||
| --> tests/panic/mir-validation.rs:LL:CC | ||
| | | ||
| LL | *(tuple.0) = 1; | ||
| | ^^^^^^^^^^^^^^ | ||
|
|
||
|
|
||
|
|
||
|
|
||
| query stack during panic: | ||
| #0 [optimized_mir] optimizing MIR for `main` | ||
| end of query stack | ||
|
|
||
| Miri caused an ICE during evaluation. Here's the interpreter backtrace at the time of the panic: | ||
| --> RUSTLIB/core/src/ops/function.rs:LL:CC | ||
| | | ||
| LL | extern "rust-call" fn call_once(self, args: Args) -> Self::Output; | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| //@ known-bug: #140850 | ||
| //@ compile-flags: -Zvalidate-mir | ||
| fn A() -> impl { | ||
| fn A() -> impl Copy { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why did this need to change?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the syntax error would hide the validation ICE. |
||
| while A() {} | ||
| loop {} | ||
| } | ||
|
|
||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| error: future cannot be sent between threads safely | ||
| --> $DIR/non-send-dyn-send-ice.rs:8:5 | ||
| | | ||
| LL | / Box::new(async { | ||
| LL | | let non_send = null::<()>(); | ||
| LL | | &non_send; | ||
| LL | | async {}.await | ||
| LL | | }) | ||
| | |______^ future created by async block is not `Send` | ||
| | | ||
| = help: within `{async block@$DIR/non-send-dyn-send-ice.rs:8:14: 8:19}`, the trait `Send` is not implemented for `*const ()` | ||
| note: future is not `Send` as this value is used across an await | ||
| --> $DIR/non-send-dyn-send-ice.rs:11:18 | ||
| | | ||
| LL | let non_send = null::<()>(); | ||
| | -------- has type `*const ()` which is not `Send` | ||
| LL | &non_send; | ||
| LL | async {}.await | ||
| | ^^^^^ await occurs here, with `non_send` maybe used later | ||
| = note: required for the cast from `Box<{async block@$DIR/non-send-dyn-send-ice.rs:8:14: 8:19}>` to `Box<dyn Send>` | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| error: item does not constrain `Bar::{opaque#0}` | ||
| --> $DIR/branch-closure-parameter.rs:14:4 | ||
| | | ||
| LL | fn foo() -> A { | ||
| | ^^^ | ||
| | | ||
| = note: consider removing `#[define_opaque]` or adding an empty `#[define_opaque()]` | ||
| note: this opaque type is supposed to be constrained | ||
| --> $DIR/branch-closure-parameter.rs:5:12 | ||
| | | ||
| LL | type Bar = impl std::fmt::Display; | ||
| | ^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure delaying this is the right strategy? The reason the validator exists is that a bunch of things working on MIR (interpreter and MIR transforms) make assumptions about MIR having some reasonable shape. We don't want to litter them all with delay_span_bug, that will take up way too much room in the code. Delaying the bug in the validator will, I think, just push these ICEs to later in the execution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could split the validator in two:
All crashes are from the second category.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validator is already split, right? There's the parts that need to be re-checked after a substitution is applied, and the parts that can't be broken by substitution.
What are examples of these "type and layout checks"?
For the interpreter we've so far had the rule that if the body doesn't typeck, it shouldn't be fed to the interpreter. A lot of that was type and layout stuff, which can have entirely nonsensical consequences, like nonsensical reference layout when it thinks
[T]is sized (and therefore&[T]is a thin ptr), or when an unsized type gets plugged in for aT: Sizedand so a ptr-sized&Tsuddenly is two prts large. If every MIR transform needs to be robust against such nonsense, we'll never be able to stop all the ICEs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking that the source and destination of a
Unsizecast are related by a trait. Checking that the source and destination of aSubtypeprojection are related...This is ok for the interpreter, as it can only call functions that are reachable. But the MIR optimizer is called on all bodies. We filter out those that are obviously erroneous, and try to filter out those that have impossible predicates. But those filters are leaky, and don't catch spurious normalization failures for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some MIR opts call into the interpreter, so it does end up being called on dead code. IIRC @BoxyUwU was working on improving the guards that avoid nonsense code from getting so far into the compilation; not sure what the status of that is or whether it would help with the issues you are trying to fix here.