Skip to content

Commit b72836b

Browse files
committed
Store evicted cache entry so it can be restored
This makes recursively calling `resolve` safe even if the cache is filled.
1 parent d902726 commit b72836b

2 files changed

Lines changed: 61 additions & 2 deletions

File tree

src/symbolize/gimli.rs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -414,12 +414,19 @@ impl Cache {
414414
.next()
415415
}
416416

417-
fn mapping_for_lib<'a>(&'a mut self, lib: usize) -> Option<(&'a mut Context<'a>, &'a Stash)> {
417+
fn mapping_for_lib<'a>(
418+
&'a mut self,
419+
lib: usize,
420+
) -> Option<(&'a mut Context<'a>, &'a Stash, Option<(usize, Mapping)>)> {
421+
let mut evicted = None;
418422
let cache_idx = self.mappings.iter().position(|(lib_id, _)| *lib_id == lib);
419423

420424
let cache_entry = if let Some(idx) = cache_idx {
421425
self.mappings.move_to_front(idx)
422426
} else {
427+
if self.mappings.is_full() {
428+
evicted = self.mappings.pop_back();
429+
}
423430
// When the mapping is not in the cache, create a new mapping and insert it,
424431
// which will also evict the oldest entry.
425432
create_mapping(&self.libraries[lib])
@@ -434,6 +441,7 @@ impl Cache {
434441
Some((
435442
unsafe { mem::transmute::<&'a mut Context<'static>, &'a mut Context<'a>>(cx) },
436443
stash,
444+
evicted,
437445
))
438446
}
439447
}
@@ -456,10 +464,26 @@ pub unsafe fn resolve(what: ResolveWhat<'_>, cb: &mut dyn FnMut(&super::Symbol))
456464
None => return,
457465
};
458466

467+
// If the cache needs to evict an entry to add a new one, we store
468+
// the evicted entry so we can restore it in case of recursion.
469+
struct CacheGuard<'a>(&'a mut Cache, Option<(usize, Mapping)>);
470+
impl Drop for CacheGuard<'_> {
471+
fn drop(&mut self) {
472+
if let Some(entry) = self.1.take() {
473+
self.0.mappings.push_back(entry);
474+
}
475+
}
476+
}
477+
let mut guard = CacheGuard(cache, None);
478+
let cache = &mut guard.0;
479+
459480
// Finally, get a cached mapping or create a new mapping for this file, and
460481
// evaluate the DWARF info to find the file/line/name for this address.
461482
let (cx, stash) = match cache.mapping_for_lib(lib) {
462-
Some((cx, stash)) => (cx, stash),
483+
Some((cx, stash, evicted)) => {
484+
guard.1 = evicted;
485+
(cx, stash)
486+
}
463487
None => return,
464488
};
465489
let mut any_frames = false;

src/symbolize/gimli/lru.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,41 @@ impl<T, const N: usize> Lru<T, N> {
3434
.map(|init| unsafe { init.assume_init_ref() })
3535
}
3636

37+
#[inline]
38+
pub fn is_full(&self) -> bool {
39+
self.len == N
40+
}
41+
42+
#[inline]
43+
pub fn pop_back(&mut self) -> Option<T> {
44+
if self.len == 0 {
45+
None
46+
} else {
47+
self.len -= 1;
48+
// SAFETY: we maintain len invariant and bail if len was equal to 0
49+
Some(unsafe { mem::transmute_copy(&self.arr[self.len]) })
50+
}
51+
}
52+
53+
#[inline]
54+
pub fn push_back(&mut self, value: T) {
55+
if N == 0 {
56+
return;
57+
} else if self.len == N {
58+
self.len = N - 1;
59+
// SAFETY: we bail on N == 0 but the first entry will be invalid
60+
// until the loop below rotates the array.
61+
unsafe { ptr::drop_in_place(self.arr.as_mut_ptr().cast::<T>()) };
62+
}
63+
let len_to_init = self.len + 1;
64+
let mut last = MaybeUninit::new(value);
65+
for elem in self.arr[0..len_to_init].iter_mut().rev() {
66+
// OPT(size): using `mem::swap` allows surprising size regressions
67+
last = mem::replace(elem, last);
68+
}
69+
self.len = len_to_init;
70+
}
71+
3772
#[inline]
3873
pub fn push_front(&mut self, value: T) -> Option<&mut T> {
3974
if N == 0 {

0 commit comments

Comments
 (0)