ICE for wcslen on Windows

[cgettys-microsoft]

Good day folks,
I hit a ICE today. I installed nightly today (and it's reporting 1.81) so I think it's up to date.

This might be related to #2904, but it's not a no_std crate. I see a shim for wcslen in https://github.com/rust-lang/miri/blob/dc286ac1432932b2a710789de40ae8bc4e7ea7c1/src/shims/foreign_items.rs - but it looks like that shim doesn't work for msvc windows targets at a minimum.
But I'm not an expert on Miri.
windows_core::PCWSTR and windows_core::PWSTR both trivially trigger in via their .len() method - meaning a lot of windows code could benefit.
Edit: repro has been minimized further, see #3692 (comment)

// windows-core 0.57.0 is the original source of the struct involved
// I've minimized the example (Trimming unnecessary derive macros, unused functions, et cetera)
// As best I can.
// https://github.com/microsoft/windows-rs/commit/15947886be041bebd0fe670fd4e73f18d95100d0

/// A pointer to a constant null-terminated string of 16-bit Unicode characters.
#[repr(transparent)]
pub struct PCWSTR(pub *const u16);

impl PCWSTR {
    /// Construct a new `PCWSTR` from a raw pointer
    pub const fn from_raw(ptr: *const u16) -> Self {
        Self(ptr)
    }

    /// String length without the trailing 0
    ///
    /// # Safety
    ///
    /// The `PCWSTR`'s pointer needs to be valid for reads up until and including the next `\0`.
    pub unsafe fn len(&self) -> usize {
        //#[cfg(windows)]
        // It's specifically the inclusion of this that makes or breaks the repro
        // The other non-windows one does not trigger the ICE.
        let len = {
            extern "C" {
                fn wcslen(s: *const u16) -> usize;
            }
            wcslen(self.0)
        };

        // #[cfg(not(windows))]
        // let len = {
        //     let mut len = 0;
        //     let mut ptr = self.0;
        //     while ptr.read() != 0 {
        //         len += 1;
        //         ptr = ptr.add(1);
        //     }
        //     len
        // };

        len
    }
}

pub fn does_not_trigger_ice_without_proc_macro()
{
    // This isn't the most portable from endianness perspective. But it'll do.
    let my_str: [u16; 2] = ['a' as u16 , 0];
    let my_pcwstr = PCWSTR::from_raw(my_str.as_ptr());
    // SAFETY: doesn't matter if it's safe, point is that this doesn't trigger an ICE.
    let _my_ice_trigger = unsafe { my_pcwstr.len() };
}

#[cfg(test)]
mod tests
{
    use crate::{does_not_trigger_ice_without_proc_macro, PCWSTR};
    // /// This works as well to trigger it.
    // #[test]
    // pub fn cause_ice2()
    // {
    //     does_not_trigger_ice_without_proc_macro()
    // }

    #[test]
    pub fn cause_ice()
    {
        // This isn't the most portable from endianness perspective. But it'll do.
        let my_str: [u16; 2] = ['a' as u16 , 0];
        let my_pcwstr = PCWSTR::from_raw(my_str.as_ptr());
        // SAFETY: doesn't matter if it's safe, point is that this doesn't trigger an ICE.
        let _my_ice_trigger = unsafe { my_pcwstr.len() };
    }
}

It also triggers on PWSTR. I don't think it repros on strlen though - which makes sense with the backtrace.
wcslen is utf16, not utf32, on Windows.

rustc-ice-2024-06-21T01_12_48-101996.txt

[cgettys-microsoft]

Doing a bit more digging

miri/src/helpers.rs

Line 1049 in cc9699d

fn read_wchar_t_str(&self, ptr: Pointer) -> InterpResult<'tcx, Vec<u32>> {

    /// Read a sequence of wchar_t until the first null terminator.
    /// Always returns a `Vec<u32>` no matter the size of `wchar_t`.
    fn read_wchar_t_str(&self, ptr: Pointer) -> InterpResult<'tcx, Vec<u32>> {
        let this = self.eval_context_ref();
        let wchar_t = this.libc_ty_layout("wchar_t");
        self.read_c_str_with_char_size(ptr, wchar_t.size, wchar_t.align.abi)
    }

If it's pulling type layouts from Rust's libc implementation (which seems to make sense, but there's a lot of indirection going on here and I'm not a Rust compiler guru), then I'm really not sure why it doesn't work, because wchar_t should be u16 for windows msvc:
https://github.com/rust-lang/libc/blob/7d7151c51285bf238d2d02dc736c64b3c6dfbcc8/src/windows/mod.rs#L26

[cgettys-microsoft]

PWCSTR from windows-core is explicitly utf-16 / u16. So I further simplified the repro. THis works fine on
cargo miri test --target x86_64-unknown-linux-gnu
but still ICEs exactly like the previous for cargo miri test --target x86_64-pc-windows-msvc

#[cfg(test)]
mod tests
{
    #[cfg(target_os = "windows")]
    type NativeWchar = u16;
    // This is a hack, but is useful for this repro
    #[cfg(not(target_os = "windows"))]
    type NativeWchar = u32;
    #[test]
    pub fn cause_ice()
    {
        // This isn't the most portable from endianness perspective. But it'll do.
        let my_str: [NativeWchar; 2] = ['a' as NativeWchar  , 0];
        let _len = {
            extern "C" {
                fn wcslen(s: *const NativeWchar) -> usize;
            }
            // SAFETY: doesn't matter if it's safe, point is that this triggers an ICE.
            let _my_ice_trigger = unsafe { wcslen(my_str.as_ptr()) };
        };
    }
}

To add to the mystery, both x86_64-pc-windows-gnullvm and x86_64-pc-windows-gnu both seem to be fine. and I believe gnullvm is using ucrt. So maybe it's related to just the tools, not even the c library in use? I'm not sure what to make of this.

Edit: cargo miri test --target aarch64-pc-windows-msvc and cargo miri test --target arm64ec-pc-windows-msvc also ICE

[ChrisDenton]

Could you provide the error message? Also please see Creating and highlighting code blocks for how to format code blocks so they're more readable.

[cgettys-microsoft]

Whoops, sorry about the formatting... fixed.
Here's the console output, minus the stack trace that I already attached in my original message

    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.56s
     Running unittests src/main.rs (LOCAL_SOURCE_PATH_REDACTED\target\miri\x86_64-pc-windows-msvc\debug\deps\playground01-809fb30126744ceb.exe)

running 1 test
thread 'rustc' panicked at src\tools\miri\src\helpers.rs:254:32:
failed to find required Rust item: ["libc", "wchar_t"]
stack backtrace:
   0:     0x7ff96bb23c3d - std::backtrace_rs::backtrace::dbghelp64::trace
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\std\src\..\..\backtrace\src\backtrace\dbghelp64.rs:91
   1:     0x7ff96bb23c3d - std::backtrace_rs::backtrace::trace_unsynchronized
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\std\src\..\..\backtrace\src\backtrace\mod.rs:66
   2:     0x7ff96bb23c3d - std::sys::backtrace::_print_fmt
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\std\src\sys\backtrace.rs:68
   3:     0x7ff96bb23c3d - std::sys::backtrace::_print::impl$0::fmt
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\std\src\sys\backtrace.rs:44
   4:     0x7ff96bb54629 - core::fmt::rt::Argument::fmt
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\core\src\fmt\rt.rs:165
   5:     0x7ff96bb54629 - core::fmt::write
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\core\src\fmt\mod.rs:1168
   6:     0x7ff96bb1a391 - std::io::Write::write_fmt<std::sys::pal::windows::stdio::Stderr>
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\std\src\io\mod.rs:1835
   7:     0x7ff96bb23a16 - std::sys::backtrace::print
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\std\src\sys\backtrace.rs:34
   8:     0x7ff96bb26bf8 - std::panicking::default_hook::closure$1
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\std\src\panicking.rs:265
   9:     0x7ff96bb268a9 - std::panicking::default_hook
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\std\src\panicking.rs:292
  10:     0x7ff8622681ac - memchr
  11:     0x7ff96bb27377 - alloc::boxed::impl$50::call
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\alloc\src\boxed.rs:2076
  12:     0x7ff96bb27377 - std::panicking::rust_panic_with_hook
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\std\src\panicking.rs:804
  13:     0x7ff96bb27186 - std::panicking::begin_panic_handler::closure$0
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\std\src\panicking.rs:670
  14:     0x7ff96bb245af - std::sys::backtrace::__rust_end_short_backtrace<std::panicking::begin_panic_handler::closure_env$0,never$>
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\std\src\sys\backtrace.rs:171
  15:     0x7ff96bb26d66 - std::panicking::begin_panic_handler
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\std\src\panicking.rs:661
  16:     0x7ff96bb7cfa4 - core::panicking::panic_fmt
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\core\src\panicking.rs:74
  17:     0x7ff618b0597b - <unknown>
  18:     0x7ff618b0d678 - <unknown>
  19:     0x7ff618bb1ca2 - <unknown>
  20:     0x7ff618bad00b - <unknown>
  21:     0x7ff618a8a939 - <unknown>
  22:     0x7ff618af013c - <unknown>
  23:     0x7ff618b01801 - <unknown>
  24:     0x7ff618a198dd - <unknown>
  25:     0x7ff85dfa563b - __ImageBase
  26:     0x7ff85dfa296f - __ImageBase
  27:     0x7ff85dfaa6c9 - __ImageBase
  28:     0x7ff96bb3838d - alloc::boxed::impl$48::call_once
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\alloc\src\boxed.rs:2062
  29:     0x7ff96bb3838d - alloc::boxed::impl$48::call_once
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\alloc\src\boxed.rs:2062
  30:     0x7ff96bb3838d - std::sys::pal::windows::thread::impl$0::new::thread_start
                               at /rustc/d8a38b00024cd7156dea4ce8fd8ae113a2745e7f/library\std\src\sys\pal\windows\thread.rs:52
  31:     0x7ff9e5bd257d - BaseThreadInitThunk
  32:     0x7ff9e69aaf28 - RtlUserThreadStart

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

note: we would appreciate a bug report: https://github.com/rust-lang/miri/issues/new

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

note: please attach the file at `LOCAL_SOURCE_PATH_REDACTED\rustc-ice-2024-06-21T02_01_40-93788.txt` to your bug report

note: compiler flags: -C embed-bitcode=no -C debuginfo=2 -C incremental=[REDACTED] -C link-args=/DYNAMICBASE /CETCOMPAT

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

query stack during panic:
end of query stack

Miri caused an ICE during evaluation. Here's the interpreter backtrace at the time of the panic:
note: the place in the program where the ICE was triggered
  --> src\rust\playground01\src/main.rs:26:44
   |
26 |             let _my_ice_trigger = unsafe { wcslen(my_str.as_ptr()) };
   |                                            ^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: BACKTRACE on thread `tests::cause_ice`:
   = note: inside `tests::cause_ice` at src\rust\playground01\src/main.rs:26:44: 26:67
note: inside closure
  --> src\rust\playground01\src/main.rs:17:23
   |
16 |     #[test]
   |     ------- in this procedural macro expansion
17 |     pub fn cause_ice()
   |                       ^
   = note: inside `<{closure@src\rust\playground01\src/main.rs:17:5: 28:6} as std::ops::FnOnce<()>>::call_once - shim` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ops\function.rs:250:5: 250:71
   = note: inside `<fn() -> std::result::Result<(), std::string::String> as std::ops::FnOnce<()>>::call_once - shim(fn() -> std::result::Result<(), std::string::String>)` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ops\function.rs:250:5: 250:71
   = note: inside `tests::test::__rust_begin_short_backtrace::<std::result::Result<(), std::string::String>, fn() -> std::result::Result<(), std::string::String>>` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\test\src\lib.rs:625:18: 625:21
   = note: inside `tests::test::types::RunnableTest::run` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\test\src\types.rs:146:40: 146:71    
   = note: inside closure at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\test\src\lib.rs:648:60: 648:79
   = note: inside `<std::panic::AssertUnwindSafe<{closure@tests::test::run_test_in_process::{closure#0}}> as std::ops::FnOnce<()>>::call_once` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\panic\unwind_safe.rs:272:9: 272:19
   = note: inside `std::panicking::r#try::do_call::<std::panic::AssertUnwindSafe<{closure@tests::test::run_test_in_process::{closure#0}}>, std::result::Result<(), std::string::String>>` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:553:40: 553:43
   = note: inside `std::panicking::r#try::<std::result::Result<(), std::string::String>, std::panic::AssertUnwindSafe<{closure@tests::test::run_test_in_process::{closure#0}}>>` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:517:19: 517:88
   = note: inside `std::panic::catch_unwind::<std::panic::AssertUnwindSafe<{closure@tests::test::run_test_in_process::{closure#0}}>, std::result::Result<(), std::string::String>>` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panic.rs:350:14: 350:33
   = note: inside `tests::test::run_test_in_process` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\test\src\lib.rs:648:27: 648:81
   = note: inside closure at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\test\src\lib.rs:569:43: 577:18
   = note: inside closure at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\test\src\lib.rs:599:41: 599:83
   = note: inside `std::sys::backtrace::__rust_begin_short_backtrace::<{closure@tests::test::run_test::{closure#1}}, ()>` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\sys\backtrace.rs:155:18: 155:21
   = note: inside closure at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\thread\mod.rs:542:17: 542:71
   = note: inside `<std::panic::AssertUnwindSafe<{closure@std::thread::Builder::spawn_unchecked_<'_, '_, {closure@tests::test::run_test::{closure#1}}, ()>::{closure#2}::{closure#0}}> as std::ops::FnOnce<()>>::call_once` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\panic\unwind_safe.rs:272:9: 272:19
   = note: inside `std::panicking::r#try::do_call::<std::panic::AssertUnwindSafe<{closure@std::thread::Builder::spawn_unchecked_<'_, '_, {closure@tests::test::run_test::{closure#1}}, ()>::{closure#2}::{closure#0}}>, ()>` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:553:40: 553:43
   = note: inside `std::panicking::r#try::<(), std::panic::AssertUnwindSafe<{closure@std::thread::Builder::spawn_unchecked_<'_, '_, {closure@tests::test::run_test::{closure#1}}, ()>::{closure#2}::{closure#0}}>>` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:517:19: 517:88
   = note: inside `std::panic::catch_unwind::<std::panic::AssertUnwindSafe<{closure@std::thread::Builder::spawn_unchecked_<'_, '_, {closure@tests::test::run_test::{closure#1}}, ()>::{closure#2}::{closure#0}}>, ()>` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panic.rs:350:14: 350:33
   = note: inside closure at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\thread\mod.rs:541:30: 543:16
   = note: inside `<{closure@std::thread::Builder::spawn_unchecked_<'_, '_, {closure@tests::test::run_test::{closure#1}}, ()>::{closure#2}} as std::ops::FnOnce<()>>::call_once - shim(vtable)` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ops\function.rs:250:5: 250:71
   = note: inside `<std::boxed::Box<dyn std::ops::FnOnce()> as std::ops::FnOnce<()>>::call_once` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\boxed.rs:2062:9: 2062:52
   = note: inside `<std::boxed::Box<std::boxed::Box<dyn std::ops::FnOnce()>> as std::ops::FnOnce<()>>::call_once` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\alloc\src\boxed.rs:2062:9: 2062:52
   = note: inside `std::sys::pal::windows::thread::Thread::new::thread_start` at C:\Users\REDACTED\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\sys\pal\windows\thread.rs:52:13: 52:60
   = note: this note originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

test tests::cause_ice ... error: test failed, to rerun pass `--bin playground01`

Let me know if there's other information that would help.

[ChrisDenton]

Ah, you posted the ICE file earlier but I overlooked it. So the important part is that miri can't find wchar_t in the libc crate. @saethlin could this be because the windows-msvc standard library no longer depends on the libc crate and miri was implicitly relying on it?

[cgettys-microsoft]

Can't blame you for missing it given my crummy formatting 😄.

If it's not related to the libc crate (keeping in mind I'm not familiar with that history), the other theory that comes to mind as I think on it is msvc and msvc library conformance RE: wchar_t. Once upon a time I believe the default was that it just was a typedef, and it's still possible to opt into the non-conformant behavior I believe.

Keeping in mind I'm not clear on whether Miri is relying on Rust type metadata (From libc crate?) or debug info or RTTI info or what. Also keeping in mind I have no idea if it's relevant or not, but thought it was worth mentioning in case it is.
/Zc:wchar_t page

Maybe the lookup needs to be for __wchar_t?

[RalfJung]

Miri relies on the Rust types declared in the libc crate.

could this be because the windows-msvc standard library no longer depends on the libc crate and miri was implicitly relying on it?

Yes. Our tests call wcslen via libc to avoid us having to repeat the signature, but that means we do have libc available even on Windows so we are missing the ICE.

We should probably add an assertion to eval_libc to make sure it only gets called on Unix targets.

[cgettys-microsoft]

FWIW @RalfJung - I did try adding libc as a crate dependency locally after Chris suggested that that might be the issue. It didn't seem to resolve the issue. But I'm guessing that adding libc as a crate dependency for my test crate wouldn't be expected to do it, , as that wouldn't make Miri magically depend on libc/load libc.

Thanks for the fast turnaround on the fix, and for all the awesome work you guys have done on Miri!

[RalfJung]

Hm, adding it as a dependency should have helped; that's how the test was passing on CI even when it was ICEing for you... maybe you have to also add extern crate libc or so.