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

rustfmt crashes with ignore on Windows #6178

Closed
danakj opened this issue May 30, 2024 · 6 comments · Fixed by #6272
Closed

rustfmt crashes with ignore on Windows #6178

danakj opened this issue May 30, 2024 · 6 comments · Fixed by #6272
Labels
only-with-option requires a non-default option value to reproduce os-windows

Comments

@danakj
Copy link

danakj commented May 30, 2024

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 ytmimi added only-with-option requires a non-default option value to reproduce os-windows labels May 30, 2024
@ytmimi
Copy link
Contributor

ytmimi commented May 30, 2024

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

danakj commented May 30, 2024

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

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:

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

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

anforowicz commented May 31, 2024

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 added a commit to anforowicz/rustfmt that referenced this issue Aug 7, 2024
This commit ensures that `fn config_path` in `rustfmt/src/config/mod.rs`
always returns canonicalized paths.  This is achieved by canonicalizing
both: 1) paths received via `CliOptions` (after checking if the path
exists) as well as 2) paths returned via `get_toml_path` (canonicalizing
within `fn get_toml_path`).

Fixes rust-lang#6178
@anforowicz
Copy link
Contributor

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.

ytmimi pushed a commit to anforowicz/rustfmt that referenced this issue Aug 14, 2024
This commit ensures that `fn config_path` in `rustfmt/src/config/mod.rs`
always returns canonicalized paths.  This is achieved by canonicalizing
both: 1) paths received via `CliOptions` (after checking if the path
exists) as well as 2) paths returned via `get_toml_path` (canonicalizing
within `fn get_toml_path`).

Fixes rust-lang#6178
@ytmimi ytmimi closed this as completed in caaa612 Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
only-with-option requires a non-default option value to reproduce os-windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants