Skip to content

Commit 988d6a8

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 988d6a8

2 files changed

Lines changed: 58 additions & 2 deletions

File tree

src/symbolize/gimli.rs

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -414,12 +414,16 @@ 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>(&'a mut self, lib: usize) -> Option<(&'a mut Context<'a>, &'a Stash, Option<(usize, Mapping)>)> {
418+
let mut evicted = None;
418419
let cache_idx = self.mappings.iter().position(|(lib_id, _)| *lib_id == lib);
419420

420421
let cache_entry = if let Some(idx) = cache_idx {
421422
self.mappings.move_to_front(idx)
422423
} else {
424+
if self.mappings.is_full() {
425+
evicted = self.mappings.pop_back();
426+
}
423427
// When the mapping is not in the cache, create a new mapping and insert it,
424428
// which will also evict the oldest entry.
425429
create_mapping(&self.libraries[lib])
@@ -434,6 +438,7 @@ impl Cache {
434438
Some((
435439
unsafe { mem::transmute::<&'a mut Context<'static>, &'a mut Context<'a>>(cx) },
436440
stash,
441+
evicted
437442
))
438443
}
439444
}
@@ -456,10 +461,26 @@ pub unsafe fn resolve(what: ResolveWhat<'_>, cb: &mut dyn FnMut(&super::Symbol))
456461
None => return,
457462
};
458463

464+
// If the cache needs to evict an entry to add a new one, we store
465+
// the evicted entry so we can restore it in case of recursion.
466+
struct CacheGuard<'a>(&'a mut Cache, Option<(usize, Mapping)>);
467+
impl Drop for CacheGuard<'_> {
468+
fn drop(&mut self) {
469+
if let Some(entry) = self.1.take() {
470+
self.0.mappings.push_back(entry);
471+
}
472+
}
473+
}
474+
let mut guard = CacheGuard(cache, None);
475+
let cache = &mut guard.0;
476+
459477
// Finally, get a cached mapping or create a new mapping for this file, and
460478
// evaluate the DWARF info to find the file/line/name for this address.
461479
let (cx, stash) = match cache.mapping_for_lib(lib) {
462-
Some((cx, stash)) => (cx, stash),
480+
Some((cx, stash, evicted)) => {
481+
guard.1 = evicted;
482+
(cx, stash)
483+
},
463484
None => return,
464485
};
465486
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)