Skip to content

Commit d6e17b9

Browse files
rootbeerrootbeer
committed
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 wasi-musl has lots of unused, different definitions of these constants). Rename the "read" and "write" properties in the `O` struct to match the wasm-wasi-musl header file property names (`RDONLY` and `WRONLY`), which also matches how Zig defines these fields for other libc. For no good reason that I can see, wasm-wasi-musl does not have the RDONLY and WRONLY bits adjacent in its open flags. So the `ACCMODE` `u2` that Zig uses for read/write/read-write in all the other POSIXy APIs can't be used with the wasm-wasi open flags. 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 ziglang#20890.
1 parent 53598e3 commit d6e17b9

File tree

6 files changed

+108
-63
lines changed

6 files changed

+108
-63
lines changed

‎lib/std/c.zig

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -674,6 +674,7 @@ pub const F = switch (native_os) {
674674
.linux => linux.F,
675675
.emscripten => emscripten.F,
676676
.wasi => struct {
677+
// Match `F_*` constants from lib/libc/include/wasm-wasi-musl/__header_fcntl.h
677678
pub const GETFD = 1;
678679
pub const SETFD = 2;
679680
pub const GETFL = 3;
@@ -1705,17 +1706,50 @@ pub const S = switch (native_os) {
17051706
.linux => linux.S,
17061707
.emscripten => emscripten.S,
17071708
.wasi => struct {
1708-
pub const IEXEC = @compileError("TODO audit this");
1709+
// Match `S_*` constants from lib/libc/include/wasm-wasi-musl/__mode_t.h
1710+
//
1711+
// Note, a bug in wasi-libc means both IFIFO and IFSOCK have the same value (0xc000).
1712+
// IFIFO should be 0x1000 (see https://github.com/WebAssembly/wasi-libc/pull/463), and
1713+
// 0x1000 is used by the wasi-libc bottom-half implementation. So we use 0x1000 here.
1714+
// But the actual bit values we get back from a wasi-libc may get masked with the wrong
1715+
// values, or may get mistranslated. So the FIFO and FSOCK file-type bits are not
1716+
// trustworthy.
17091717
pub const IFBLK = 0x6000;
17101718
pub const IFCHR = 0x2000;
17111719
pub const IFDIR = 0x4000;
1712-
pub const IFIFO = 0xc000;
1720+
pub const IFIFO = 0x1000; // also IFSOCK
17131721
pub const IFLNK = 0xa000;
17141722
pub const IFMT = IFBLK | IFCHR | IFDIR | IFIFO | IFLNK | IFREG | IFSOCK;
17151723
pub const IFREG = 0x8000;
1716-
/// There's no concept of UNIX domain socket but we define this value here
1717-
/// in order to line with other OSes.
1718-
pub const IFSOCK = 0x1;
1724+
pub const IFSOCK = 0xc000;
1725+
1726+
pub fn ISBLK(m: u32) bool {
1727+
return m & IFMT == IFBLK;
1728+
}
1729+
1730+
pub fn ISCHR(m: u32) bool {
1731+
return m & IFMT == IFCHR;
1732+
}
1733+
1734+
pub fn ISDIR(m: u32) bool {
1735+
return m & IFMT == IFDIR;
1736+
}
1737+
1738+
pub fn ISFIFO(m: u32) bool {
1739+
return m & IFMT == IFIFO;
1740+
}
1741+
1742+
pub fn ISLNK(m: u32) bool {
1743+
return m & IFMT == IFLNK;
1744+
}
1745+
1746+
pub fn ISREG(m: u32) bool {
1747+
return m & IFMT == IFREG;
1748+
}
1749+
1750+
pub fn ISSOCK(m: u32) bool {
1751+
return m & IFMT == IFSOCK;
1752+
}
17191753
},
17201754
.macos, .ios, .tvos, .watchos, .visionos => struct {
17211755
pub const IFMT = 0o170000;
@@ -6751,6 +6785,7 @@ pub const Stat = switch (native_os) {
67516785
},
67526786
.emscripten => emscripten.Stat,
67536787
.wasi => extern struct {
6788+
// Match wasi-libc's `struct stat` in lib/libc/include/wasm-wasi-musl/__struct_stat.h
67546789
dev: dev_t,
67556790
ino: ino_t,
67566791
nlink: nlink_t,
@@ -7451,17 +7486,18 @@ pub const AT = switch (native_os) {
74517486
pub const RECURSIVE = 0x8000;
74527487
},
74537488
.wasi => struct {
7454-
pub const SYMLINK_NOFOLLOW = 0x100;
7455-
pub const SYMLINK_FOLLOW = 0x400;
7456-
pub const REMOVEDIR: u32 = 0x4;
7489+
// Match `AT_*` constants in lib/libc/include/wasm-wasi-musl/__header_fcntl.h
7490+
pub const EACCESS = 0x0;
7491+
pub const SYMLINK_NOFOLLOW = 0x1;
7492+
pub const SYMLINK_FOLLOW = 0x2;
7493+
pub const REMOVEDIR = 0x4;
74577494
/// When linking libc, we follow their convention and use -2 for current working directory.
74587495
/// However, without libc, Zig does a different convention: it assumes the
74597496
/// current working directory is the first preopen. This behavior can be
74607497
/// overridden with a public function called `wasi_cwd` in the root source
74617498
/// file.
74627499
pub const FDCWD: fd_t = if (builtin.link_libc) -2 else 3;
74637500
},
7464-
74657501
else => void,
74667502
};
74677503

@@ -7490,6 +7526,7 @@ pub const O = switch (native_os) {
74907526
_: u9 = 0,
74917527
},
74927528
.wasi => packed struct(u32) {
7529+
// Match `O_*` bits from lib/libc/include/wasm-wasi-musl/__header_fcntl.h
74937530
APPEND: bool = false,
74947531
DSYNC: bool = false,
74957532
NONBLOCK: bool = false,
@@ -7502,10 +7539,19 @@ pub const O = switch (native_os) {
75027539
TRUNC: bool = false,
75037540
_16: u8 = 0,
75047541
NOFOLLOW: bool = false,
7542+
// Logical O_ACCMODE is the following 4 bits. Note O_SEARCH bit
7543+
// is between O_RDONLY and O_WRONLY, so can't easily reuse
7544+
// std.posix.ACCMODE in this struct like other platforms.
75057545
EXEC: bool = false,
7506-
read: bool = false,
7546+
// NOTE: on most platforms "ACCMODE defaults to .RDONLY", because its 0x0.
7547+
// RDONLY is not the default on this platform. (Unless we set this to true?)
7548+
RDONLY: bool = false,
75077549
SEARCH: bool = false,
7508-
write: bool = false,
7550+
WRONLY: bool = false,
7551+
7552+
// O_CLOEXEC, O_TTY_ININT, O_NOCTTY are 0 in wasi-musl, so they're silently
7553+
// ignored in C code. Thus no mapping in Zig.
7554+
75097555
_: u3 = 0,
75107556
},
75117557
.solaris, .illumos => packed struct(u32) {

‎lib/std/fs/Dir.zig

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -850,8 +850,8 @@ pub fn openFileZ(self: Dir, sub_path: [*:0]const u8, flags: File.OpenFlags) File
850850

851851
var os_flags: posix.O = switch (native_os) {
852852
.wasi => .{
853-
.read = flags.mode != .write_only,
854-
.write = flags.mode != .read_only,
853+
.RDONLY = flags.mode != .write_only,
854+
.WRONLY = flags.mode != .read_only,
855855
},
856856
else => .{
857857
.ACCMODE = switch (flags.mode) {
@@ -1295,17 +1295,10 @@ pub fn realpathZ(self: Dir, pathname: [*:0]const u8, out_buffer: []u8) RealPathE
12951295
return self.realpathW(pathname_w.span(), out_buffer);
12961296
}
12971297

1298-
const flags: posix.O = switch (native_os) {
1299-
.linux => .{
1300-
.NONBLOCK = true,
1301-
.CLOEXEC = true,
1302-
.PATH = true,
1303-
},
1304-
else => .{
1305-
.NONBLOCK = true,
1306-
.CLOEXEC = true,
1307-
},
1308-
};
1298+
var flags: posix.O = .{};
1299+
if (@hasField(posix.O, "NONBLOCK")) flags.NONBLOCK = true;
1300+
if (@hasField(posix.O, "CLOEXEC")) flags.CLOEXEC = true;
1301+
if (@hasField(posix.O, "PATH")) flags.PATH = true;
13091302

13101303
const fd = posix.openatZ(self.fd, pathname, flags, 0) catch |err| switch (err) {
13111304
error.FileLocksNotSupported => return error.Unexpected,
@@ -1520,7 +1513,7 @@ pub fn openDirZ(self: Dir, sub_path_c: [*:0]const u8, args: OpenOptions) OpenErr
15201513

15211514
var symlink_flags: posix.O = switch (native_os) {
15221515
.wasi => .{
1523-
.read = true,
1516+
.RDONLY = true,
15241517
.NOFOLLOW = args.no_follow,
15251518
.DIRECTORY = true,
15261519
},

‎lib/std/fs/test.zig

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -361,9 +361,12 @@ test "openDirAbsolute" {
361361
}
362362

363363
test "openDir cwd parent '..'" {
364-
if (native_os == .wasi) return error.SkipZigTest;
365-
366-
var dir = try fs.cwd().openDir("..", .{});
364+
var dir = fs.cwd().openDir("..", .{}) catch |err| {
365+
if (native_os == .wasi and err == error.AccessDenied) {
366+
return; // This is okay. WASI disallows escaping from the fs sandbox
367+
}
368+
return err;
369+
};
367370
defer dir.close();
368371
}
369372

@@ -1678,8 +1681,6 @@ test "read from locked file" {
16781681
}
16791682

16801683
test "walker" {
1681-
if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest;
1682-
16831684
var tmp = tmpDir(.{ .iterate = true });
16841685
defer tmp.cleanup();
16851686

@@ -1731,8 +1732,6 @@ test "walker" {
17311732
}
17321733

17331734
test "walker without fully iterating" {
1734-
if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest;
1735-
17361735
var tmp = tmpDir(.{ .iterate = true });
17371736
defer tmp.cleanup();
17381737

@@ -1754,8 +1753,6 @@ test "walker without fully iterating" {
17541753
}
17551754

17561755
test "'.' and '..' in fs.Dir functions" {
1757-
if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest;
1758-
17591756
if (native_os == .windows and builtin.cpu.arch == .aarch64) {
17601757
// https://github.com/ziglang/zig/issues/17134
17611758
return error.SkipZigTest;

‎lib/std/os/wasi.zig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! wasi_snapshot_preview1 spec available (in witx format) here:
22
//! * typenames -- https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/witx/typenames.witx
33
//! * module -- https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/witx/wasi_snapshot_preview1.witx
4-
//! Note that libc API does *not* go in this file. wasi libc API goes into std/c/wasi.zig instead.
4+
//! Note that libc API does *not* go in this file. wasi libc API goes into std/c.zig instead.
55
const builtin = @import("builtin");
66
const std = @import("std");
77
const assert = std.debug.assert;

‎lib/std/posix.zig

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1685,7 +1685,7 @@ pub fn openat(dir_fd: fd_t, file_path: []const u8, flags: O, mode: mode_t) OpenE
16851685
);
16861686
errdefer close(fd);
16871687

1688-
if (flags.write) {
1688+
if (flags.WRONLY) {
16891689
const info = try std.os.fstat_wasi(fd);
16901690
if (info.filetype == .DIRECTORY)
16911691
return error.IsDir;
@@ -1756,11 +1756,11 @@ fn openOptionsFromFlagsWasi(oflag: O) OpenError!WasiOpenOptions {
17561756
// Next, calculate the read/write rights to request, depending on the
17571757
// provided POSIX access mode
17581758
var rights: w.rights_t = .{};
1759-
if (oflag.read) {
1759+
if (oflag.RDONLY) {
17601760
rights.FD_READ = true;
17611761
rights.FD_READDIR = true;
17621762
}
1763-
if (oflag.write) {
1763+
if (oflag.WRONLY) {
17641764
rights.FD_DATASYNC = true;
17651765
rights.FD_WRITE = true;
17661766
rights.FD_ALLOCATE = true;
@@ -5105,10 +5105,10 @@ pub fn sysctl(
51055105
newlen: usize,
51065106
) SysCtlError!void {
51075107
if (native_os == .wasi) {
5108-
@panic("unsupported"); // TODO should be compile error, not panic
5108+
@compileError("sysctl not supported on WASI");
51095109
}
51105110
if (native_os == .haiku) {
5111-
@panic("unsupported"); // TODO should be compile error, not panic
5111+
@compileError("sysctl not supported on Haiku");
51125112
}
51135113

51145114
const name_len = cast(c_uint, name.len) orelse return error.NameTooLong;
@@ -5130,10 +5130,10 @@ pub fn sysctlbynameZ(
51305130
newlen: usize,
51315131
) SysCtlError!void {
51325132
if (native_os == .wasi) {
5133-
@panic("unsupported"); // TODO should be compile error, not panic
5133+
@compileError("sysctl not supported on WASI");
51345134
}
51355135
if (native_os == .haiku) {
5136-
@panic("unsupported"); // TODO should be compile error, not panic
5136+
@compileError("sysctl not supported on Haiku");
51375137
}
51385138

51395139
switch (errno(system.sysctlbyname(name, oldp, oldlenp, newp, newlen))) {

‎lib/std/posix/test.zig

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,19 @@ test "WTF-8 to WTF-16 conversion buffer overflows" {
3131
try expectError(error.NameTooLong, posix.chdirZ(input_wtf8));
3232
}
3333

34+
test "check WASI CWD" {
35+
if (native_os == .wasi) {
36+
if (std.options.wasiCwd() != 3) {
37+
@panic("WASI code that uses cwd (like this test) needs a preopen for cwd (add '--dir=.' to wasmtime)");
38+
}
39+
40+
if (!builtin.link_libc) {
41+
// WASI without-libc hardcodes fd 3 as the FDCWD token so it can be passed directly to WASI calls
42+
try expectEqual(3, posix.AT.FDCWD);
43+
}
44+
}
45+
}
46+
3447
test "chdir smoke test" {
3548
if (native_os == .wasi) return error.SkipZigTest;
3649

@@ -151,7 +164,6 @@ test "open smoke test" {
151164
}
152165

153166
test "openat smoke test" {
154-
if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest;
155167
if (native_os == .windows) return error.SkipZigTest;
156168

157169
// TODO verify file attributes using `fstatat`
@@ -200,10 +212,13 @@ test "openat smoke test" {
200212
}), mode);
201213
posix.close(fd);
202214

203-
// Try opening as file which should fail.
204-
try expectError(error.IsDir, posix.openat(tmp.dir.fd, "some_dir", CommonOpenFlags.lower(.{
205-
.ACCMODE = .RDWR,
206-
}), mode));
215+
// Try opening as file which should fail (skip on wasi+libc due to
216+
// https://github.com/bytecodealliance/wasmtime/issues/9054)
217+
if (native_os != .wasi or !builtin.link_libc) {
218+
try expectError(error.IsDir, posix.openat(tmp.dir.fd, "some_dir", CommonOpenFlags.lower(.{
219+
.ACCMODE = .RDWR,
220+
}), mode));
221+
}
207222
}
208223

209224
test "symlink with relative paths" {
@@ -366,8 +381,7 @@ test "fstatat" {
366381
defer file.close();
367382

368383
// now repeat but using `fstatat` instead
369-
const flags = if (native_os == .wasi) 0x0 else posix.AT.SYMLINK_NOFOLLOW;
370-
const statat = try posix.fstatat(tmp.dir.fd, "file.txt", flags);
384+
const statat = try posix.fstatat(tmp.dir.fd, "file.txt", posix.AT.SYMLINK_NOFOLLOW);
371385

372386
// s390x-linux does not have nanosecond precision for fstat(), but it does for fstatat(). As a
373387
// result, comparing the two structures is doomed to fail.
@@ -1308,22 +1322,17 @@ const CommonOpenFlags = packed struct {
13081322
NONBLOCK: bool = false,
13091323

13101324
pub fn lower(cof: CommonOpenFlags) posix.O {
1311-
if (native_os == .wasi) return .{
1312-
.read = cof.ACCMODE != .WRONLY,
1313-
.write = cof.ACCMODE != .RDONLY,
1314-
.CREAT = cof.CREAT,
1315-
.EXCL = cof.EXCL,
1316-
.DIRECTORY = cof.DIRECTORY,
1317-
.NONBLOCK = cof.NONBLOCK,
1318-
};
1319-
var result: posix.O = .{
1325+
var result: posix.O = if (native_os == .wasi) .{
1326+
.RDONLY = cof.ACCMODE != .WRONLY,
1327+
.WRONLY = cof.ACCMODE != .RDONLY,
1328+
} else .{
13201329
.ACCMODE = cof.ACCMODE,
1321-
.CREAT = cof.CREAT,
1322-
.EXCL = cof.EXCL,
1323-
.DIRECTORY = cof.DIRECTORY,
1324-
.NONBLOCK = cof.NONBLOCK,
1325-
.CLOEXEC = cof.CLOEXEC,
13261330
};
1331+
result.CREAT = cof.CREAT;
1332+
result.EXCL = cof.EXCL;
1333+
result.DIRECTORY = cof.DIRECTORY;
1334+
result.NONBLOCK = cof.NONBLOCK;
1335+
if (@hasField(posix.O, "CLOEXEC")) result.CLOEXEC = cof.CLOEXEC;
13271336
if (@hasField(posix.O, "LARGEFILE")) result.LARGEFILE = cof.LARGEFILE;
13281337
return result;
13291338
}

0 commit comments

Comments
 (0)