From 2c2ee8bf082e850a81003abcdfab87704e3be095 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Tue, 15 Aug 2023 09:40:32 -0400 Subject: [PATCH] Validate splits don't overlap symbols (or each other) Use DoubleEndedIterator::next_back instead of Iterator::last to avoid unnecessary iteration. --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/analysis/slices.rs | 2 +- src/analysis/tracker.rs | 2 +- src/cmd/dol.rs | 2 +- src/obj/mod.rs | 16 +++++++--- src/obj/split.rs | 66 ++++++++++++++++++++++++++++++++++++++++- 7 files changed, 82 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6adccc2..6fdc11e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -214,7 +214,7 @@ dependencies = [ [[package]] name = "decomp-toolkit" -version = "0.3.5" +version = "0.3.6" dependencies = [ "anyhow", "ar", diff --git a/Cargo.toml b/Cargo.toml index f6676fc..b3b70f4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ name = "decomp-toolkit" description = "Yet another GameCube/Wii decompilation toolkit." authors = ["Luke Street "] license = "MIT OR Apache-2.0" -version = "0.3.5" +version = "0.3.6" edition = "2021" publish = false build = "build.rs" diff --git a/src/analysis/slices.rs b/src/analysis/slices.rs index 9150e4a..99cfcc3 100644 --- a/src/analysis/slices.rs +++ b/src/analysis/slices.rs @@ -80,7 +80,7 @@ impl FunctionSlices { pub fn add_block_start(&mut self, addr: u32) -> bool { // Slice previous block. - if let Some((_, end)) = self.blocks.range_mut(..addr).last() { + if let Some((_, end)) = self.blocks.range_mut(..addr).next_back() { let last_end = *end; if last_end > addr { *end = addr; diff --git a/src/analysis/tracker.rs b/src/analysis/tracker.rs index 7f4b123..96e7173 100644 --- a/src/analysis/tracker.rs +++ b/src/analysis/tracker.rs @@ -376,7 +376,7 @@ impl Tracker { if self.ignore_addresses.contains(&addr) { return false; } - if let Some((&start, &end)) = obj.blocked_ranges.range(..=from).last() { + if let Some((&start, &end)) = obj.blocked_ranges.range(..=from).next_back() { if from >= start && from < end { return false; } diff --git a/src/cmd/dol.rs b/src/cmd/dol.rs index b604ac8..73ef88a 100644 --- a/src/cmd/dol.rs +++ b/src/cmd/dol.rs @@ -509,7 +509,7 @@ fn validate>(obj: &ObjInfo, elf_file: P, state: &AnalyzerState) - continue; } if !real_functions.contains_key(&start) { - let (real_addr, real_name) = real_functions.range(..start).last().unwrap(); + let (real_addr, real_name) = real_functions.range(..start).next_back().unwrap(); log::warn!( "Function {:#010X} not real (actually a part of {} @ {:#010X})", start, diff --git a/src/obj/mod.rs b/src/obj/mod.rs index 7816c1b..ce296de 100644 --- a/src/obj/mod.rs +++ b/src/obj/mod.rs @@ -478,7 +478,10 @@ impl ObjSymbols { Ok(result) } - pub fn by_kind(&self, kind: ObjSymbolKind) -> impl Iterator { + pub fn by_kind( + &self, + kind: ObjSymbolKind, + ) -> impl DoubleEndedIterator { self.symbols.iter().enumerate().filter(move |(_, sym)| sym.kind == kind) } @@ -661,15 +664,20 @@ impl ObjInfo { /// Locate an existing split for the given address. pub fn split_for(&self, address: u32) -> Option<(u32, &ObjSplit)> { - match self.splits_for_range(..=address).last() { + match self.splits_for_range(..=address).next_back() { Some((addr, split)) if split.end == 0 || split.end > address => Some((addr, split)), _ => None, } } /// Locate existing splits within the given address range. - pub fn splits_for_range(&self, range: R) -> impl Iterator - where R: RangeBounds { + pub fn splits_for_range( + &self, + range: R, + ) -> impl DoubleEndedIterator + where + R: RangeBounds, + { self.splits.range(range).flat_map(|(addr, v)| v.iter().map(move |u| (*addr, u))) } diff --git a/src/obj/split.rs b/src/obj/split.rs index b811878..7f308ba 100644 --- a/src/obj/split.rs +++ b/src/obj/split.rs @@ -386,6 +386,67 @@ fn update_common_splits(obj: &mut ObjInfo) -> Result<()> { Ok(()) } +/// Final validation of splits. +fn validate_splits(obj: &ObjInfo) -> Result<()> { + let mut last_split_end = 0; + for (addr, split) in obj.splits_for_range(..) { + let section = obj.section_at(addr)?; + ensure!( + addr >= last_split_end, + "Split {} {} {:#010X}..{:#010X} overlaps with previous split", + split.unit, + section.name, + addr, + split.end + ); + ensure!( + split.end > 0 && split.end > addr, + "Invalid split end {} {} {:#010X}..{:#010X}", + split.unit, + section.name, + addr, + split.end + ); + last_split_end = split.end; + + if let Some((_, symbol)) = + obj.symbols.for_range(..addr).filter(|&(_, s)| s.size_known && s.size > 0).next_back() + { + ensure!( + addr >= symbol.address as u32 + symbol.size as u32, + "Split {} {} {:#010X}..{:#010X} overlaps symbol '{}' {:#010X}..{:#010X}", + split.unit, + section.name, + addr, + split.end, + symbol.name, + symbol.address, + symbol.address + symbol.size + ); + } + + if let Some((_, symbol)) = obj + .symbols + .for_range(..split.end) + .filter(|&(_, s)| s.size_known && s.size > 0) + .next_back() + { + ensure!( + split.end >= symbol.address as u32 + symbol.size as u32, + "Split {} {} ({:#010X}..{:#010X}) ends within symbol '{}' ({:#010X}..{:#010X})", + split.unit, + section.name, + addr, + split.end, + symbol.name, + symbol.address, + symbol.address + symbol.size + ); + } + } + Ok(()) +} + /// Perform any necessary adjustments to allow relinking. /// This includes: /// - Ensuring .ctors & .dtors entries are split with their associated function @@ -418,6 +479,9 @@ pub fn update_splits(obj: &mut ObjInfo) -> Result<()> { // Update common BSS splits update_common_splits(obj)?; + // Ensure splits don't overlap symbols or each other + validate_splits(obj)?; + // Resolve link order obj.link_order = resolve_link_order(obj)?; @@ -908,7 +972,7 @@ pub fn end_for_section(obj: &ObjInfo, section_index: usize) -> Result { .symbols .for_range(section_start..section_end) .filter(|(_, s)| s.kind == ObjSymbolKind::Object && s.size_known && s.size > 0) - .last(); + .next_back(); match last_symbol { Some((_, symbol)) if is_linker_generated_object(&symbol.name) => { log::debug!(