Skip to content

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

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

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

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

alexrp opened this issue Jul 22, 2024 · 8 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
Member

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
Member 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

@rootbeer
Copy link
Contributor

The upstream bug about losing track of O_RDWR applies to basic files, too (not just directories). Using the basic posix APIs to create a file and write to it fails when going through the wasi-libc path.

For example:

const std = @import("std");
const builtin = @import("builtin");
const tmpDir = std.testing.tmpDir;
const testing = std.testing;

pub fn main() !void {
    var tmp = tmpDir(.{});
    defer tmp.cleanup();

    const file_name = "afile.txt";

    // This write works because Zig's std.fs.Dir.createFile() avoids libc and invokes std.os.wasi APIs directly (so the FD has the write flag)
    try tmp.dir.writeFile(.{ .sub_path = file_name, .data = "non-empty file" });

    // But doing an open manually ignores the write flag (on wasi without libc)
    const os_flags: std.posix.O = switch (builtin.os.tag) {
        .wasi => .{
            .read = true,
            .write = true,
        },
        else => .{
            .ACCMODE = .RDWR,
        },
    };

    const fd = try std.posix.openat(tmp.dir.fd, file_name, os_flags, 0);
    defer std.posix.close(fd);

    // This works on wasi-without-libc (or native linux), but fails on wasi-with-libc:
    std.debug.print("write: {any}\n", .{ std.posix.write(fd, "new content") });
}

If you run the above natively (with or without libc), on wasi-without-libc and wasi-with-libc you'll see:

$ zig run main.zig
write: 11
$ zig run  -lc main.zig
write: 11
$ zig build-exe main.zig -target wasm32-wasi && wasmtime --dir=. main.wasm
write: 11
$ zig build-exe main.zig -lc -target wasm32-wasi && wasmtime --dir=. main.wasm
write: error.NotOpenForWriting

Inspecting the descriptor returned from openat (e.g., with std.os.wasi.fd_fdstat_get(fd, &stat);) shows that FD_WRITE is not present in fs_rights_base or fs_rights_inheriting on the wasi-with-libc path (but it is present in both on the wasi-without-libc path).

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

4 participants