rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.16k stars 318 forks source link

Miri tests on Windows require admin for symlinks #3587

Closed RossSmyth closed 1 month ago

RossSmyth commented 1 month ago

This is on Windows 10 on commit 28ab963bc9abe6999f4acd126c865ec59bf4a503. I can replicate the issue on both of my computers.

  1. .\miri toolchain
  2. .\miri test

Tests pass successfully

  1. .\miri toolchain
  2. $Env:MIRI_TEST_TARGET="x86_64-unknown-linux-gnu"
  3. .\miri test
FAILED TEST: tests/pass\shims\fs.rs
command: "C:\\Users\\rsmyth\\Documents\\miri\\target\\debug\\miri.exe" "--error-format=json" "--sysroot=C:\\Users\\rsmyth\\AppData\\Local\\rust-lang\\miri\\cache" "-Dwarnings" "-Dunused" "-Ainternal_features" "-Zui-testing" "--target" "x86_64-unknown-linux-gnu" "--out-dir" "C:\\Users\\rsmyth\\Documents\\miri\\target\\ui\\tests\\pass\\shims" "tests/pass\\shims\\fs.rs" "-Zmiri-disable-isolation" "--edition" "2021"

error: pass test got exit code: 1, but expected 0

error: actual output differed from expected
Execute `./miri test --bless` to update `tests/pass\shims\fs.stderr` to the actual output
--- tests/pass\shims\fs.stderr
+++ <stderr output>
+error: unsupported operation: io error Uncategorized cannot be translated into a raw os error
+  --> RUSTLIB/std/src/sys/pal/PLATFORM/fs.rs:LL:CC
+   |
+LL |             cvt(unsafe { libc::symlink(original.as_ptr(), link.as_ptr()) }).map(|_| ())
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ io error Uncategorized cannot be translated into a raw os error
+   |
+   = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support
+   = note: BACKTRACE:
+   = note: inside closure at RUSTLIB/std/src/sys/pal/PLATFORM/fs.rs:LL:CC
+   = note: inside `std::sys::pal::PLATFORM::small_c_string::run_with_cstr_stack::<()>` at RUSTLIB/std/src/sys/pal/PLATFORM/small_c_string.rs:LL:CC
+   = note: inside `std::sys::pal::PLATFORM::small_c_string::run_with_cstr::<()>` at RUSTLIB/std/src/sys/pal/PLATFORM/small_c_string.rs:LL:CC
+   = note: inside `std::sys::pal::PLATFORM::small_c_string::run_path_with_cstr::<()>` at RUSTLIB/std/src/sys/pal/PLATFORM/small_c_string.rs:LL:CC
+   = note: inside closure at RUSTLIB/std/src/sys/pal/PLATFORM/fs.rs:LL:CC
+   = note: inside `std::sys::pal::PLATFORM::small_c_string::run_with_cstr_stack::<()>` at RUSTLIB/std/src/sys/pal/PLATFORM/small_c_string.rs:LL:CC
+   = note: inside `std::sys::pal::PLATFORM::small_c_string::run_with_cstr::<()>` at RUSTLIB/std/src/sys/pal/PLATFORM/small_c_string.rs:LL:CC
+   = note: inside `std::sys::pal::PLATFORM::small_c_string::run_path_with_cstr::<()>` at RUSTLIB/std/src/sys/pal/PLATFORM/small_c_string.rs:LL:CC
+   = note: inside `std::sys::pal::PLATFORM::fs::symlink` at RUSTLIB/std/src/sys/pal/PLATFORM/fs.rs:LL:CC
+   = note: inside `std::os::unix::fs::symlink::<&std::path::PathBuf, &std::path::PathBuf>` at RUSTLIB/std/src/os/unix/fs.rs:LL:CC
+note: inside `test_symlink`
+  --> $DIR/fs.rs:LL:CC
+   |
+LL |     std::os::unix::fs::symlink(&path, &symlink_path).unwrap();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: inside `main`
+  --> $DIR/fs.rs:LL:CC
+   |
+LL |     test_symlink();
+   |     ^^^^^^^^^^^^^^
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

This seems to happen on nix targets as it happens on `-apple-darwin` as well.

Based upon the instructions inside the test I looked for miri_ files, but there were none in the tmp directory. Also setting TMPDIR or MIRI_TEMP to other paths had no affect.

RossSmyth commented 1 month ago

I'll look into this later if there is a shim missing

RalfJung commented 1 month ago

Does .\miri test without any env var work?

RossSmyth commented 1 month ago

Yes, that is the first case.

RossSmyth commented 1 month ago

Tests also pass when cross-interpreting i686-pc-windows-msvc

RalfJung commented 1 month ago

Ah, sorry, I read too quickly. Well that's something at least. :)

So there's a problem creating symlinks. Wasn't there something where creating symlinks on Windows needs admin permissions or so? It works on CI though...

We should definitely do a better job at saying what the error was, though... Uncategorized is not super useful.^^

Based upon the instructions inside the test I looked for miri_ files, but there were none in the tmp directory. Also setting TMPDIR or MIRI_TEMP to other paths had no affect.

The directory gets cleaned up when tests are done (even if they fail), so you wouldn't see any trace of that. If it gets to test_symlink it means these other tests already passed

    test_path_conversion();
    test_file();
    test_file_clone();
    test_file_create_new();
    test_seek();
    test_metadata();
    test_file_set_len();
    test_file_sync();

So I think it's safe to say the directory exists and works as expected, just symlinks don't.

RossSmyth commented 1 month ago

TBH I have no idea how symlinks work on Windows. I've never used them.

And since you mentioned it, I ran the tests under admin and they passed. So yeah, that seems to be correct.

RalfJung commented 1 month ago

Okay, good to know that we figured out the cause of this. :)

Not sure what the best way is to deal with this. We could ignore permission errors (if we can detect them, Uncategorized is not very encouraging). We could move symlink tests into a separate file and disable them on Windows hosts... that's probably the best option, but it's kind of a shame.

@ChrisDenton do you have any better ideas?

ChrisDenton commented 1 month ago

You can enable "developer mode" in settings to allow unprivileged symlinks creation. So one option would just be to document this as a requirement.

Otherwise maybe there could be a check to see if the host supports symlinks (e.g. by trying to create one)?

RossSmyth commented 1 month ago

With #3589 it does make it clearer. At least it is an error code that google can tell me about.

error: unsupported operation: unsupported io error: A required privilege is not held by the client. (os error 1314)
  --> C:\Users\rsmyth\.rustup\toolchains\miri\lib\rustlib\src\rust\library\std\src\sys\pal\unix\fs.rs:1732:26
   |
LL |             cvt(unsafe { libc::symlink(original.as_ptr(), link.as_ptr()) }).map(|_| ())
   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unsupported io error: A required privilege is not held by the client. (os error 1314)
   |
   = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support
   = note: BACKTRACE:
   = note: inside closure at C:\Users\rsmyth\.rustup\toolchains\miri\lib\rustlib\src\rust\library\std\src\sys\pal\unix\fs.rs:1732:26: 1732:73
   = note: inside `std::sys::pal::common::small_c_string::run_with_cstr_stack::<()>` at C:\Users\rsmyth\.rustup\toolchains\miri\lib\rustlib\src\rust\library\std\src\sys\pal\common\small_c_string.rs:49:18: 49:22
   = note: inside `std::sys::pal::common::small_c_string::run_with_cstr::<()>` at C:\Users\rsmyth\.rustup\toolchains\miri\lib\rustlib\src\rust\library\std\src\sys\pal\common\small_c_string.rs:29:18: 29:47
   = note: inside `std::sys::pal::common::small_c_string::run_path_with_cstr::<()>` at C:\Users\rsmyth\.rustup\toolchains\miri\lib\rustlib\src\rust\library\std\src\sys\pal\common\small_c_string.rs:19:5: 19:58
   = note: inside closure at C:\Users\rsmyth\.rustup\toolchains\miri\lib\rustlib\src\rust\library\std\src\sys\pal\unix\fs.rs:1731:9: 1733:11
   = note: inside `std::sys::pal::common::small_c_string::run_with_cstr_stack::<()>` at C:\Users\rsmyth\.rustup\toolchains\miri\lib\rustlib\src\rust\library\std\src\sys\pal\common\small_c_string.rs:49:18: 49:22
   = note: inside `std::sys::pal::common::small_c_string::run_with_cstr::<()>` at C:\Users\rsmyth\.rustup\toolchains\miri\lib\rustlib\src\rust\library\std\src\sys\pal\common\small_c_string.rs:29:18: 29:47
   = note: inside `std::sys::pal::common::small_c_string::run_path_with_cstr::<()>` at C:\Users\rsmyth\.rustup\toolchains\miri\lib\rustlib\src\rust\library\std\src\sys\pal\common\small_c_string.rs:19:5: 19:58
   = note: inside `std::sys::pal::unix::fs::symlink` at C:\Users\rsmyth\.rustup\toolchains\miri\lib\rustlib\src\rust\library\std\src\sys\pal\unix\fs.rs:1730:5: 1734:7
   = note: inside `std::os::unix::fs::symlink::<&std::path::PathBuf, &std::path::PathBuf>` at C:\Users\rsmyth\.rustup\toolchains\miri\lib\rustlib\src\rust\library\std\src\os\unix\fs.rs:951:5: 951:55
note: inside `test_symlink`
  --> tests/pass\shims\fs.rs:233:5
   |
LL |     std::os::unix::fs::symlink(&path, &symlink_path).unwrap();
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `main`
  --> tests/pass\shims\fs.rs:32:5
   |
LL |     test_symlink();
   |     ^^^^^^^^^^^^^^

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

error: aborting due to 1 previous error
RalfJung commented 1 month ago

Thanks for checking that!