Skip to content

Commit f683ada

Browse files
alexcrichtonalexcrichton
authored
Improve stdio lazy init (#650)
Ensure it's always initialized in descriptor table functions instead of only having initialization in some functions. Fixes `fstat`, for example, on 0/1/2. The previous initialization functions documented that the `file_handle` wasn't ever touched so it could stay uninitialized, but this isn't true given other functions throughout wasi-libc. Eventually I'd like to have a custom descriptor type just for stdio so stdio doesn't have a "null" `file_handle` field but for now that's the easiest way to slot this in. While I'm getting other tests working in Python this is in theory enough to get up off the ground. More functions will need to grow tests for `__handle == 0` to handle stdio in the future, but I plan on handling them as they arise.
1 parent 0bddaa0 commit f683ada

File tree

10 files changed

+149
-97
lines changed

10 files changed

+149
-97
lines changed

‎libc-bottom-half/cloudlibc/src/libc/fcntl/posix_fadvise.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) {
3333
errno = EBADF;
3434
return EBADF;
3535
}
36+
if (file_handle.__handle == 0) {
37+
errno = EBADF;
38+
return EBADF;
39+
}
3640

3741
filesystem_error_code_t error_code;
3842
filesystem_advice_t fs_advice;

‎libc-bottom-half/cloudlibc/src/libc/sys/stat/fstat.c‎

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,11 @@
1616
#include "stat_impl.h"
1717

1818
#ifdef __wasilibc_use_wasip2
19-
int fstat(int fildes, struct stat *buf) {
20-
// Translate the file descriptor to an internal handle
21-
filesystem_borrow_descriptor_t file_handle;
22-
if (!fd_to_file_handle_allow_open(fildes, &file_handle)) {
23-
errno = EBADF;
24-
return -1;
25-
}
26-
19+
static int fstat_file(filesystem_borrow_descriptor_t file, struct stat *buf) {
2720
// Get the metadata hash for the file descriptor
2821
filesystem_metadata_hash_value_t metadata;
2922
filesystem_error_code_t error;
30-
if (!filesystem_method_descriptor_metadata_hash(file_handle,
23+
if (!filesystem_method_descriptor_metadata_hash(file,
3124
&metadata,
3225
&error)) {
3326
translate_error(error);
@@ -36,7 +29,7 @@ int fstat(int fildes, struct stat *buf) {
3629

3730
// Get the file metadata
3831
filesystem_descriptor_stat_t internal_stat;
39-
bool stat_result = filesystem_method_descriptor_stat(file_handle,
32+
bool stat_result = filesystem_method_descriptor_stat(file,
4033
&internal_stat,
4134
&error);
4235
if (!stat_result) {
@@ -48,6 +41,37 @@ int fstat(int fildes, struct stat *buf) {
4841
to_public_stat(&metadata, &internal_stat, buf);
4942
return 0;
5043
}
44+
45+
int fstat(int fildes, struct stat *buf) {
46+
// Translate the file descriptor to an internal handle
47+
descriptor_table_entry_t *entry;
48+
if (!descriptor_table_get_ref(fildes, &entry)) {
49+
errno = EBADF;
50+
return -1;
51+
}
52+
53+
int st_mode = 0;
54+
55+
switch (entry->tag) {
56+
case DESCRIPTOR_TABLE_ENTRY_FILE_HANDLE:
57+
return fstat_file(entry->file.file_handle, buf);
58+
case DESCRIPTOR_TABLE_ENTRY_FILE_STREAM:
59+
if (entry->stream.file_info.file_handle.__handle != 0)
60+
return fstat_file(entry->stream.file_info.file_handle, buf);
61+
break;
62+
case DESCRIPTOR_TABLE_ENTRY_TCP_SOCKET:
63+
case DESCRIPTOR_TABLE_ENTRY_UDP_SOCKET:
64+
st_mode = S_IFSOCK;
65+
break;
66+
default:
67+
errno = EBADF;
68+
return -1;
69+
}
70+
71+
memset(buf, 0, sizeof(*buf));
72+
buf->st_mode = st_mode;
73+
return 0;
74+
}
5175
#else
5276
int fstat(int fildes, struct stat *buf) {
5377
__wasi_filestat_t internal_stat;

‎libc-bottom-half/cloudlibc/src/libc/unistd/read.c‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,6 @@ ssize_t read(int fildes, void *buf, size_t nbyte) {
1717
#ifdef __wasilibc_use_wasip2
1818
bool ok = false;
1919

20-
// Check for stdin
21-
if (fildes == 0) {
22-
if (!init_stdin()) {
23-
errno = EINVAL;
24-
return -1;
25-
}
26-
}
27-
2820
// Translate the file descriptor to an internal handle
2921
descriptor_table_entry_t* entry = 0;
3022
if (!descriptor_table_get_ref(fildes, &entry)) {

‎libc-bottom-half/cloudlibc/src/libc/unistd/write.c‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,20 +24,6 @@ ssize_t write(int fildes, const void *buf, size_t nbyte) {
2424
filesystem_error_code_t error_code;
2525
descriptor_table_entry_t* entry = 0;
2626

27-
// Check for stdout/stderr
28-
if (fildes == 1) {
29-
if (!init_stdout()) {
30-
errno = EINVAL;
31-
return -1;
32-
}
33-
}
34-
else if (fildes == 2) {
35-
if (!init_stderr()) {
36-
errno = EINVAL;
37-
return -1;
38-
}
39-
}
40-
4127
// Translate the file descriptor to an internal handle
4228
if (!descriptor_table_get_ref(fildes, &entry)) {
4329
errno = EBADF;

‎libc-bottom-half/headers/private/wasi/file_utils.h‎

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -41,66 +41,6 @@ static bool fd_to_file_handle_allow_open(int fd, filesystem_borrow_descriptor_t*
4141
return true;
4242
}
4343

44-
// The following three functions are used for lazily initializing stdin/stdout/stderr in
45-
// the descriptor table. These file descriptors never need to be removed from the
46-
// descriptor table.
47-
static bool init_stdin() {
48-
descriptor_table_entry_t entry;
49-
descriptor_table_entry_t *entry_p;
50-
51-
if (!descriptor_table_get_ref(0, &entry_p)) {
52-
entry.tag = DESCRIPTOR_TABLE_ENTRY_FILE_STREAM;
53-
entry.stream.read_stream = streams_borrow_input_stream(stdin_get_stdin());
54-
entry.stream.offset = 0;
55-
entry.stream.read_pollable.__handle = 0;
56-
entry.stream.write_pollable.__handle = 0;
57-
entry.stream.file_info.readable = true;
58-
entry.stream.file_info.writable = false;
59-
// entry.stream.file_info.file_handle is uninitialized, but it will never be used
60-
61-
return descriptor_table_update(0, entry);
62-
}
63-
return true;
64-
}
65-
66-
static bool init_stdout() {
67-
descriptor_table_entry_t entry;
68-
descriptor_table_entry_t *entry_p;
69-
70-
if (!descriptor_table_get_ref(1, &entry_p)) {
71-
entry.tag = DESCRIPTOR_TABLE_ENTRY_FILE_STREAM;
72-
entry.stream.write_stream = streams_borrow_output_stream(stdout_get_stdout());
73-
entry.stream.offset = 0;
74-
entry.stream.read_pollable.__handle = 0;
75-
entry.stream.write_pollable.__handle = 0;
76-
entry.stream.file_info.readable = false;
77-
entry.stream.file_info.writable = true;
78-
// entry.stream.file_info.file_handle is uninitialized, but it will never be used
79-
80-
return descriptor_table_update(1, entry);
81-
}
82-
return true;
83-
}
84-
85-
static bool init_stderr() {
86-
descriptor_table_entry_t entry;
87-
descriptor_table_entry_t *entry_p;
88-
89-
if (!descriptor_table_get_ref(2, &entry_p)) {
90-
entry.tag = DESCRIPTOR_TABLE_ENTRY_FILE_STREAM;
91-
entry.stream.write_stream = streams_borrow_output_stream(stderr_get_stderr());
92-
entry.stream.offset = 0;
93-
entry.stream.read_pollable.__handle = 0;
94-
entry.stream.write_pollable.__handle = 0;
95-
entry.stream.file_info.readable = false;
96-
entry.stream.file_info.writable = true;
97-
// entry.stream.file_info.file_handle is uninitialized, but it will never be used
98-
99-
return descriptor_table_update(2, entry);
100-
}
101-
return true;
102-
}
103-
10444
static unsigned dir_entry_type_to_d_type(filesystem_descriptor_type_t ty) {
10545
switch(ty) {
10646
case FILESYSTEM_DESCRIPTOR_TYPE_UNKNOWN:

‎libc-bottom-half/sources/descriptor_table.c‎

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -205,23 +205,71 @@ static bool remove(int fd, descriptor_table_entry_t *entry,
205205
}
206206
}
207207

208+
static bool stdio_initialized = false;
209+
210+
static bool init_stdio() {
211+
stdio_initialized = true;
212+
213+
descriptor_table_entry_t entry;
214+
215+
memset(&entry, 0, sizeof(entry));
216+
entry.tag = DESCRIPTOR_TABLE_ENTRY_FILE_STREAM;
217+
entry.stream.read_stream = streams_borrow_input_stream(stdin_get_stdin());
218+
entry.stream.offset = 0;
219+
entry.stream.file_info.readable = true;
220+
entry.stream.file_info.writable = false;
221+
222+
if (!descriptor_table_update(0, entry))
223+
return false;
224+
225+
memset(&entry, 0, sizeof(entry));
226+
entry.tag = DESCRIPTOR_TABLE_ENTRY_FILE_STREAM;
227+
entry.stream.write_stream = streams_borrow_output_stream(stdout_get_stdout());
228+
entry.stream.offset = 0;
229+
entry.stream.file_info.readable = false;
230+
entry.stream.file_info.writable = true;
231+
232+
if (!descriptor_table_update(1, entry))
233+
return false;
234+
235+
memset(&entry, 0, sizeof(entry));
236+
entry.tag = DESCRIPTOR_TABLE_ENTRY_FILE_STREAM;
237+
entry.stream.write_stream = streams_borrow_output_stream(stderr_get_stderr());
238+
entry.stream.offset = 0;
239+
entry.stream.file_info.readable = false;
240+
entry.stream.file_info.writable = true;
241+
242+
if (!descriptor_table_update(2, entry))
243+
return false;
244+
245+
return true;
246+
}
247+
208248
bool descriptor_table_insert(descriptor_table_entry_t entry, int *fd)
209249
{
210-
*fd = ++next_fd;
211-
return insert(entry, *fd, &global_table, false);
250+
if (!stdio_initialized && !init_stdio())
251+
return false;
252+
*fd = ++next_fd;
253+
return insert(entry, *fd, &global_table, false);
212254
}
213255

214256
bool descriptor_table_get_ref(int fd, descriptor_table_entry_t **entry)
215257
{
216-
return get(fd, entry, &global_table);
258+
if (!stdio_initialized && !init_stdio())
259+
return false;
260+
return get(fd, entry, &global_table);
217261
}
218262

219263
bool descriptor_table_update(int fd, descriptor_table_entry_t entry)
220264
{
221-
return insert(entry, fd, &global_table, true);
265+
if (!stdio_initialized && !init_stdio())
266+
return false;
267+
return insert(entry, fd, &global_table, true);
222268
}
223269

224270
bool descriptor_table_remove(int fd, descriptor_table_entry_t *entry)
225271
{
226-
return remove(fd, entry, &global_table);
272+
if (!stdio_initialized && !init_stdio())
273+
return false;
274+
return remove(fd, entry, &global_table);
227275
}

‎libc-bottom-half/sources/isatty.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ int __isatty(int fd) {
1818
return 0;
1919
}
2020

21+
// TODO: handle stdio and call get-terminal functions
22+
if (file_handle.__handle == 0) {
23+
errno = ENOTTY;
24+
return 0;
25+
}
26+
2127
// Stat the file to determine if it's a tty
2228
filesystem_descriptor_stat_t statbuf;
2329
filesystem_error_code_t error_code;

‎test/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ add_wasilibc_test(fdopen.c FS)
273273
add_wasilibc_test(feof.c FS)
274274
add_wasilibc_test(file_permissions.c FS)
275275
add_wasilibc_test(fseek.c FS)
276+
add_wasilibc_test(fstat.c FS)
276277
add_wasilibc_test(fsync.c FS)
277278
add_wasilibc_test(ftruncate.c FS)
278279
add_wasilibc_test(fts.c FS)

‎test/src/fstat.c‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#include <sys/stat.h>
2+
#include <errno.h>
3+
#include <stdio.h>
4+
#include <stdlib.h>
5+
#include <string.h>
6+
#include "test.h"
7+
8+
#define TEST(c) do { \
9+
errno = 0; \
10+
if (!(c)) \
11+
t_error("%s failed (errno = %d)\n", #c, errno); \
12+
} while(0)
13+
14+
static FILE *make_temp_file() {
15+
const char* path = "temp_file";
16+
FILE *f = fopen(path, "w");
17+
if (f == NULL) {
18+
printf("Error: fopen(%s) failed: %s\n", path, strerror(errno));
19+
exit(1);
20+
}
21+
return f;
22+
}
23+
24+
int main(void) {
25+
struct stat stat;
26+
27+
for (int i = 0; i < 3; i++) {
28+
printf("testing fd %d\n", i);
29+
TEST(fstat(i, &stat) == 0);
30+
printf("st_mode is 0o%o\n", stat.st_mode);
31+
}
32+
33+
FILE *f = make_temp_file();
34+
TEST(f);
35+
int fd = fileno(f);
36+
37+
TEST(fstat(fd, &stat) == 0);
38+
TEST((stat.st_mode & S_IFMT) == S_IFREG);
39+
fclose(f);
40+
41+
return t_status;
42+
}

‎test/src/isatty.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,5 +26,14 @@ int main(void)
2626

2727
TEST(unlink(tmp) != -1);
2828

29+
// make sure isatty can be called on stdio fds, but don't test what the
30+
// actual value is yet.
31+
isatty(0);
32+
TEST(errno != EBADF);
33+
isatty(1);
34+
TEST(errno != EBADF);
35+
isatty(2);
36+
TEST(errno != EBADF);
37+
2938
return t_status;
3039
}

0 commit comments

Comments
 (0)