-
Notifications
You must be signed in to change notification settings - Fork 60
deliberate UB: add crossbeam-deque #568
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
Conversation
@@ -19,6 +19,8 @@ We should evaluate whether there truly is some use-case here that is not current | |||
see the [discussion here](https://github.com/rust-lang/unsafe-code-guidelines/issues/449).<br> | |||
The alternative is to not use the "fast path" for problematic types (and fall back to the SeqLock), but that requires some way to query at `const`-time whether the type contains padding (or provenance). | |||
(Or of course one can use inline assembly, but it would be better if that was not required.) | |||
* crossbeam's deque uses [volatile accesses that really should be atomic instead](https://github.com/crossbeam-rs/crossbeam/blob/5a154def002304814d50f3c7658bd30eb46b2fad/crossbeam-deque/src/deque.rs#L70-L88). | |||
They cannot use atomic accesses as those are not possible for arbitrary `T`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@taiki-e do you know if atomic bytewise memcpy (rust-lang/rfcs#3301) would solve this problem? It sure looks like it would.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I mentioned the use in crossbeam-deque in that RFC over 2 years ago.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, as mentioned in crossbeam-rs/crossbeam#1015 ("AtomicCell: Use atomic-maybe-uninit"), crossbeam-deque will likely be using atomic-maybe-uninit for non-{miri,tsan} cases to resolve this issue for now.
It is possible to use atomic-maybe-uninit crate to fix UB of concurrent access with volatile read/write in seqlock/deque (#744, #859), but that is not included in this PR and is left to subsequent PRs.
resources/deliberate-ub.md
Outdated
@@ -19,6 +19,8 @@ We should evaluate whether there truly is some use-case here that is not current | |||
see the [discussion here](https://github.com/rust-lang/unsafe-code-guidelines/issues/449).<br> | |||
The alternative is to not use the "fast path" for problematic types (and fall back to the SeqLock), but that requires some way to query at `const`-time whether the type contains padding (or provenance). | |||
(Or of course one can use inline assembly, but it would be better if that was not required.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, this ("use inline assembly") has been done in crossbeam's master branch (crossbeam-rs/crossbeam#1015).
Also see the discussion on Zulip.