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

std.fs.test file operations on directories fails with Wasmtime 23.0.1 #20747

Open
alexrp opened this issue Jul 22, 2024 · 7 comments
Open

std.fs.test file operations on directories fails with Wasmtime 23.0.1 #20747

alexrp opened this issue Jul 22, 2024 · 7 comments
Labels
bug Observed behavior contradicts documented or intended behavior downstream An issue with a third party project that uses Zig. os-wasi
Milestone

Comments

@alexrp
Copy link
Contributor

alexrp commented Jul 22, 2024

Zig Version

1fc42ed

Steps to Reproduce and Observed Behavior

  1. Install Wasmtime 23.0.1 (maybe earlier?).
  2. Run the standard library tests with -fwasmtime (but see std.Build.Step.Run: Fix invocation syntax for Wasmtime 14+. #20745).
error: 'fs.test.test.file operations on directories' failed: expected error.IsDir, found fs.File{ .handle = 4 }
Unable to dump stack trace: not implemented for Wasm

Fails here:

zig/lib/std/fs/test.zig

Lines 767 to 769 in eac7fd4

// Note: The `.mode = .read_write` is necessary to ensure the error occurs on all platforms.
// TODO: Add a read-only test as well, see https://github.com/ziglang/zig/issues/5732
try testing.expectError(error.IsDir, ctx.dir.openFile(test_dir_name, .{ .mode = .read_write }));

Expected Behavior

No failure.

@alexrp alexrp added the bug Observed behavior contradicts documented or intended behavior label Jul 22, 2024
@squeek502
Copy link
Collaborator

Just to clarify, what's the host system? Context: #13157

@alexrp
Copy link
Contributor Author

alexrp commented Jul 23, 2024

uname -srm
Linux 6.5.0-44-generic x86_64

@squeek502
Copy link
Collaborator

This could very easily be a bug in wasmtime. From the POSIX specificaiton of open:

[EISDIR]
    The named file is a directory and oflag includes O_WRONLY or O_RDWR.

@squeek502
Copy link
Collaborator

squeek502 commented Jul 23, 2024

This is actually pretty likely to be a bug in wasmtime, and it's one that manifested in exactly the same way in the past (but was fixed, so this seems like a regression):

EDIT: I'm not so certain that this is the same bug, I was doing something dumb while testing.

@squeek502
Copy link
Collaborator

squeek502 commented Jul 23, 2024

Seems to be related to wasi-libc:

Without linking libc, the tests pass:

$ zig test lib/std/std.zig --zig-lib-dir lib -target wasm32-wasi -femit-bin=test-wasi.wasm --test-no-exec
$ wasmtime --dir=. test-wasi.wasm
2592 passed; 115 skipped; 0 failed.

When linking libc, that test fails:

$ zig test lib/std/std.zig --zig-lib-dir lib -target wasm32-wasi -femit-bin=test-wasi-libc.wasm -lc --test-no-exec
$ wasmtime --dir=. test-wasi-libc.wasm
...
expected error.IsDir, found fs.File{ .handle = 7 }
1192/2707 fs.test.test.file operations on directories...FAIL (TestUnexpectedError)
Unable to dump stack trace: not implemented for Wasm
...
2589 passed; 117 skipped; 1 failed.

EDIT: The first wasmtime version that fails is 15.0.0:

$ ~/Downloads/wasmtime-v15.0.0-x86_64-linux/wasmtime --dir=. test-wasi-libc.wasm
2589 passed; 117 skipped; 1 failed.

$ ~/Downloads/wasmtime-v14.0.4-x86_64-linux/wasmtime --dir=. test-wasi-libc.wasm
2590 passed; 117 skipped; 0 failed.

@squeek502
Copy link
Collaborator

squeek502 commented Jul 23, 2024

The plot thickens...

The following test cases are built/run with

$ zig test open-dir-rw.zig -target wasm32-wasi -lc -femit-bin=open-dir-rw-test-libc.wasm --test-no-exec
$ wasmtime --dir=. open-dir-rw-test-libc.wasm
// This fails:
test "file operations on directories in tmpDir" {
    var tmp_dir = testing.tmpDir(.{});
    defer tmp_dir.cleanup();

    const test_dir_name = "testdir";
    try tmp_dir.dir.makeDir(test_dir_name);

    try testing.expectError(error.IsDir, tmp_dir.dir.openFile(test_dir_name, .{ .mode = .read_write }));
}

// But this passes:
test "file operations on directories in cwd" {
    const test_dir_name = "testdir";
    try std.fs.cwd().deleteTree(test_dir_name);
    try std.fs.cwd().makeDir(test_dir_name);

    try testing.expectError(error.IsDir, std.fs.cwd().openFile(test_dir_name, .{ .mode = .read_write }));
}

So somehow the usage of tmpDir is involved in the failure.

@squeek502
Copy link
Collaborator

squeek502 commented Jul 23, 2024

Found a reproduction with wasi-sdk 23.0, so this is indeed an upstream wasi-libc/wasmtime bug:

#include <sys/stat.h>
#include <sys/types.h>
#include <fcntl.h>
#include <unistd.h>

int main() {
    mkdirat(AT_FDCWD, "testdir", 0777);
    {
        // Opening with O_RDWR fails as expected if the dir fd is AT_FDCWD
        int dir = openat(AT_FDCWD, "testdir", O_RDWR);
        if (dir != -1) return 1;
    }

    // Open "testdir" for real now since we want to use it as the fd for openat
    int dir = openat(AT_FDCWD, "testdir", O_RDONLY|O_DIRECTORY);
    mkdirat(dir, "subdir", 0777);

    // Try to open the subdir with O_RDWR, this should fail but it succeeds
    // because the O_RDWR is lost and the openat2 syscall is called with O_RDONLY
    // instead
    int testdir = openat(dir, "subdir", O_RDWR);

    if (testdir != -1) return 1;
    return 0;
}
$ WASI_SDK=/home/ryan/Downloads/wasi-sdk-23.0-x86_64-linux
$ $WASI_SDK/bin/clang --sysroot=$WASI_SDK/share/wasi-sysroot open-dir-tmpdir-rw.c -o open-dir-tmpdir-rw-sdk-23.wasm
$ strace -e trace=openat2 wasmtime --dir=. open-dir-tmpdir-rw-sdk-23.wasm
openat2(3, "testdir", {flags=O_RDWR|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = -1 EISDIR (Is a directory)
openat2(3, "testdir", {flags=O_RDONLY|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 11
openat2(11, "subdir", {flags=O_RDONLY|O_LARGEFILE|O_CLOEXEC, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 12
+++ exited with 1 +++

The O_RDWR is lost when calling openat with an fd that's not AT_FDCWD.

EDIT: Created a wasmtime issue: bytecodealliance/wasmtime#9054

@squeek502 squeek502 added the upstream An issue with a third party project that Zig uses. label Jul 23, 2024
@andrewrk andrewrk added downstream An issue with a third party project that uses Zig. and removed upstream An issue with a third party project that Zig uses. labels Jul 26, 2024
@andrewrk andrewrk added this to the unplanned milestone Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior downstream An issue with a third party project that uses Zig. os-wasi
Projects
None yet
Development

No branches or pull requests

3 participants