search

rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.14k stars 318 forks source link

Fix futex with large timeout ICE #3653

Closed tiif closed 3 weeks ago

tiif commented 3 weeks ago

Fixes #3647.

This PR changed the type of nanoseconds from u64 to u128. In duration_since, nanoseconds is manually converted to second by dividing it with 1e9. But overflow is still possible.

tiif commented 3 weeks ago

Initially there is a test, but it took way too long for ./miri test to finish, so I removed it. But surprisingly, it completed fairly fast with ./miri run --dep ./tests/pass-dep/concurrency/linux-futex.rs.

Test:

// Reproduce https://github.com/rust-lang/miri/issues/3647. This should not ICE.
fn large_timeout() {
    let futex: i32 = 123;

    unsafe {
        libc::syscall(
            libc::SYS_futex,
            addr_of!(futex),
            libc::FUTEX_WAIT,
            123,
            &libc::timespec { tv_sec: 184467440839020, tv_nsec: 117558982 },
        );
    }
}
tiif commented 3 weeks ago

The linux-futex test became very slow after this new commit?

RalfJung commented 3 weeks ago

@rustbot author

tiif commented 3 weeks ago

@rustbot ready

RalfJung commented 3 weeks ago

Looks good, thanks! @bors r+

bors commented 3 weeks ago

:pushpin: Commit f2c5071b09aef74906c202c7617b20111b563c0b has been approved by RalfJung

It is now in the queue for this repository.

bors commented 3 weeks ago

:hourglass: Testing commit f2c5071b09aef74906c202c7617b20111b563c0b with merge e09827881fb388b8012a6c3c021d357ead61507d...

tiif commented 3 weeks ago

Thanks, but no squashing? The commit messages are not very neat.

bors commented 3 weeks ago

:sunny: Test successful - checks-actions Approved by: RalfJung Pushing e09827881fb388b8012a6c3c021d357ead61507d to master...

RalfJung commented 3 weeks ago

Ah yeah, I forgot about squashing oops

tiif commented 3 weeks ago

Hmm how the unmerge works? Should I open a new pr with the squashed commits?

RalfJung commented 3 weeks ago

No, it's too late. Once this landed it can never be un-done.