From c66f67d4baf4e57c25114d6c15ad0fb2c17af23c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20M=C3=BCller?= Date: Tue, 7 Nov 2023 14:59:29 -0800 Subject: [PATCH] Vendor std::cell::OnceCell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With recent changes we required the use of std::cell::OnceCell. This type stabilized with Rust 1.70, which we cannot yet upgrade to while adhering to our self-imposed minimum supported Rust version policy (which would only allow us to upgrade once 1.75 is released, but the most recent release at this point is 1.73). What's more, some of our consumers were already forced to patch out constructs that their Rust version does not yet support. For OnceCell and its pervasive use, this would easily turn into a major effort. As such, with this change we "vendor" the OnceCell code from the standard library. It was adjusted to remove compiler specific attributes and unnecessary functionality but otherwise kept as-is. Signed-off-by: Daniel Müller --- src/file_cache.rs | 2 +- src/inspect/inspector.rs | 7 +- src/lib.rs | 1 + src/once.rs | 194 ++++++++++++++++++++++++++++++++++++ src/symbolize/symbolizer.rs | 9 +- src/util.rs | 23 ----- 6 files changed, 203 insertions(+), 33 deletions(-) create mode 100644 src/once.rs diff --git a/src/file_cache.rs b/src/file_cache.rs index 4774e9c1..55b7d630 100644 --- a/src/file_cache.rs +++ b/src/file_cache.rs @@ -1,10 +1,10 @@ -use std::cell::OnceCell; use std::fs::File; use std::os::unix::io::AsRawFd; use std::path::Path; use std::path::PathBuf; use crate::insert_map::InsertMap; +use crate::once::OnceCell; use crate::util::fstat; use crate::ErrorExt as _; use crate::Result; diff --git a/src/inspect/inspector.rs b/src/inspect/inspector.rs index bfb59e38..cefb7d62 100644 --- a/src/inspect/inspector.rs +++ b/src/inspect/inspector.rs @@ -1,4 +1,3 @@ -use std::cell::OnceCell; use std::path::Path; use std::rc::Rc; @@ -8,7 +7,7 @@ use crate::elf::ElfBackend; use crate::elf::ElfParser; use crate::elf::ElfResolver; use crate::file_cache::FileCache; -use crate::util::OnceCellExt as _; +use crate::once::OnceCell; use crate::Result; use crate::SymResolver; @@ -84,7 +83,7 @@ impl Inspector { let (file, cell) = self.elf_cache.entry(path)?; let resolver = if let Some(data) = cell.get() { if debug_info { - data.dwarf.get_or_try_init_(|| { + data.dwarf.get_or_try_init(|| { // SANITY: We *know* a `ResolverData` object is present and // given that we are initializing the `dwarf` part // of it, the `elf` part *must* be present. @@ -92,7 +91,7 @@ impl Inspector { self.elf_resolver_from_parser(path, parser, true) })? } else { - data.elf.get_or_try_init_(|| { + data.elf.get_or_try_init(|| { // SANITY: We *know* a `ResolverData` object is present and // given that we are initializing the `elf` part of // it, the `dwarf` part *must* be present. diff --git a/src/lib.rs b/src/lib.rs index b300c31a..2f8bbdaf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -54,6 +54,7 @@ mod ksym; mod maps; mod mmap; pub mod normalize; +mod once; mod resolver; pub mod symbolize; mod util; diff --git a/src/once.rs b/src/once.rs new file mode 100644 index 00000000..4cedc26b --- /dev/null +++ b/src/once.rs @@ -0,0 +1,194 @@ +//! A copy of std::once::OnceCell. +// TODO: Remove this module once our minimum supported Rust version is greater +// 1.70 and/or `OnceCell::get_or_try_init` is stable. + +use std::cell::UnsafeCell; +use std::convert::Infallible; +use std::fmt; +use std::hint::unreachable_unchecked; + +/// A cell which can be written to only once. +/// +/// This allows obtaining a shared `&T` reference to its inner value without copying or replacing +/// it (unlike [`Cell`]), and without runtime borrow checks (unlike [`RefCell`]). However, +/// only immutable references can be obtained unless one has a mutable reference to the cell +/// itself. +/// +/// For a thread-safe version of this struct, see [`std::sync::OnceLock`]. +/// +/// [`RefCell`]: crate::cell::RefCell +/// [`Cell`]: crate::cell::Cell +/// [`std::sync::OnceLock`]: ../../std/sync/struct.OnceLock.html +pub struct OnceCell { + // Invariant: written to at most once. + inner: UnsafeCell>, +} + +impl OnceCell { + /// Creates a new empty cell. + #[inline] + #[must_use] + pub const fn new() -> OnceCell { + OnceCell { + inner: UnsafeCell::new(None), + } + } + + /// Gets the reference to the underlying value. + /// + /// Returns `None` if the cell is empty. + #[inline] + pub fn get(&self) -> Option<&T> { + // SAFETY: Safe due to `inner`'s invariant + unsafe { &*self.inner.get() }.as_ref() + } + + /// Sets the contents of the cell to `value`. + /// + /// # Errors + /// + /// This method returns `Ok(())` if the cell was empty and `Err(value)` if + /// it was full. + #[inline] + pub fn set(&self, value: T) -> Result<(), T> { + match self.try_insert(value) { + Ok(_) => Ok(()), + Err((_, value)) => Err(value), + } + } + + /// Sets the contents of the cell to `value` if the cell was empty, then + /// returns a reference to it. + /// + /// # Errors + /// + /// This method returns `Ok(&value)` if the cell was empty and + /// `Err(¤t_value, value)` if it was full. + #[inline] + pub fn try_insert(&self, value: T) -> Result<&T, (&T, T)> { + if let Some(old) = self.get() { + return Err((old, value)); + } + + // SAFETY: This is the only place where we set the slot, no races + // due to reentrancy/concurrency are possible, and we've + // checked that slot is currently `None`, so this write + // maintains the `inner`'s invariant. + let slot = unsafe { &mut *self.inner.get() }; + Ok(slot.insert(value)) + } + + /// Gets the contents of the cell, initializing it with `f` + /// if the cell was empty. + /// + /// # Panics + /// + /// If `f` panics, the panic is propagated to the caller, and the cell + /// remains uninitialized. + /// + /// It is an error to reentrantly initialize the cell from `f`. Doing + /// so results in a panic. + #[inline] + pub fn get_or_init(&self, f: F) -> &T + where + F: FnOnce() -> T, + { + match self.get_or_try_init(|| Ok::(f())) { + Ok(val) => val, + Err(_) => unsafe { unreachable_unchecked() }, + } + } + + /// Gets the contents of the cell, initializing it with `f` if + /// the cell was empty. If the cell was empty and `f` failed, an + /// error is returned. + /// + /// # Panics + /// + /// If `f` panics, the panic is propagated to the caller, and the cell + /// remains uninitialized. + /// + /// It is an error to reentrantly initialize the cell from `f`. Doing + /// so results in a panic. + pub fn get_or_try_init(&self, f: F) -> Result<&T, E> + where + F: FnOnce() -> Result, + { + if let Some(val) = self.get() { + return Ok(val); + } + /// Avoid inlining the initialization closure into the common path that fetches + /// the already initialized value + #[cold] + fn outlined_call(f: F) -> Result + where + F: FnOnce() -> Result, + { + f() + } + let val = outlined_call(f)?; + // Note that *some* forms of reentrant initialization might lead to + // UB (see `reentrant_init` test). I believe that just removing this + // `panic`, while keeping `try_insert` would be sound, but it seems + // better to panic, rather than to silently use an old value. + if let Ok(val) = self.try_insert(val) { + Ok(val) + } else { + panic!("reentrant init") + } + } +} + +impl Default for OnceCell { + #[inline] + fn default() -> Self { + Self::new() + } +} + +impl fmt::Debug for OnceCell { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut d = f.debug_tuple("OnceCell"); + match self.get() { + Some(v) => d.field(v), + None => d.field(&format_args!("")), + }; + d.finish() + } +} + +impl Clone for OnceCell { + #[inline] + fn clone(&self) -> OnceCell { + let res = OnceCell::new(); + if let Some(value) = self.get() { + match res.set(value.clone()) { + Ok(()) => (), + Err(_) => unreachable!(), + } + } + res + } +} + +impl PartialEq for OnceCell { + #[inline] + fn eq(&self, other: &Self) -> bool { + self.get() == other.get() + } +} + +impl Eq for OnceCell {} + +impl From for OnceCell { + /// Creates a new `OnceCell` which already contains the given `value`. + #[inline] + fn from(value: T) -> Self { + OnceCell { + inner: UnsafeCell::new(Some(value)), + } + } +} + +// Just like for `Cell` this isn't needed, but results in nicer error messages. +//impl !Sync for OnceCell {} diff --git a/src/symbolize/symbolizer.rs b/src/symbolize/symbolizer.rs index d6c454e8..00386dcd 100644 --- a/src/symbolize/symbolizer.rs +++ b/src/symbolize/symbolizer.rs @@ -29,7 +29,6 @@ use crate::normalize::normalize_sorted_user_addrs_with_entries; use crate::normalize::Handler as _; use crate::util; use crate::util::uname_release; -use crate::util::OnceCellExt as _; use crate::zip; use crate::Addr; use crate::Error; @@ -397,7 +396,7 @@ impl Symbolizer { fn elf_resolver<'slf>(&'slf self, path: &Path) -> Result<&'slf Rc> { let (file, cell) = self.elf_cache.entry(path)?; - let resolver = cell.get_or_try_init_(|| self.create_elf_resolver(path, file))?; + let resolver = cell.get_or_try_init(|| self.create_elf_resolver(path, file))?; Ok(resolver) } @@ -408,7 +407,7 @@ impl Symbolizer { fn gsym_resolver<'slf>(&'slf self, path: &Path) -> Result<&'slf Rc>> { let (file, cell) = self.gsym_cache.entry(path)?; - let resolver = cell.get_or_try_init_(|| self.create_gsym_resolver(path, file))?; + let resolver = cell.get_or_try_init(|| self.create_gsym_resolver(path, file))?; Ok(resolver) } @@ -460,7 +459,7 @@ impl Symbolizer { file_off: u64, ) -> Result, Addr)>> { let (file, cell) = self.apk_cache.entry(path)?; - let (apk, resolvers) = cell.get_or_try_init_(|| { + let (apk, resolvers) = cell.get_or_try_init(|| { let apk = zip::Archive::with_mmap(Mmap::builder().map(file)?)?; let resolvers = InsertMap::new(); Result::<_, Error>::Ok((apk, resolvers)) @@ -572,7 +571,7 @@ impl Symbolizer { fn ksym_resolver<'slf>(&'slf self, path: &Path) -> Result<&'slf Rc> { let (file, cell) = self.ksym_cache.entry(path)?; - let resolver = cell.get_or_try_init_(|| self.create_ksym_resolver(path, file))?; + let resolver = cell.get_or_try_init(|| self.create_ksym_resolver(path, file))?; Ok(resolver) } diff --git a/src/util.rs b/src/util.rs index e07bbe49..15790b1a 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,4 +1,3 @@ -use std::cell::OnceCell; use std::cmp::Ordering; use std::ffi::CStr; use std::ffi::CString; @@ -12,28 +11,6 @@ use std::ptr::NonNull; use std::slice; -// TODO: Remove once `OnceCell::get_or_try_init()` is stable. -pub(crate) trait OnceCellExt { - fn get_or_try_init_(&self, f: F) -> Result<&T, E> - where - F: FnOnce() -> Result; -} - -impl OnceCellExt for OnceCell { - fn get_or_try_init_(&self, f: F) -> Result<&T, E> - where - F: FnOnce() -> Result, - { - if let Some(value) = self.get() { - Ok(value) - } else { - let value = f()?; - Ok(self.get_or_init(|| value)) - } - } -} - - /// Reorder elements of `array` based on index information in `indices`. fn reorder(array: &mut [T], indices: Vec<(U, usize)>) { debug_assert_eq!(array.len(), indices.len());