Skip to content

Implement WASI fd_pread and fd_pwrite #571

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 29, 2024
Merged

Implement WASI fd_pread and fd_pwrite #571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a229e83
Use error code for unsupported WASI clock ID
electrum Oct 11, 2024
1910a12
Use FileChannel in WASI
electrum Oct 10, 2024
767e930
Implement WASI fd_pread and fd_pwrite
electrum Oct 10, 2024
4938f79
Rename options method on WasiPreview1.Builder
electrum Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions docs/docs/usage/wasi.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,10 @@ For the most up-to-date info, and to see what specific functions we support, see
| fd_filestat_get | ✅ | |
| fd_filestat_set_size | ✅ | |
| fd_filestat_set_times | ✅ | |
| fd_pread | ❌ | |
| fd_pread | ✅ | |
| fd_prestat_dir_name | ✅ | |
| fd_prestat_get | ✅ | |
| fd_pwrite | ❌ | |
| fd_pwrite | 🟡 | Not supported for files opened in append mode. |
| fd_read | ✅ | |
| fd_readdir | ✅ | |
| fd_renumber | ✅ | |
Expand All @@ -177,4 +177,3 @@ For the most up-to-date info, and to see what specific functions we support, see
| sock_recv | ❌ | |
| sock_send | ❌ | |
| sock_shutdown | ✅ | |

3 changes: 1 addition & 2 deletions machine-tests/src/test/java/com/dylibso/chicory/testing/MachinesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@ public void shouldUseMachineCallOnlyForExport() throws Exception {
.withDirectory(target.toString(), target)
.withArguments(List.of("wat2wasm", path.toString(), "--output=-"))
.build();
var logger = new SystemLogger();
try (var wasi = WasiPreview1.builder().withLogger(logger).withOpts(wasiOpts).build()) {
try (var wasi = WasiPreview1.builder().withOptions(wasiOpts).build()) {
ImportValues imports = new ImportValues(wasi.toHostFunctions());
var wat2WasmModule = Parser.parse(new File("../wabt/src/main/resources/wat2wasm"));
var startFunctionIndex = new AtomicInteger();
Expand Down
2 changes: 1 addition & 1 deletion wabt/src/main/java/com/dylibso/chicory/wabt/Wast2Json.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public void process() {
try (var wasi =
WasiPreview1.builder()
.withLogger(logger)
.withOpts(wasiOpts.build())
.withOptions(wasiOpts.build())
.build()) {
ImportValues imports = new ImportValues(wasi.toHostFunctions());

Expand Down
2 changes: 1 addition & 1 deletion wabt/src/main/java/com/dylibso/chicory/wabt/Wat2Wasm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ private static byte[] parse(InputStream is, String fileName) {
.build();

try (var wasi =
WasiPreview1.builder().withLogger(logger).withOpts(wasiOpts).build()) {
WasiPreview1.builder().withLogger(logger).withOptions(wasiOpts).build()) {
ImportValues imports = new ImportValues(wasi.toHostFunctions());
Instance.builder(MODULE)
.withMachineFactory(Wat2WasmModuleMachineFactory::create)
Expand Down
7 changes: 1 addition & 6 deletions wasi/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,7 @@
<excludes>
<!-- Java does not support READ+APPEND -->
<exclude>tests/rust/testsuite/fd_flags_set.wasm</exclude>
<!-- fd_pread is not implemented -->
<exclude>tests/c/testsuite/pread-with-access.wasm</exclude>
<exclude>tests/rust/testsuite/dir_fd_op_failures.wasm</exclude>
<exclude>tests/rust/testsuite/file_pread_pwrite.wasm</exclude>
<!-- fd_pwrite is not implemented -->
<exclude>tests/c/testsuite/pwrite-with-access.wasm</exclude>
<!-- Linux and Jimfs always append with APPEND -->
<exclude>tests/c/testsuite/pwrite-with-append.wasm</exclude>
<!-- poll_oneoff is not implemented -->
<exclude>tests/rust/testsuite/poll_oneoff_stdio.wasm</exclude>
Expand Down
16 changes: 12 additions & 4 deletions wasi/src/main/java/com/dylibso/chicory/wasi/Descriptors.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.ByteBuffer;
import java.nio.channels.SeekableByteChannel;
import java.nio.channels.FileChannel;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -140,11 +140,11 @@ public Path path() {

static final class OpenFile implements Descriptor, Closeable, DataReader, DataWriter {
private final Path path;
private final SeekableByteChannel channel;
private final FileChannel channel;
private final int fdFlags;
private final long rights;

public OpenFile(Path path, SeekableByteChannel channel, int fdFlags, long rights) {
public OpenFile(Path path, FileChannel channel, int fdFlags, long rights) {
this.path = requireNonNull(path);
this.channel = requireNonNull(channel);
this.fdFlags = fdFlags;
Expand All @@ -155,7 +155,7 @@ public Path path() {
return path;
}

public SeekableByteChannel channel() {
public FileChannel channel() {
return channel;
}

Expand All @@ -172,11 +172,19 @@ public int read(byte[] data) throws IOException {
return channel.read(ByteBuffer.wrap(data));
}

public int read(byte[] data, long position) throws IOException {
return channel.read(ByteBuffer.wrap(data), position);
}

@Override
public int write(byte[] data) throws IOException {
return channel.write(ByteBuffer.wrap(data));
}

public int write(byte[] data, long position) throws IOException {
return channel.write(ByteBuffer.wrap(data), position);
}

@Override
public void close() throws IOException {
channel.close();
Expand Down
124 changes: 109 additions & 15 deletions wasi/src/main/java/com/dylibso/chicory/wasi/WasiPreview1.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public Builder withLogger(Logger logger) {
return this;
}

public Builder withOpts(WasiOptions opts) {
public Builder withOptions(WasiOptions opts) {
this.opts = opts;
return this;
}
Expand Down Expand Up @@ -181,9 +181,8 @@ public int clockResGet(Memory memory, int clockId, int resultPtr) {
memory.writeLong(resultPtr, 1L);
return wasiResult(WasiErrno.ESUCCESS);
case WasiClockId.PROCESS_CPUTIME_ID:
throw new WASMRuntimeException("We don't yet support clockid process_cputime_id");
case WasiClockId.THREAD_CPUTIME_ID:
throw new WASMRuntimeException("We don't yet support clockid thread_cputime_id");
return wasiResult(WasiErrno.ENOTSUP);
default:
return wasiResult(WasiErrno.EINVAL);
}
Expand All @@ -202,9 +201,8 @@ public int clockTimeGet(Memory memory, int clockId, long precision, int resultPt
memory.writeLong(resultPtr, System.nanoTime());
return wasiResult(WasiErrno.ESUCCESS);
case WasiClockId.PROCESS_CPUTIME_ID:
throw new WASMRuntimeException("We don't yet support clockid process_cputime_id");
case WasiClockId.THREAD_CPUTIME_ID:
throw new WASMRuntimeException("We don't yet support clockid thread_cputime_id");
return wasiResult(WasiErrno.ENOTSUP);
default:
return wasiResult(WasiErrno.EINVAL);
}
Expand Down Expand Up @@ -521,9 +519,58 @@ public int fdFilestatSetTimes(int fd, long accessTime, long modifiedTime, int fs
}

@WasmExport
public int fdPread(int fd, int iovs, int iovsLen, long offset, int nreadPtr) {
public int fdPread(Memory memory, int fd, int iovs, int iovsLen, long offset, int nreadPtr) {
logger.tracef("fd_pread: [%s, %s, %s, %s, %s]", fd, iovs, iovsLen, offset, nreadPtr);
throw new WASMRuntimeException("We don't yet support this WASI call: fd_pread");

if (offset < 0) {
return wasiResult(WasiErrno.EINVAL);
}

var descriptor = descriptors.get(fd);
if (descriptor == null) {
return wasiResult(WasiErrno.EBADF);
}

if (descriptor instanceof InStream) {
return wasiResult(WasiErrno.ESPIPE);
}
if (descriptor instanceof OutStream) {
return wasiResult(WasiErrno.EBADF);
}
if (descriptor instanceof Directory) {
return wasiResult(WasiErrno.EISDIR);
}
if (!(descriptor instanceof OpenFile)) {
throw unhandledDescriptor(descriptor);
}
var file = (OpenFile) descriptor;

int totalRead = 0;
for (var i = 0; i < iovsLen; i++) {
int base = iovs + (i * 8);
int iovBase = memory.readInt(base);
var iovLen = memory.readInt(base + 4);
try {
byte[] data = new byte[iovLen];
int read = file.read(data, offset);
if (read < 0) {
break;
}
memory.write(iovBase, data, 0, read);
offset += read;
totalRead += read;
if (read < iovLen) {
break;
}
} catch (NonReadableChannelException e) {
return wasiResult(WasiErrno.ENOTCAPABLE);
} catch (IOException e) {
return wasiResult(WasiErrno.EIO);
}
}

memory.writeI32(nreadPtr, totalRead);
return wasiResult(WasiErrno.ESUCCESS);
}

@WasmExport
Expand Down Expand Up @@ -566,9 +613,60 @@ public int fdPrestatGet(Memory memory, int fd, int buf) {
}

@WasmExport
public int fdPwrite(int fd, int iovs, int iovsLen, long offset, int nwrittenPtr) {
public int fdPwrite(
Memory memory, int fd, int iovs, int iovsLen, long offset, int nwrittenPtr) {
logger.tracef("fd_pwrite: [%s, %s, %s, %s, %s]", fd, iovs, iovsLen, offset, nwrittenPtr);
throw new WASMRuntimeException("We don't yet support this WASI call: fd_pwrite");

if (offset < 0) {
return wasiResult(WasiErrno.EINVAL);
}

var descriptor = descriptors.get(fd);
if (descriptor == null) {
return wasiResult(WasiErrno.EBADF);
}

if (descriptor instanceof InStream) {
return wasiResult(WasiErrno.EBADF);
}
if (descriptor instanceof OutStream) {
return wasiResult(WasiErrno.ESPIPE);
}
if (descriptor instanceof Directory) {
return wasiResult(WasiErrno.EISDIR);
}

if (!(descriptor instanceof OpenFile)) {
throw unhandledDescriptor(descriptor);
}
var file = (OpenFile) descriptor;

if (flagSet(file.fdFlags(), WasiFdFlags.APPEND)) {
return wasiResult(WasiErrno.ENOTSUP);
}

var totalWritten = 0;
for (var i = 0; i < iovsLen; i++) {
var base = iovs + (i * 8);
var iovBase = memory.readInt(base);
var iovLen = memory.readInt(base + 4);
var data = memory.readBytes(iovBase, iovLen);
try {
int written = file.write(data, offset);
offset += written;
totalWritten += written;
if (written < iovLen) {
break;
}
} catch (NonWritableChannelException e) {
return wasiResult(WasiErrno.ENOTCAPABLE);
} catch (IOException e) {
return wasiResult(WasiErrno.EIO);
}
}

memory.writeI32(nwrittenPtr, totalWritten);
return wasiResult(WasiErrno.ESUCCESS);
}

@WasmExport
Expand Down Expand Up @@ -1048,7 +1146,7 @@ public int pathOpen(

int fd;
try {
SeekableByteChannel channel = Files.newByteChannel(path, openOptions);
FileChannel channel = FileChannel.open(path, openOptions);
fd = descriptors.allocate(new OpenFile(path, channel, fdFlags, rightsBase));
} catch (FileAlreadyExistsException e) {
return wasiResult(WasiErrno.EEXIST);
Expand Down Expand Up @@ -1349,13 +1447,9 @@ private WasiErrno fileSync(int fd, boolean metadata) {
throw unhandledDescriptor(descriptor);
}
var channel = ((OpenFile) descriptor).channel();
if (!(channel instanceof FileChannel)) {
return WasiErrno.ENOTSUP;
}
var fileChannel = (FileChannel) channel;

try {
fileChannel.force(metadata);
channel.force(metadata);
} catch (IOException e) {
return WasiErrno.EIO;
}
Expand Down
46 changes: 46 additions & 0 deletions wasi/src/test/java/com/dylibso/chicory/wasi/WasiPreview1Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.dylibso.chicory.wasi;

import static com.dylibso.chicory.wasi.Files.copyDirectory;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -14,8 +15,14 @@
import com.dylibso.chicory.wasm.Module;
import com.dylibso.chicory.wasm.Parser;
import com.dylibso.chicory.wasm.types.MemoryLimits;
import com.google.common.jimfs.Configuration;
import com.google.common.jimfs.Jimfs;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.nio.file.FileSystem;
import java.nio.file.Path;
import java.util.List;
import java.util.Random;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -226,4 +233,43 @@ public void wasiRandom() {
assertArrayEquals(first, memory.readBytes(0, 123_456));
assertArrayEquals(second, memory.readBytes(222_222, 87_654));
}

@Test
public void wasiPositionedWriteWithAppendShouldFail() throws IOException {
try (FileSystem fs =
Jimfs.newFileSystem(
Configuration.unix().toBuilder().setAttributeViews("unix").build())) {

var dir = "fs-tests.dir";
Path source = new File("../wasi-testsuite/tests/c/testsuite").toPath().resolve(dir);
Path target = fs.getPath(dir);
copyDirectory(source, target);

var options = WasiOptions.builder().withDirectory(target.toString(), target).build();

try (var wasi = WasiPreview1.builder().withOptions(options).build()) {
var memory = new Memory(new MemoryLimits(1));

int fdPtr = 0;
int result =
wasi.pathOpen(
memory,
3,
WasiLookupFlags.SYMLINK_FOLLOW,
"pwrite.cleanup",
WasiOpenFlags.CREAT,
0,
0,
WasiFdFlags.APPEND,
fdPtr);
assertEquals(WasiErrno.ESUCCESS.value(), result);

int fd = memory.readInt(fdPtr);
assertEquals(4, fd);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to forget what is this 4 in a couple of hours 😅
But it's an improvement over my version of it 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the next open descriptor (0-2 are stdin/out/err, 3 is the pre-opened directory). We don't actually need to assert this, but doesn't hurt to validate anyway.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I know because I wrote the test with the debugger on my side 😅 but I'm going to forget about it quickly.


result = wasi.fdPwrite(memory, fd, 0, 0, 0, 0);
assertEquals(WasiErrno.ENOTSUP.value(), result);
}
}
}
}
Loading