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

Detect mixed-size and mixed-atomicity non-synchronized accesses #3137

Merged
merged 8 commits into from
Oct 24, 2023

Conversation

RalfJung
Copy link
Member

Fixes #2303

@RalfJung
Copy link
Member Author

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Oct 24, 2023

📌 Commit d7206eb has been approved by oli-obk

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 24, 2023

⌛ Testing commit d7206eb with merge f36a7d1...

bors added a commit that referenced this pull request Oct 24, 2023
Detect mixed-size and mixed-atomicity non-synchronized accesses

Fixes #2303
tests/pass/concurrency/simple.rs Outdated Show resolved Hide resolved
@@ -267,7 +267,7 @@ fn concurrent_wait_wake() {
FUTEX.store(FREE, Ordering::Relaxed);
unsafe {
DATA = 1;
libc::syscall(libc::SYS_futex, &FUTEX as *const AtomicI32, libc::FUTEX_WAKE, 1);
libc::syscall(libc::SYS_futex, addr_of!(FUTEX), libc::FUTEX_WAKE, 1);
Copy link
Member

@taiki-e taiki-e Oct 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused about the changes to this file. Does this mean that creating a reference to an atomic type is considered a data race?

And if so, is Atomic*::as_ptr not safe as well?

(Or maybe the problem was only in the code creating references to non-atomic integers, and was done for code style consistency?)

Copy link
Member Author

@RalfJung RalfJung Oct 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff you are commenting on was not necessary, interior mutability blocks the spurious reads.

But other parts of this test use static mut FUTEX: i32, and &FUTEX then induces a non-atomic read that was now causing problems. I figured I'd change everything to use addr_of! consistently while I am at it.

(Or maybe the problem was only in the code creating references to non-atomic integers, and was done for code style consistency?)

Yes, that.

@bors
Copy link
Contributor

bors commented Oct 24, 2023

💔 Test failed - checks-actions

@RalfJung
Copy link
Member Author

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Oct 24, 2023

📌 Commit a380383 has been approved by oli-obk

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 24, 2023

⌛ Testing commit a380383 with merge d7278f9...

@bors
Copy link
Contributor

bors commented Oct 24, 2023

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing d7278f9 to master...

@bors bors merged commit d7278f9 into rust-lang:master Oct 24, 2023
4 checks passed
@RalfJung RalfJung deleted the data-race branch October 25, 2023 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Detect non-perfectly-overlapping unordered atomic accesses as UB?
4 participants