Tighten the bounds on atomic Ordering in std::sys::unix::weak::Weak #79039
Conversation
rust-highfive
added
the
S-waiting-on-review
jyn514
added
the
T-libs
| // invoking `mprotect`). | ||
| // | ||
| // That said, none of that's *guaranteed*, and so we fence. | ||
| atomic::fence(Ordering::Acquire); |
There was a problem hiding this comment.
How does the performance of the fence compare to using a 2nd load with acquire order? My understanding is that a fence inhibits more reorderings than an atomic targeting a single memory location.
|
You can just use |
|
I think the concern here is multiple threads racing to load a symbol, one of them winning the race and initializing that memory and another thread then observing the new pointer. You need a happens-before relationship between using the function and the function's code being written to memory. |
|
|
If RTLD_LAZY is specified on the loaded library, it can do actual loading I believe. RTLD_DEFAULT looks at all libraries loaded in the global namespace, including ones which may be loaded with RTLD_LAZY. |
|
@bors r+ |
|
📌 Commit 55d7f73 has been approved by |
bors
added
S-waiting-on-bors
Tighten the bounds on atomic Ordering in std::sys::unix::weak::Weak This moves reading this from multiple SeqCst reads to Relaxed read + Acquire fence if we are actually going to use the data. Would love to avoid the Acquire fence, but doing so would need Ordering::Consume, which neither Rust, nor LLVM supports (a shame, since this fence is hardly free on ARM, which is what I was hoping to improve). r? `@Amanieu` (Sorry for always picking you, but I know a lot of people wouldn't feel comfortable reviewing atomic ordering changes)
Rollup of 11 pull requests Successful merges: - rust-lang#78361 (Updated the list of white-listed target features for x86) - rust-lang#78785 (linux: try to use libc getrandom to allow interposition) - rust-lang#78999 (stability: More precise location for deprecation lint on macros) - rust-lang#79039 (Tighten the bounds on atomic Ordering in std::sys::unix::weak::Weak) - rust-lang#79079 (Turn top-level comments into module docs in MIR visitor) - rust-lang#79114 (add trailing_zeros and leading_zeros to non zero types) - rust-lang#79131 (Enable AVX512 *epi64 variants by updating stdarch) - rust-lang#79133 (bootstrap: use the same version number for rustc and cargo) - rust-lang#79145 (Fix handling of panic calls) - rust-lang#79151 (Fix typo in `std::io::Write` docs) - rust-lang#79158 (type is too big -> values of the type are too big) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This moves reading this from multiple SeqCst reads to Relaxed read + Acquire fence if we are actually going to use the data.
Would love to avoid the Acquire fence, but doing so would need Ordering::Consume, which neither Rust, nor LLVM supports (a shame, since this fence is hardly free on ARM, which is what I was hoping to improve).
r? @Amanieu (Sorry for always picking you, but I know a lot of people wouldn't feel comfortable reviewing atomic ordering changes)