rustfmt crashes with `ignore` on Windows

danakj commented 4 months ago

Using nightly rust:

PS C:\src\test> C:\src\c\src\third_party\rust-toolchain\bin\rustfmt.exe --version
rustfmt 1.7.0-dev (27eb5e46111 2024-05-14)

My directory layout:

PS C:\src\test> ls

    Directory: C:\src\test

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
d-----         5/30/2024  12:45 PM                foo
-a----         5/30/2024  12:45 PM             22 .rustfmt.toml
-a----         5/30/2024  12:47 PM              0 ffi.rs

The foo directory is empty. The bug occurs with or without it being present. The ffi.rs file is empty, its contents are not relevant.

My .rustfmt.toml:

ignore = [
  "foo",
]

Panic when running rustfmt:

PS C:\src\test> C:\src\c\src\third_party\rust-toolchain\bin\rustfmt.exe --config-path=.rustfmt.toml ffi.rs
thread 'main' panicked at C:\b\s\w\ir\cache\builder\src\third_party\rust-src\cargo-home\registry\src\index.crates.io-6f17d22bba15001f\ignore-0.4.20\src\gitignore.rs:227:9:
path is expected to be under the root
stack backtrace:
   0:     0x7ffa0c14dd4b - std::backtrace_rs::backtrace::trace_unsynchronized::h5f93b2abd0db91f0
   1:     0x7ffa0c133607 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hce6ce78ac92839e4
   2:     0x7ffa0c165849 - core::fmt::write::ha5fb765460c424ba
   3:     0x7ffa0c13e1b2 - std::io::Write::write_fmt::habae0783318d8fea
   4:     0x7ffa0c1333b6 - std::sys_common::backtrace::lock::hb3e74337ee21512b
   5:     0x7ffa0c134a36 - std::panicking::default_hook::h7607b7ddd5c7b6ec
   6:     0x7ffa0c134695 - std::panicking::default_hook::h7607b7ddd5c7b6ec
   7:     0x7ffa032f19bd - RNvXsM_NtCs3lievmH1lCy_5alloc5boxedINtB5_3BoxNCNvCsb1rAIG9dYoS_17rustc_driver_impl16install_ice_hook0EINtNtNtCs5nNRtGYlUPI_4core3ops8function2FnTRDG0_IB1D_TRL1_INtNtNtB1J_5panic10panic_info9PanicInfoL0_EEEp6OutputuNtNtB1J_6marker4SendNtB3s_4SyncEL_RB2z_EE
   8:     0x7ffa0c135697 - std::panicking::rust_panic_with_hook::haf133a654d379e78
   9:     0x7ff6ebefe31e - <unknown>
  10:     0x7ff6ebefd9a9 - <unknown>
  11:     0x7ff6ec1c02dd - <unknown>
  12:     0x7ff6ebf15f50 - <unknown>
  13:     0x7ff6ebf131cd - <unknown>
  14:     0x7ff6ebe35662 - <unknown>
  15:     0x7ff6ebe0ada3 - <unknown>
  16:     0x7ff6ebe339c3 - <unknown>
  17:     0x7ff6ebe2c4b5 - <unknown>
  18:     0x7ff6ebe29b43 - <unknown>
  19:     0x7ff6ebe37585 - <unknown>
  20:     0x7ff6ebe1b7d3 - <unknown>
  21:     0x7ff6ebe1957c - <unknown>
  22:     0x7ff6ebe17046 - <unknown>
  23:     0x7ff6ebe3a046 - <unknown>
  24:     0x7ff6ebe36a8c - <unknown>
  25:     0x7ffa0c134dce - std::panicking::try::h55cdbfa07f843c24
  26:     0x7ffa0c145d1e - std::rt::lang_start_internal::ha0abe746fee78982
  27:     0x7ff6ebe1c6dc - <unknown>
  28:     0x7ff6ec1a51c0 - <unknown>
  29:     0x7ffa5c877344 - BaseThreadInitThunk
  30:     0x7ffa5d0626b1 - RtlUserThreadStart

error: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rustfmt/issues/new?labels=bug

note: please make sure that you have updated to the latest nightly

There is no such problem on Linux.

The assertion failing is here: https://github.com/BurntSushi/ripgrep/blob/fe97c0a152cabc1bc07ec36b4b1e27cd230c3014/crates/ignore/src/gitignore.rs#L227

Chromium bug for cross-linking: https://crbug.com/40248903

ytmimi commented 4 months ago

Thanks for the report. Linking the tracking issue for ignore #3395. You mentioned that you're using rustfmt 1.7.0-dev (27eb5e46111 2024-05-14). Any chance that a more recent nightly version fixes this issue?

danakj commented 4 months ago

It's possible and we can find out when the next roll gets through into Chromium (probably next week it looks like). I don't know that this is a new/recent issue FWIW, it's the first time I tried to verify formatting would work on Windows.

anforowicz commented 4 months ago

Below is a snippet of a Chromium-independent repro at the tip-of-the tree of rustfmt, with some extra dbg! in ignore_paths.rs. I think one thing worth noting is that ignore::gitignore::Gitignore::matched_path_or_any_parents requires that the given file path must be under the root path of the matcher. And the dbg! show that here the path is under the root, but only kind-of, sort-of:

Root: c:\test
Path: \\?\C:\test\bar.rs

In particular:

The drive in the root is spelled with c:\ rather than C:\
The child path uses the \\?\ prefix

I note that fn strip_prefix in the ignore crate uses exact string matching, so the 2 differences above mean that the child path is not considered to be under the root.

I think the paths should be normalized to remove the benign differences between the root vs child path syntax:

Normalized somehow - maybe via std::path::Path::canonicalize
Normalized somewhere - either in the ignore crate or in rustfmt.

c:\src\rustfmt>dir c:\test
 Volume in drive C has no label.
 Volume Serial Number is 90E8-8A5D

 Directory of c:\test

05/31/2024  02:32 PM    <DIR>          .
05/31/2024  02:32 PM    <DIR>          ..
05/31/2024  02:31 PM                 0 bar.rs
05/31/2024  02:31 PM                18 rustfmt.toml
               2 File(s)             18 bytes
               2 Dir(s)  866,253,344,768 bytes free

c:\src\rustfmt>type c:\test\rustfmt.toml
ignore = ["foo"]

c:\src\rustfmt>cargo run --bin rustfmt -- --config-path=c:\test\rustfmt.toml c:\test\bar.rs
    Finished dev [unoptimized + debuginfo] target(s) in 0.36s
     Running `target\debug\rustfmt.exe --config-path=c:\test\rustfmt.toml c:\test\bar.rs`
[src\ignore_path.rs:23:9] self.ignore_set.path() = "c:\\test"
[src\ignore_path.rs:27:17] &p = "\\\\?\\C:\\test\\bar.rs"
thread 'main' panicked at C:\Users\lukasza\.cargo\registry\src\index.crates.io-6f17d22bba15001f\ignore-0.4.18\src\gitignore.rs:227:9:
path is expected to be under the root
stack backtrace:
   0:     0x7ffbf0d32252 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::he7d1ca341df54a85
   1:     0x7ffbf0d64c4d - core::fmt::write::he1928d3c9fd3e726
   2:     0x7ffbf0d28d41 - <std::io::IoSlice as core::fmt::Debug>::fmt::h75106291429179c2
   3:     0x7ffbf0d3207a - std::sys_common::backtrace::lock::hcadabe22c39262a1
   4:     0x7ffbf0d354b9 - std::panicking::default_hook::h44204a891c2eb359
   5:     0x7ffbf0d35175 - std::panicking::default_hook::h44204a891c2eb359
   6:     0x7ffbe0bbda43 - <tracing_subscriber[d07e7bfc55775049]::fmt::format::Writer>::write_fmt
   7:     0x7ffbf0d35ad3 - std::panicking::rust_panic_with_hook::h1e10d5fa9efdead2
   8:     0x7ff6d9787608 - std::panicking::begin_panic::closure$0<ref$<str$> >
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\std\src\panicking.rs:687
   9:     0x7ff6d9787529 - core::hint::black_box
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\core\src\hint.rs:334
  10:     0x7ff6d9787529 - std::sys_common::backtrace::__rust_end_short_backtrace<std::panicking::begin_panic::closure_env$0<ref$<str$> >,never$>
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\std\src\sys_common\backtrace.rs:171
  11:     0x7ff6d978757b - std::panicking::begin_panic<ref$<str$> >
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\std\src\panicking.rs:686
  12:     0x7ff6d932f5fa - ignore::gitignore::Gitignore::matched_path_or_any_parents<ref$<std::path::PathBuf> >
                               at C:\Users\lukasza\.cargo\registry\src\index.crates.io-6f17d22bba15001f\ignore-0.4.18\src\gitignore.rs:227
  13:     0x7ff6d93d7ce1 - rustfmt_nightly::ignore_path::IgnorePathSet::is_match
                               at c:\src\rustfmt\src\ignore_path.rs:28
  14:     0x7ff6d91d510c - rustfmt_nightly::parse::session::ParseSess::ignore_file
                               at c:\src\rustfmt\src\parse\session.rs:217
  15:     0x7ff6d8fd9eb7 - rustfmt_nightly::formatting::FormatContext<rustfmt_nightly::Session<std::io::stdio::Stdout> >::ignore_file<rustfmt_nightly::Session<std::io::stdio::Stdout> >
                               at c:\src\rustfmt\src\formatting.rs:204
  16:     0x7ff6d8fd94ac - rustfmt_nightly::formatting::should_skip_module<rustfmt_nightly::Session<std::io::stdio::Stdout> >
                               at c:\src\rustfmt\src\formatting.rs:75
  17:     0x7ff6d8fd8ffa - rustfmt_nightly::formatting::format_project::closure$1<rustfmt_nightly::Session<std::io::stdio::Stdout> >
                               at c:\src\rustfmt\src\formatting.rs:143
  18:     0x7ff6d8fda2cb - core::ops::function::impls::impl$3::call_mut<tuple$<ref$<tuple$<enum2$<rustfmt_nightly::config::file_lines::FileName>,rustfmt_nightly::modules::Module> > >,rustfmt_nightly::formatting::format_project::closure_env$1<rustfmt_nightly::Session<std::io::stdio:
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\core\src\ops\function.rs:294
  19:     0x7ff6d8fe56d4 - core::iter::traits::iterator::Iterator::find::check::closure$0<tuple$<enum2$<rustfmt_nightly::config::file_lines::FileName>,rustfmt_nightly::modules::Module>,ref_mut$<rustfmt_nightly::formatting::format_project::closure_env$1<rustfmt_nightly::Session<std:
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\core\src\iter\traits\iterator.rs:2927
  20:     0x7ff6d8fd4a32 - core::iter::traits::iterator::Iterator::try_fold<alloc::collections::btree::map::IntoIter<enum2$<rustfmt_nightly::config::file_lines::FileName>,rustfmt_nightly::modules::Module,alloc::alloc::Global>,tuple$<>,core::iter::traits::iterator::Iterator::find::c
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\core\src\iter\traits\iterator.rs:2462
  21:     0x7ff6d8fd484b - core::iter::traits::iterator::Iterator::find<alloc::collections::btree::map::IntoIter<enum2$<rustfmt_nightly::config::file_lines::FileName>,rustfmt_nightly::modules::Module,alloc::alloc::Global>,ref_mut$<rustfmt_nightly::formatting::format_project::closur
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\core\src\iter\traits\iterator.rs:2931
  22:     0x7ff6d8fd35ed - core::iter::adapters::filter::impl$2::next<alloc::collections::btree::map::IntoIter<enum2$<rustfmt_nightly::config::file_lines::FileName>,rustfmt_nightly::modules::Module,alloc::alloc::Global>,rustfmt_nightly::formatting::format_project::closure_env$1<rus
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\core\src\iter\adapters\filter.rs:60
  23:     0x7ff6d8fc7377 - alloc::vec::spec_from_iter_nested::impl$0::from_iter<tuple$<enum2$<rustfmt_nightly::config::file_lines::FileName>,rustfmt_nightly::modules::Module>,core::iter::adapters::filter::Filter<alloc::collections::btree::map::IntoIter<enum2$<rustfmt_nightly::confi
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\alloc\src\vec\spec_from_iter_nested.rs:26
  24:     0x7ff6d8fc8b71 - alloc::vec::spec_from_iter::impl$0::from_iter<tuple$<enum2$<rustfmt_nightly::config::file_lines::FileName>,rustfmt_nightly::modules::Module>,core::iter::adapters::filter::Filter<alloc::collections::btree::map::IntoIter<enum2$<rustfmt_nightly::config::file
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\alloc\src\vec\spec_from_iter.rs:33
  25:     0x7ff6d8fc892a - alloc::vec::impl$14::from_iter<tuple$<enum2$<rustfmt_nightly::config::file_lines::FileName>,rustfmt_nightly::modules::Module>,core::iter::adapters::filter::Filter<alloc::collections::btree::map::IntoIter<enum2$<rustfmt_nightly::config::file_lines::FileNam
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\alloc\src\vec\mod.rs:2791
  26:     0x7ff6d8fd3661 - core::iter::traits::iterator::Iterator::collect<core::iter::adapters::filter::Filter<alloc::collections::btree::map::IntoIter<enum2$<rustfmt_nightly::config::file_lines::FileName>,rustfmt_nightly::modules::Module,alloc::alloc::Global>,rustfmt_nightly::for
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\core\src\iter\traits\iterator.rs:2054
  27:     0x7ff6d8fd86b2 - rustfmt_nightly::formatting::format_project<rustfmt_nightly::Session<std::io::stdio::Stdout> >
                               at c:\src\rustfmt\src\formatting.rs:134
  28:     0x7ff6d8fda059 - rustfmt_nightly::formatting::impl$0::format_input_inner::closure$0<std::io::stdio::Stdout>
                               at c:\src\rustfmt\src\formatting.rs:48
  29:     0x7ff6d8fcfd34 - scoped_tls::ScopedKey<rustc_span::SessionGlobals>::with<rustc_span::SessionGlobals,rustfmt_nightly::formatting::impl$0::format_input_inner::closure_env$0<std::io::stdio::Stdout>,enum2$<core::result::Result<rustfmt_nightly::FormatReport,enum2$<rustfmt_nigh
                               at /rust/deps\scoped-tls-1.0.1\src\lib.rs:171
  30:     0x7ff6d8fd5c94 - rustc_span::create_session_if_not_set_then::closure$0<enum2$<core::result::Result<rustfmt_nightly::FormatReport,enum2$<rustfmt_nightly::ErrorKind> > >,rustfmt_nightly::formatting::impl$0::format_input_inner::closure_env$0<std::io::stdio::Stdout> >
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\compiler\rustc_span\src\lib.rs:148
  31:     0x7ff6d8fcfba6 - scoped_tls::ScopedKey<rustc_span::SessionGlobals>::set<rustc_span::SessionGlobals,rustc_span::create_session_if_not_set_then::closure_env$0<enum2$<core::result::Result<rustfmt_nightly::FormatReport,enum2$<rustfmt_nightly::ErrorKind> > >,rustfmt_nightly::f
                               at /rust/deps\scoped-tls-1.0.1\src\lib.rs:137
  32:     0x7ff6d8fd5bd9 - rustc_span::create_session_if_not_set_then<enum2$<core::result::Result<rustfmt_nightly::FormatReport,enum2$<rustfmt_nightly::ErrorKind> > >,rustfmt_nightly::formatting::impl$0::format_input_inner::closure_env$0<std::io::stdio::Stdout> >
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\compiler\rustc_span\src\lib.rs:148
  33:     0x7ff6d8fe42c1 - rustfmt_nightly::Session<std::io::stdio::Stdout>::format_input_inner<std::io::stdio::Stdout>
                               at c:\src\rustfmt\src\formatting.rs:38
  34:     0x7ff6d8fe4719 - rustfmt_nightly::Session<std::io::stdio::Stdout>::format<std::io::stdio::Stdout>
                               at c:\src\rustfmt\src\lib.rs:468
  35:     0x7ff6d8fde805 - rustfmt::format_and_emit_report<std::io::stdio::Stdout>
                               at c:\src\rustfmt\src\bin\main.rs:371
  36:     0x7ff6d8fcba57 - rustfmt::format
                               at c:\src\rustfmt\src\bin\main.rs:346
  37:     0x7ff6d8fc99bf - rustfmt::execute
                               at c:\src\rustfmt\src\bin\main.rs:257
  38:     0x7ff6d8fc8d01 - rustfmt::main
                               at c:\src\rustfmt\src\bin\main.rs:38
  39:     0x7ff6d8fd039b - core::ops::function::FnOnce::call_once<void (*)(),tuple$<> >
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\core\src\ops\function.rs:250
  40:     0x7ff6d8fe5cbe - core::hint::black_box
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\core\src\hint.rs:334
  41:     0x7ff6d8fe5cbe - std::sys_common::backtrace::__rust_begin_short_backtrace<void (*)(),tuple$<> >
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\std\src\sys_common\backtrace.rs:155
  42:     0x7ff6d8fc54b1 - std::rt::lang_start::closure$0<tuple$<> >
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\std\src\rt.rs:166
  43:     0x7ffbf0d1ae32 - std::rt::lang_start_internal::hfc2680ea62bd7a5d
  44:     0x7ff6d8fc548a - std::rt::lang_start<tuple$<> >
                               at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1\library\std\src\rt.rs:165
  45:     0x7ff6d8fcf9b9 - main
  46:     0x7ff6d97a5bf0 - invoke_main
                               at D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
  47:     0x7ff6d97a5bf0 - __scrt_common_main_seh
                               at D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
  48:     0x7ffc51ac7344 - BaseThreadInitThunk
  49:     0x7ffc51c026b1 - RtlUserThreadStart

error: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rustfmt/issues/new?labels=bug

note: please attach the file at `c:\src\rustfmt\rustc-ice-2024-05-31T21_32_38-2332.txt` to your bug report

query stack during panic:
end of query stack
error: process didn't exit successfully: `target\debug\rustfmt.exe --config-path=c:\test\rustfmt.toml c:\test\bar.rs` (exit code: 101)

anforowicz commented 4 months ago

One way to avoid this crash is to modify rustfmt/src/ignore_path.rs as follows:

Add a call to canonicalize in IgnorePathSet::from_ignore_list (error handling seems ok to me):

impl IgnorePathSet {
    pub(crate) fn from_ignore_list(ignore_list: &IgnoreList) -> Result<Self, ignore::Error> {
        let mut ignore_builder = gitignore::GitignoreBuilder::new(ignore_list.rustfmt_toml_path().canonicalize()?);
        // ...

Add a call to canonicalize in IgnorePathSet::is_match. I am less sure about the correct error handling behavior in this case - what I have below seems like a bit of a hack... (still useful to illustrate/show that this change helps to avoid the crash):

    pub(crate) fn is_match(&self, file_name: &FileName) -> bool {
        match file_name {
            FileName::Stdin => false,
            FileName::Real(p) => {
                match p.canonicalize() {  // <= NEW CALL TO `canonicalize` HERE
                    Ok(p) => self
                       .ignore_set
                       .matched_path_or_any_parents(p, false)
                       .is_ignore(),
                    Err(_) => false,  // <= THIS IS PROBABLY A WRONG THING TO DO
                }
            },
        }
    }

anforowicz commented 4 months ago

Testing

I am not sure why this problem is not caught by cargo test in rustfmt. I am guessing that rustfmt\src\test\mod.rs always operates on canonicalized paths and therefore avoids this issue? (FWIW one test config does use the ignore property: tests/config/issue-3779.toml)

I guess Windows-specific unit-testing rustfmt/src/ignore_path.rs may be sufficient when/if we want to proceed with the fix.

Canonicalization open questions

IMO the main blocker is figuring out how/where to handle canonicalization errors:

Should IgnorePathSet::is_match somehow handle the errors? (Treating them as a match? As a mismatch?)
Should canonicalization be done at a different level (e.g. as soon as the path is consumed by rustfmt)?
- Maybe in fn format in rustfmt\src\bin\main.rs inside the for file in files loop? But this would only help when using the rustfmt binary, but it wouldn't help other users of the rustfmt library, since they could still pass a non-canonicalized path to the Input::File constructor.

anforowicz commented 1 month ago

It seems that paths of files being formatted are already being canonicalized in src/bin/main.rs, in fn determine_operation / when building Operation::Format::files. And I've tested that therefore it is sufficient to canonicalized the ignore_list.rustfmt_toml_path() - it seems best to canonicalize in fn get_toml_path and fn config_path in rustfmt/src/config/mod.rs.

rust-lang / rustfmt

rustfmt crashes with `ignore` on Windows #6178

Testing

Canonicalization open questions