Miri no longer works on Windows MSVC due to unimplemented function GetFileInformationByHandleEx

[complexspaces]

As of nightly-2022-10-16, miri test now unconditionally fails with an error about GetFileInformationByHandleEx being unimplemented. I first noticed this here after a merge of an older PR failed CI. At a glance, this seems to be a result of rust-lang/rust#98033. I noticed that the function is mentioned in #2057, but nothing more than that. What would be involved in shimming this function out, since it seems rather bad that all Windows Miri testing no longer works?

cargo +nightly-2022-10-16 miri test
Preparing a sysroot for Miri (target: x86_64-pc-windows-msvc)... done
   Compiling miri-test v0.1.0 (C:\1Password\tmp\miri-test)
    Finished test [unoptimized + debuginfo] target(s) in 0.05s
     Running unittests src\lib.rs (target\miri\x86_64-pc-windows-msvc\debug\deps\miri_test-0cf529eb468479a8.exe)
error: unsupported operation: can't call foreign function: GetFileInformationByHandleEx
   --> C:\Users\User\.rustup\toolchains\nightly-2022-10-16-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\sys\windows\io.rs:125:15
    |
125 |       let res = c::GetFileInformationByHandleEx(
    |  _______________^
126 | |         handle,
127 | |         c::FileNameInfo,
128 | |         name_info_bytes.0.as_mut_ptr() as *mut libc::c_void,
129 | |         SIZE as u32,
130 | |     );
    | |_____^ can't call foreign function: GetFileInformationByHandleEx
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
    = note: BACKTRACE:
    = note: inside `std::sys::windows::io::msys_tty_on` at C:\Users\User\.rustup\toolchains\nightly-2022-10-16-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\sys\windows\io.rs:125:15
    = note: inside `std::sys::windows::io::handle_is_console` at C:\Users\User\.rustup\toolchains\nightly-2022-10-16-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\sys\windows\io.rs:119:5
    = note: inside `std::sys::windows::io::is_terminal::<std::io::Stdout>` at C:\Users\User\.rustup\toolchains\nightly-2022-10-16-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\sys\windows\io.rs:87:14
    = note: inside `<std::io::Stdout as std::io::IsTerminal>::is_terminal` at C:\Users\User\.rustup\toolchains\nightly-2022-10-16-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\io\stdio.rs:1059:17
    = note: inside `test::TestOpts::use_color` at C:\Users\User\.rustup\toolchains\nightly-2022-10-16-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\test\src\cli.rs:35:58
    = note: inside `test::run_tests_console` at C:\Users\User\.rustup\toolchains\nightly-2022-10-16-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\test\src\console.rs:274:13
    = note: inside `test::test_main` at C:\Users\User\.rustup\toolchains\nightly-2022-10-16-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\test\src\lib.rs:115:15
    = note: inside `test::test_main_static` at C:\Users\User\.rustup\toolchains\nightly-2022-10-16-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\test\src\lib.rs:134:5
    = note: inside `main`

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to previous error

error: test failed, to rerun pass `--lib`

Version info:

cargo +nightly-2022-10-16 --version -v
cargo 1.66.0-nightly (b332991a5 2022-10-13)
release: 1.66.0-nightly
commit-hash: b332991a57c9d055f1864de1eed93e2178d49440
commit-date: 2022-10-13
host: x86_64-pc-windows-msvc
libgit2: 1.5.0 (sys:0.15.0 vendored)
libcurl: 7.83.1-DEV (sys:0.4.55+curl-7.83.1 vendored ssl:Schannel)
os: Windows 10.0.19044 (Windows 10 Pro) [64-bit]

Reproduction case:

src/lib.rs:

#[cfg(test)]
fn return_value() -> u32 {
    5
}

#[test]
fn do_some_check() {
    assert_eq!(return_value(), 5);
}

Cargo.toml:

[package]
name = "miri-test"
version = "0.1.0"
edition = "2021"

[dependencies]

[thomcc]

Hm, this flew under my radar (it landed long after the R+ I gave). I'm not sure the best approach here -- the way we implement this in std is a huge hack since we're doing this to see if it's obviously a cygwin or msys pty, I think the best approach might be an additional hack for miri in std, unless we want miri to pretend to be cygwin/msys in this case...

[thomcc]

Perhaps having miri report "miri-askdjfqhadslkfj-pty" for the filename and adding a check for "miri-" to https://github.com/rust-lang/rust/blob/194140bef501ad3acb00d57c20fb80ee34aa1d3b/library/std/src/sys/windows/io.rs#L144 would be reasonable. Kinda jank, tho...

[saethlin]

Is there any reason this didn't cause a test failure somewhere? Is this again our missing Windows target coverage for Miri tests?

[thomcc]

I don't know if we run miri in CI for windows.

[ChrisDenton]

I would strongly prefer adding a temporary #[cfg(miri)] over using any gross hacks.

But this code is (almost) a direct copy from the isatty crate and I thought we already had shims to handle that?

[thomcc]

@ChrisDenton What would the cfg(miri) look like? I don't see a way for that to work unless miri somehow reports to std whether or not the given handle is actually a tty.

[ChrisDenton]

Does miri actually need to care right now? Does it matter if it always reports true (or always false)?

[thomcc]

My main concern there is regressing libtest output compared to whatever we had previously, but perhaps I don't need to be worried about that?

[ChrisDenton]

It looks like it worked before by, effectively, returning false each time which is why it reaches the fallback in the new code. See:

https://github.com/rust-lang/rust/blob/bf15a9e5263fcea065a7ae9c179b2d24c2deb670/library/test/src/helpers/isatty.rs#L10-L25

https://github.com/rust-lang/rust/blob/27579a214dc2a918e67bffb51a0825005e8bd42e/src/tools/miri/src/shims/windows/foreign_items.rs#L334-L343

[thomcc]

Oh, yeah, that's a much better way to do it.

[RalfJung]

Yeah we should make is_terminal work on Miri for any target.

Looks like we have two options -- actually make GetConsoleMode do something, or adding a cfg(miri) to avoid the false negative handling fallback in Miri?

Cc @drmeepster

[RalfJung]

Actually turns out we can just use a simple stub implementation of GetFileInformationByHandleEx that always returns null, to get things to work as before.