ICE when documenting code with intra-doc-links with types with the same name but different crates

[shepmaster]

Cargo.toml

[package]
name = "repro"
version = "0.0.1"
edition = "2018"

[dependencies]
backtrace = { version = "=0.3.40" }

src/lib.rs

//! a [`Backtrace`] b [`backtrace::Backtrace`] c

% RUST_BACKTRACE=full cargo +nightly doc
 Documenting repro v0.0.1 (/private/tmp/reduce)
warning: `[Backtrace]` cannot be resolved, ignoring it...
 --> src/lib.rs:1:8
  |
1 | //! a [`Backtrace`] b [`backtrace::Backtrace`] c
  |        ^^^^^^^^^^^ cannot be resolved, ignoring
  |
  = note: `#[warn(intra_doc_link_resolution_failure)]` on by default
  = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`

thread 'rustc' panicked at 'index out of bounds: the len is 15 but the index is 15', /rustc/1423bec54cf2db283b614e527cfd602b481485d1/src/libcore/slice/mod.rs:2796:10
stack backtrace:
   0:        0x111a6bb65 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h0b2af991c2fe744d
   1:        0x111aa2cb0 - core::fmt::write::h9accbbe8984e0e42
   2:        0x111a5f46b - std::io::Write::write_fmt::h043704431db5a03f
   3:        0x111a6ff73 - std::panicking::default_hook::{{closure}}::h6c92fb3ec83e7ead
   4:        0x111a6fc7a - std::panicking::default_hook::hc450333d5d96e7ef
   5:        0x111a706bb - std::panicking::rust_panic_with_hook::h917f6fb05cbf8c98
   6:        0x111a70219 - std::panicking::continue_panic_fmt::hf6c906ba6ebd15f2
   7:        0x111a70119 - rust_begin_unwind
   8:        0x111a9c32c - core::panicking::panic_fmt::h3875a43afa9dedf3
   9:        0x111a9c2f9 - core::panicking::panic_bounds_check::hf760d11171dae66b
  10:        0x10ff603c2 - rustc_metadata::cstore_impl::<impl rustc::middle::cstore::CrateStore for rustc_metadata::cstore::CStore>::crate_data_as_any::h71b302c11ca9df21
  11:        0x10fedbc97 - rustc_metadata::cstore_impl::provide_extern::crate_name::h78403914ba38d538
  12:        0x10ec026d2 - rustc::ty::query::__query_compute::crate_name::h5a4e514932eb45ca
  13:        0x10ec67097 - rustc::dep_graph::graph::DepGraph::with_task_impl::h703b19fef3ff8af1
  14:        0x10eabc122 - rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query::h2515a1874185b65a
  15:        0x10ebae230 - rustdoc::clean::inline::record_extern_fqn::h3bee56eed20d57c9
  16:        0x10ec18db4 - rustdoc::clean::register_res::ha531ea399e3772b2
  17:        0x10ec8237c - <rustdoc::passes::collect_intra_doc_links::LinkCollector as rustdoc::fold::DocFolder>::fold_item::h55459422e80c1ced
  18:        0x10ec84703 - <rustdoc::passes::collect_intra_doc_links::LinkCollector as rustdoc::fold::DocFolder>::fold_crate::h2b1acb5c03f4d105
  19:        0x10ec7e786 - rustdoc::passes::collect_intra_doc_links::collect_intra_doc_links::hd1d0aadccdea5918
  20:        0x10ea7b5f0 - rustc::ty::context::tls::enter_global::hdf2dfee3160e05c1
  21:        0x10eaf387e - rustc_interface::passes::BoxedGlobalCtxt::access::{{closure}}::h54cd1c2851a0b918
  22:        0x10f026eeb - rustc_interface::passes::create_global_ctxt::{{closure}}::h52bddd6fee95c3ac
  23:        0x10eaf3474 - rustc_interface::passes::BoxedGlobalCtxt::enter::h2cb1dcf96523f357
  24:        0x10ebb6966 - rustdoc::core::run_core::h74a886bd0ae312ba
  25:        0x10ea690a0 - <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::h69bb8ac4b07d52e9
  26:        0x10eb8ddc0 - std::panicking::try::do_call::h86523fc6f43abb8c
  27:        0x111a7fd1f - __rust_maybe_catch_panic
  28:        0x10ebe8a0e - rustc_driver::catch_fatal_errors::h8687599a5d02e530
  29:        0x10ec87570 - rustdoc::main_options::h70f4a3ec203f5ffb
  30:        0x10eb43141 - std::thread::local::LocalKey<T>::with::ha42f6555d2118b0f
  31:        0x10eaeebb2 - scoped_tls::ScopedKey<T>::set::h95496a5f95698873
  32:        0x10ec06be5 - syntax::with_globals::h7ba77a695cd1ab99
  33:        0x10ea6cc11 - std::sys_common::backtrace::__rust_begin_short_backtrace::hc4944de7af8b6487
  34:        0x111a7fd1f - __rust_maybe_catch_panic
  35:        0x10ea6f807 - core::ops::function::FnOnce::call_once{{vtable.shim}}::heae349a5cee8e57d
  36:        0x111a51a0e - <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once::h9b41d7adac010663
  37:        0x111a7ea5e - std::sys::unix::thread::Thread::new::thread_start::h0725fe9398379348
  38:     0x7fff6bad0d76 - _pthread_start
error: Could not document `repro`.

Caused by:
  process didn't exit successfully: `rustdoc --edition=2018 --crate-type lib --crate-name repro src/lib.rs -o /private/tmp/reduce/target/doc --error-format=json --json=diagnostic-rendered-ansi -L dependency=/private/tmp/reduce/target/debug/deps --extern backtrace=/private/tmp/reduce/target/debug/deps/libbacktrace-422eef4db1c9a8ca.rmeta` (exit code: 1)

rustc 1.38.0 (625451e37 2019-09-23)
binary: rustc
commit-hash: 625451e376bb2e5283fc4741caa0a3e8a2ca4d54
commit-date: 2019-09-23
host: x86_64-apple-darwin
release: 1.38.0
LLVM version: 9.0

/cc @kinnison

[kinnison]

So the explosion is that somehow the crate index in the DefId is broken, or at least not entirely right.

Interestingly if you add extern crate backtrace to the lib.rs then it no longer ICEs (but has the same DefId)

Edit: So this looks like it's somehow a bug when trying to resolve it as a global name (both extern crate backtrace; and use backtrace; prevent the iCE)

[GuillaumeGomez]

Maybe it's linked to #65840 ? If so, it's not in rustdoc but below.

[kinnison]

I believe I've worked out approximately what's going on:

Rustdoc freezes the compiler state (resolver, etc) as part of the setup to ready the DocContext instance.

Since extern crate was made optional, and crate names are automatically part of the global scope, people stopped necessarily telling the compiler about the crates in their code. If a comment references a path in this manner, but the code itself doesn't, then the resolver frozen into the DocContext doesn't have that crate's metadata loaded into it. When we then ask the resolver to resolve a new path (e.g. backtrace::Backtrace) it goes ahead and loads the crate into a different instance of the CStore than will be used when we then try and use the DefId later.

It feels like the moment a CStore is clone()d it should also be locked so that it can never be modified again.

It's also possible that rustdoc could be forced to enter the right context so that we never do this wrongly, but that's beyond me right now.

Given that, @GuillaumeGomez , I feel this is probably either a bug in how rustdoc is handling the resolver, or else a change in how the resolver behaves which either needs fixing, or changing in rustdoc to cope.

[kinnison]

My gut feeling is that we probably need to stop capturing/cloning the resolver as we do here: https://github.com/rust-lang/rust/blob/master/src/librustdoc/core.rs#L346 and instead try and find a resolver as part of the closure here: https://github.com/rust-lang/rust/blob/master/src/librustdoc/core.rs#L354

Of course, it looks like a TyCtxt has an immutable copy of the resolver's outputs, and no access to the resolver itself, urgh, and we have no easy way to inject changes into the context at this point.

[shepmaster]

I wonder why that type can be cloned in the first place...