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

Rustup #330

Merged
merged 4 commits into from
Sep 6, 2017
Merged

Rustup #330

merged 4 commits into from
Sep 6, 2017

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Sep 6, 2017

No description provided.

@RalfJung
Copy link
Member Author

RalfJung commented Sep 6, 2017

Now that is odd. Some (MIR-ful) tests fail with

error: compilation failed!

status: exit code: 101

command: target/release/miri tests/run-pass-fullmir/unsized-tuple-impls.rs -L /tmp/compiletest.zVoSw4nmrRp3 --target=x86_64-unknown-linux-gnu -Z dump-mir=all -Z mir-opt-level=3 -Z dump-mir-dir=/tmp/compiletest.zVoSw4nmrRp3/unsized-tuple-impls -L /tmp/compiletest.zVoSw4nmrRp3/unsized-tuple-impls.stage-id.mir-opt.libaux -C prefer-dynamic -o /tmp/compiletest.zVoSw4nmrRp3/unsized-tuple-impls.stage-id --sysroot /home/travis/.xargo/HOST -Zmir-opt-level=3 --miri_host_target

stdout:

------------------------------------------

------------------------------------------

stderr:

------------------------------------------

thread 'main' panicked at 'assertion failed: value < ((u32::MAX) as usize)', /checkout/src/librustc/mir/mod.rs:383:0

note: Run with `RUST_BACKTRACE=1` for a backtrace.

However, they work locally...

@RalfJung
Copy link
Member Author

RalfJung commented Sep 6, 2017

Ah never mind, this is the inliner in rustc crashing:

$ RUST_BACKTRACE=1 MIRI_SYSROOT=~/.xargo/HOST cargo run --bin miri -- -Zmir-opt-level=3 tests/run-pass-fullmir/unsized-tuple-impls.rs
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs
     Running `target/debug/miri -Zmir-opt-level=3 tests/run-pass-fullmir/unsized-tuple-impls.rs`
thread 'main' panicked at 'assertion failed: value < ((u32::MAX) as usize)', /checkout/src/librustc/mir/mod.rs:383:0
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:381
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:397
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:577
   5: std::panicking::begin_panic
   6: <rustc::mir::Local as rustc_data_structures::indexed_vec::Idx>::new
   7: <rustc_mir::transform::inline::Integrator<'a, 'tcx> as rustc::mir::visit::MutVisitor<'tcx>>::visit_local
   8: <rustc_mir::transform::inline::Integrator<'a, 'tcx> as rustc::mir::visit::MutVisitor<'tcx>>::visit_lvalue
   9: <rustc_mir::transform::inline::Integrator<'a, 'tcx> as rustc::mir::visit::MutVisitor<'tcx>>::visit_basic_block_data
  10: rustc_mir::transform::inline::Inliner::run_pass
  11: <rustc_mir::transform::inline::Inline as rustc::mir::transform::MirPass>::run_pass
  12: rustc_mir::transform::run_suite
  13: rustc_mir::transform::optimized_mir
  14: rustc::dep_graph::graph::DepGraph::with_task
  15: rustc::ty::maps::<impl rustc::ty::maps::queries::optimized_mir<'tcx>>::try_get
  16: rustc::ty::maps::TyCtxtAt::optimized_mir
  17: rustc::ty::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::maybe_optimized_mir
  18: <rustc_miri::interpret::eval_context::EvalContext<'a, 'tcx, M>>::load_mir
             at ./src/librustc_mir/interpret/eval_context.rs:272
  19: miri::eval_main::run_main
             at miri/lib.rs:51

@oli-obk
Copy link
Contributor

oli-obk commented Sep 6, 2017

Yea... I'll upstream the fix in another PR that we can get through faster than the miri PR

@RalfJung
Copy link
Member Author

RalfJung commented Sep 6, 2017

Strange enough though I cannot get this crash to happen when just invoking rustc. This works:

rustc -Zmir-opt-level=3 -Zalways-encode-mir -Zdump-mir=all tests/run-pass-fullmir/unsized-tuple-impls.rs

Also, why are all the flags passed to rustc 2 or 3 times in the CI?

@oli-obk
Copy link
Contributor

oli-obk commented Sep 6, 2017

Also, why are all the flags passed to rustc 2 or 3 times in the CI?

because we set the flags, then if it's a compiletest, compiletest sets the flags set in the file, too

@oli-obk
Copy link
Contributor

oli-obk commented Sep 6, 2017

This works

Well... it's not dumping all MIR. There's some libstd MIR that probably never gets loaded or optimized. Especially if it's a generic function that is never called in rustc itself. I'm not sure what exactly is going on, but you can see it if you put a catch_panic around our load_mir call and report a regular miri-error instead.

@RalfJung
Copy link
Member Author

RalfJung commented Sep 6, 2017

because we set the flags, then if it's a compiletest, compiletest sets the flags set in the file, too

But this file doesn't set any flags.

Strange enough though I cannot get this crash to happen when just invoking rustc.

Ah, I also have to set the sysroot. Then it crashes as expected. Should I report upstream? It seems you're already on it anyway.

rustc -Zmir-opt-level=3 --sysroot=$HOME/.xargo/HOST tests/run-pass-fullmir/unsized-tuple-impls.rs

@oli-obk
Copy link
Contributor

oli-obk commented Sep 6, 2017

Already opened a PR upstream: rust-lang/rust#44362

@oli-obk
Copy link
Contributor

oli-obk commented Sep 6, 2017

But this file doesn't set any flags.

We also set some more flags in our compiletest.rs, maybe these are the ones you're looking for? We should probably clean up our CI at some point ^^

@RalfJung
Copy link
Member Author

RalfJung commented Sep 6, 2017

We also set some more flags in our compiletest.rs, maybe these are the ones you're looking for? We should probably clean up our CI at some point ^^

We do, but not twice, do we? The additional flags miri sets before internally calling rustc should not appear here.

@RalfJung
Copy link
Member Author

RalfJung commented Sep 6, 2017

Probably some of this comes from using the "mir-opt" tester.

But yeah, the CI has accumulated an impressive amount of hacks, with all this parameter passing and so on. We have MIRI_SYSROOT and --sysroot and hooks into our own magic --miri_host_target flag and whatnot...

@RalfJung
Copy link
Member Author

RalfJung commented Sep 6, 2017

Are you okay with disabling the optimized tests for now or do you rather want to wait until your PR lands?

@oli-obk
Copy link
Contributor

oli-obk commented Sep 6, 2017

Nah, just merge it and reenable later. We should probably add a miri variant to compiletest, which will try out a combination of all flags, and reports if removing flags doesn't change the test outcome.

@RalfJung RalfJung merged commit b456b7c into rust-lang:master Sep 6, 2017
@RalfJung RalfJung deleted the rustup branch September 6, 2017 08:52
@@ -190,7 +190,8 @@ fn run_pass_miri_noopt() {

#[test]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use #[ignore] here instead of making the content a commit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good catch! I will try to remember this next time. I did not know about this attribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants