From 138d30bb4763a5831ace21d916bde669b4ff5efb Mon Sep 17 00:00:00 2001 From: Pat Tullmann Date: Mon, 9 Sep 2024 19:51:15 -0700 Subject: [PATCH] wasi: fix wasm-wasi-musl constants Zig's copy of the `SYMLINK_{NO,}FOLLOW` constants from wasi-musl was wrong, as were the `IFIFO` and `IFSOCK` file type flags. Fix these up, and add comments pointing to exactly where they come from (as the wasi-musl source has lots of unused, different definitions of these constants). Add tests for the Zig convention that WASM preopen 3 is the current working directory. This is true for WASM with or without libc. Enable several fs and posix tests that are now passing (not necessarily because of this change) on wasm targets. Fixes #20890. --- lib/std/c.zig | 50 ++++++++++++++++++++++++++++++++++-------- lib/std/fs/Dir.zig | 15 ++++--------- lib/std/fs/test.zig | 15 +++++-------- lib/std/os/wasi.zig | 2 +- lib/std/posix.zig | 8 +++---- lib/std/posix/test.zig | 47 +++++++++++++++++++++++---------------- 6 files changed, 84 insertions(+), 53 deletions(-) diff --git a/lib/std/c.zig b/lib/std/c.zig index d3418c7d8a07f8a8286b00817e3ff41ee2d0ca23..a366c0ca63ae94e088145852a43e2b405b44ae09 100644 --- a/lib/std/c.zig +++ b/lib/std/c.zig @@ -692,6 +692,7 @@ pub const F = switch (native_os) { .linux => linux.F, .emscripten => emscripten.F, .wasi => struct { + // Match `F_*` constants from lib/libc/include/wasm-wasi-musl/__header_fcntl.h pub const GETFD = 1; pub const SETFD = 2; pub const GETFL = 3; @@ -1723,17 +1724,43 @@ pub const S = switch (native_os) { .linux => linux.S, .emscripten => emscripten.S, .wasi => struct { - pub const IEXEC = @compileError("TODO audit this"); + // Match `S_*` constants from lib/libc/include/wasm-wasi-musl/__mode_t.h pub const IFBLK = 0x6000; pub const IFCHR = 0x2000; pub const IFDIR = 0x4000; - pub const IFIFO = 0xc000; + pub const IFIFO = 0x1000; pub const IFLNK = 0xa000; pub const IFMT = IFBLK | IFCHR | IFDIR | IFIFO | IFLNK | IFREG | IFSOCK; pub const IFREG = 0x8000; - /// There's no concept of UNIX domain socket but we define this value here - /// in order to line with other OSes. - pub const IFSOCK = 0x1; + pub const IFSOCK = 0xc000; + + pub fn ISBLK(m: u32) bool { + return m & IFMT == IFBLK; + } + + pub fn ISCHR(m: u32) bool { + return m & IFMT == IFCHR; + } + + pub fn ISDIR(m: u32) bool { + return m & IFMT == IFDIR; + } + + pub fn ISFIFO(m: u32) bool { + return m & IFMT == IFIFO; + } + + pub fn ISLNK(m: u32) bool { + return m & IFMT == IFLNK; + } + + pub fn ISREG(m: u32) bool { + return m & IFMT == IFREG; + } + + pub fn ISSOCK(m: u32) bool { + return m & IFMT == IFSOCK; + } }, .macos, .ios, .tvos, .watchos, .visionos => struct { pub const IFMT = 0o170000; @@ -6802,6 +6829,7 @@ pub const Stat = switch (native_os) { }, .emscripten => emscripten.Stat, .wasi => extern struct { + // Match wasi-libc's `struct stat` in lib/libc/include/wasm-wasi-musl/__struct_stat.h dev: dev_t, ino: ino_t, nlink: nlink_t, @@ -7502,9 +7530,11 @@ pub const AT = switch (native_os) { pub const RECURSIVE = 0x8000; }, .wasi => struct { - pub const SYMLINK_NOFOLLOW = 0x100; - pub const SYMLINK_FOLLOW = 0x400; - pub const REMOVEDIR: u32 = 0x4; + // Match `AT_*` constants in lib/libc/include/wasm-wasi-musl/__header_fcntl.h + pub const EACCESS = 0x0; + pub const SYMLINK_NOFOLLOW = 0x1; + pub const SYMLINK_FOLLOW = 0x2; + pub const REMOVEDIR = 0x4; /// When linking libc, we follow their convention and use -2 for current working directory. /// However, without libc, Zig does a different convention: it assumes the /// current working directory is the first preopen. This behavior can be @@ -7512,7 +7542,6 @@ pub const AT = switch (native_os) { /// file. pub const FDCWD: fd_t = if (builtin.link_libc) -2 else 3; }, - else => void, }; @@ -7541,6 +7570,7 @@ pub const O = switch (native_os) { _: u9 = 0, }, .wasi => packed struct(u32) { + // Match `O_*` bits from lib/libc/include/wasm-wasi-musl/__header_fcntl.h APPEND: bool = false, DSYNC: bool = false, NONBLOCK: bool = false, @@ -7557,6 +7587,8 @@ pub const O = switch (native_os) { read: bool = false, SEARCH: bool = false, write: bool = false, + // O_CLOEXEC, O_TTY_ININT, O_NOCTTY are 0 in wasi-musl, so they're silently + // ignored in C code. Thus no mapping in Zig. _: u3 = 0, }, .solaris, .illumos => packed struct(u32) { diff --git a/lib/std/fs/Dir.zig b/lib/std/fs/Dir.zig index d0776ca3088c71344e6c8f8cf84072b4d89a87ae..eda4067bebe8283bd832f0b56e3c9112d85ffb85 100644 --- a/lib/std/fs/Dir.zig +++ b/lib/std/fs/Dir.zig @@ -1295,17 +1295,10 @@ pub fn realpathZ(self: Dir, pathname: [*:0]const u8, out_buffer: []u8) RealPathE return self.realpathW(pathname_w.span(), out_buffer); } - const flags: posix.O = switch (native_os) { - .linux => .{ - .NONBLOCK = true, - .CLOEXEC = true, - .PATH = true, - }, - else => .{ - .NONBLOCK = true, - .CLOEXEC = true, - }, - }; + var flags: posix.O = .{}; + if (@hasField(posix.O, "NONBLOCK")) flags.NONBLOCK = true; + if (@hasField(posix.O, "CLOEXEC")) flags.CLOEXEC = true; + if (@hasField(posix.O, "PATH")) flags.PATH = true; const fd = posix.openatZ(self.fd, pathname, flags, 0) catch |err| switch (err) { error.FileLocksNotSupported => return error.Unexpected, diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index 618323dce364e0b32069a2af801850c4f418a902..98fef1b2ce147b239fb5ddb42e445764d195f18c 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -361,9 +361,12 @@ test "openDirAbsolute" { } test "openDir cwd parent '..'" { - if (native_os == .wasi) return error.SkipZigTest; - - var dir = try fs.cwd().openDir("..", .{}); + var dir = fs.cwd().openDir("..", .{}) catch |err| { + if (native_os == .wasi and err == error.AccessDenied) { + return; // This is okay. WASI disallows escaping from the fs sandbox + } + return err; + }; defer dir.close(); } @@ -1678,8 +1681,6 @@ test "read from locked file" { } test "walker" { - if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest; - var tmp = tmpDir(.{ .iterate = true }); defer tmp.cleanup(); @@ -1731,8 +1732,6 @@ test "walker" { } test "walker without fully iterating" { - if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest; - var tmp = tmpDir(.{ .iterate = true }); defer tmp.cleanup(); @@ -1754,8 +1753,6 @@ test "walker without fully iterating" { } test "'.' and '..' in fs.Dir functions" { - if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest; - if (native_os == .windows and builtin.cpu.arch == .aarch64) { // https://github.com/ziglang/zig/issues/17134 return error.SkipZigTest; diff --git a/lib/std/os/wasi.zig b/lib/std/os/wasi.zig index 5ffde3c1f67af6422eaee81d6db403d4e5ec1679..caef010683b99f36d482901a6b2d09e86b702ab3 100644 --- a/lib/std/os/wasi.zig +++ b/lib/std/os/wasi.zig @@ -1,7 +1,7 @@ //! wasi_snapshot_preview1 spec available (in witx format) here: //! * typenames -- https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/witx/typenames.witx //! * module -- https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/witx/wasi_snapshot_preview1.witx -//! Note that libc API does *not* go in this file. wasi libc API goes into std/c/wasi.zig instead. +//! Note that libc API does *not* go in this file. wasi libc API goes into std/c.zig instead. const builtin = @import("builtin"); const std = @import("std"); const assert = std.debug.assert; diff --git a/lib/std/posix.zig b/lib/std/posix.zig index 924969195018f39a3f7e544058dc809ade6bbd59..f7ee1e7ddc89e2380fda4d44bc38fdcf7143b811 100644 --- a/lib/std/posix.zig +++ b/lib/std/posix.zig @@ -5143,10 +5143,10 @@ pub fn sysctl( newlen: usize, ) SysCtlError!void { if (native_os == .wasi) { - @panic("unsupported"); // TODO should be compile error, not panic + @compileError("sysctl not supported on WASI"); } if (native_os == .haiku) { - @panic("unsupported"); // TODO should be compile error, not panic + @compileError("sysctl not supported on Haiku"); } const name_len = cast(c_uint, name.len) orelse return error.NameTooLong; @@ -5168,10 +5168,10 @@ pub fn sysctlbynameZ( newlen: usize, ) SysCtlError!void { if (native_os == .wasi) { - @panic("unsupported"); // TODO should be compile error, not panic + @compileError("sysctl not supported on WASI"); } if (native_os == .haiku) { - @panic("unsupported"); // TODO should be compile error, not panic + @compileError("sysctl not supported on Haiku"); } switch (errno(system.sysctlbyname(name, oldp, oldlenp, newp, newlen))) { diff --git a/lib/std/posix/test.zig b/lib/std/posix/test.zig index 89346e66fe8276aae1ad6c18692d2673f0cd5083..3254faedfbc032d080abe22a35cb1fa2bb839af5 100644 --- a/lib/std/posix/test.zig +++ b/lib/std/posix/test.zig @@ -31,6 +31,19 @@ test "WTF-8 to WTF-16 conversion buffer overflows" { try expectError(error.NameTooLong, posix.chdirZ(input_wtf8)); } +test "check WASI CWD" { + if (native_os == .wasi) { + if (std.options.wasiCwd() != 3) { + @panic("WASI code that uses cwd (like this test) needs a preopen for cwd (add '--dir=.' to wasmtime)"); + } + + if (!builtin.link_libc) { + // WASI without-libc hardcodes fd 3 as the FDCWD token so it can be passed directly to WASI calls + try expectEqual(3, posix.AT.FDCWD); + } + } +} + test "chdir smoke test" { if (native_os == .wasi) return error.SkipZigTest; @@ -151,7 +164,6 @@ test "open smoke test" { } test "openat smoke test" { - if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest; if (native_os == .windows) return error.SkipZigTest; // TODO verify file attributes using `fstatat` @@ -200,10 +212,13 @@ test "openat smoke test" { }), mode); posix.close(fd); - // Try opening as file which should fail. - try expectError(error.IsDir, posix.openat(tmp.dir.fd, "some_dir", CommonOpenFlags.lower(.{ - .ACCMODE = .RDWR, - }), mode)); + // Try opening as file which should fail (skip on wasi+libc due to + // https://github.com/bytecodealliance/wasmtime/issues/9054) + if (native_os != .wasi or !builtin.link_libc) { + try expectError(error.IsDir, posix.openat(tmp.dir.fd, "some_dir", CommonOpenFlags.lower(.{ + .ACCMODE = .RDWR, + }), mode)); + } } test "symlink with relative paths" { @@ -366,8 +381,7 @@ test "fstatat" { defer file.close(); // now repeat but using `fstatat` instead - const flags = if (native_os == .wasi) 0x0 else posix.AT.SYMLINK_NOFOLLOW; - const statat = try posix.fstatat(tmp.dir.fd, "file.txt", flags); + const statat = try posix.fstatat(tmp.dir.fd, "file.txt", posix.AT.SYMLINK_NOFOLLOW); // s390x-linux does not have nanosecond precision for fstat(), but it does for fstatat(). As a // result, comparing the two structures is doomed to fail. @@ -1308,22 +1322,17 @@ const CommonOpenFlags = packed struct { NONBLOCK: bool = false, pub fn lower(cof: CommonOpenFlags) posix.O { - if (native_os == .wasi) return .{ + var result: posix.O = if (native_os == .wasi) .{ .read = cof.ACCMODE != .WRONLY, .write = cof.ACCMODE != .RDONLY, - .CREAT = cof.CREAT, - .EXCL = cof.EXCL, - .DIRECTORY = cof.DIRECTORY, - .NONBLOCK = cof.NONBLOCK, - }; - var result: posix.O = .{ + } else .{ .ACCMODE = cof.ACCMODE, - .CREAT = cof.CREAT, - .EXCL = cof.EXCL, - .DIRECTORY = cof.DIRECTORY, - .NONBLOCK = cof.NONBLOCK, - .CLOEXEC = cof.CLOEXEC, }; + result.CREAT = cof.CREAT; + result.EXCL = cof.EXCL; + result.DIRECTORY = cof.DIRECTORY; + result.NONBLOCK = cof.NONBLOCK; + if (@hasField(posix.O, "CLOEXEC")) result.CLOEXEC = cof.CLOEXEC; if (@hasField(posix.O, "LARGEFILE")) result.LARGEFILE = cof.LARGEFILE; return result; } -- 2.54.0