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

Fix futex with large timeout ICE #3653

Merged
merged 9 commits into from
Jun 9, 2024
Merged

Fix futex with large timeout ICE #3653

merged 9 commits into from
Jun 9, 2024

Conversation

tiif
Copy link
Contributor

@tiif tiif commented Jun 7, 2024

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
Copy link
Contributor Author

tiif commented Jun 7, 2024

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 },
        );
    }
}

src/clock.rs Outdated Show resolved Hide resolved
@tiif
Copy link
Contributor Author

tiif commented Jun 7, 2024

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

src/clock.rs Outdated Show resolved Hide resolved
src/clock.rs Outdated Show resolved Hide resolved
src/clock.rs Outdated Show resolved Hide resolved
src/clock.rs Show resolved Hide resolved
src/clock.rs Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member

RalfJung commented Jun 8, 2024

@rustbot author

@rustbot rustbot added the S-waiting-on-author Status: Waiting for the PR author to address review comments label Jun 8, 2024
@tiif
Copy link
Contributor Author

tiif commented Jun 9, 2024

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Jun 9, 2024
@RalfJung
Copy link
Member

RalfJung commented Jun 9, 2024

Looks good, thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Jun 9, 2024

📌 Commit f2c5071 has been approved by RalfJung

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jun 9, 2024

⌛ Testing commit f2c5071 with merge e098278...

@tiif
Copy link
Contributor Author

tiif commented Jun 9, 2024

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

@bors
Copy link
Collaborator

bors commented Jun 9, 2024

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing e098278 to master...

@bors bors merged commit e098278 into rust-lang:master Jun 9, 2024
8 checks passed
@RalfJung
Copy link
Member

RalfJung commented Jun 9, 2024

Ah yeah, I forgot about squashing oops

@tiif
Copy link
Contributor Author

tiif commented Jun 9, 2024

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

@RalfJung
Copy link
Member

RalfJung commented Jun 9, 2024

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

@tiif tiif deleted the bug/futex_ice branch June 9, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Waiting for a review to complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE due to overflow when using a large timeout with futexes
5 participants