rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.05k stars 12.69k forks source link

`Map::find` should not index, but call `get` instead to prevent ICEs #112534

Open Centri3 opened 1 year ago

Centri3 commented 1 year ago

rustc_middle::hir::map::Map::find will panic even though it's meant to return None, since it uses indexing instead of get.

This was ran into while writing a clippy lint. This visited every HirId using visit_id. It would be expected that if it failed to find it, it would ignore that id. Instead, it panics. For now, a reimplementation has been created.

Code

https://github.com/Centri3/rust-clippy/blob/676ad8d65cfc5c9c383c7dcde30026ad7152cf88/clippy_lints/src/min_ident_chars.rs#L85-L87, not really applicable.

Backtrace

``` thread 'rustc' panicked at 'index out of bounds: the len is 3 but the index is 3', compiler/rustc_middle/src/hir/map/mod.rs:322:24 stack backtrace: 0: 0x7fd35bbc5bb1 - std::backtrace_rs::backtrace::libunwind::trace::h4e09b4719f180222 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5 1: 0x7fd35bbc5bb1 - std::backtrace_rs::backtrace::trace_unsynchronized::hd7cdeae81cc896d9 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5 2: 0x7fd35bbc5bb1 - std::sys_common::backtrace::_print_fmt::hd49c3b478db8c635 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/sys_common/backtrace.rs:65:5 3: 0x7fd35bbc5bb1 - ::fmt::h59edacfaa3bc83ad at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/sys_common/backtrace.rs:44:22 4: 0x7fd35bc2619f - core::fmt::rt::Argument::fmt::h4d6e5a3b8dd7874d at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/core/src/fmt/rt.rs:138:9 5: 0x7fd35bc2619f - core::fmt::write::haf2f7e5235ba578a at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/core/src/fmt/mod.rs:1094:21 6: 0x7fd35bbb8dc1 - std::io::Write::write_fmt::h4a6949a8bf6e6368 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/io/mod.rs:1713:15 7: 0x7fd35bbc59c5 - std::sys_common::backtrace::_print::hff5de9ef9631778c at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/sys_common/backtrace.rs:47:5 8: 0x7fd35bbc59c5 - std::sys_common::backtrace::print::he929539ea17f974e at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/sys_common/backtrace.rs:34:9 9: 0x7fd35bbc8687 - std::panicking::default_hook::{{closure}}::hf81d17fa0bdad902 10: 0x7fd35bbc8474 - std::panicking::default_hook::hf7cf1a888245839f at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/panicking.rs:288:9 11: 0x7fd35ed9c92b - rustc_driver_impl[56cae5af60b3bf59]::install_ice_hook::{closure#0} 12: 0x7fd35bbc8dcd - as core::ops::function::Fn>::call::h15d48301475b02a0 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/alloc/src/boxed.rs:1999:9 13: 0x7fd35bbc8dcd - std::panicking::rust_panic_with_hook::h057616b5a1d41480 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/panicking.rs:709:13 14: 0x7fd35bbc8b67 - std::panicking::begin_panic_handler::{{closure}}::h5ec862423d83e4d3 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/panicking.rs:597:13 15: 0x7fd35bbc5fe6 - std::sys_common::backtrace::__rust_end_short_backtrace::h4c1dca17bc43c37e at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/sys_common/backtrace.rs:151:18 16: 0x7fd35bbc88b2 - rust_begin_unwind at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/panicking.rs:593:5 17: 0x7fd35bc22423 - core::panicking::panic_fmt::h704203a971c03be5 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/core/src/panicking.rs:67:14 18: 0x7fd35bc22582 - core::panicking::panic_bounds_check::hd086f175ae4832c3 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/core/src/panicking.rs:162:5 19: 0x7fd35cf4f934 - ::find 20: 0x55c159342b13 - ::visit_id::ha653009bcd00a3b1 at /home/centri/GitHub/rust-clippy/clippy_lints/src/min_ident_chars.rs:88:26 21: 0x55c15905e75f - rustc_hir::intravisit::walk_param_bound::hcf81da3ee637b560 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/compiler/rustc_hir/src/intravisit.rs:1065:13 22: 0x55c158ce1a34 - rustc_hir::intravisit::Visitor::visit_param_bound::h094792a64fd53dd6 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/compiler/rustc_hir/src/intravisit.rs:393:9 23: 0x55c159169fe1 - rustc_hir::intravisit::walk_item::h270fa81635496322 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/compiler/rustc_hir/src/intravisit.rs:505:13 24: 0x55c1593428d2 - ::check_item::hfe7d6c5b6b848b08 at /home/centri/GitHub/rust-clippy/clippy_lints/src/min_ident_chars.rs:57:9 25: 0x7fd35f155eb3 - ::check_local 26: 0x7fd35f13fa17 - as rustc_hir[55fad958df311873]::intravisit::Visitor>::visit_nested_item 27: 0x7fd35f118eee - rustc_hir[55fad958df311873]::intravisit::walk_ty::> 28: 0x7fd35f118e4a - rustc_hir[55fad958df311873]::intravisit::walk_fn::> 29: 0x7fd35f13f37a - as rustc_hir[55fad958df311873]::intravisit::Visitor>::visit_fn 30: 0x7fd35f119843 - rustc_hir[55fad958df311873]::intravisit::walk_item::> 31: 0x7fd35f13fa22 - as rustc_hir[55fad958df311873]::intravisit::Visitor>::visit_nested_item 32: 0x7fd35f1190c8 - rustc_hir[55fad958df311873]::intravisit::walk_mod::> 33: 0x7fd35e451cfb - rustc_lint[427e7e775f1dde36]::late::late_lint_crate:: 34: 0x7fd35e451168 - ::time::<(), rustc_lint[427e7e775f1dde36]::late::check_crate::{closure#0}::{closure#0}> 35: 0x7fd35e25d163 - ::time::<(), rustc_interface[f00e945cb6d9f8c2]::passes::analysis::{closure#5}::{closure#1}::{closure#2}::{closure#0}> 36: 0x7fd35e25ccde - as core[6c80c2e01350ba19]::ops::function::FnOnce<()>>::call_once 37: 0x7fd35e25c7d8 - ::time::<(), rustc_interface[f00e945cb6d9f8c2]::passes::analysis::{closure#5}> 38: 0x7fd35e25bf4d - rustc_interface[f00e945cb6d9f8c2]::passes::analysis 39: 0x7fd35e2c4efa - rustc_query_impl[d4eb4aa411611910]::plumbing::__rust_begin_short_backtrace::> 40: 0x7fd35e2c4ee9 - >::call_once 41: 0x7fd35e47b1c8 - rustc_query_system[9ea2071ffed97fb3]::query::plumbing::try_execute_query::>, false, false, false>, rustc_query_impl[d4eb4aa411611910]::plumbing::QueryCtxt, false> 42: 0x7fd35e47af99 - rustc_query_impl[d4eb4aa411611910]::query_impl::analysis::get_query_non_incr::__rust_end_short_backtrace 43: 0x7fd35e4529f5 - ::enter::> 44: 0x7fd35e001800 - ::enter::, rustc_span[9ffd76ecb71b4ec4]::ErrorGuaranteed>> 45: 0x7fd35dfff39f - >::set::, rustc_driver_impl[56cae5af60b3bf59]::run_compiler::{closure#1}>::{closure#0}, core[6c80c2e01350ba19]::result::Result<(), rustc_span[9ffd76ecb71b4ec4]::ErrorGuaranteed>> 46: 0x7fd35dffe806 - std[cd42f2f269856667]::sys_common::backtrace::__rust_begin_short_backtrace::, rustc_driver_impl[56cae5af60b3bf59]::run_compiler::{closure#1}>::{closure#0}, core[6c80c2e01350ba19]::result::Result<(), rustc_span[9ffd76ecb71b4ec4]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[6c80c2e01350ba19]::result::Result<(), rustc_span[9ffd76ecb71b4ec4]::ErrorGuaranteed>> 47: 0x7fd35dffe5b5 - <::spawn_unchecked_, rustc_driver_impl[56cae5af60b3bf59]::run_compiler::{closure#1}>::{closure#0}, core[6c80c2e01350ba19]::result::Result<(), rustc_span[9ffd76ecb71b4ec4]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[6c80c2e01350ba19]::result::Result<(), rustc_span[9ffd76ecb71b4ec4]::ErrorGuaranteed>>::{closure#1} as core[6c80c2e01350ba19]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0} 48: 0x7fd35bbd32d5 - as core::ops::function::FnOnce>::call_once::h04ff4859749c6ea9 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/alloc/src/boxed.rs:1985:9 49: 0x7fd35bbd32d5 - as core::ops::function::FnOnce>::call_once::hc63f4d2c9651d617 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/alloc/src/boxed.rs:1985:9 50: 0x7fd35bbd32d5 - std::sys::unix::thread::Thread::new::thread_start::h5594a02e634c76b0 at /rustc/d59363ad0b6391b7fc5bbb02c9ccf9300eef3753/library/std/src/sys/unix/thread.rs:108:17 51: 0x7fd352785b43 - start_thread at ./nptl/pthread_create.c:442:8 52: 0x7fd352817a00 - clone3 at ./misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:81 53: 0x0 - error: the compiler unexpectedly panicked. this is a bug. note: we would appreciate a bug report: https://github.com/rust-lang/rust-clippy/issues/new note: rustc 1.72.0-nightly (d59363ad0 2023-06-01) running on x86_64-unknown-linux-gnu note: compiler flags: -C prefer-dynamic -Z ui-testing query stack during panic: #0 [analysis] running analysis passes on this crate end of query stack note: Clippy version: clippy 0.1.72 (7f43e0f10 2023-06-09) ```

compiler-errors commented 1 year ago

Do you know what hir id is causing the ICE? That's a strange behavior, hir::Map::find should work on any valid hir id, even if it doesn't have a hir::Node associated with it 🤔

compiler-errors commented 1 year ago

That is to say, I wonder if this is a bug in ast lowering, and if so, I'd rather fix it in rustc rather than making hir::Map::find more fallible 🤔

blyxyas commented 1 year ago

Replying to those comments, it seems like async items panic with find. I'm not sure which case, but that's what this comment talks about.

compiler-errors commented 1 year ago

If you can tell me what owns the HirId that you're finding trouble with, and where that HirId is actually located in that owner, then I might be able to help. But there's not much information to work with here :sweat:

Centri3 commented 1 year ago

This code would trigger the ICE:

async fn a() {}

It looks like visit_id is called in walk_param_bound on LangItemTrait, though I have no idea what this HirId is there for; it has no associated node, afaict, and comes from async desugaring. This should really have a node corresponding to this (compiler-inserted) bound. I think this would normally return a Node::Ty (for other bounds) so maybe this should do the same? But I'm not sure.