Use AtomicPtr instead of AtomicUsize for Weak

This allows Strict Provenance to work properly, fixing #262. It also
now matches what `libstd` does:
https://github.com/rust-lang/rust/blob/9f7e997c8bc3cacd2ab4eb75e63cb5fa9279c7b0/library/std/src/sys/unix/weak.rs#L85-L141

Also, while reading the `libstd` code, I noticed that they use an
`Acquire` fence and `Release` store as the returned pointer should
have "consume" semantics. I changed our code to do something
slightly stronger (Acquire load and Release store) for consistancy.

Signed-off-by: Joe Richey <[email protected]>

src/util_libc.rs

@@ -6,8 +6,13 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 #![allow(dead_code)]
-use crate::{util::LazyUsize, Error};
-use core::{num::NonZeroU32, ptr::NonNull};
+use crate::Error;
+use core::{
+    num::NonZeroU32,
+    ptr::NonNull,
+    sync::atomic::{AtomicPtr, Ordering},
+};
+use libc::c_void;
 
 cfg_if! {
     if #[cfg(any(target_os = "netbsd", target_os = "openbsd", target_os = "android"))] {
@@ -76,29 +81,48 @@ pub fn sys_fill_exact(
 
 // A "weak" binding to a C function that may or may not be present at runtime.
 // Used for supporting newer OS features while still building on older systems.
-// F must be a function pointer of type `unsafe extern "C" fn`. Based off of the
-// weak! macro in libstd.
+// Based off of the DlsymWeak struct in libstd (src/sys/unix/weak.rs), except
+// that the caller must cast self.ptr() to a function pointer.
 pub struct Weak {
     name: &'static str,
-    addr: LazyUsize,
+    addr: AtomicPtr<c_void>,
 }
 
 impl Weak {
+    // A non-null pointer value which indicates we are uninitialized. This
+    // constant should ideally not be a valid address of a function pointer.
+    // However, if by chance libc::dlsym does return UNINIT, there will not
+    // be undefined behavior. libc::dlsym will just be called each time ptr()
+    // is called. This would be inefficient, but correct.
+    // TODO: Replace with core::ptr::invalid_mut(1) when that is stable.
+    const UNINIT: *mut c_void = 1 as *mut c_void;
+
     // Construct a binding to a C function with a given name. This function is
     // unsafe because `name` _must_ be null terminated.
     pub const unsafe fn new(name: &'static str) -> Self {
         Self {
             name,
-            addr: LazyUsize::new(),
+            addr: AtomicPtr::new(Self::UNINIT),
         }
     }
 
-    // Return a function pointer if present at runtime. Otherwise, return null.
-    pub fn ptr(&self) -> Option<NonNull<libc::c_void>> {
-        let addr = self.addr.unsync_init(|| unsafe {
-            libc::dlsym(libc::RTLD_DEFAULT, self.name.as_ptr() as *const _) as usize
-        });
-        NonNull::new(addr as *mut _)
+    // Return the address of a function if present at runtime. Otherwise,
+    // return null. Multiple callers can call ptr() concurrently. It will
+    // always return _some_ value returned by libc::dlsym. However, the
+    // dlsym function may be called multiple times.
+    pub fn ptr(&self) -> Option<NonNull<c_void>> {
+        // Despite having only a single atomic variable (self.addr), we still
+        // need a "consume" ordering as we will generally be reading though
+        // this value (by calling the function we dlsym'ed). Rust lacks this
+        // ordering, so we have to go with the next strongest: Acquire/Release.
+        // As noted in libstd, this might be unnecessary.
+        let mut addr = self.addr.load(Ordering::Acquire);
+        if addr == Self::UNINIT {
+            let symbol = self.name.as_ptr() as *const _;
+            addr = unsafe { libc::dlsym(libc::RTLD_DEFAULT, symbol) };
+            self.addr.store(addr, Ordering::Release);
+        }
+        NonNull::new(addr)
     }
 }