From cb8c3add453911df4d5d7177fcb2caf91cdcbb4f Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Wed, 1 Jul 2026 13:24:38 +0000 Subject: [PATCH] Store evicted cache entry so it can be restored This makes recursively calling `resolve` safe even if the cache is filled. --- src/symbolize/gimli.rs | 28 ++++++++++++++++++++++++++-- src/symbolize/gimli/lru.rs | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/symbolize/gimli.rs b/src/symbolize/gimli.rs index cbc0b768..84d2430a 100644 --- a/src/symbolize/gimli.rs +++ b/src/symbolize/gimli.rs @@ -414,12 +414,19 @@ impl Cache { .next() } - fn mapping_for_lib<'a>(&'a mut self, lib: usize) -> Option<(&'a mut Context<'a>, &'a Stash)> { + fn mapping_for_lib<'a>( + &'a mut self, + lib: usize, + ) -> Option<(&'a mut Context<'a>, &'a Stash, Option<(usize, Mapping)>)> { + let mut evicted = None; let cache_idx = self.mappings.iter().position(|(lib_id, _)| *lib_id == lib); let cache_entry = if let Some(idx) = cache_idx { self.mappings.move_to_front(idx) } else { + if self.mappings.is_full() { + evicted = self.mappings.pop_back(); + } // When the mapping is not in the cache, create a new mapping and insert it, // which will also evict the oldest entry. create_mapping(&self.libraries[lib]) @@ -434,6 +441,7 @@ impl Cache { Some(( unsafe { mem::transmute::<&'a mut Context<'static>, &'a mut Context<'a>>(cx) }, stash, + evicted, )) } } @@ -456,10 +464,26 @@ pub unsafe fn resolve(what: ResolveWhat<'_>, cb: &mut dyn FnMut(&super::Symbol)) None => return, }; + // If the cache needs to evict an entry to add a new one, we store + // the evicted entry so we can restore it in case of recursion. + struct CacheGuard<'a>(&'a mut Cache, Option<(usize, Mapping)>); + impl Drop for CacheGuard<'_> { + fn drop(&mut self) { + if let Some(entry) = self.1.take() { + self.0.mappings.push_back(entry); + } + } + } + let mut guard = CacheGuard(cache, None); + let cache = &mut guard.0; + // Finally, get a cached mapping or create a new mapping for this file, and // evaluate the DWARF info to find the file/line/name for this address. let (cx, stash) = match cache.mapping_for_lib(lib) { - Some((cx, stash)) => (cx, stash), + Some((cx, stash, evicted)) => { + guard.1 = evicted; + (cx, stash) + } None => return, }; let mut any_frames = false; diff --git a/src/symbolize/gimli/lru.rs b/src/symbolize/gimli/lru.rs index b7cf5a5b..d8276f78 100644 --- a/src/symbolize/gimli/lru.rs +++ b/src/symbolize/gimli/lru.rs @@ -34,6 +34,41 @@ impl Lru { .map(|init| unsafe { init.assume_init_ref() }) } + #[inline] + pub fn is_full(&self) -> bool { + self.len == N + } + + #[inline] + pub fn pop_back(&mut self) -> Option { + if self.len == 0 { + None + } else { + self.len -= 1; + // SAFETY: we maintain len invariant and bail if len was equal to 0 + Some(unsafe { mem::transmute_copy(&self.arr[self.len]) }) + } + } + + #[inline] + pub fn push_back(&mut self, value: T) { + if N == 0 { + return; + } else if self.len == N { + self.len = N - 1; + // SAFETY: we bail on N == 0 but the first entry will be invalid + // until the loop below rotates the array. + unsafe { ptr::drop_in_place(self.arr.as_mut_ptr().cast::()) }; + } + let len_to_init = self.len + 1; + let mut last = MaybeUninit::new(value); + for elem in self.arr[0..len_to_init].iter_mut().rev() { + // OPT(size): using `mem::swap` allows surprising size regressions + last = mem::replace(elem, last); + } + self.len = len_to_init; + } + #[inline] pub fn push_front(&mut self, value: T) -> Option<&mut T> { if N == 0 {