Skip to content
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.

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

utimensat() with timespec=NULL sets wrong time #4184

Closed
tiran opened this issue May 24, 2022 · 7 comments
Closed

utimensat() with timespec=NULL sets wrong time #4184

tiran opened this issue May 24, 2022 · 7 comments
Labels
bug Incorrect behavior in the current implementation that needs fixing

Comments

@tiran
Copy link

tiran commented May 24, 2022

WASI libc or wasmtime set wrong atime and mtime when utimensat is used with NULL timespec. The call is suppose to set the atime and mtime of the file to NOW, but it sets both to 0. I can't say if the problem is in wasi-libc __wasilibc_nocwd_utimensat, utimens_get_timestamps, or in wasmtime path_filestat_set_times implementation.

Test Case

Reproducer:

#include <fcntl.h>
#include <sys/stat.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>

#define TESTFN "utime.test"

int pstat(const char *name) {
    struct stat st;
    if (stat(TESTFN, &st) < 0) {
        perror(TESTFN);
        exit(1);
    }
    printf(
        "atime: %lld.%09ld, mtime: %lld.%09ld\n",
        st.st_atim.tv_sec, st.st_atim.tv_nsec,
        st.st_mtim.tv_sec, st.st_mtim.tv_nsec
    );
    return 0;
}

int main(void) {
    int fd;
    
    fd = open(TESTFN, O_CREAT | O_WRONLY);
    write(fd, "test\n", 5);
    close(fd);

    pstat(TESTFN);
    utimensat(AT_FDCWD, TESTFN, NULL, 0);
    pstat(TESTFN);

    return 0;
}

Steps to Reproduce

Compile and run the reproducer with WASI-SDK's clang and wasmtime

$ gcc -o utime utime.c && ./utime
$ /opt/wasi-sdk/bin/clang -o utime.wasm utime.c && wasmtime run --dir . -- utime.wasm

Expected Results

utimensat with timespec NULL should set the mtime and atime of the file to the current time.

$ gcc -o utime utime.c && ./utime
atime: 1653412556.499033757, mtime: 1653412556.499033757
atime: 1653412558.795059730, mtime: 1653412558.795059730

Actual Results

wasmtime sets atime and mtime to 0.

$ /opt/wasi-sdk/bin/clang -o utime.wasm utime.c && wasmtime run --dir . -- utime.wasm 
atime: 0.000000000, mtime: 1653412625.340814414
atime: 0.000000000, mtime: 0.000000000

Versions and Environment

WASI SDK: 15.0

Wasmtime version or commit: 0.36.0

Operating system: Linux (Ubuntu 20.04 in podman on Fedora 36)

Architecture: x86_64

Extra Info

Anything else you'd like to add?

@tiran tiran added the bug Incorrect behavior in the current implementation that needs fixing label May 24, 2022
@alexcrichton
Copy link
Member

Could you provide the wasm file here to help reproduce?

You can also try running locally with RUST_LOG=wasi_common which will print out an strace-ish format of wasi calls executed and can help determine whether the issue is with Wasmtime or with wasi-sdk.

@sunfishcode
Copy link
Member

I'm able to reproduce this. The RUST_LOG trace shows the flags values having ATIM | MTIM, and not ATIM_NOW | MTIM_NOW, and these appear to be the flags passed in from the wasm. I'll look at wasi-libc next.

@tiran
Copy link
Author

tiran commented May 24, 2022

 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > wiggle abi; module="wasi_snapshot_preview1" function="path_filestat_get"
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > fd=Fd(3) flags=SYMLINK_FOLLOW path=*guest 0x41d/10
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > result=Ok(Filestat { dev: 64771, ino: 8937277, filetype: RegularFile, nlink: 1, size: 5, atim: 0, mtim: 1653416690266718793, ctim: 0 })
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > wiggle abi; module="wasi_snapshot_preview1" function="fd_fdstat_get"
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > fd=Fd(1)
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > result=Ok(Fdstat { fs_filetype: CharacterDevice, fs_flags: APPEND, fs_rights_base: FD_DATASYNC | FD_READ | FD_FDSTAT_SET_FLAGS | FD_SYNC | FD_WRITE | FD_ADVISE | FD_ALLOCATE | FD_FILESTAT_GET | FD_FILESTAT_SET_SIZE | FD_FILESTAT_SET_TIMES | POLL_FD_READWRITE, fs_rights_inheriting: (empty) })
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > wiggle abi; module="wasi_snapshot_preview1" function="fd_write"
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > fd=Fd(1) iovs=*guest 0x10fb0/2
atime: 0.000000000, mtime: 1653416690.266718793
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > result=Ok(48)
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > wiggle abi; module="wasi_snapshot_preview1" function="path_filestat_set_times"
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > fd=Fd(3) flags=SYMLINK_FOLLOW path=*guest 0x41d/10 atim=0 mtim=0 fst_flags=ATIM | MTIM
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > result=Ok(())
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > wiggle abi; module="wasi_snapshot_preview1" function="path_filestat_get"
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > fd=Fd(3) flags=SYMLINK_FOLLOW path=*guest 0x41d/10
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > result=Ok(Filestat { dev: 64771, ino: 8937277, filetype: RegularFile, nlink: 1, size: 5, atim: 0, mtim: 0, ctim: 0 })
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > wiggle abi; module="wasi_snapshot_preview1" function="fd_write"
 TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > fd=Fd(1) iovs=*guest 0x10fb0/2

@tiran
Copy link
Author

tiran commented May 24, 2022

utime.zip

@sunfishcode
Copy link
Member

This turned out to be a bug in wasi-libc. It was passing uninitialized values into WASI functions, which previously worked because it only does so when those values aren't being used, however with the recent LLVM update, LLVM is now optimizing in a way which actively breaks this code.

I've now opened WebAssembly/wasi-libc#291 with a fix.

@sunfishcode
Copy link
Member

The fix is now in the wasi-sdk-16 release.

@tiran
Copy link
Author

tiran commented Jun 3, 2022

The fix is now in the wasi-sdk-16 release.

Thanks! Your patch fixed the utime tests in CPython's test suite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior in the current implementation that needs fixing
Projects
None yet
Development

No branches or pull requests

3 participants