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

Compiler panic on fairly simple match expression #51415

Closed
mulkieran opened this issue Jun 7, 2018 · 15 comments
Closed

Compiler panic on fairly simple match expression #51415

mulkieran opened this issue Jun 7, 2018 · 15 comments
Assignees
Labels
A-borrow-checker Area: The borrow checker C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@mulkieran
Copy link

Here's the stack trace

> make build
PKG_CONFIG_ALLOW_CROSS=1 \
RUSTFLAGS='-D warnings' \
RUST_BACKTRACE=1 \
cargo build --target "x86_64-unknown-linux-gnu"
   Compiling libstratis v0.5.3 (file:///home/mulhern/my-contributions/stratisd)
thread 'rustc' panicked at 'called `Result::unwrap()` on an `Err` value: (MoveData { move_paths: [MovePath { place: _0 }, MovePath { place: _1 }, MovePath { place: _2 }, MovePath { place: _3 }, MovePath { place: _4 }], moves: [mp4@bb0[4], mp4@bb2[0], mp3@bb2[1], mp3@bb3[0], mp3@bb4[0], mp0@bb4[1]], loc_map: LocationMap { map: [[[], [], [], [], [mo0]], [[]], [[mo1], [mo2]], [[mo3]], [[mo4], [mo5]]] }, path_map: [[mo5], [], [], [mo2, mo3, mo4], [mo0, mo1]], rev_lookup: MovePathLookup { locals: [mp0, mp1, mp2, mp3, mp4], projections: {} }, inits: [mp1@src/engine/strat_engine/cmd.rs:54:32: 54:59 (Deep), mp2@src/engine/strat_engine/cmd.rs:54:33: 54:43 (Deep), mp3@src/engine/strat_engine/cmd.rs:54:38: 54:42 (Deep), mp4@src/engine/strat_engine/cmd.rs:54:45: 54:49 (Deep), mp0@src/engine/strat_engine/cmd.rs:54:45: 54:59 (NonPanicPathOnly)], init_loc_map: LocationMap { map: [[[], [in2], [], [in3], [in4]], [[]], [[], []], [[]], [[], []]] }, init_path_map: [[in4], [in0], [in1], [in2], [in3]] }, [IllegalMove { cannot_move_out_of: IllegalMoveOrigin { span: src/engine/strat_engine/cmd.rs:54:38: 54:42, kind: BorrowedContent } }])', libcore/result.rs:945:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: core::ops::function::Fn::call
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::begin_panic_fmt
   7: rust_begin_unwind
   8: core::panicking::panic_fmt
   9: core::result::unwrap_failed
  10: <rustc_mir::transform::elaborate_drops::ElaborateDrops as rustc_mir::transform::MirPass>::run_pass
  11: rustc_mir::transform::optimized_mir::{{closure}}
  12: rustc_mir::transform::optimized_mir
  13: rustc::dep_graph::graph::DepGraph::with_task_impl
  14: rustc_errors::Handler::track_diagnostics
  15: rustc::ty::maps::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::cycle_check
  16: rustc::ty::maps::<impl rustc::ty::maps::queries::optimized_mir<'tcx>>::force
  17: rustc::ty::maps::<impl rustc::ty::maps::queries::optimized_mir<'tcx>>::try_get
  18: rustc::ty::maps::TyCtxtAt::optimized_mir
  19: rustc::ty::maps::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::optimized_mir
  20: rustc_metadata::encoder::<impl rustc_metadata::isolated_encoder::IsolatedEncoder<'a, 'b, 'tcx>>::encode_optimized_mir
  21: rustc_metadata::encoder::<impl rustc_metadata::isolated_encoder::IsolatedEncoder<'a, 'b, 'tcx>>::encode_info_for_closure
  22: rustc::dep_graph::graph::DepGraph::with_ignore
  23: rustc_metadata::encoder::<impl rustc_metadata::index_builder::IndexBuilder<'a, 'b, 'tcx>>::encode_info_for_expr
  24: rustc::hir::intravisit::walk_expr
  25: rustc::hir::intravisit::walk_expr
  26: rustc::hir::intravisit::walk_expr
  27: rustc::hir::intravisit::Visitor::visit_nested_body
  28: <rustc_metadata::encoder::EncodeVisitor<'a, 'b, 'tcx> as rustc::hir::intravisit::Visitor<'tcx>>::visit_item
  29: rustc::hir::Crate::visit_all_item_likes
  30: rustc_metadata::encoder::encode_metadata
  31: rustc_metadata::cstore_impl::<impl rustc::middle::cstore::CrateStore for rustc_metadata::cstore::CStore>::encode_metadata
  32: rustc::ty::context::TyCtxt::encode_metadata
  33: rustc_trans::base::write_metadata
  34: rustc::util::common::time
  35: rustc_trans::base::trans_crate
  36: <rustc_trans::LlvmTransCrate as rustc_trans_utils::trans_crate::TransCrate>::trans_crate
  37: rustc::util::common::time
  38: rustc_driver::driver::phase_4_translate_to_llvm
  39: rustc_driver::driver::compile_input::{{closure}}
  40: <std::thread::local::LocalKey<T>>::with
  41: <std::thread::local::LocalKey<T>>::with
  42: rustc::ty::context::TyCtxt::create_and_enter
  43: rustc_driver::driver::compile_input
  44: rustc_driver::run_compiler_impl
  45: syntax::with_globals

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.26.0 running on x86_64-unknown-linux-gnu

note: compiler flags: -C debuginfo=2 -C incremental --crate-type lib

note: some of the compiler flags provided by cargo are hidden

error: Could not compile `libstratis`.

To learn more, run the command again with --verbose.
make: *** [Makefile:27: build] Error 101
@mulkieran
Copy link
Author

Here's a PR with the code that caused the crash:

stratis-storage/stratisd#983

@mulkieran
Copy link
Author

I compile frequently, and had made only a few changes between this and the last time I had compiled. I think the crash has something to do with the implementation of verify_binaries because I had just been juggling with that. I expect it has something to do with the Some match arm.

@hellow554
Copy link
Contributor

hellow554 commented Jun 7, 2018

Can you provide a mcve?

@mulkieran
Copy link
Author

What's an "mcve"?

@mulkieran
Copy link
Author

Whoops, I thought that was an actual word, not an initialism. Giving it the 15 minute try...

@mulkieran
Copy link
Author

mulkieran commented Jun 7, 2018

Well, I make no assertions about minimality, but this code snippet does fail on the rust playground w/ all six possible configurations and it's pretty short.

#[macro_use]
extern crate lazy_static;

use std::collections::HashMap;
use std::path::PathBuf;

lazy_static! {
    static ref BINARIES: HashMap<String, Option<PathBuf>> = [].iter().cloned().collect();
}

fn main() {
    let _ = match BINARIES.iter().find(|(_, &path)| path.is_none()) {
        None => Ok(()),
        Some((ref name, _)) => Err(name.to_string()),
    };
}

@mulkieran
Copy link
Author

Smaller:

use std::collections::HashMap;
use std::path::PathBuf;

fn main() {
    let junk: HashMap<String, Option<PathBuf>> = HashMap::new();
    let _ = match junk.iter().find(|(_, &path)| path.is_none()) {
        None => Ok(()),
        Some((ref name, _)) => Err(name.to_string()),
    };
}

@nagisa nagisa added the I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ label Jun 7, 2018
@mulkieran
Copy link
Author

Smidge smaller:

use std::collections::HashMap;
use std::path::PathBuf;

fn main() {
    let junk: HashMap<String, Option<PathBuf>> = HashMap::new();
    let _ = match junk.iter().find(|(_, &path)| path.is_none()) {
        None => "none".to_string(),
        Some((ref name, _)) => name.to_string(),
    };
}

@hellow554
Copy link
Contributor

ICE does not happen with #![feature(nll)]

@hellow554
Copy link
Contributor

hellow554 commented Jun 7, 2018

You can make your example working by changing the &path to either path (match ergnomics) or ref path (explicit ref), but yes. You're right. It's an ICE, should not happen :)

@hellow554
Copy link
Contributor

Worked on 1.25.0 with an explicit error message.

error[E0658]: non-reference pattern used to match a reference (see issue #42640)
 --> src/main.rs:6:37
  |
6 |     let _ = match junk.iter().find(|(_, path)| path.is_none()) {
  |                                     ^^^^^^^^^^^^^ help: consider using a reference: `&(_, path)`

@estebank
Copy link
Contributor

estebank commented Jun 7, 2018

I think might have been caused by the match ergonomics pr.

@pietroalbini pietroalbini added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. C-bug Category: This is a bug. labels Jun 8, 2018
@alberdingk-thijm
Copy link

Had a similar compiler panic with this snippet of code:

fn main() {
    let a = vec![String::from("a"), String::from("b"), String::from("c")];
    let opt = a.iter().enumerate().find(|(_, &s)| {
        *s == String::from("d")
    }).map(|(i, _)| i);
    println!("{:?}", opt);
}

Changing it to the snippet below fixes the problem, so I definitely think the match ergonomics is doing something.

fn main() {
    let a = vec![String::from("a"), String::from("b"), String::from("c")];
    let opt = a.iter().enumerate().find(|&(_, s)| {
        *s == String::from("d")
    }).map(|(i, _)| i);
    println!("{:?}", opt);
}

See it in the playground here.

@nikomatsakis
Copy link
Contributor

triage: P-high

Hmm, this might be a problem with borrowck failing to catch a borrow (the NLL borrow checker seems to catch the error correctly).

@nikomatsakis nikomatsakis added A-borrow-checker Area: The borrow checker I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness labels Jun 21, 2018
@nikomatsakis nikomatsakis self-assigned this Jun 21, 2018
@nikomatsakis nikomatsakis added P-high High priority and removed I-nominated labels Jun 21, 2018
@nikomatsakis
Copy link
Contributor

The problem seems to be specific to argument patterns. This code fails to compile as expected:

fn main() {
    let a = vec![String::from("a")];
    let opt = a.iter().enumerate().find(|param| {
        let (_, &s) = param; //~ ERROR
        *s == String::from("d")
    }).map(|(i, _)| i);
    println!("{:?}", opt);
}

bors added a commit that referenced this issue Jun 22, 2018
…t-bindings-bug, r=eddyb

yet another "old borrowck" bug around match default bindings

We were getting the type of the parameter from its pattern, but that didn't include adjustments. I did a `ripgrep` around and this seemed to be the only affected case.

The reason this didn't show up as an ICE earlier is that mem-categorization is lenient with respect to weird discrepancies. I am going to add more delay-span-bug calls shortly around that (I'll push onto the PR).

This example is an ICE, but I presume that there is a way to make a soundness example out of this -- it basically ignores borrows occuring inside match-default-bindings in a closure, though only if the implicit deref is at the top-level. It happens though that this occurs frequently in iterators, which often give a `&T` parameter.

Fixes #51415
Fixes #49534

r? @eddyb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants