Skip to content

Commit 82f5cdf

Browse files
committed
std: improve the dlsym! macro and add a test for it
* Ensure nul-termination of the symbol name at compile-time * Use an acquire load instead of a relaxed load and acquire fence * Properly use `unsafe` and add safety comments * Add tests
1 parent fe55364 commit 82f5cdf

File tree

3 files changed

+102
-51
lines changed

3 files changed

+102
-51
lines changed

library/std/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@
350350
#![feature(float_gamma)]
351351
#![feature(float_minimum_maximum)]
352352
#![feature(fmt_internals)]
353+
#![feature(fn_ptr_trait)]
353354
#![feature(generic_atomic)]
354355
#![feature(hasher_prefixfree_extras)]
355356
#![feature(hashmap_internals)]

library/std/src/sys/pal/unix/weak.rs

Lines changed: 69 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,14 @@
2222
#![allow(dead_code, unused_macros)]
2323
#![forbid(unsafe_op_in_unsafe_fn)]
2424

25-
use crate::ffi::CStr;
26-
use crate::marker::PhantomData;
27-
use crate::sync::atomic::{self, Atomic, AtomicPtr, Ordering};
25+
use crate::ffi::{CStr, c_char, c_void};
26+
use crate::marker::{FnPtr, PhantomData};
27+
use crate::sync::atomic::{Atomic, AtomicPtr, Ordering};
2828
use crate::{mem, ptr};
2929

30+
#[cfg(test)]
31+
mod tests;
32+
3033
// We can use true weak linkage on ELF targets.
3134
#[cfg(all(unix, not(target_vendor = "apple")))]
3235
pub(crate) macro weak {
@@ -64,7 +67,7 @@ impl<F: Copy> ExternWeak<F> {
6467

6568
pub(crate) macro dlsym {
6669
(fn $name:ident($($param:ident : $t:ty),* $(,)?) -> $ret:ty;) => (
67-
dlsym!(
70+
dlsym!(
6871
#[link_name = stringify!($name)]
6972
fn $name($($param : $t),*) -> $ret;
7073
);
@@ -73,84 +76,99 @@ pub(crate) macro dlsym {
7376
#[link_name = $sym:expr]
7477
fn $name:ident($($param:ident : $t:ty),* $(,)?) -> $ret:ty;
7578
) => (
76-
static DLSYM: DlsymWeak<unsafe extern "C" fn($($t),*) -> $ret> =
77-
DlsymWeak::new(concat!($sym, '\0'));
79+
static DLSYM: DlsymWeak<unsafe extern "C" fn($($t),*) -> $ret> = {
80+
let Ok(name) = CStr::from_bytes_with_nul(concat!($sym, '\0').as_bytes()) else {
81+
panic!("symbol name may not contain NUL")
82+
};
83+
84+
// SAFETY: Whoever calls the function pointer returned by `get()`
85+
// is responsible for ensuring that the signature is correct. Just
86+
// like with extern blocks, this is syntactically enforced by making
87+
// the function pointer be unsafe.
88+
unsafe { DlsymWeak::new(name) }
89+
};
90+
7891
let $name = &DLSYM;
7992
)
8093
}
94+
8195
pub(crate) struct DlsymWeak<F> {
82-
name: &'static str,
96+
/// A pointer to the nul-terminated name of the symbol.
97+
// Use a pointer instead of `&'static CStr` to save space.
98+
name: *const c_char,
8399
func: Atomic<*mut libc::c_void>,
84100
_marker: PhantomData<F>,
85101
}
86102

87-
impl<F> DlsymWeak<F> {
88-
pub(crate) const fn new(name: &'static str) -> Self {
103+
impl<F: FnPtr> DlsymWeak<F> {
104+
/// # Safety
105+
///
106+
/// If the signature of `F` does not match the signature of the symbol (if
107+
/// it exists), calling the function pointer returned by `get()` is
108+
/// undefined behaviour.
109+
pub(crate) const unsafe fn new(name: &'static CStr) -> Self {
89110
DlsymWeak {
90-
name,
111+
name: name.as_ptr(),
91112
func: AtomicPtr::new(ptr::without_provenance_mut(1)),
92113
_marker: PhantomData,
93114
}
94115
}
95116

96117
#[inline]
97118
pub(crate) fn get(&self) -> Option<F> {
98-
unsafe {
99-
// Relaxed is fine here because we fence before reading through the
100-
// pointer (see the comment below).
101-
match self.func.load(Ordering::Relaxed) {
102-
func if func.addr() == 1 => self.initialize(),
103-
func if func.is_null() => None,
104-
func => {
105-
let func = mem::transmute_copy::<*mut libc::c_void, F>(&func);
106-
// The caller is presumably going to read through this value
107-
// (by calling the function we've dlsymed). This means we'd
108-
// need to have loaded it with at least C11's consume
109-
// ordering in order to be guaranteed that the data we read
110-
// from the pointer isn't from before the pointer was
111-
// stored. Rust has no equivalent to memory_order_consume,
112-
// so we use an acquire fence (sorry, ARM).
113-
//
114-
// Now, in practice this likely isn't needed even on CPUs
115-
// where relaxed and consume mean different things. The
116-
// symbols we're loading are probably present (or not) at
117-
// init, and even if they aren't the runtime dynamic loader
118-
// is extremely likely have sufficient barriers internally
119-
// (possibly implicitly, for example the ones provided by
120-
// invoking `mprotect`).
121-
//
122-
// That said, none of that's *guaranteed*, and so we fence.
123-
atomic::fence(Ordering::Acquire);
124-
Some(func)
125-
}
126-
}
119+
// The caller is presumably going to read through this value
120+
// (by calling the function we've dlsymed). This means we'd
121+
// need to have loaded it with at least C11's consume
122+
// ordering in order to be guaranteed that the data we read
123+
// from the pointer isn't from before the pointer was
124+
// stored. Rust has no equivalent to memory_order_consume,
125+
// so we use an acquire load (sorry, ARM).
126+
//
127+
// Now, in practice this likely isn't needed even on CPUs
128+
// where relaxed and consume mean different things. The
129+
// symbols we're loading are probably present (or not) at
130+
// init, and even if they aren't the runtime dynamic loader
131+
// is extremely likely have sufficient barriers internally
132+
// (possibly implicitly, for example the ones provided by
133+
// invoking `mprotect`).
134+
//
135+
// That said, none of that's *guaranteed*, so we use acquire.
136+
match self.func.load(Ordering::Acquire) {
137+
func if func.addr() == 1 => self.initialize(),
138+
func if func.is_null() => None,
139+
// SAFETY:
140+
// `func` is not null and `F` implements `FnPtr`, thus this
141+
// transmutation is well-defined. It is the responsibility of the
142+
// creator of this `DlsymWeak` to ensure that calling the resulting
143+
// function pointer does not result in undefined behaviour (though
144+
// the `dlsym!` macro delegates this responsibility to the caller
145+
// of the function by using `unsafe` function pointers).
146+
// FIXME: use `transmute` once it stops complaining about generics.
147+
func => Some(unsafe { mem::transmute_copy::<*mut c_void, F>(&func) }),
127148
}
128149
}
129150

130151
// Cold because it should only happen during first-time initialization.
131152
#[cold]
132-
unsafe fn initialize(&self) -> Option<F> {
133-
assert_eq!(size_of::<F>(), size_of::<*mut libc::c_void>());
134-
135-
let val = unsafe { fetch(self.name) };
136-
// This synchronizes with the acquire fence in `get`.
153+
fn initialize(&self) -> Option<F> {
154+
// SAFETY: `self.name` was created from a `&'static CStr` and is
155+
// therefore a valid C string pointer.
156+
let val = unsafe { libc::dlsym(libc::RTLD_DEFAULT, self.name) };
157+
// This synchronizes with the acquire load in `get`.
137158
self.func.store(val, Ordering::Release);
138159

139160
if val.is_null() {
140161
None
141162
} else {
163+
// SAFETY: see the comment in `get`.
164+
// FIXME: use `transmute` once it stops complaining about generics.
142165
Some(unsafe { mem::transmute_copy::<*mut libc::c_void, F>(&val) })
143166
}
144167
}
145168
}
146169

147-
unsafe fn fetch(name: &str) -> *mut libc::c_void {
148-
let name = match CStr::from_bytes_with_nul(name.as_bytes()) {
149-
Ok(cstr) => cstr,
150-
Err(..) => return ptr::null_mut(),
151-
};
152-
unsafe { libc::dlsym(libc::RTLD_DEFAULT, name.as_ptr()) }
153-
}
170+
unsafe impl<F> Send for DlsymWeak<F> {}
171+
unsafe impl<F> Sync for DlsymWeak<F> {}
154172

155173
#[cfg(not(any(target_os = "linux", target_os = "android")))]
156174
pub(crate) macro syscall {
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
use super::*;
2+
3+
#[test]
4+
fn dlsym_existing() {
5+
const TEST_STRING: &'static CStr = c"Ferris!";
6+
7+
// Try to find a symbol that definitely exists.
8+
dlsym! {
9+
fn strlen(cs: *const c_char) -> usize;
10+
}
11+
12+
dlsym! {
13+
#[link_name = "strlen"]
14+
fn custom_name(cs: *const c_char) -> usize;
15+
}
16+
17+
let strlen = strlen.get().unwrap();
18+
assert_eq!(unsafe { strlen(TEST_STRING.as_ptr()) }, TEST_STRING.count_bytes());
19+
20+
let custom_name = custom_name.get().unwrap();
21+
assert_eq!(unsafe { custom_name(TEST_STRING.as_ptr()) }, TEST_STRING.count_bytes());
22+
}
23+
24+
#[test]
25+
fn dlsym_missing() {
26+
// Try to find a symbol that definitely does not exist.
27+
dlsym! {
28+
fn test_symbol_that_does_not_exist() -> i32;
29+
}
30+
31+
assert!(test_symbol_that_does_not_exist.get().is_none());
32+
}

0 commit comments

Comments
 (0)