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

[cargo doc ICE] SBC-Case parentheses result in ICE #55723

Closed
ZhangHanDong opened this issue Nov 6, 2018 · 14 comments · Fixed by #55962
Closed

[cargo doc ICE] SBC-Case parentheses result in ICE #55723

ZhangHanDong opened this issue Nov 6, 2018 · 14 comments · Fixed by #55962
Assignees
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name A-Unicode Area: Unicode 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

@ZhangHanDong
Copy link

ZhangHanDong commented Nov 6, 2018

///  this is sbc-case parentheses ->(sum )
fn test(){}

ICE happened when run cargo doc:

thread '<unnamed>' panicked at 'assertion failed: bpos.to_u32() >= mbc.pos.to_u32() + mbc.bytes as u32', libsyntax/source_map.rs:842:17
note: Run with `RUST_BACKTRACE=1` for a backtrace.

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.32.0-nightly (8b096314a 2018-11-02) running on x86_64-apple-darwin

error: Could not document `xxx`.

Caused by:
  process didn't exit successfully: `rustdoc --edition=2018 --crate-name interview_500 src/lib.rs --color always -o /path/target/doc -L dependency=/path/target/debug/deps` (exit code: 1)
@Aaron1011 Aaron1011 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 7, 2018
@GuillaumeGomez GuillaumeGomez self-assigned this Nov 8, 2018
@GuillaumeGomez
Copy link
Member

Works fine for me. Anything else besides the given information?

@QuietMisdreavus
Copy link
Member

I can't seem to reproduce that ICE with that nightly. Are there any other details about your project that you can tell us about? The panic itself seems to originate in libsyntax, in a function about tracking multi-byte characters in source files:

// We should never see a byte position in the middle of a
// character
assert!(bpos.to_u32() >= mbc.pos.to_u32() + mbc.bytes as u32);

@ZhangHanDong
Copy link
Author

@GuillaumeGomez @QuietMisdreavus

try this:

/// (sum - arr[i])

@GuillaumeGomez
Copy link
Member

Still working fine.

@ZhangHanDong
Copy link
Author

@GuillaumeGomez

you can try this github reps: test_ice

@ZhangHanDong
Copy link
Author

ZhangHanDong commented Nov 10, 2018

@GuillaumeGomez

I think the bug is caused by that lookup_source_file_idx looked for the wrong location in bytepos_to_file_charpos method.

@GuillaumeGomez
Copy link
Member

Ah nice, thanks!

@QuietMisdreavus QuietMisdreavus added A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name A-Unicode Area: Unicode labels Nov 14, 2018
@QuietMisdreavus
Copy link
Member

Log with backtrace from a local rustdoc build:

Full backtrace
$ RUST_BACKTRACE=1 cargo +other2 doc
thread '<unnamed>' panicked at 'assertion failed: bpos.to_u32() >= mbc.pos.to_u32() + mbc.bytes as u32', libsyntax/source_map.rs:842:17
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:476
   5: std::panicking::begin_panic
             at /home/misdreavus/git/rust-builds/src/libstd/panicking.rs:410
   6: syntax::source_map::SourceMap::bytepos_to_file_charpos
             at libsyntax/source_map.rs:842
   7: syntax::source_map::SourceMap::lookup_char_pos
             at libsyntax/source_map.rs:326
   8: <syntax::source_map::SourceMap as rustc_errors::SourceMapper>::lookup_char_pos
             at libsyntax/source_map.rs:955
   9: rustc_errors::emitter::EmitterWriter::get_multispan_max_line_num                                                                                              at librustc_errors/emitter.rs:736
  10: <rustc_errors::emitter::EmitterWriter as rustc_errors::emitter::Emitter>::emit
             at librustc_errors/emitter.rs:759                                                                                                 [80/136]
             at librustc_errors/emitter.rs:1330
             at librustc_errors/emitter.rs:81
  11: rustc_errors::Handler::emit_db
             at librustc_errors/lib.rs:722
  12: rustc_errors::diagnostic_builder::DiagnosticBuilder::emit
             at librustc_errors/diagnostic_builder.rs:98
  13: rustdoc::passes::collect_intra_doc_links::resolution_failure
             at librustdoc/passes/collect_intra_doc_links.rs:573
  14: <rustdoc::passes::collect_intra_doc_links::LinkCollector<'a, 'tcx, 'rcx, 'cstore> as rustdoc::fold::DocFolder>::fold_item
             at librustdoc/passes/collect_intra_doc_links.rs:432
  15: <alloc::vec::Vec<T> as alloc::vec::SpecExtend<T, I>>::from_iter
             at librustdoc/fold.rs:110
             at /home/misdreavus/git/rust-builds/src/libcore/iter/mod.rs:1640
             at /home/misdreavus/git/rust-builds/src/liballoc/vec.rs:1788
  16: rustdoc::fold::DocFolder::fold_inner_recur
             at /home/misdreavus/git/rust-builds/src/liballoc/vec.rs:1700
             at /home/misdreavus/git/rust-builds/src/libcore/iter/iterator.rs:1476
             at librustdoc/fold.rs:110
             at librustdoc/fold.rs:37
  17: rustdoc::fold::DocFolder::fold_item_recur
             at librustdoc/fold.rs:100
  18: <rustdoc::passes::collect_intra_doc_links::LinkCollector<'a, 'tcx, 'rcx, 'cstore> as rustdoc::fold::DocFolder>::fold_item
             at librustdoc/passes/collect_intra_doc_links.rs:461
  19: rustdoc::passes::collect_intra_doc_links::collect_intra_doc_links
             at librustdoc/fold.rs:115
             at /home/misdreavus/git/rust-builds/src/libcore/option.rs:632
             at librustdoc/fold.rs:115
             at librustdoc/passes/collect_intra_doc_links.rs:41
  20: rustdoc::core::run_core::{{closure}}::{{closure}}
             at librustdoc/core.rs:613
  21: rustc::ty::context::tls::enter_global
             at /home/misdreavus/git/rust-builds/src/librustc_driver/driver.rs:1355
             at /home/misdreavus/git/rust-builds/src/librustc/ty/context.rs:2082
             at /home/misdreavus/git/rust-builds/src/librustc/ty/context.rs:2050
             at /home/misdreavus/git/rust-builds/src/librustc/ty/context.rs:1989
             at /home/misdreavus/git/rust-builds/src/librustc/ty/context.rs:2049
             at /home/misdreavus/git/rust-builds/src/librustc/ty/context.rs:2081                                                                                    at /home/misdreavus/git/rust-builds/src/librustc/ty/context.rs:2039
             at /home/misdreavus/git/rust-builds/src/libstd/thread/local.rs:309
             at /home/misdreavus/git/rust-builds/src/libstd/thread/local.rs:255                                                                [40/136]
             at /home/misdreavus/git/rust-builds/src/librustc/ty/context.rs:2031
             at /home/misdreavus/git/rust-builds/src/libstd/thread/local.rs:309
             at /home/misdreavus/git/rust-builds/src/libstd/thread/local.rs:255
             at /home/misdreavus/git/rust-builds/src/librustc/ty/context.rs:2023
             at /home/misdreavus/git/rust-builds/src/librustc/ty/context.rs:2061
  22: rustc::ty::context::TyCtxt::create_and_enter
             at /home/misdreavus/git/rust-builds/src/librustc/ty/context.rs:1247
  23: rustdoc::core::run_core::{{closure}}
             at /home/misdreavus/git/rust-builds/src/librustc_driver/driver.rs:1263
             at librustdoc/core.rs:495
  24: syntax::with_globals
             at /home/misdreavus/git/rust-builds/src/librustc_driver/driver.rs:76
             at /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-0.1.2/src/lib.rs:155
             at /home/misdreavus/git/rust-builds/src/librustc_driver/driver.rs:75
             at librustdoc/core.rs:402
             at librustdoc/lib.rs:413
             at /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-0.1.2/src/lib.rs:155
             at /home/misdreavus/git/rust-builds/src/libsyntax/lib.rs:123
             at /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-0.1.2/src/lib.rs:155
             at /home/misdreavus/git/rust-builds/src/libsyntax/lib.rs:122
  25: std::panicking::try::do_call
             at librustdoc/lib.rs:410
             at /home/misdreavus/git/rust-builds/src/librustc_driver/lib.rs:1640
             at /home/misdreavus/git/rust-builds/src/libstd/panic.rs:319
             at /home/misdreavus/git/rust-builds/src/libstd/panicking.rs:310
  26: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:102
  27: std::panicking::try
             at /home/misdreavus/git/rust-builds/src/libstd/panicking.rs:289
  28: rustc_driver::monitor
             at /home/misdreavus/git/rust-builds/src/libstd/panic.rs:398
             at /home/misdreavus/git/rust-builds/src/librustc_driver/lib.rs:1554
             at /home/misdreavus/git/rust-builds/src/librustc_driver/lib.rs:1565
             at /home/misdreavus/git/rust-builds/src/librustc_driver/lib.rs:1639
  29: rustdoc::main_args
             at librustdoc/lib.rs:410
             at librustdoc/lib.rs:385                                                                                                                    30: syntax::with_globals
             at librustdoc/lib.rs:108
             at /home/misdreavus/git/rust-builds/src/libcore/option.rs:424                                                                      [0/136]
             at librustdoc/lib.rs:108
             at /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-0.1.2/src/lib.rs:155
             at /home/misdreavus/git/rust-builds/src/libsyntax/lib.rs:123
             at /home/misdreavus/.cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-0.1.2/src/lib.rs:155
             at /home/misdreavus/git/rust-builds/src/libsyntax/lib.rs:122
  31: std::panicking::try::do_call
             at /home/misdreavus/git/rust-builds/src/libstd/thread/mod.rs:477
             at /home/misdreavus/git/rust-builds/src/libstd/panic.rs:319
             at /home/misdreavus/git/rust-builds/src/libstd/panicking.rs:310
  32: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:102
  33: std::panicking::try
             at /home/misdreavus/git/rust-builds/src/libstd/panicking.rs:289
  34: <F as alloc::boxed::FnBox<A>>::call_box
             at /home/misdreavus/git/rust-builds/src/libstd/panic.rs:398
             at /home/misdreavus/git/rust-builds/src/libstd/thread/mod.rs:476
             at /home/misdreavus/git/rust-builds/src/liballoc/boxed.rs:672
  35: std::sys_common::thread::start_thread
             at /home/misdreavus/git/rust-builds/src/liballoc/boxed.rs:682
             at libstd/sys_common/thread.rs:24
  36: std::sys::unix::thread::Thread::new::thread_start
             at libstd/sys/unix/thread.rs:90
  37: start_thread
  38: clone

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.32.0-dev running on x86_64-unknown-linux-gnu

error: Could not document `test_ice`.

Caused by:
  process didn't exit successfully: `rustdoc --edition=2018 --crate-name test_ice src/lib.rs --color always -o /home/misdreavus/clones/test_ice/target/doc -L dependency=/home/misdreavus/clones/test_ice/target/debug/deps` (exit code: 1)

Based on that backtrace, it looks like it's a problem with the reporting of intra-doc link resolution errors, and the span we're calculating from the link reference. These spans are constructed from byte ranges we calculate in html::markdown::markdown_links, when scanning markdown for link references:

let locate = |s: &str| unsafe {
let s_start = s.as_ptr();
let s_end = s_start.add(s.len());
let md_start = md.as_ptr();
let md_end = md_start.add(md.len());
if md_start <= s_start && s_end <= md_end {
let start = s_start.offset_from(md_start) as usize;
let end = s_end.offset_from(md_start) as usize;
Some(start..end)
} else {
None
}
};

Oddly enough, vim seems to re-encode those parentheses in a way that makes the problem go away, so writing this into a test will be exceedingly difficult. This may explain why i couldn't reproduce the issue, because when i copy/pasted the sample into vim, it no longer had the encoding problem.

An immediate work-around for you is to backslash-escape the square brackets on that line, to stop the markdown parser from thinking it's a link.

@QuietMisdreavus
Copy link
Member

Oh hang on, i think i've found what's going on. Vim only led me astray because of a plugin i had that caused me to change the test and "solve" the issue.

If you look closely at the reproduction...

/// ## For example:
///  
/// (arr[i])
pub fn test_ice() {
    unimplemented!();
}

The "blank" line in the middle is actually hiding something important: trailing whitespace. This causes some code later on to fail, that tries to convert the byte range given in the sample i linked into a byte range representative of the code:

let line_offset = dox[..link_range.start].lines().count();
// The span starts in the `///`, so we don't have to account for the leading whitespace
let code_dox_len = if line_offset <= 1 {
doc_comment_padding
} else {
// The first `///`
doc_comment_padding +
// Each subsequent leading whitespace and `///`
code_dox.lines().skip(1).take(line_offset - 1).fold(0, |sum, line| {
sum + doc_comment_padding + line.len() - line.trim().len()
})
};
// Extract the specific span
let sp = sp.from_inner_byte_pos(
link_range.start + code_dox_len,
link_range.end + code_dox_len,
);

That sum + doc_comment_padding + line.len() - line.trim().len() is likely the culprit. (I'm waiting on a build right now, but i'll test this hunch once it's available.) I bet that calculation is incorrectly trimming off that trailing whitespace, which is creating a offset that is causing the index of the span to be incorrectly offset from its actual position... straight into the middle of a multibyte codepoint. Change that line.trim().len() to line.trim_start().len() and the issue will probably disappear.

Another workaround (which will make the error properly appear) is to trim any trailing whitespace in your file, which will cause this calculation use the correct range for the text.

@QuietMisdreavus
Copy link
Member

I've opened #55962 to fix this issue. There are still problems with this calculation, but it should fix your situation, at least.

pietroalbini added a commit to pietroalbini/rust that referenced this issue Nov 16, 2018
…llaumeGomez

rustdoc: properly calculate spans for intra-doc link resolution errors

Fixes rust-lang#55723

When rustdoc is reporting a resolution error for intra-doc links, it needs to convert a span from one relative to the *markdown* (as the links are only found on the final markdown text) to one relative to the *source code* (as the error reporting is meant to show where the line is in the source, so the user can fix it). However, a calculation for how much "offset" to apply had a subtle error: it trimmed the whole line when attempting to account for leading indentation. This caused it to add in *trailing* whitespace into this calculation, which created an incorrect span.

In a lot of situations, this isn't a problem - the span will be shifted in the code slightly, but the warning will still be displayed and mostly legible. However, there is one important situation where this can cause an ICE: multi-byte codepoints. If a shifted span now has a starting point in the middle of a multi-byte codepoint, libsyntax will panic when trying to track what source item it corresponds to. This flew under our radar because trailing whitespace and multi-byte codepoints are both situations that we don't run into in the compiler repo.

(There is one more situation where this can error, that will be much harder to fix: block-style doc comments. Lines in a block-style doc comment have a zero-or-more (usually one) character offset per line, causing this calculation to be way off. I'm punting that to another issue, though...)
kennytm added a commit to kennytm/rust that referenced this issue Nov 17, 2018
…llaumeGomez

rustdoc: properly calculate spans for intra-doc link resolution errors

Fixes rust-lang#55723

When rustdoc is reporting a resolution error for intra-doc links, it needs to convert a span from one relative to the *markdown* (as the links are only found on the final markdown text) to one relative to the *source code* (as the error reporting is meant to show where the line is in the source, so the user can fix it). However, a calculation for how much "offset" to apply had a subtle error: it trimmed the whole line when attempting to account for leading indentation. This caused it to add in *trailing* whitespace into this calculation, which created an incorrect span.

In a lot of situations, this isn't a problem - the span will be shifted in the code slightly, but the warning will still be displayed and mostly legible. However, there is one important situation where this can cause an ICE: multi-byte codepoints. If a shifted span now has a starting point in the middle of a multi-byte codepoint, libsyntax will panic when trying to track what source item it corresponds to. This flew under our radar because trailing whitespace and multi-byte codepoints are both situations that we don't run into in the compiler repo.

(There is one more situation where this can error, that will be much harder to fix: block-style doc comments. Lines in a block-style doc comment have a zero-or-more (usually one) character offset per line, causing this calculation to be way off. I'm punting that to another issue, though...)
pietroalbini added a commit to pietroalbini/rust that referenced this issue Nov 18, 2018
…llaumeGomez

rustdoc: properly calculate spans for intra-doc link resolution errors

Fixes rust-lang#55723

When rustdoc is reporting a resolution error for intra-doc links, it needs to convert a span from one relative to the *markdown* (as the links are only found on the final markdown text) to one relative to the *source code* (as the error reporting is meant to show where the line is in the source, so the user can fix it). However, a calculation for how much "offset" to apply had a subtle error: it trimmed the whole line when attempting to account for leading indentation. This caused it to add in *trailing* whitespace into this calculation, which created an incorrect span.

In a lot of situations, this isn't a problem - the span will be shifted in the code slightly, but the warning will still be displayed and mostly legible. However, there is one important situation where this can cause an ICE: multi-byte codepoints. If a shifted span now has a starting point in the middle of a multi-byte codepoint, libsyntax will panic when trying to track what source item it corresponds to. This flew under our radar because trailing whitespace and multi-byte codepoints are both situations that we don't run into in the compiler repo.

(There is one more situation where this can error, that will be much harder to fix: block-style doc comments. Lines in a block-style doc comment have a zero-or-more (usually one) character offset per line, causing this calculation to be way off. I'm punting that to another issue, though...)
kennytm pushed a commit to pietroalbini/rust that referenced this issue Nov 19, 2018
…llaumeGomez

rustdoc: properly calculate spans for intra-doc link resolution errors

Fixes rust-lang#55723

When rustdoc is reporting a resolution error for intra-doc links, it needs to convert a span from one relative to the *markdown* (as the links are only found on the final markdown text) to one relative to the *source code* (as the error reporting is meant to show where the line is in the source, so the user can fix it). However, a calculation for how much "offset" to apply had a subtle error: it trimmed the whole line when attempting to account for leading indentation. This caused it to add in *trailing* whitespace into this calculation, which created an incorrect span.

In a lot of situations, this isn't a problem - the span will be shifted in the code slightly, but the warning will still be displayed and mostly legible. However, there is one important situation where this can cause an ICE: multi-byte codepoints. If a shifted span now has a starting point in the middle of a multi-byte codepoint, libsyntax will panic when trying to track what source item it corresponds to. This flew under our radar because trailing whitespace and multi-byte codepoints are both situations that we don't run into in the compiler repo.

(There is one more situation where this can error, that will be much harder to fix: block-style doc comments. Lines in a block-style doc comment have a zero-or-more (usually one) character offset per line, causing this calculation to be way off. I'm punting that to another issue, though...)
@ZhangHanDong
Copy link
Author

ZhangHanDong commented Dec 2, 2018

@QuietMisdreavus

Ask a question:

 // The span starts in the `///`, so we don't have to account for the leading whitespace
            let code_dox_len = if line_offset <= 1 {
                doc_comment_padding
            } else {
                // The first `///`
                doc_comment_padding +
                    // Each subsequent leading whitespace and `///`
                    code_dox.lines().skip(1).take(line_offset - 1).fold(0, |sum, line| {
                        sum + doc_comment_padding + line.len() - line.trim_start().len()
                    })
            };

I don't understand why not write directly like this:

let code_dox_len =  line_offset * doc_comment_padding;

@ZhangHanDong
Copy link
Author

@QuietMisdreavus

ok, I see,

/// ## For example:
   ///  
///   (arr[i])

In order to calculate the preceding whitespaces. But rustfmt can delete the whitespace.

@QuietMisdreavus
Copy link
Member

Right, the extra calculation is meant to deal with the indentation level, since we're dealing with the plain-text source code there. Also, if line_offset is zero (i haven't read that code in a while, so i'm not sure if it's supposed to be) then that can also lead to an incorrect calculation with your version.

But rustfmt can delete the whitespace.

I'm not sure i understand the reference? Rustdoc can't assume the presence of other tools, and we can't force everyone to start using rustfmt to be able to generate docs.

@ZhangHanDong
Copy link
Author

@QuietMisdreavus Ok, I see. thanks for your reply.

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 A-Unicode Area: Unicode 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