Skip to content

Commit 782231d

Browse files
authored
[FS] Fix partial write case when using _fd_write/doWritev/writev (#22261)
doWritev iterates through the iovs structure writing the data for each entry. The bug fixed here is that if a write does not write all data, the loop continues. If the subsequent write also accepts data, it has accepted data out of order. Fixes #22258
1 parent a26f20f commit 782231d

File tree

4 files changed

+114
-0
lines changed

4 files changed

+114
-0
lines changed

src/library_wasi.js

+4
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,10 @@ var WasiLibrary = {
223223
var curr = FS.write(stream, HEAP8, ptr, len, offset);
224224
if (curr < 0) return -1;
225225
ret += curr;
226+
if (curr < len) {
227+
// No more space to write.
228+
break;
229+
}
226230
if (typeof offset != 'undefined') {
227231
offset += curr;
228232
}

test/fs/test_writev_partial_write.c

+94
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/*
2+
* Copyright 2024 The Emscripten Authors. All rights reserved.
3+
* Emscripten is available under two separate licenses, the MIT license and the
4+
* University of Illinois/NCSA Open Source License. Both these licenses can be
5+
* found in the LICENSE file.
6+
*/
7+
8+
#include <assert.h>
9+
#include <fcntl.h>
10+
#include <stdio.h>
11+
#include <string.h>
12+
#include <sys/stat.h>
13+
#include <sys/uio.h>
14+
#include <unistd.h>
15+
#include <emscripten.h>
16+
17+
EM_JS_DEPS(main, "$ERRNO_CODES");
18+
19+
void setup(void) {
20+
EM_ASM(
21+
var device = FS.makedev(80, 0);
22+
FS.registerDevice(device, {
23+
write: function(stream, buffer, offset, length, pos) {
24+
if (length === 0) {
25+
return 0;
26+
}
27+
// Only do a partial write of one byte each time
28+
out('TO DEVICE: ' + JSON.stringify(String.fromCharCode(buffer[offset])));
29+
return 1;
30+
}
31+
});
32+
FS.mkdev('/device', device);
33+
);
34+
}
35+
36+
// Run the test with writev directly
37+
void test_writev_direct(void) {
38+
int fd = open("/device", O_WRONLY);
39+
assert(fd);
40+
struct iovec iovs[] = {{.iov_base = "ABC", .iov_len = 3},
41+
{.iov_base = "XYZ", .iov_len = 3}};
42+
struct iovec* iov = iovs;
43+
44+
size_t rem = iov[0].iov_len + iov[1].iov_len;
45+
int iovcnt = 2;
46+
ssize_t cnt;
47+
for (;;) {
48+
cnt = writev(fd, iov, iovcnt);
49+
assert(cnt >= 0);
50+
if (cnt == rem) {
51+
// All data written
52+
break;
53+
}
54+
rem -= cnt;
55+
if (cnt > iov[0].iov_len) {
56+
cnt -= iov[0].iov_len;
57+
iov++;
58+
iovcnt--;
59+
}
60+
iov[0].iov_base = (char*)iov[0].iov_base + cnt;
61+
iov[0].iov_len -= cnt;
62+
}
63+
64+
close(fd);
65+
}
66+
67+
// Run the test using stdio, this test is dependent on specific buffering used
68+
// and is included here as this code most closely matches the original bug
69+
// report
70+
void test_via_stdio(void) {
71+
// XXX: We open in append mode because truncating JS based files is not
72+
// supported yet. See #22262
73+
#ifdef WASMFS
74+
FILE* f = fopen("/device", "a");
75+
#else
76+
FILE* f = fopen("/device", "w");
77+
#endif
78+
assert(f);
79+
// Use line buffering. The bug is exposed with line buffering because with
80+
// line buffering two entries in __stdio_write's iovs are used.
81+
setvbuf(f, NULL, _IOLBF, 0);
82+
fputs("abc", f);
83+
fputs("\n", f);
84+
fflush(f);
85+
fclose(f);
86+
}
87+
88+
int main() {
89+
setup();
90+
test_writev_direct();
91+
test_via_stdio();
92+
printf("done\n");
93+
return 0;
94+
}

test/fs/test_writev_partial_write.out

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
TO DEVICE: "A"
2+
TO DEVICE: "B"
3+
TO DEVICE: "C"
4+
TO DEVICE: "X"
5+
TO DEVICE: "Y"
6+
TO DEVICE: "Z"
7+
TO DEVICE: "a"
8+
TO DEVICE: "b"
9+
TO DEVICE: "c"
10+
TO DEVICE: "\n"
11+
done

test/test_other.py

+5
Original file line numberDiff line numberDiff line change
@@ -15028,3 +15028,8 @@ def test_extra_struct_info(self):
1502815028
stderr = self.run_process([EMCC, test_file('hello_world.c'), '--js-library', test_file('other/test_extra_struct_info.js')], stderr=PIPE).stderr
1502915029
self.assertContained('(before) AF_INET=2', stderr)
1503015030
self.assertContained('(after) AF_INET=42', stderr)
15031+
15032+
@also_with_wasmfs
15033+
def test_fs_writev_partial_write(self):
15034+
self.set_setting('FORCE_FILESYSTEM')
15035+
self.do_run_in_out_file_test('fs/test_writev_partial_write.c')

0 commit comments

Comments
 (0)