rust-lang / rust

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

ICE with non-ascii byte string in #[path] attribute #81208

Closed Nemo157 closed 3 years ago

Nemo157 commented 3 years ago

Code

#[path = b"ffi.rs"]
mod ffi;

fn main() {
}

Meta

rustc 1.51.0-nightly (c5a96fb79 2021-01-19)
binary: rustc
commit-hash: c5a96fb7973649807a7943e7395456db158dcab6
commit-date: 2021-01-19
host: x86_64-unknown-linux-gnu
release: 1.51.0-nightly
LLVM version: 11.0.1

Also tested on rustc 1.49.0 (e1884a8e3 2020-12-29) and rustc 1.50.0-beta.6 (ea20aa255 2021-01-14), it errored without ICEing on 1.49 and ICEd on 1.50 so

@rustbot modify labels: +regression-from-stable-to-beta

Error output

   Compiling foo v0.1.0 (/tmp/tmp.BmkigLzpQI/foo)
error: byte constant must be ASCII. Use a \xHH escape for a non-ASCII byte
 --> src/main.rs:1:12
  |
1 | #[path = b"ffi.rs"]
  |            ^

error[E0583]: file not found for module `ffi`
 --> src/main.rs:2:1
  |
2 | mod ffi;
  | ^^^^^^^^
  |
  = help: to create the module `ffi`, create file "src/ffi.rs"

error: malformed `path` attribute input
 --> src/main.rs:1:1
  |
1 | #[path = b"ffi.rs"]
  | ^^^^^^^^^^^^^^^^^ help: must be of the form: `#[path = "file"]`

thread 'rustc' panicked at 'assertion failed: bpos.to_u32() >= mbc.pos.to_u32() + mbc.bytes as u32', compiler/rustc_span/src/lib.rs:1511:17

error: internal compiler error: unexpected panic

note: rustc 1.51.0-nightly (c5a96fb79 2021-01-19) running on x86_64-unknown-linux-gnu

note: compiler flags: -C embed-bitcode=no -C debuginfo=2 -C incremental -C link-arg=-fuse-ld=lld -C target-cpu=native --crate-type bin

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
end of query stack
error: aborting due to 3 previous errors
Backtrace

``` 0: rust_begin_unwind at /rustc/c5a96fb7973649807a7943e7395456db158dcab6/library/std/src/panicking.rs:493:5 1: core::panicking::panic_fmt at /rustc/c5a96fb7973649807a7943e7395456db158dcab6/library/core/src/panicking.rs:92:14 2: core::panicking::panic at /rustc/c5a96fb7973649807a7943e7395456db158dcab6/library/core/src/panicking.rs:50:5 3: rustc_span::SourceFile::lookup_file_pos 4: rustc_span::SourceFile::lookup_file_pos_with_col_display 5: rustc_span::source_map::SourceMap::lookup_char_pos 6: rustc_errors::emitter::EmitterWriter::get_multispan_max_line_num 7: ::emit_diagnostic 8: rustc_errors::json::Diagnostic::from_errors_diagnostic 9: ::emit_diagnostic 10: rustc_errors::HandlerInner::emit_diagnostic 11: rustc_errors::Handler::emit_diag_at_span 12: rustc_errors::Handler::span_err 13: rustc_parse::lexer::unescape_error_reporting::emit_unescape_error 14: rustc_lexer::unescape::unescape_literal 15: rustc_parse::lexer::StringReader::next_token 16: rustc_parse::lexer::tokentrees::TokenTreesReader::parse_all_token_trees 17: rustc_parse::lexer::parse_token_trees 18: rustc_parse::maybe_file_to_stream 19: rustc_parse::source_file_to_stream 20: rustc_parse::parse_stream_from_source_str 21: rustc_parse::fake_token_stream 22: rustc_parse::nt_to_tokenstream 23: rustc_ast_lowering::TokenStreamLowering::lower_token 24: rustc_ast_lowering::LoweringContext::lower_mac_args 25: rustc_ast_lowering::LoweringContext::lower_attr 26: as core::iter::traits::collect::Extend<::Item>>::extend 27: rustc_ast_lowering::LoweringContext::lower_attrs 28: rustc_ast_lowering::item::::lower_item 29: rustc_ast_lowering::LoweringContext::with_hir_id_owner 30: ::visit_mod 31: rustc_ast_lowering::lower_crate 32: rustc_interface::passes::BoxedResolver::access::{{closure}} 33: rustc_interface::passes::configure_and_expand::{{closure}} 34: rustc_interface::passes::BoxedResolver::access 35: rustc_interface::queries::Queries::lower_to_hir 36: rustc_interface::queries::Queries::global_ctxt 37: rustc_interface::queries::::enter 38: rustc_span::with_source_map 39: rustc_interface::interface::create_compiler_and_run 40: rustc_span::with_session_globals ```

apiraino commented 3 years ago

Assigning P-medium as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

tesuji commented 3 years ago

searched nightlies: from nightly-2020-12-01 to nightly-2021-01-21 regressed nightly: nightly-2020-12-11 searched commits: from f0f68778f798d6d34649745b41770829b17ba5b8...d32c320d7eee56706486fef6be778495303afe9e regressed commit: 58d2bad9f7ab0971495247b6c94978848760ca9d

It's #78837 cc @petrochenkov

bisected with cargo-bisect-rustc v0.5.2 Host triple: x86_64-unknown-linux-gnu Reproduce with: ```bash cargo bisect-rustc 2020-12-1 --end 2021-01-21 --preserve --regress=ice -- check ```
camelid commented 3 years ago

@rustbot glacier "https://gist.github.com/rust-play/47376b89a715489824408a48804d68f4"

estebank commented 3 years ago

I spent some time looking at this.

  1. This will not ICE in stable because the assert should be compiled away. What will happen is an incorrect span in the output duplicating a prior error.
    error: non-ASCII in byte constant
    --> f4.rs:1:11
    |
    1 | #[path = b"ffi.rs"]
    |           ^
    |
    = help: use a \xHH escape for a non-ASCII byte
  2. I think this is caused by the double duty that Spans and BytePos have. We sometimes treat BytePos as indices into a byte buffer, but there's an additional piece of info in Spans that isn't carried over with BytePos, namely whether the byte position is the beginning or the end of a Span. This matters because when doing operations being or not being the end is the difference between having to do + BytePos(1) or not. I believe this is the underlying cause of these issues. @wesleywiser if you're taking on this you might want to look at modifying lookup_file_pos to accept a new enum stating whether the BytePos is the start or end (and depending on that whether we should +1 the assertion and likely other logic or not).
  3. There are a bunch of small tweaks to these diagnostics that can be done as a separate PR by changing span_err with handler.struct_span_err(span, "non-ASCII in byte constant").span_label(span, "byte constant must be ASCII").help("use a \\xHH escape for a non-ASCII byte").emit(); and even provide a structured suggestion instead of the help.
camelid commented 3 years ago
  1. This will not ICE in stable because the assert should be compiled away.

Why would the assert be compiled away in stable but not beta and nightly? I'm pretty sure the issue here is that the PR that causes this ICE was merged for 1.50 (which is the current beta version), not 1.49 (which is the current stable).

estebank commented 3 years ago

Am I wrong to understand that debug-assertions in config.toml controls whether assertions get evaluated, which in turn is only enabled if debug = true?

Regardless, a small patch to remove that assertion would get around the ICE without major negative impact (the Span label is shifted one char to the left and nothing else), if we don't cut a more comprehensive PR before the next release.

camelid commented 3 years ago

Am I wrong to understand that debug-assertions in config.toml controls whether assertions get evaluated, which in turn is only enabled if debug = true?

I'm not sure if that gets rid of assert! calls (it may only get rid of debug_assert! calls). However, what I'm referring to is that you said it wouldn't ICE in stable, whereas my understanding is that if it ICEs in beta, it will ICE in the next stable.

estebank commented 3 years ago

@wesleywiser taking over, I've figured out the problem.

wesleywiser commented 3 years ago

Re-opening because this still affects beta and the PR needs to be backported.

pietroalbini commented 3 years ago

The fix landed in 1.51.0, and I cherry-picked a fix for the 1.50.0 stable branch. If the cherry-pick is accepted by CI then we can close this issue!

pietroalbini commented 3 years ago

CI passed on the test runner in the 1.50.0 promotion, but it timed out on an unrelated builder, so the fix worked! Closing this.