Skip to content

Commit

Permalink
fix(symcache): Explicitly map "holes" between functions
Browse files Browse the repository at this point in the history
Currently, if we look up an address that falls "between"
two functions/symbols, we will always resolve it to the
first of the two functions/symbols because each range
implicitly extends to the start of the next one. This can
lead to bad user experience when we map (invalid) addresses
to entirely nonsensical functions.

Therefore, we now explicitly insert a mapping at the end
of the function that maps to "nowhere" to denote the
function's end.
  • Loading branch information
loewenheim committed Feb 10, 2025
1 parent 5b4babb commit 22979db
Showing 1 changed file with 30 additions and 0 deletions.
30 changes: 30 additions & 0 deletions symbolic-symcache/src/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use symbolic_debuginfo::{DebugSession, FileFormat, Function, ObjectLike, Symbol}
use watto::{Pod, StringTable, Writer};

use super::{raw, transform};
use crate::raw::NO_SOURCE_LOCATION;
use crate::{Error, ErrorKind};

/// The SymCache Converter.
Expand Down Expand Up @@ -373,6 +374,19 @@ impl<'a> SymCacheConverter<'a> {
if function_end > *last_addr {
*last_addr = function_end;
}

// Insert an explicit "empty" mapping for the end of the function.
// This is to ensure that addresses that fall "between" functions don't get
// erroneously mapped to the previous function.
//
// We only do this if there is no previous mapping for the end address—we don't
// want to overwrite valid mappings.
//
// If the next function starts right at this function's end, that's no trouble,
// it will just overwrite this mapping with one of its ranges.
if let btree_map::Entry::Vacant(vacant_entry) = self.ranges.entry(function_end) {
vacant_entry.insert(NO_SOURCE_LOCATION);
}
}

/// Processes an individual [`Symbol`].
Expand Down Expand Up @@ -429,6 +443,22 @@ impl<'a> SymCacheConverter<'a> {
if symbol.address as u32 >= *last_addr {
self.last_addr = None;
}

// Insert an explicit "empty" mapping for the end of the symbol.
// This is to ensure that addresses that fall "between" symbols don't get
// erroneously mapped to the previous symbol.
//
// We only do this if there is no previous mapping for the end address—we don't
// want to overwrite valid mappings.
//
// If the next symbol starts right at this symbols's end, that's no trouble,
// it will just overwrite this mapping.
if symbol.size > 0 {
let end_address = (symbol.address + symbol.size) as u32;
if let btree_map::Entry::Vacant(vacant_entry) = self.ranges.entry(end_address) {
vacant_entry.insert(NO_SOURCE_LOCATION);
}
}
}

// Methods for serializing to a [`Write`] below:
Expand Down

0 comments on commit 22979db

Please sign in to comment.