Skip to content

Commit

Permalink
fix(symcache): Use saturating addition in writer
Browse files Browse the repository at this point in the history
Addition wraps in release mode. For large line size values,
this resulted in ranges whose end was before their start,
which caused us to skip a function's inlinees in some cases.

Instead of using bare addition on `u64` and casting to `u32`
at the end, we now use saturating addition on `u32` directly.
  • Loading branch information
loewenheim committed Dec 17, 2024
1 parent 242fd76 commit dbe96bc
Showing 1 changed file with 11 additions and 8 deletions.
19 changes: 11 additions & 8 deletions symbolic-symcache/src/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ impl<'a> SymCacheConverter<'a> {
for inlinee in &function.inlinees {
for line in &inlinee.lines {
let start = line.address as u32;
let end = (line.address + line.size.unwrap_or(1)) as u32;
let end = start.saturating_add(line.size.unwrap_or(1) as u32);
inlinee_ranges.push(start..end);
}
}
Expand All @@ -214,7 +214,7 @@ impl<'a> SymCacheConverter<'a> {
// Iterate over the line records.
while let Some(line) = next_line.take() {
let line_range_start = line.address as u32;
let line_range_end = (line.address + line.size.unwrap_or(1)) as u32;
let line_range_end = line_range_start.saturating_add(line.size.unwrap_or(1) as u32);

// Find the call location for this line.
while next_call_location.is_some() && next_call_location.unwrap().0 <= line_range_start
Expand Down Expand Up @@ -308,16 +308,18 @@ impl<'a> SymCacheConverter<'a> {
let mut current_address = line_range_start;
while current_address < line_range_end {
// Emit our source location at current_address if current_address is not covered by an inlinee.
if next_inline.is_none() || next_inline.as_ref().unwrap().start > current_address {
if next_inline
.as_ref()
.is_none_or(|next| next.start > current_address)
{
// "insert_range"
self.ranges.insert(current_address, source_location.clone());
}

// If there is an inlinee range covered by this line record, turn this line into that
// call's "call line". Make a `call_location_idx` for it and store it in `callee_call_locations`.
if next_inline.is_some() && next_inline.as_ref().unwrap().start < line_range_end {
let inline_range = next_inline.take().unwrap();

if let Some(inline_range) = next_inline.take_if(|next| next.start < line_range_end)
{
// "make_call_location"
let (call_location_idx, _) =
self.call_locations.insert_full(source_location.clone());
Expand All @@ -341,8 +343,9 @@ impl<'a> SymCacheConverter<'a> {
// multiple identical small "call line" records instead of one combined record
// covering the entire inlinee range. We can't have different "call lines" for a single
// inlinee range anyway, so it's fine to skip these.
while next_line.is_some()
&& (next_line.as_ref().unwrap().address as u32) < current_address
while next_line
.as_ref()
.is_some_and(|next| (next.address as u32) < current_address)
{
next_line = line_iter.next();
}
Expand Down

0 comments on commit dbe96bc

Please sign in to comment.