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

Index out of bounds when running cargo doc in rustc_metadata/src/creader.rs #84738

Closed
XAMPPRocky opened this issue Apr 30, 2021 · 35 comments · Fixed by #85749 or #90489
Closed

Index out of bounds when running cargo doc in rustc_metadata/src/creader.rs #84738

XAMPPRocky opened this issue Apr 30, 2021 · 35 comments · Fixed by #85749 or #90489
Labels
A-resolve Area: Name resolution C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-critical Critical priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Milestone

Comments

@XAMPPRocky
Copy link
Member

XAMPPRocky commented Apr 30, 2021

EDIT(camelid): This issue is now being used to track a different, but related, bug. The discussion starting here is current.

Steps

  • Clone rust-gpu.
  • cargo doc -p spirv-std --no-deps

Meta

rustc --version --verbose:

rustc 1.53.0-nightly (42816d61e 2021-04-24)
binary: rustc
commit-hash: 42816d61ead7e46d462df997958ccfd514f8c21c
commit-date: 2021-04-24
host: x86_64-apple-darwin
release: 1.53.0-nightly
LLVM version: 12.0.0

Error output

thread 'rustc' panicked at 'index out of bounds: the len is 8 but the index is 8', compiler/rustc_metadata/src/creader.rs:137:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

error: internal compiler error: unexpected panic

error: Unrecognized option: 'crate-version'

error: could not document `spirv-std`

Caused by:
  process didn't exit successfully: `rustdoc --edition=2018 --crate-type lib --crate-name spirv_std crates/spirv-std/src/lib.rs -o /Users/erin.power/src/rust-gpu2/target/doc --cfg 'feature="default"' --error-format=json --json=diagnostic-rendered-ansi -L dependency=/Users/erin.power/src/rust-gpu2/target/debug/deps --extern bitflags=/Users/erin.power/src/rust-gpu2/target/debug/deps/libbitflags-7c13a8e06c86cae3.rmeta --extern num_traits=/Users/erin.power/src/rust-gpu2/target/debug/deps/libnum_traits-226168b7b64579d8.rmeta --extern spirv_std_macros=/Users/erin.power/src/rust-gpu2/target/debug/deps/libspirv_std_macros-63f1f4de237cb2bc.dylib --extern spirv_types=/Users/erin.power/src/rust-gpu2/target/debug/deps/libspirv_types-7559b6c8fd9bbf59.rmeta --crate-version 0.4.0-alpha.7` (exit status: 1)
Backtrace

   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::panic_bounds_check
   3: rustc_metadata::rmeta::decoder::cstore_impl::provide_extern::crate_name
   4: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
   5: rustc_data_structures::stack::ensure_sufficient_stack
   6: rustc_query_system::query::plumbing::force_query_with_job
   7: rustc_query_system::query::plumbing::get_query_impl
   8: <rustc_query_impl::Queries as rustc_middle::ty::query::QueryEngine>::crate_name
   9: rustdoc::clean::inline::record_extern_fqn
  10: rustdoc::clean::utils::register_res
  11: rustdoc::passes::collect_intra_doc_links::LinkCollector::resolve_link
  12: <rustdoc::passes::collect_intra_doc_links::LinkCollector as rustdoc::fold::DocFolder>::fold_item
  13: alloc::vec::source_iter_marker::<impl alloc::vec::spec_from_iter::SpecFromIter<T,I> for alloc::vec::Vec<T>>::from_iter
  14: rustdoc::fold::DocFolder::fold_inner_recur
  15: rustdoc::fold::DocFolder::fold_item_recur
  16: <rustdoc::passes::collect_intra_doc_links::LinkCollector as rustdoc::fold::DocFolder>::fold_item
  17: rustdoc::passes::collect_intra_doc_links::collect_intra_doc_links
  18: rustdoc::core::run_global_ctxt
  19: rustc_interface::passes::QueryContext::enter
  20: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
  21: rustc_span::with_source_map
  22: rustc_interface::interface::create_compiler_and_run
  23: rustdoc::main_options
  24: scoped_tls::ScopedKey<T>::set

@XAMPPRocky XAMPPRocky added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Apr 30, 2021
@jyn514
Copy link
Member

jyn514 commented Apr 30, 2021

@XAMPPRocky this is probably related to #83761 somehow. I don't plan to investigate any of the proximate issues until the underlying problem is fixed. Feel free to take a look yourself if this is blocking your work, but it won't be easy.

@jyn514 jyn514 added A-resolve Area: Name resolution T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 30, 2021
@jyn514
Copy link
Member

jyn514 commented Apr 30, 2021

Oh, I guess this could also be a missed case from #83738. That should be easier to fix.

@jyn514
Copy link
Member

jyn514 commented Apr 30, 2021

If you can come up with an MCVE it would be easier to tell which of the two issues it is.

@jyn514 jyn514 added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Apr 30, 2021
@mautamu
Copy link
Contributor

mautamu commented May 3, 2021

Not exactly an MCVE, but I think I'm able to reproduce this with only a handful of files @jyn514. The line
thread 'rustc' panicked at 'index out of bounds: the len is 8 but the index is 8', compiler/rustc_metadata/src/creader.rs:137:21
does change slightly though:
thread 'rustc' panicked at 'index out of bounds: the len is 20 but the index is 20', compiler/rustc_metadata/src/creader.rs:137:21

https://github.com/mautamu/spirv-std-3

(Same rustc version output as above).

Hope this helps :).

Best,
Mautamu

@jyn514
Copy link
Member

jyn514 commented May 3, 2021

DEBUG rustdoc::passes::collect_intra_doc_links combined_docs=[ImageFormat]: spirv_types::image_params::ImageFormat
DEBUG rustdoc::passes::collect_intra_doc_links resolving spirv_types::image_params::ImageFormat as a macro in the module DefId(19:0 ~ spirv_std_macros[a0e2])

I think the issue is that the crate is first used in a dependency, so it isn't loaded in

if let Some(doc) = attr.doc_str() {
for link in markdown_links(&doc.as_str()) {

Maybe a fix is to recursively traverse the docs of re-exports? That sounds kind of expensive, but the only other option (besides fixing #83761) is to revert and re-open #68427.

@jyn514 jyn514 added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label May 3, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 3, 2021
@jyn514
Copy link
Member

jyn514 commented May 3, 2021

An easy workaround is to link to spirv_types somewhere in the crate re-exporting the docs, even something like this would work:

diff --git a/src/lib.rs b/src/lib.rs
index d4de24e..81c278f 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1 +1,5 @@
 pub use spirv_std_macros::Image;
+
+#[doc(hidden)]
+/// [spirv_types]
+pub fn workaround_rustdoc_ice_84738() {}

@jyn514 jyn514 removed the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label May 3, 2021
@jyn514
Copy link
Member

jyn514 commented May 3, 2021

Hmm, this setup didn't reproduce the issue, not sure why:

inner/src/lib.rs:
/// Some docs from original crate
pub fn bar() {}
outer/src/lib.rs:
/// [inner::bar]
pub fn foo() {}
re_export/src/lib.rs:
pub use outer::foo;

@jyn514 jyn514 added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label May 3, 2021
@jyn514
Copy link
Member

jyn514 commented May 3, 2021

Maybe the proc macro is related somehow?

@hameerabbasi
Copy link
Contributor


Regression in 640ce99


searched nightlies: from nightly-2021-01-01 to nightly-2021-04-24
regressed nightly: nightly-2021-04-04
searched commits: from 138fd56 to 0b417ab
regressed commit: 640ce99

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --test-dir=. --start=2021-01-01 --script=./script.sh

That's #83738, good guess, @jyn514.

@pietroalbini pietroalbini added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels May 4, 2021
@pietroalbini pietroalbini added this to the 1.53.0 milestone May 4, 2021
@pietroalbini
Copy link
Member

This regression now slipped into beta 1.53.0.

@jyn514
Copy link
Member

jyn514 commented May 30, 2021

@Aaron1011 came up with a slightly smaller reproduction, the proc macro wasn't related: https://github.com/Aaron1011/spirv-std-3/tree/no-macro

@TheBlueMatt
Copy link

We've also run into this recently with a rather trivial crate https://docs.rs/crate/lightning-background-processor/0.0.98

@jyn514
Copy link
Member

jyn514 commented Oct 25, 2021

#90283 reports the same issue in an open source codebase: penumbra-zone/penumbra@1bf5d1b

@drewcrawford
Copy link
Contributor

Also seeing this in another project in 1.56 stable. Would put together a reproducer but there already is one.

@jyn514
Copy link
Member

jyn514 commented Oct 30, 2021

@drewcrawford If you could minimize the crash that would be helpful, the one at the top was already fixed by #88215

@drewcrawford
Copy link
Contributor

I've reduced it to https://github.com/drewcrawford/rustc-84738. Still seems a bit large but having trouble getting it any smaller.

This reproducer involves rust-lang/cargo#6313. I suspect that is the root cause in this case, not sure if that tracks with the other reports.

@jyn514
Copy link
Member

jyn514 commented Oct 30, 2021

@drewcrawford how can I build that package? running cargo doc gives me

warning: gcc: error: src/hard-exception.m: Objective-C compiler not installed on this system

error: failed to run custom build command for `objr v0.1.0 (https://github.com/drewcrawford/objr#84d444b4)`

even though I have gcc installed.

@drewcrawford
Copy link
Contributor

The dependency is macOS only. GitHub runners can do it.

There is probably a way to shrink the dependency enough to build on some other platform, can look into it a bit more

@drewcrawford
Copy link
Contributor

@jyn514 Updated to ICE on windows, which I suspect is enough for linux as well. May require cargo clean or cargo update

drewcrawford added a commit to drewcrawford/corevideor that referenced this issue Oct 30, 2021
@jyn514
Copy link
Member

jyn514 commented Nov 1, 2021

Hmm, so something strange is going on here - I minimized @drewcrawford's example a little more, down to four local crates:

$ fd --maxdepth 1 .
README.md
REPRODUCE.sh
coregraphicsr
corevideor
objr
objr-upstream

If objr-upstream has the contents of https://github.com/drewcrawford/objr/tree/winbuild, then the issue reproduces correctly, but if it's anything smaller, then coregraphicsr documents fine. At first I suspected the crate hash was different or something, but cargo is passing different -C metadata hashes even when objr and objr-upstream are identical, so that's not it.

@jyn514
Copy link
Member

jyn514 commented Nov 1, 2021

@drewcrawford FWIW you can work around the ICE with this diff:

diff --git a/src/objcinstance.rs b/src/objcinstance.rs
index f876cd5..1264f19 100644
--- a/src/objcinstance.rs
+++ b/src/objcinstance.rs
@@ -126,7 +126,7 @@ pub trait ObjcInstanceBehavior {
     ///Safely casts the object to an `Option<NonNullImmutable>`.  Suitable for implementing nullable functions.
     fn nullable(ptr: *const Self) -> Option<NonNullImmutable<Self>>;
 
-    ///Allows you to call [objr::bindings::PerformsSelector::perform] from a nonmutating context.
+    ///Allows you to call [crate::bindings::PerformsSelector::perform] from a nonmutating context.
     ///
     /// This function should not be used for general-purpose pointer casting.
     ///

@drewcrawford
Copy link
Contributor

Really interesting workaround! Those ought to be the same due to #56409, maybe there’s some interaction there?

@camelid camelid added P-high High priority P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. P-high High priority labels Nov 2, 2021
@jyn514
Copy link
Member

jyn514 commented Nov 7, 2021

@drewcrawford see #83761

jyn514 added a commit to jyn514/rust that referenced this issue Nov 8, 2021
This helps with (but does not fix)
rust-lang#84738. I tested on
jyn514/objr@edcee7b
and still hit ICEs.
@jyn514
Copy link
Member

jyn514 commented Nov 8, 2021

MCVE: #90489 (comment)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 11, 2021
…trochenkov

rustdoc: Go back to loading all external crates unconditionally

This *continues* to cause regressions. This code will be unnecessary
once access to the resolver happens fully before creating the tyctxt
(rust-lang#83761), so load all crates unconditionally for now. To minimize churn, this leaves in the code for loading crates selectively.

"Fixes" rust-lang#84738. Previously: rust-lang#83738, rust-lang#85749, rust-lang#88215

r? `@petrochenkov` cc `@camelid` (this should fix the "index out of bounds" error you had while looking up `crate_name`).
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 11, 2021
…ochenkov

rustdoc: Go back to loading all external crates unconditionally

This *continues* to cause regressions. This code will be unnecessary
once access to the resolver happens fully before creating the tyctxt
(rust-lang#83761), so load all crates unconditionally for now. To minimize churn, this leaves in the code for loading crates selectively.

"Fixes" rust-lang#84738. Previously: rust-lang#83738, rust-lang#85749, rust-lang#88215

r? `@petrochenkov` cc `@camelid` (this should fix the "index out of bounds" error you had while looking up `crate_name`).
cuviper pushed a commit to cuviper/rust that referenced this issue Nov 16, 2021
This helps with (but does not fix)
rust-lang#84738. I tested on
jyn514/objr@edcee7b
and still hit ICEs.

(cherry picked from commit cdafe99)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolve Area: Name resolution C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-critical Critical priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet