Skip to content

Commit 1a7d709

Browse files
authored
[WasmFS] Allow backends to have short reads and writes (#16895)
We previously tried to prevent reads and writes beyond the ends of files by calling `getSize` on files in the backend-independent implementations of `read` and `write` and adjusting the length we passed to the backend accordingly. This works fine when files have meaningful lengths, reads and writes are guaranteed not to be shortened by the underlying APIs, and files cannot change size concurrently with the syscall execution, but those requirements are not always met. Instead, change the backend API to match the `pread` and `pwrite` syscall APIs, which allow accesses to be shorter than requested. Specifically, change the return types to be `ssize_t` so that the accessed lengths can be returned. Error codes are now returned as negative values, just like in the syscalls themselves. This change will also have performance benefits, especially for the OPFS backend, in which getting the size of a file is an asynchronous operation that is much slower than a small read or write.
1 parent 970998b commit 1a7d709

13 files changed

+134
-152
lines changed

src/library_wasmfs_js_file.js

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,28 +22,24 @@ mergeInto(LibraryManager.library, {
2222
// Initialize typed array on first write operation.
2323
wasmFS$JSMemoryFiles[file] = new Uint8Array(offset + length);
2424
}
25-
2625
if (offset + length > wasmFS$JSMemoryFiles[file].length) {
2726
// Resize the typed array if the length of the write buffer exceeds its capacity.
2827
var oldContents = wasmFS$JSMemoryFiles[file];
2928
var newContents = new Uint8Array(offset + length);
3029
newContents.set(oldContents);
3130
wasmFS$JSMemoryFiles[file] = newContents;
3231
}
33-
3432
wasmFS$JSMemoryFiles[file].set(HEAPU8.subarray(buffer, buffer + length), offset);
35-
return 0;
33+
return length;
3634
} catch (err) {
37-
return {{{ cDefine('EIO') }}};
35+
return -{{{ cDefine('EIO') }}};
3836
}
3937
},
4038
read: (file, buffer, length, offset) => {
41-
try {
42-
HEAPU8.set(wasmFS$JSMemoryFiles[file].subarray(offset, offset + length), buffer);
43-
return 0;
44-
} catch (err) {
45-
return {{{ cDefine('EIO') }}};
46-
}
39+
var fileData = wasmFS$JSMemoryFiles[file];
40+
length = Math.max(0, fileData.length - offset);
41+
HEAPU8.set(fileData.subarray(offset, offset + length), buffer);
42+
return length;
4743
},
4844
getSize: (file) => {
4945
return wasmFS$JSMemoryFiles[file] ? wasmFS$JSMemoryFiles[file].length : 0;

src/library_wasmfs_jsimpl.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ mergeInto(LibraryManager.library, {
8484
assert(wasmFS$backends[backend]);
8585
#endif
8686
var result = await wasmFS$backends[backend].write(file, buffer, length, offset);
87-
{{{ makeSetValue('result_p', 0, 'result', 'i64') }}};
87+
{{{ makeSetValue('result_p', 0, 'result', SIZE_TYPE) }}};
8888
_emscripten_proxy_finish(ctx);
8989
},
9090

@@ -95,7 +95,7 @@ mergeInto(LibraryManager.library, {
9595
assert(wasmFS$backends[backend]);
9696
#endif
9797
var result = await wasmFS$backends[backend].read(file, buffer, length, offset);
98-
{{{ makeSetValue('result_p', 0, 'result', 'i32') }}};
98+
{{{ makeSetValue('result_p', 0, 'result', SIZE_TYPE) }}};
9999
_emscripten_proxy_finish(ctx);
100100
},
101101

system/lib/wasmfs/backends/memory_backend.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,22 @@
1313

1414
namespace wasmfs {
1515

16-
__wasi_errno_t MemoryFile::write(const uint8_t* buf, size_t len, off_t offset) {
16+
ssize_t MemoryFile::write(const uint8_t* buf, size_t len, off_t offset) {
1717
if (offset + len > buffer.size()) {
1818
buffer.resize(offset + len);
1919
}
2020
std::memcpy(&buffer[offset], buf, len);
21-
22-
return __WASI_ERRNO_SUCCESS;
21+
return len;
2322
}
2423

25-
__wasi_errno_t MemoryFile::read(uint8_t* buf, size_t len, off_t offset) {
26-
// The caller should have already checked that the offset + len does
27-
// not exceed the file's size.
28-
assert(offset + len <= buffer.size());
24+
ssize_t MemoryFile::read(uint8_t* buf, size_t len, off_t offset) {
25+
if (offset >= buffer.size()) {
26+
len = 0;
27+
} else if (offset + len >= buffer.size()) {
28+
len = buffer.size() - offset;
29+
}
2930
std::memcpy(buf, &buffer[offset], len);
30-
31-
return __WASI_ERRNO_SUCCESS;
31+
return len;
3232
}
3333

3434
std::vector<MemoryDirectory::ChildEntry>::iterator

system/lib/wasmfs/backends/node_backend.cpp

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -163,25 +163,21 @@ class NodeFile : public DataFile {
163163

164164
void close() override { state.close(); }
165165

166-
__wasi_errno_t read(uint8_t* buf, size_t len, off_t offset) override {
166+
ssize_t read(uint8_t* buf, size_t len, off_t offset) override {
167167
uint32_t nread;
168168
if (auto err = _wasmfs_node_read(state.getFD(), buf, len, offset, &nread)) {
169-
return err;
169+
return -err;
170170
}
171-
// TODO: Add a way to report the actual bytes read. We currently assume the
172-
// available bytes can't change under us.
173-
return __WASI_ERRNO_SUCCESS;
171+
return nread;
174172
}
175173

176-
__wasi_errno_t write(const uint8_t* buf, size_t len, off_t offset) override {
174+
ssize_t write(const uint8_t* buf, size_t len, off_t offset) override {
177175
uint32_t nwritten;
178176
if (auto err =
179177
_wasmfs_node_write(state.getFD(), buf, len, offset, &nwritten)) {
180-
return err;
178+
return -err;
181179
}
182-
// TODO: Add a way to report the actual bytes written. We currently assume
183-
// the write cannot be short.
184-
return __WASI_ERRNO_SUCCESS;
180+
return nwritten;
185181
}
186182

187183
void flush() override {

system/lib/wasmfs/backends/opfs_backend.cpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,20 +131,16 @@ class OPFSFile : public DataFile {
131131
}
132132
}
133133

134-
__wasi_errno_t read(uint8_t* buf, size_t len, off_t offset) override {
134+
ssize_t read(uint8_t* buf, size_t len, off_t offset) override {
135135
uint32_t nread;
136136
proxy([&]() { nread = _wasmfs_opfs_read(accessID, buf, len, offset); });
137-
// TODO: Add a way to report the actual bytes read. We currently assume the
138-
// available bytes can't change under us.
139-
return __WASI_ERRNO_SUCCESS;
137+
return nread;
140138
}
141139

142-
__wasi_errno_t write(const uint8_t* buf, size_t len, off_t offset) override {
140+
ssize_t write(const uint8_t* buf, size_t len, off_t offset) override {
143141
uint32_t nwritten;
144142
proxy([&]() { nwritten = _wasmfs_opfs_write(accessID, buf, len, offset); });
145-
// TODO: Add a way to report the actual bytes written. We currently assume
146-
// the write cannot be short.
147-
return __WASI_ERRNO_SUCCESS;
143+
return nwritten;
148144
}
149145

150146
void flush() override {

system/lib/wasmfs/backends/proxied_file_backend.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,14 @@ class ProxiedFile : public DataFile {
3131
}
3232

3333
// Read and write operations are forwarded via the proxying mechanism.
34-
__wasi_errno_t write(const uint8_t* buf, size_t len, off_t offset) override {
35-
__wasi_errno_t result;
34+
ssize_t write(const uint8_t* buf, size_t len, off_t offset) override {
35+
ssize_t result;
3636
proxy([&]() { result = baseFile->locked().write(buf, len, offset); });
3737
return result;
3838
}
3939

40-
__wasi_errno_t read(uint8_t* buf, size_t len, off_t offset) override {
41-
__wasi_errno_t result;
40+
ssize_t read(uint8_t* buf, size_t len, off_t offset) override {
41+
ssize_t result;
4242
proxy([&]() { result = baseFile->locked().read(buf, len, offset); });
4343
return result;
4444
}

system/lib/wasmfs/file.h

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,12 @@ class DataFile : public File {
130130
virtual void open(oflags_t flags) = 0;
131131
virtual void close() = 0;
132132

133-
// TODO: Allow backends to override the version of read with multiple iovecs
134-
// to make it possible to implement pipes. See #16269.
135-
virtual __wasi_errno_t read(uint8_t* buf, size_t len, off_t offset) = 0;
136-
virtual __wasi_errno_t
137-
write(const uint8_t* buf, size_t len, off_t offset) = 0;
133+
// Return the accessed length or a negative error code. It is not an error to
134+
// access fewer bytes than requested.
135+
// TODO: Allow backends to override the version of read with
136+
// multiple iovecs to make it possible to implement pipes. See #16269.
137+
virtual ssize_t read(uint8_t* buf, size_t len, off_t offset) = 0;
138+
virtual ssize_t write(const uint8_t* buf, size_t len, off_t offset) = 0;
138139

139140
// Sets the size of the file to a specific size. If new space is allocated, it
140141
// should be zero-initialized (often backends have an efficient way to do this
@@ -283,10 +284,10 @@ class DataFile::Handle : public File::Handle {
283284
void open(oflags_t flags) { getFile()->open(flags); }
284285
void close() { getFile()->close(); }
285286

286-
__wasi_errno_t read(uint8_t* buf, size_t len, off_t offset) {
287+
ssize_t read(uint8_t* buf, size_t len, off_t offset) {
287288
return getFile()->read(buf, len, offset);
288289
}
289-
__wasi_errno_t write(const uint8_t* buf, size_t len, off_t offset) {
290+
ssize_t write(const uint8_t* buf, size_t len, off_t offset) {
290291
return getFile()->write(buf, len, offset);
291292
}
292293

system/lib/wasmfs/js_impl_backend.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,12 @@ class JSImplFile : public DataFile {
8080
void open(oflags_t) override {}
8181
void close() override {}
8282

83-
__wasi_errno_t write(const uint8_t* buf, size_t len, off_t offset) override {
83+
ssize_t write(const uint8_t* buf, size_t len, off_t offset) override {
8484
return _wasmfs_jsimpl_write(
8585
getBackendIndex(), getFileIndex(), buf, len, offset);
8686
}
8787

88-
__wasi_errno_t read(uint8_t* buf, size_t len, off_t offset) override {
89-
// The caller should have already checked that the offset + len does
90-
// not exceed the file's size.
91-
assert(offset + len <= getSize());
88+
ssize_t read(uint8_t* buf, size_t len, off_t offset) override {
9289
return _wasmfs_jsimpl_read(
9390
getBackendIndex(), getFileIndex(), buf, len, offset);
9491
}

system/lib/wasmfs/memory_backend.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ class MemoryFile : public DataFile {
2121

2222
void open(oflags_t) override {}
2323
void close() override {}
24-
__wasi_errno_t write(const uint8_t* buf, size_t len, off_t offset) override;
25-
__wasi_errno_t read(uint8_t* buf, size_t len, off_t offset) override;
24+
ssize_t write(const uint8_t* buf, size_t len, off_t offset) override;
25+
ssize_t read(uint8_t* buf, size_t len, off_t offset) override;
2626
void flush() override {}
2727
size_t getSize() override { return buffer.size(); }
2828
void setSize(size_t size) override { return buffer.resize(size); }

system/lib/wasmfs/pipe_backend.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,22 @@ class PipeFile : public DataFile {
2626
void open(oflags_t) override {}
2727
void close() override {}
2828

29-
__wasi_errno_t write(const uint8_t* buf, size_t len, off_t offset) override {
29+
ssize_t write(const uint8_t* buf, size_t len, off_t offset) override {
3030
for (size_t i = 0; i < len; i++) {
3131
data->push(buf[i]);
3232
}
33-
return __WASI_ERRNO_SUCCESS;
33+
return len;
3434
}
3535

36-
__wasi_errno_t read(uint8_t* buf, size_t len, off_t offset) override {
36+
ssize_t read(uint8_t* buf, size_t len, off_t offset) override {
3737
for (size_t i = 0; i < len; i++) {
3838
if (data->empty()) {
39-
return __WASI_ERRNO_INVAL;
39+
return i;
4040
}
4141
buf[i] = data->front();
4242
data->pop();
4343
}
44-
return __WASI_ERRNO_SUCCESS;
44+
return len;
4545
}
4646

4747
void flush() override {}

0 commit comments

Comments
 (0)