Skip to content

Commit

Permalink
read: improve handling of 0 for tombstones
Browse files Browse the repository at this point in the history 
Where possible, ignore these instead of returning them or returning
an error. This often isn't possible because 0 is a valid address,
but we can handle it for `DW_LNE_set_address` in the middle of a
line sequence, and for address pairs.
  • Loading branch information
@philipc
philipc committed Sep 3, 2024
1 parent f49dfeb commit 67dbf2f
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 87 deletions.
2 changes: 2 additions & 0 deletions src/read/aranges.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,8 @@ impl<R: Reader> ArangeEntryIter<R> {
#[doc(hidden)]
pub fn convert_raw(&self, mut entry: ArangeEntry) -> Result<Option<ArangeEntry>> {
// Skip tombstone entries.
// DWARF specifies a tombstone value of -1, but many linkers use 0.
// However, 0 may be a valid address, so the caller must handle that case.
let address_size = self.encoding.address_size;
let tombstone_address = !0 >> (64 - self.encoding.address_size * 8);
if entry.range.begin == tombstone_address {
Expand Down
23 changes: 14 additions & 9 deletions src/read/line.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -828,12 +828,16 @@ impl LineRow {
}

LineInstruction::SetAddress(address) => {
// If the address is a tombstone, then skip instructions until the next address.
// DWARF specifies a tombstone value of -1, but many linkers use 0.
// However, 0 may be a valid address, so we only skip that if we have previously
// seen a higher address. Additionally, gold may keep the relocation addend,
// so we treat all lower addresses as tombstones instead of just 0.
// This works because DWARF specifies that addresses are monotonically increasing
// within a sequence; the alternative is to return an error.
let tombstone_address = !0 >> (64 - program.header().encoding.address_size * 8);
self.tombstone = address == tombstone_address;
self.tombstone = address < self.address || address == tombstone_address;
if !self.tombstone {
if address < self.address {
return Err(Error::InvalidAddressRange);
}
self.address = address;
self.op_index.0 = 0;
}
Expand Down Expand Up @@ -2877,13 +2881,14 @@ mod tests {
#[test]
fn test_exec_set_address_backwards() {
let header = make_test_header(EndianSlice::new(&[], LittleEndian));
let mut registers = LineRow::new(&header);
registers.address = 1;
let mut initial_registers = LineRow::new(&header);
initial_registers.address = 1;
let opcode = LineInstruction::SetAddress(0);

let mut program = IncompleteLineProgram { header };
let result = registers.execute(opcode, &mut program);
assert_eq!(result, Err(Error::InvalidAddressRange));
let mut expected_registers = initial_registers;
expected_registers.tombstone = true;

assert_exec_opcode(header, initial_registers, opcode, expected_registers, false);
}

#[test]
Expand Down
53 changes: 12 additions & 41 deletions src/read/loclists.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,7 @@ impl<R: Reader> LocListIter<R> {
),
RawLocListEntry::AddressOrOffsetPair { begin, end, data }
| RawLocListEntry::OffsetPair { begin, end, data } => {
// Skip tombstone entries (see below).
if self.base_address == tombstone {
return Ok(None);
}
Expand All @@ -651,7 +652,17 @@ impl<R: Reader> LocListIter<R> {
}
};

if range.begin == tombstone || range.begin > range.end {
// Skip tombstone entries.
//
// DWARF specifies a tombstone value of -1 or -2, but many linkers use 0 or 1.
// However, 0/1 may be a valid address, so we can't always reliably skip them.
// One case where we can skip them is for address pairs, where both values are
// replaced by tombstones and thus `begin` equals `end`. Since these entries
// are empty, it's safe to skip them even if they aren't tombstones.
//
// In addition to skipping tombstone entries, we also skip invalid entries
// where `begin` is greater than `end`. This can occur due to compiler bugs.
if range.begin == tombstone || range.begin >= range.end {
return Ok(None);
}

Expand Down Expand Up @@ -806,16 +817,6 @@ mod tests {
);

// An empty location range followed by a normal location.
assert_eq!(
locations.next(),
Ok(Some(LocationListEntry {
range: Range {
begin: 0x0201_0600,
end: 0x0201_0600,
},
data: Expression(EndianSlice::new(&[4, 0, 0, 0], LittleEndian)),
}))
);
assert_eq!(
locations.next(),
Ok(Some(LocationListEntry {
Expand Down Expand Up @@ -1070,16 +1071,6 @@ mod tests {
);

// An empty location range followed by a normal location.
assert_eq!(
locations.next(),
Ok(Some(LocationListEntry {
range: Range {
begin: 0x0201_0600,
end: 0x0201_0600,
},
data: Expression(EndianSlice::new(&[4, 0, 0, 0], LittleEndian)),
}))
);
assert_eq!(
locations.next(),
Ok(Some(LocationListEntry {
Expand Down Expand Up @@ -1290,16 +1281,6 @@ mod tests {
);

// An empty location range followed by a normal location.
assert_eq!(
locations.next(),
Ok(Some(LocationListEntry {
range: Range {
begin: 0x0201_0600,
end: 0x0201_0600,
},
data: Expression(EndianSlice::new(&[4, 0, 0, 0], LittleEndian)),
}))
);
assert_eq!(
locations.next(),
Ok(Some(LocationListEntry {
Expand Down Expand Up @@ -1426,16 +1407,6 @@ mod tests {
);

// An empty location range followed by a normal location.
assert_eq!(
locations.next(),
Ok(Some(LocationListEntry {
range: Range {
begin: 0x0201_0600,
end: 0x0201_0600,
},
data: Expression(EndianSlice::new(&[4, 0, 0, 0], LittleEndian)),
}))
);
assert_eq!(
locations.next(),
Ok(Some(LocationListEntry {
Expand Down
59 changes: 22 additions & 37 deletions src/read/rnglists.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ impl<R: Reader> RngListIter<R> {
}
RawRngListEntry::AddressOrOffsetPair { begin, end }
| RawRngListEntry::OffsetPair { begin, end } => {
// Skip tombstone entries (see below).
if self.base_address == tombstone {
return Ok(None);
}
Expand All @@ -570,7 +571,17 @@ impl<R: Reader> RngListIter<R> {
}
};

if range.begin == tombstone || range.begin > range.end {
// Skip tombstone entries.
//
// DWARF specifies a tombstone value of -1 or -2, but many linkers use 0 or 1.
// However, 0/1 may be a valid address, so we can't always reliably skip them.
// One case where we can skip them is for address pairs, where both values are
// replaced by tombstones and thus `begin` equals `end`. Since these entries
// are empty, it's safe to skip them even if they aren't tombstones.
//
// In addition to skipping tombstone entries, we also skip invalid entries
// where `begin` is greater than `end`. This can occur due to compiler bugs.
if range.begin == tombstone || range.begin >= range.end {
return Ok(None);
}

Expand Down Expand Up @@ -697,8 +708,9 @@ mod tests {
.L8(7).L32(0x201_0c00).uleb(0x100)
// An OffsetPair that starts at 0.
.L8(4).uleb(0).uleb(1)
// An OffsetPair that starts and ends at 0.
// An OffsetPair that starts and ends at 0 followed by a normal OffsetPair.
.L8(4).uleb(0).uleb(0)
.L8(4).uleb(0x10e00).uleb(0x10f00)
// An OffsetPair that ends at -1.
.L8(5).L32(0)
.L8(4).uleb(0).uleb(0xffff_ffff)
Expand Down Expand Up @@ -762,13 +774,6 @@ mod tests {
);

// An empty range followed by a normal range.
assert_eq!(
ranges.next(),
Ok(Some(Range {
begin: 0x0201_0600,
end: 0x0201_0600,
}))
);
assert_eq!(
ranges.next(),
Ok(Some(Range {
Expand Down Expand Up @@ -804,12 +809,12 @@ mod tests {
}))
);

// A range that starts and ends at 0.
// A range that starts and ends at 0 followed by a normal range.
assert_eq!(
ranges.next(),
Ok(Some(Range {
begin: 0x0200_0000,
end: 0x0200_0000,
begin: 0x0201_0e00,
end: 0x0201_0f00,
}))
);

Expand Down Expand Up @@ -921,8 +926,9 @@ mod tests {
.L8(7).L64(0x201_0c00).uleb(0x100)
// An OffsetPair that starts at 0.
.L8(4).uleb(0).uleb(1)
// An OffsetPair that starts and ends at 0.
// An OffsetPair that starts and ends at 0 followed by a normal OffsetPair.
.L8(4).uleb(0).uleb(0)
.L8(4).uleb(0x10e00).uleb(0x10f00)
// An OffsetPair that ends at -1.
.L8(5).L64(0)
.L8(4).uleb(0).uleb(0xffff_ffff)
Expand Down Expand Up @@ -986,13 +992,6 @@ mod tests {
);

// An empty range followed by a normal range.
assert_eq!(
ranges.next(),
Ok(Some(Range {
begin: 0x0201_0600,
end: 0x0201_0600,
}))
);
assert_eq!(
ranges.next(),
Ok(Some(Range {
Expand Down Expand Up @@ -1028,12 +1027,12 @@ mod tests {
}))
);

// A range that starts and ends at 0.
// A range that starts and ends at 0 followed by a normal range.
assert_eq!(
ranges.next(),
Ok(Some(Range {
begin: 0x0200_0000,
end: 0x0200_0000,
begin: 0x0201_0e00,
end: 0x0201_0f00,
}))
);

Expand Down Expand Up @@ -1199,13 +1198,6 @@ mod tests {
);

// An empty range followed by a normal range.
assert_eq!(
ranges.next(),
Ok(Some(Range {
begin: 0x0201_0600,
end: 0x0201_0600,
}))
);
assert_eq!(
ranges.next(),
Ok(Some(Range {
Expand Down Expand Up @@ -1317,13 +1309,6 @@ mod tests {
);

// An empty range followed by a normal range.
assert_eq!(
ranges.next(),
Ok(Some(Range {
begin: 0x0201_0600,
end: 0x0201_0600,
}))
);
assert_eq!(
ranges.next(),
Ok(Some(Range {
Expand Down

0 comments on commit 67dbf2f

Please sign in to comment.