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

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

Closed
shepmaster opened this issue Nov 6, 2019 · 5 comments · Fixed by #66211
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@shepmaster
Copy link
Member

shepmaster commented Nov 6, 2019

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

@shepmaster shepmaster added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ labels Nov 6, 2019
@jonas-schievink jonas-schievink added A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-bug Category: This is a bug. labels Nov 6, 2019
@kinnison
Copy link
Contributor

kinnison commented Nov 6, 2019

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
Copy link
Member

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

@kinnison
Copy link
Contributor

kinnison commented Nov 6, 2019

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
Copy link
Contributor

kinnison commented Nov 6, 2019

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
Copy link
Member Author

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

shepmaster added a commit to shepmaster/snafu that referenced this issue Nov 7, 2019
bors added a commit that referenced this issue Nov 13, 2019
Fix ICE when documentation includes intra-doc-link

When collecting intra-doc-links we could trigger the loading of extra crates into the crate store due to name resolution finding crates referred to in documentation but not in code.  This might be due to
configuration differences or simply referring to something else.

This would cause an ICE because the newly loaded crate metadata existed in a crate store associated with the rustdoc global context, but the resolver had its own crate store cloned just before the documentation processing began and as such it could try and look up crates in a store which lacked them.

In this PR, I add support for `--extern-private` to the `rustdoc` tool so that it is supported for `compiletest` to then pass the crates in; and then I fix the issue by forcing the resolver to look over all the crates before we then lower the input ready for processing into documentation.

The first commit (the `--extern-private`) could be replaced with a commit which adds support for `--extern` to `compiletest` if preferred, though I think that adding `--extern-private` to `rustdoc` is more useful anyway since it makes the CLI a little more like `rustc`'s which might help reduce surprise for someone running it by hand or in their own test code.

The PR is meant to fix #66159 though it may also fix #65840.

cc @GuillaumeGomez
@bors bors closed this as completed in 33ded3e Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants