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

Improve errors when invoked with wrong sysroot (missing MIR) #1834

Closed
ojeda opened this issue Jun 13, 2021 · 13 comments · Fixed by #1856
Closed

Improve errors when invoked with wrong sysroot (missing MIR) #1834

ojeda opened this issue Jun 13, 2021 · 13 comments · Fixed by #1856
Labels
A-ux Area: This affects the general user experience C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement E-good-first-issue A good way to start contributing, mentoring is available

Comments

@ojeda
Copy link
Contributor

ojeda commented Jun 13, 2021

miri behaves like a strange rustc driver:

$ miri main.rs # stable
error: the option `Z` is only accepted on the nightly compiler

$ miri main.rs # nightly
error: internal compiler error: compiler/rustc_metadata/src/rmeta/decoder.rs:1157:17: get_optimized_mir: missing MIR for `DefId(1:8654 ~ std[e170]::rt::lang_start_internal)`
@bjorn3
Copy link
Member

bjorn3 commented Jun 13, 2021

$ miri main.rs # stable
error: the option `Z` is only accepted on the nightly compiler

It shouldn't even be possible to install miri on stable AFAIK.

$ miri main.rs # nightly
error: internal compiler error: compiler/rustc_metadata/src/rmeta/decoder.rs:1157:17: get_optimized_mir: missing MIR for `DefId(1:8654 ~ std[e170]::rt::lang_start_internal)`

I guess this error could be better. The sysroot needs to be compiled with -Zalways-encode-mir. Otherwise MIR is missing for non-generic non-#[inline] functions.

@RalfJung
Copy link
Member

It shouldn't even be possible to install miri on stable AFAIK.

Indeed, how did you install this?

I guess this error could be better. The sysroot needs to be compiled with -Zalways-encode-mir. Otherwise MIR is missing for non-generic non-#[inline] functions.

Yeah... we used to have a nicer error but that probably bitrotted now that cargo miri can auto-compile the sysroot with xargo.

@ojeda
Copy link
Contributor Author

ojeda commented Jun 14, 2021

Indeed, how did you install this?

It appears to come by default via the standalone installers:

$ curl https://static.rust-lang.org/dist/rust-1.52.1-x86_64-unknown-linux-gnu.tar.gz | tar xz
$ rust-1.52.1-x86_64-unknown-linux-gnu/install.sh
$ miri
error: the option `Z` is only accepted on the nightly compiler
$ RUSTC_BOOTSTRAP=1 miri --version
rustc 1.52.1 (9bc8c42bb 2021-05-09)

@RalfJung
Copy link
Member

oops that's a bug. I reported an issue for this: rust-lang/rust#86286. Good thing that Miri dosn't work there, it's not supposed to even be part of this release. ;)

@RalfJung
Copy link
Member

RalfJung commented Jun 14, 2021

For the ICE when a MIR body is missing, we need to check is_mir_available before calling instance_mir. However, this patch does not work:

--- a/compiler/rustc_mir/src/interpret/machine.rs
+++ b/compiler/rustc_mir/src/interpret/machine.rs
@@ -144,7 +144,13 @@ fn load_mir(
         ecx: &InterpCx<'mir, 'tcx, Self>,
         instance: ty::InstanceDef<'tcx>,
     ) -> InterpResult<'tcx, &'tcx mir::Body<'tcx>> {
-        Ok(ecx.tcx.instance_mir(instance))
+        let def_id = instance.def_id();
+        // Avoid ICE when MIR is missing: first check if it is available.
+        if ecx.tcx.is_mir_available(def_id) {
+            Ok(ecx.tcx.instance_mir(instance))
+        } else {
+            throw_unsup!(NoMirFor(def_id))
+        }
     }
 
     /// Entry point to all function calls.

This will now ICE for shims because they claim to have no MIR available.

@oli-obk any suggestions for what to do here? I'd rather not duplicate the entire logic of instance_mir, but I also see no way to avoid that. Can we make instance_mir return an Option?

@oli-obk
Copy link
Contributor

oli-obk commented Jun 15, 2021

Can we make instance_mir return an Option?

that would invoke is_mir_available on every instance_mir call, probably not a problem, but should bench it at least.

@RalfJung
Copy link
Member

I'm open to other options. :D

@oli-obk
Copy link
Contributor

oli-obk commented Jun 15, 2021

I'm open to other options. :D

I see what you did there, but libcore only has Option

Make instance_mir_inner that does the main logic and takes a closure argument for loading the MIR. instance_mir calls it and passes Some(optimized_mir()) and unwraps the result of instance_mir_inner.

@RalfJung
Copy link
Member

Honestly I didn't even see what I did there. Now I do. ;)

Can't the queries called by instance_mir be made to return an Option, instead of ICEing on missing MIR?

@oli-obk
Copy link
Contributor

oli-obk commented Jun 16, 2021

if you add _opt to their name and add inherent functions of the original name that unwrap, i think that would be fine

@RalfJung
Copy link
Member

Yeah that sounds like the best approach to me then.

@RalfJung RalfJung changed the title Improve errors when invoked directly Improve errors when invoked with wrong sysroot (missing MIR) Jun 16, 2021
@bjorn3
Copy link
Member

bjorn3 commented Jun 16, 2021

Maybe you could show a warning at startup if tcx.is_mir_available for a known non-generic function in libcore returns false?

@RalfJung
Copy link
Member

FWIW, this is the backtrace of the ICE:

error: internal compiler error: compiler/rustc_metadata/src/rmeta/decoder.rs:1167:17: get_optimized_mir: missing MIR for `DefId(1:9633 ~ std[8eb6]::rt::lang_start_internal)`

thread 'rustc' panicked at 'Box<dyn Any>', compiler/rustc_errors/src/lib.rs:1007:9
stack backtrace:
   0: std::panicking::begin_panic
   1: std::panic::panic_any
   2: rustc_errors::HandlerInner::bug
   3: rustc_errors::Handler::bug
   4: rustc_middle::ty::context::tls::with_opt
   5: rustc_middle::util::bug::opt_span_bug_fmt
   6: rustc_middle::util::bug::bug_fmt
   7: rustc_metadata::rmeta::decoder::<impl rustc_metadata::creader::CrateMetadataRef>::get_optimized_mir
   8: rustc_metadata::rmeta::decoder::cstore_impl::provide_extern::optimized_mir
   9: rustc_query_system::query::plumbing::get_query_impl
  10: <rustc_query_impl::Queries as rustc_middle::ty::query::QueryEngine>::optimized_mir
  11: rustc_middle::ty::<impl rustc_middle::ty::context::TyCtxt>::instance_mir
  12: rustc_mir::interpret::machine::Machine::load_mir
             at /rustc/a50d72158e08e02cfc051b863017bdbd2c45b637/compiler/rustc_mir/src/interpret/machine.rs:147:12
  13: rustc_mir::interpret::eval_context::InterpCx<M>::load_mir
             at /rustc/a50d72158e08e02cfc051b863017bdbd2c45b637/compiler/rustc_mir/src/interpret/eval_context.rs:504:9
  14: miri::shims::EvalContextExt::find_mir_or_eval_fn
             at ./src/shims/mod.rs:57:19
  15: <miri::machine::Evaluator as rustc_mir::interpret::machine::Machine>::find_mir_or_eval_fn
             at ./src/machine.rs:407:9
  16: rustc_mir::interpret::terminator::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::eval_fn_call
             at /rustc/a50d72158e08e02cfc051b863017bdbd2c45b637/compiler/rustc_mir/src/interpret/terminator.rs:303:27
  17: rustc_mir::interpret::terminator::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::eval_terminator
             at /rustc/a50d72158e08e02cfc051b863017bdbd2c45b637/compiler/rustc_mir/src/interpret/terminator.rs:108:17
  18: rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::terminator
             at /rustc/a50d72158e08e02cfc051b863017bdbd2c45b637/compiler/rustc_mir/src/interpret/step.rs:313:9
  19: rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::step
             at /rustc/a50d72158e08e02cfc051b863017bdbd2c45b637/compiler/rustc_mir/src/interpret/step.rs:73:9
  20: miri::eval::eval_main::{{closure}}
             at ./src/eval.rs:264:29
  21: miri::eval::eval_main
             at ./src/eval.rs:258:38
  22: <miri::MiriCompilerCalls as rustc_driver::Callbacks>::after_analysis::{{closure}}
             at ./src/bin/miri.rs:80:40

@RalfJung RalfJung added A-ux Area: This affects the general user experience C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement E-good-first-issue A good way to start contributing, mentoring is available labels Jul 19, 2021
@bors bors closed this as completed in eb9e307 Jul 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ux Area: This affects the general user experience C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement E-good-first-issue A good way to start contributing, mentoring is available
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants