Validate splits don't overlap symbols (or each other)

Use DoubleEndedIterator::next_back instead of Iterator::last to avoid unnecessary iteration.

Cargo.lock

@@ -214,7 +214,7 @@ dependencies = [
 
 [[package]]
 name = "decomp-toolkit"
-version = "0.3.5"
+version = "0.3.6"
 dependencies = [
  "anyhow",
  "ar",

Cargo.toml

@@ -3,7 +3,7 @@ name = "decomp-toolkit"
 description = "Yet another GameCube/Wii decompilation toolkit."
 authors = ["Luke Street <luke@street.dev>"]
 license = "MIT OR Apache-2.0"
-version = "0.3.5"
+version = "0.3.6"
 edition = "2021"
 publish = false
 build = "build.rs"

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;

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;
             }

src/cmd/dol.rs

@@ -509,7 +509,7 @@ fn validate<P: AsRef<Path>>(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,

src/obj/mod.rs

@@ -478,7 +478,10 @@ impl ObjSymbols {
         Ok(result)
     }
 
-    pub fn by_kind(&self, kind: ObjSymbolKind) -> impl Iterator<Item = (SymbolIndex, &ObjSymbol)> {
+    pub fn by_kind(
+        &self,
+        kind: ObjSymbolKind,
+    ) -> impl DoubleEndedIterator<Item = (SymbolIndex, &ObjSymbol)> {
         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<R>(&self, range: R) -> impl Iterator<Item = (u32, &ObjSplit)>
-    where R: RangeBounds<u32> {
+    pub fn splits_for_range<R>(
+        &self,
+        range: R,
+    ) -> impl DoubleEndedIterator<Item = (u32, &ObjSplit)>
+    where
+        R: RangeBounds<u32>,
+    {
         self.splits.range(range).flat_map(|(addr, v)| v.iter().map(move |u| (*addr, u)))
     }
 

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<u32> {
             .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!(