Improve automatic symbol pairing for nameless literals (#247)

* Improve automatic symbol pairing for nameless literals

* Fix data reloc diffing when the reloc points to an in-function static symbol

* Only pair up literals that match perfectly

* Clippy

* Do two separate passes when pairing up literals

* Fix partially-matching literal pairups not working right

* Remove duplicate $ splitting code

* Implement $ splitting for section names too

* Minor cleanup

.github/workflows/build.yaml

@@ -36,9 +36,9 @@ jobs:
       - name: Cache Rust workspace
         uses: Swatinem/rust-cache@v2
       - name: Cargo check
-        run: cargo check --all-targets --all-features
+        run: cargo check --all-targets --all-features --workspace
       - name: Cargo clippy
-        run: cargo clippy --all-targets --all-features
+        run: cargo clippy --all-targets --all-features --workspace
 
   fmt:
     name: Format
@@ -92,7 +92,7 @@ jobs:
       - name: Cache Rust workspace
         uses: Swatinem/rust-cache@v2
       - name: Cargo test
-        run: cargo test --release --features all
+        run: cargo test --release --all-features --workspace
 
   build-cli:
     name: Build objdiff-cli
@@ -146,13 +146,14 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@v4
+      - name: Install uv
+        if: matrix.build == 'zigbuild'
+        uses: astral-sh/setup-uv@v6
       - name: Install cargo-zigbuild
         if: matrix.build == 'zigbuild'
         run: |
-          python3 -m venv .venv
-          . .venv/bin/activate
-          echo PATH=$PATH >> $GITHUB_ENV
-          pip install ziglang==0.13.0.post1 cargo-zigbuild==0.19.8
+          uv tool install cargo-zigbuild==0.20.1 --with-executables-from ziglang==0.15.1
+          echo "CARGO_ZIGBUILD_ZIG_PATH=$(uv tool dir)/cargo-zigbuild/bin/python-zig" >> $GITHUB_ENV
       - name: Setup Rust toolchain
         uses: dtolnay/rust-toolchain@stable
         with:
@@ -213,13 +214,14 @@ jobs:
           sudo apt-get -y install ${{ matrix.packages }}
       - name: Checkout
         uses: actions/checkout@v4
+      - name: Install uv
+        if: matrix.build == 'zigbuild'
+        uses: astral-sh/setup-uv@v6
       - name: Install cargo-zigbuild
         if: matrix.build == 'zigbuild'
         run: |
-          python3 -m venv .venv
-          . .venv/bin/activate
-          echo PATH=$PATH >> $GITHUB_ENV
-          pip install ziglang==0.13.0.post1 cargo-zigbuild==0.19.8
+          uv tool install cargo-zigbuild==0.20.1 --with-executables-from ziglang==0.15.1
+          echo "CARGO_ZIGBUILD_ZIG_PATH=$(uv tool dir)/cargo-zigbuild/bin/python-zig" >> $GITHUB_ENV
       - name: Setup Rust toolchain
         uses: dtolnay/rust-toolchain@stable
         with:

.pre-commit-config.yaml

@@ -24,7 +24,7 @@ repos:
         description: Run cargo clippy on all project files.
         language: system
         entry: cargo
-        args: ["+nightly", "clippy", "--all-targets", "--all-features"]
+        args: ["+nightly", "clippy", "--all-targets", "--all-features", "--workspace"]
         pass_filenames: false
       - id: cargo-deny
         name: cargo deny

Cargo.toml

@@ -5,18 +5,28 @@ members = [
     "objdiff-gui",
     "objdiff-wasm",
 ]
+default-members = [
+    "objdiff-cli",
+    "objdiff-core",
+    "objdiff-gui",
+    # Exclude objdiff-wasm by default
+]
 resolver = "3"
 
+[workspace.package]
+version = "3.0.1"
+authors = ["Luke Street <luke@street.dev>"]
+edition = "2024"
+license = "MIT OR Apache-2.0"
+repository = "https://github.com/encounter/objdiff"
+rust-version = "1.88"
+
 [profile.release-lto]
 inherits = "release"
 lto = "fat"
 strip = "debuginfo"
 codegen-units = 1
 
-[workspace.package]
-version = "3.0.0-beta.12"
-authors = ["Luke Street <luke@street.dev>"]
-edition = "2024"
-license = "MIT OR Apache-2.0"
-repository = "https://github.com/encounter/objdiff"
-rust-version = "1.88"
+[profile.release-min]
+inherits = "release-lto"
+opt-level = "z"

README.md

@@ -90,22 +90,31 @@ file as well. You can then add `objdiff.json` to your `.gitignore` to prevent it
   "build_base": true,
   "watch_patterns": [
     "*.c",
+    "*.cc",
     "*.cp",
     "*.cpp",
     "*.cxx",
+    "*.c++",
     "*.h",
+    "*.hh",
     "*.hp",
     "*.hpp",
     "*.hxx",
+    "*.h++",
+    "*.pch",
+    "*.pch++",
+    "*.inc",
     "*.s",
     "*.S",
     "*.asm",
-    "*.inc",
     "*.py",
     "*.yml",
     "*.txt",
     "*.json"
   ],
+  "ignore_patterns": [
+    "build/**/*"
+  ],
   "units": [
     {
       "name": "main/MetroTRK/mslsupp",
@@ -141,6 +150,10 @@ It's unlikely you'll want to disable this, unless you're using an external tool
 If any of these files change, objdiff will automatically rebuild the objects and re-compare them.  
 If not specified, objdiff will use the default patterns listed above.
 
+`ignore_patterns` _(optional)_: A list of glob patterns to explicitly ignore when watching for changes.  
+([Supported syntax](https://docs.rs/globset/latest/globset/#syntax))  
+If not specified, objdiff will use the default patterns listed above.
+
 `units` _(optional)_: If specified, objdiff will display a list of objects in the sidebar for easy navigation.
 
 > `name` _(optional)_: The name of the object in the UI. If not specified, the object's `path` will be used.

config.schema.json

@@ -57,23 +57,39 @@
       },
       "default": [
         "*.c",
+        "*.cc",
         "*.cp",
         "*.cpp",
         "*.cxx",
+        "*.c++",
         "*.h",
+        "*.hh",
         "*.hp",
         "*.hpp",
         "*.hxx",
+        "*.h++",
+        "*.pch",
+        "*.pch++",
+        "*.inc",
         "*.s",
         "*.S",
         "*.asm",
-        "*.inc",
         "*.py",
         "*.yml",
         "*.txt",
         "*.json"
       ]
     },
+    "ignore_patterns": {
+      "type": "array",
+      "description": "List of glob patterns to explicitly ignore when watching for changes.\nFiles matching these patterns will not trigger a rebuild.\nSupported syntax: https://docs.rs/globset/latest/globset/#syntax",
+      "items": {
+        "type": "string"
+      },
+      "default": [
+        "build/**/*"
+      ]
+    },
     "objects": {
       "type": "array",
       "description": "Use units instead.",

deny.toml

@@ -74,6 +74,7 @@ ignore = [
     #"a-crate-that-is-yanked@0.1.1", # you can also ignore yanked crate versions if you wish
     #{ crate = "a-crate-that-is-yanked@0.1.1", reason = "you can specify why you are ignoring the yanked crate" },
     { id = "RUSTSEC-2024-0436", reason = "Unmaintained paste crate is an indirect dependency" },
+    { id = "RUSTSEC-2025-0052", reason = "Unmaintained async-std crate is an indirect dependency" },
 ]
 # If this is true, then cargo deny will use the git executable to fetch advisory database.
 # If this is false, then it uses a built-in git library.
@@ -241,8 +242,8 @@ allow-git = []
 [sources.allow-org]
 # github.com organizations to allow git sources for
 github = [
-    "enarx", # flagset
     "encounter",
+    "gimli-rs",  # gimli
 ]
 # gitlab.com organizations to allow git sources for
 gitlab = []

objdiff-cli/src/cmd/diff.rs

@@ -19,7 +19,6 @@ use crossterm::{
     },
 };
 use objdiff_core::{
-    bindings::diff::DiffResult,
     build::{
         BuildConfig, BuildStatus,
         watcher::{Watcher, create_watcher},
@@ -28,7 +27,7 @@ use objdiff_core::{
         ProjectConfig, ProjectObject, ProjectObjectMetadata, build_globset,
         path::{check_path_buf, platform_path, platform_path_serde_option},
     },
-    diff::{self, DiffObjConfig, MappingConfig, ObjectDiff},
+    diff::{DiffObjConfig, MappingConfig, ObjectDiff},
     jobs::{
         Job, JobQueue, JobResult,
         objdiff::{ObjDiffConfig, start_build},
@@ -40,10 +39,7 @@ use typed_path::{Utf8PlatformPath, Utf8PlatformPathBuf};
 
 use crate::{
     cmd::apply_config_args,
-    util::{
-        output::{OutputFormat, write_output},
-        term::crossterm_panic_handler,
-    },
+    util::term::crossterm_panic_handler,
     views::{EventControlFlow, EventResult, UiView, function_diff::FunctionDiffUi},
 };
 
@@ -63,12 +59,6 @@ pub struct Args {
     #[argp(option, short = 'u')]
     /// Unit name within project
     unit: Option<String>,
-    #[argp(option, short = 'o', from_str_fn(platform_path))]
-    /// Output file (one-shot mode) ("-" for stdout)
-    output: Option<Utf8PlatformPathBuf>,
-    #[argp(option)]
-    /// Output format (json, json-pretty, proto) (default: json)
-    format: Option<String>,
     #[argp(positional)]
     /// Function symbol to diff
     symbol: Option<String>,
@@ -171,11 +161,7 @@ pub fn run(args: Args) -> Result<()> {
             _ => bail!("Either target and base or project and unit must be specified"),
         };
 
-    if let Some(output) = &args.output {
-        run_oneshot(&args, output, target_path.as_deref(), base_path.as_deref())
-    } else {
-        run_interactive(args, target_path, base_path, project_config)
-    }
+    run_interactive(args, target_path, base_path, project_config)
 }
 
 fn build_config_from_args(args: &Args) -> Result<(DiffObjConfig, MappingConfig)> {
@@ -194,28 +180,6 @@ fn build_config_from_args(args: &Args) -> Result<(DiffObjConfig, MappingConfig)>
     Ok((diff_config, mapping_config))
 }
 
-fn run_oneshot(
-    args: &Args,
-    output: &Utf8PlatformPath,
-    target_path: Option<&Utf8PlatformPath>,
-    base_path: Option<&Utf8PlatformPath>,
-) -> Result<()> {
-    let output_format = OutputFormat::from_option(args.format.as_deref())?;
-    let (diff_config, mapping_config) = build_config_from_args(args)?;
-    let target = target_path
-        .map(|p| obj::read::read(p.as_ref(), &diff_config).with_context(|| format!("Loading {p}")))
-        .transpose()?;
-    let base = base_path
-        .map(|p| obj::read::read(p.as_ref(), &diff_config).with_context(|| format!("Loading {p}")))
-        .transpose()?;
-    let result =
-        diff::diff_objs(target.as_ref(), base.as_ref(), None, &diff_config, &mapping_config)?;
-    let left = target.as_ref().and_then(|o| result.left.as_ref().map(|d| (o, d)));
-    let right = base.as_ref().and_then(|o| result.right.as_ref().map(|d| (o, d)));
-    write_output(&DiffResult::new(left, right), Some(output), output_format)?;
-    Ok(())
-}
-
 pub struct AppState {
     pub jobs: JobQueue,
     pub waker: Arc<TermWaker>,
@@ -378,10 +342,12 @@ fn run_interactive(
     };
     if let (Some(project_dir), Some(project_config)) = (&state.project_dir, &state.project_config) {
         let watch_patterns = project_config.build_watch_patterns()?;
+        let ignore_patterns = project_config.build_ignore_patterns()?;
         state.watcher = Some(create_watcher(
             state.modified.clone(),
             project_dir.as_ref(),
             build_globset(&watch_patterns)?,
+            build_globset(&ignore_patterns)?,
             Waker::from(state.waker.clone()),
         )?);
     }

objdiff-cli/src/cmd/report.rs

@@ -248,13 +248,12 @@ fn report_object(
             {
                 continue;
             }
-            if let Some(existing_functions) = &mut existing_functions {
-                if (symbol.flags.contains(SymbolFlag::Global)
+            if let Some(existing_functions) = &mut existing_functions
+                && (symbol.flags.contains(SymbolFlag::Global)
                     || symbol.flags.contains(SymbolFlag::Weak))
-                    && !existing_functions.insert(symbol.name.clone())
-                {
-                    continue;
-                }
+                && !existing_functions.insert(symbol.name.clone())
+            {
+                continue;
             }
             let match_percent = symbol_diff.match_percent.unwrap_or_else(|| {
                 // Support cases where we don't have a target object,

objdiff-cli/src/views/function_diff.rs

@@ -170,32 +170,31 @@ impl UiView for FunctionDiffUi {
 
         let mut prev_text = None;
         let mut prev_margin_text = None;
-        if self.three_way {
-            if let Some((obj, symbol_idx, symbol_diff)) =
+        if self.three_way
+            && let Some((obj, symbol_idx, symbol_diff)) =
                 get_symbol(state.prev_obj.as_ref(), self.prev_sym)
-            {
-                let mut text = Text::default();
-                let rect = content_chunks[4].inner(Margin::new(0, 1));
-                self.print_sym(
-                    &mut text,
-                    obj,
-                    symbol_idx,
-                    symbol_diff,
-                    &state.diff_obj_config,
-                    rect,
-                    &self.right_highlight,
-                    result,
-                    true,
-                );
-                max_width = max_width.max(text.width());
-                prev_text = Some(text);
+        {
+            let mut text = Text::default();
+            let rect = content_chunks[4].inner(Margin::new(0, 1));
+            self.print_sym(
+                &mut text,
+                obj,
+                symbol_idx,
+                symbol_diff,
+                &state.diff_obj_config,
+                rect,
+                &self.right_highlight,
+                result,
+                true,
+            );
+            max_width = max_width.max(text.width());
+            prev_text = Some(text);
 
-                // Render margin
-                let mut text = Text::default();
-                let rect = content_chunks[3].inner(Margin::new(1, 1));
-                self.print_margin(&mut text, symbol_diff, rect);
-                prev_margin_text = Some(text);
-            }
+            // Render margin
+            let mut text = Text::default();
+            let rect = content_chunks[3].inner(Margin::new(1, 1));
+            self.print_margin(&mut text, symbol_diff, rect);
+            prev_margin_text = Some(text);
         }
 
         let max_scroll_x =
@@ -561,10 +560,12 @@ impl FunctionDiffUi {
                 let len = label_text.len();
                 let highlighted =
                     highlight_kind != HighlightKind::None && *highlight == highlight_kind;
-                if let Some((cx, cy)) = result.click_xy {
-                    if cx >= sx && cx < sx + len as u16 && cy == sy {
-                        new_highlight = Some(highlight_kind);
-                    }
+                if let Some((cx, cy)) = result.click_xy
+                    && cx >= sx
+                    && cx < sx + len as u16
+                    && cy == sy
+                {
+                    new_highlight = Some(highlight_kind);
                 }
                 let mut style = Style::new().fg(match segment.color {
                     DiffTextColor::Normal => Color::Gray,

objdiff-core/Cargo.toml

@@ -62,7 +62,10 @@ config = [
     "dep:semver",
     "dep:typed-path",
 ]
-dwarf = ["dep:gimli"]
+dwarf = [
+    "dep:gimli",
+    "dep:typed-arena",
+]
 serde = [
     "dep:pbjson",
     "dep:pbjson-build",
@@ -78,6 +81,7 @@ std = [
     "prost?/std",
     "serde?/std",
     "similar?/std",
+    "typed-arena?/std",
     "typed-path?/std",
     "dep:filetime",
     "dep:memmap2",
@@ -128,12 +132,12 @@ itertools = { version = "0.14", default-features = false, features = ["use_alloc
 log = { version = "0.4", default-features = false, optional = true }
 memmap2 = { version = "0.9", optional = true }
 num-traits = { version = "0.2", default-features = false, optional = true }
-object = { git = "https://github.com/gimli-rs/object", rev = "16ff70aa6fbd97d6bb7b92375929f4d72414c32b", default-features = false, features = ["read_core", "elf", "coff"] }
+object = { version = "0.37", default-features = false, features = ["read_core", "elf", "coff"] }
 pbjson = { version = "0.8", default-features = false, optional = true }
 prost = { version = "0.14", default-features = false, features = ["derive"], optional = true }
 regex = { version = "1.11", default-features = false, features = [], optional = true }
 serde = { version = "1.0", default-features = false, features = ["derive"], optional = true }
-similar = { version = "2.7", default-features = false, features = ["hashbrown"], optional = true, git = "https://github.com/encounter/similar.git", branch = "no_std" }
+similar = { git = "https://github.com/encounter/similar.git", branch = "no_std", default-features = false, features = ["hashbrown"], optional = true }
 typed-path = { version = "0.11", default-features = false, optional = true }
 
 # config
@@ -142,7 +146,8 @@ semver = { version = "1.0", default-features = false, optional = true }
 serde_json = { version = "1.0", default-features = false, features = ["alloc"], optional = true }
 
 # dwarf
-gimli = { version = "0.32", default-features = false, features = ["read"], optional = true }
+gimli = { git = "https://github.com/gimli-rs/gimli", rev = "7335f00e7c39fd501511584fefb0ba974117c950", default-features = false, features = ["read"], optional = true }
+typed-arena = { version = "2.0", default-features = false, optional = true }
 
 # ppc
 cwdemangle = { version = "1.0", optional = true }

objdiff-core/protos/diff.proto

@@ -1,165 +0,0 @@
-syntax = "proto3";
-
-package objdiff.diff;
-
-// A symbol
-message Symbol {
-  // Name of the symbol
-  string name = 1;
-  // Demangled name of the symbol
-  optional string demangled_name = 2;
-  // Symbol address
-  uint64 address = 3;
-  // Symbol size
-  uint64 size = 4;
-  // Bitmask of SymbolFlag
-  uint32 flags = 5;
-}
-
-// Symbol visibility flags
-enum SymbolFlag {
-  SYMBOL_NONE = 0;
-  SYMBOL_GLOBAL = 1;
-  SYMBOL_LOCAL = 2;
-  SYMBOL_WEAK = 4;
-  SYMBOL_COMMON = 8;
-  SYMBOL_HIDDEN = 16;
-}
-
-// A single parsed instruction
-message Instruction {
-  // Instruction address
-  uint64 address = 1;
-  // Instruction size
-  uint32 size = 2;
-  // Instruction opcode
-  uint32 opcode = 3;
-  // Instruction mnemonic
-  string mnemonic = 4;
-  // Instruction formatted string
-  string formatted = 5;
-  // Original (unsimplified) instruction string
-  optional string original = 6;
-  // Instruction arguments
-  repeated Argument arguments = 7;
-  // Instruction relocation
-  optional Relocation relocation = 8;
-  // Instruction branch destination
-  optional uint64 branch_dest = 9;
-  // Instruction line number
-  optional uint32 line_number = 10;
-}
-
-// An instruction argument
-message Argument {
-  oneof value {
-    // Plain text
-    string plain_text = 1;
-    // Value
-    ArgumentValue argument = 2;
-    // Relocation
-    ArgumentRelocation relocation = 3;
-    // Branch destination
-    uint64 branch_dest = 4;
-  }
-}
-
-// An instruction argument value
-message ArgumentValue {
-  oneof value {
-    // Signed integer
-    int64 signed = 1;
-    // Unsigned integer
-    uint64 unsigned = 2;
-    // Opaque value
-    string opaque = 3;
-  }
-}
-
-// Marker type for relocation arguments
-message ArgumentRelocation {
-}
-
-message Relocation {
-  uint32 type = 1;
-  string type_name = 2;
-  RelocationTarget target = 3;
-}
-
-message RelocationTarget {
-  uint32 symbol_index = 1;
-  int64 addend = 2;
-}
-
-message InstructionDiffRow {
-  DiffKind diff_kind = 1;
-  optional Instruction instruction = 2;
-  optional InstructionBranchFrom branch_from = 3;
-  optional InstructionBranchTo branch_to = 4;
-  repeated ArgumentDiff arg_diff = 5;
-}
-
-message ArgumentDiff {
-  optional uint32 diff_index = 1;
-}
-
-enum DiffKind {
-  DIFF_NONE = 0;
-  DIFF_REPLACE = 1;
-  DIFF_DELETE = 2;
-  DIFF_INSERT = 3;
-  DIFF_OP_MISMATCH = 4;
-  DIFF_ARG_MISMATCH = 5;
-}
-
-message InstructionBranchFrom {
-  repeated uint32 instruction_index = 1;
-  uint32 branch_index = 2;
-}
-
-message InstructionBranchTo {
-  uint32 instruction_index = 1;
-  uint32 branch_index = 2;
-}
-
-message SymbolDiff {
-  Symbol symbol = 1;
-  repeated InstructionDiffRow instruction_rows = 2;
-  optional float match_percent = 3;
-  // The symbol index in the _other_ object that this symbol was diffed against
-  optional uint32 target_symbol = 5;
-}
-
-message DataDiff {
-  DiffKind kind = 1;
-  bytes data = 2;
-  // May be larger than data
-  uint64 size = 3;
-}
-
-message SectionDiff {
-  string name = 1;
-  SectionKind kind = 2;
-  uint64 size = 3;
-  uint64 address = 4;
-  reserved 5;
-  repeated DataDiff data = 6;
-  optional float match_percent = 7;
-}
-
-enum SectionKind {
-  SECTION_UNKNOWN = 0;
-  SECTION_TEXT = 1;
-  SECTION_DATA = 2;
-  SECTION_BSS = 3;
-}
-
-message ObjectDiff {
-  repeated SectionDiff sections = 1;
-  repeated SymbolDiff symbols = 2;
-}
-
-message DiffResult {
-  optional ObjectDiff left = 1;
-  optional ObjectDiff right = 2;
-}

objdiff-core/src/arch/arm.rs

@@ -464,6 +464,22 @@ impl Arch for ArchArm {
         }
         flags
     }
+
+    fn infer_function_size(
+        &self,
+        symbol: &Symbol,
+        section: &Section,
+        mut next_address: u64,
+    ) -> Result<u64> {
+        // Trim any trailing 4-byte zeroes from the end (padding)
+        while next_address >= symbol.address + 4
+            && let Some(data) = section.data_range(next_address - 4, 4)
+            && data == [0u8; 4]
+        {
+            next_address -= 4;
+        }
+        Ok(next_address.saturating_sub(symbol.address))
+    }
 }
 
 #[derive(Clone, Copy, Debug)]

objdiff-core/src/arch/mips.rs

@@ -15,7 +15,7 @@ use crate::{
     diff::{DiffObjConfig, MipsAbi, MipsInstrCategory, display::InstructionPart},
     obj::{
         InstructionArg, InstructionArgValue, InstructionRef, Relocation, RelocationFlags,
-        ResolvedInstructionRef, ResolvedRelocation, SymbolFlag, SymbolFlagSet,
+        ResolvedInstructionRef, ResolvedRelocation, Section, Symbol, SymbolFlag, SymbolFlagSet,
     },
 };
 
@@ -140,6 +140,14 @@ impl ArchMips {
         })
     }
 
+    fn default_instruction_flags(&self) -> rabbitizer::InstructionFlags {
+        match self.isa_extension {
+            Some(extension) => rabbitizer::InstructionFlags::new_extension(extension),
+            None => rabbitizer::InstructionFlags::new(IsaVersion::MIPS_III),
+        }
+        .with_abi(self.abi)
+    }
+
     fn instruction_flags(&self, diff_config: &DiffObjConfig) -> rabbitizer::InstructionFlags {
         let isa_extension = match diff_config.mips_instr_category {
             MipsInstrCategory::Auto => self.isa_extension,
@@ -151,7 +159,7 @@ impl ArchMips {
         };
         match isa_extension {
             Some(extension) => rabbitizer::InstructionFlags::new_extension(extension),
-            None => rabbitizer::InstructionFlags::new_isa(IsaVersion::MIPS_III, None),
+            None => rabbitizer::InstructionFlags::new(IsaVersion::MIPS_III),
         }
         .with_abi(match diff_config.mips_abi {
             MipsAbi::Auto => self.abi,
@@ -234,17 +242,16 @@ impl Arch for ArchMips {
             object::RelocationFlags::Elf { r_type } => {
                 if relocation.has_implicit_addend() {
                     // Check for paired R_MIPS_HI16 and R_MIPS_LO16 relocations.
-                    if let elf::R_MIPS_HI16 | elf::R_MIPS_LO16 = r_type {
-                        if let Some(addend) = self
+                    if let elf::R_MIPS_HI16 | elf::R_MIPS_LO16 = r_type
+                        && let Some(addend) = self
                             .paired_relocations
                             .get(section.index().0)
                             .and_then(|m| m.get(&address).copied())
-                        {
-                            return Ok(Some(RelocationOverride {
-                                target: RelocationOverrideTarget::Keep,
-                                addend,
-                            }));
-                        }
+                    {
+                        return Ok(Some(RelocationOverride {
+                            target: RelocationOverrideTarget::Keep,
+                            addend,
+                        }));
                     }
 
                     let data = section.data()?;
@@ -331,6 +338,36 @@ impl Arch for ArchMips {
         }
         flags
     }
+
+    fn infer_function_size(
+        &self,
+        symbol: &Symbol,
+        section: &Section,
+        next_address: u64,
+    ) -> Result<u64> {
+        // Trim any trailing 4-byte zeroes from the end (nops)
+        let mut new_address = next_address;
+        while new_address >= symbol.address + 4
+            && let Some(data) = section.data_range(new_address - 4, 4)
+            && data == [0u8; 4]
+        {
+            new_address -= 4;
+        }
+        // Check if the last instruction has a delay slot, if so, include the delay slot nop
+        if new_address + 4 <= next_address
+            && new_address >= symbol.address + 4
+            && let Some(data) = section.data_range(new_address - 4, 4)
+            && let instruction = rabbitizer::Instruction::new(
+                self.endianness.read_u32_bytes(data.try_into().unwrap()),
+                Vram::new((new_address - 4) as u32),
+                self.default_instruction_flags(),
+            )
+            && instruction.opcode().has_delay_slot()
+        {
+            new_address += 4;
+        }
+        Ok(new_address.saturating_sub(symbol.address))
+    }
 }
 
 fn push_args(

objdiff-core/src/arch/mod.rs

@@ -6,6 +6,7 @@ use alloc::{
     vec::Vec,
 };
 use core::{
+    any::Any,
     ffi::CStr,
     fmt::{self, Debug},
 };
@@ -215,10 +216,10 @@ impl dyn Arch {
 
         // Remove any branch destinations that are outside the function range
         for ins in result.iter_mut() {
-            if let Some(branch_dest) = ins.branch_dest {
-                if branch_dest < function_start || branch_dest >= function_end {
-                    ins.branch_dest = None;
-                }
+            if let Some(branch_dest) = ins.branch_dest
+                && (branch_dest < function_start || branch_dest >= function_end)
+            {
+                ins.branch_dest = None;
             }
         }
 
@@ -305,7 +306,7 @@ impl dyn Arch {
     }
 }
 
-pub trait Arch: Send + Sync + Debug {
+pub trait Arch: Any + Debug + Send + Sync {
     /// Finishes arch-specific initialization that must be done after sections have been combined.
     fn post_init(&mut self, _sections: &[Section], _symbols: &[Symbol]) {}
 
@@ -406,6 +407,15 @@ pub trait Arch: Send + Sync + Debug {
     ) -> Vec<ContextItem> {
         Vec::new()
     }
+
+    fn infer_function_size(
+        &self,
+        symbol: &Symbol,
+        _section: &Section,
+        next_address: u64,
+    ) -> Result<u64> {
+        Ok(next_address.saturating_sub(symbol.address))
+    }
 }
 
 pub fn new_arch(object: &object::File) -> Result<Box<dyn Arch>> {

objdiff-core/src/arch/ppc/flow_analysis.rs

@@ -514,14 +514,14 @@ pub fn ppc_data_flow_analysis(
 }
 
 fn get_string_data(obj: &Object, symbol_index: usize, offset: Simm) -> Option<&str> {
-    if let Some(sym) = obj.symbols.get(symbol_index) {
-        if sym.name.starts_with("@stringBase") && offset.0 != 0 {
-            if let Some(data) = obj.symbol_data(symbol_index) {
-                let bytes = &data[offset.0 as usize..];
-                if let Ok(Ok(str)) = CStr::from_bytes_until_nul(bytes).map(|x| x.to_str()) {
-                    return Some(str);
-                }
-            }
+    if let Some(sym) = obj.symbols.get(symbol_index)
+        && sym.name.starts_with("@stringBase")
+        && offset.0 != 0
+        && let Some(data) = obj.symbol_data(symbol_index)
+    {
+        let bytes = &data[offset.0 as usize..];
+        if let Ok(Ok(str)) = CStr::from_bytes_until_nul(bytes).map(|x| x.to_str()) {
+            return Some(str);
         }
     }
     None
@@ -577,19 +577,17 @@ fn generate_flow_analysis_result(
         let registers = register_state_at.get(index as usize).unwrap_or(&default_register_state);
         if let (powerpc::Opcode::Addi, Argument::GPR(rel), Argument::Simm(offset)) =
             (ins.op, args[1], args[2])
+            && let RegisterContent::Symbol(sym_index) = registers[rel]
+            && let Some(str) = get_string_data(obj, sym_index, offset)
         {
-            if let RegisterContent::Symbol(sym_index) = registers[rel] {
-                if let Some(str) = get_string_data(obj, sym_index, offset) {
-                    // Show the string constant in the analysis result
-                    let formatted = format!("\"{str}\"");
-                    analysis_result.set_argument_value_at_address(
-                        ins_address,
-                        2,
-                        FlowAnalysisValue::Text(clamp_text_length(formatted, 20)),
-                    );
-                    // Don't continue, we want to show the stringbase value as well
-                }
-            }
+            // Show the string constant in the analysis result
+            let formatted = format!("\"{str}\"");
+            analysis_result.set_argument_value_at_address(
+                ins_address,
+                2,
+                FlowAnalysisValue::Text(clamp_text_length(formatted, 20)),
+            );
+            // Don't continue, we want to show the stringbase value as well
         }
 
         let is_store = is_store_instruction(ins.op);

objdiff-core/src/arch/ppc/mod.rs

@@ -20,7 +20,7 @@ use crate::{
     },
     obj::{
         FlowAnalysisResult, InstructionRef, Object, Relocation, RelocationFlags,
-        ResolvedInstructionRef, ResolvedRelocation, Symbol, SymbolFlag, SymbolFlagSet,
+        ResolvedInstructionRef, ResolvedRelocation, Section, Symbol, SymbolFlag, SymbolFlagSet,
     },
 };
 
@@ -66,7 +66,9 @@ impl ArchPpc {
                 if file.is_64() {
                     powerpc::Extension::Ppc64 | powerpc::Extension::AltiVec
                 } else {
-                    powerpc::Extension::AltiVec.into()
+                    // Gekko/Broadway objects often use the EF_PPC_EMB flag,
+                    // but ProDG in particular does not emit it.
+                    powerpc::Extensions::gekko_broadway()
                 }
             }
         };
@@ -455,6 +457,22 @@ impl Arch for ArchPpc {
         }
         out
     }
+
+    fn infer_function_size(
+        &self,
+        symbol: &Symbol,
+        section: &Section,
+        mut next_address: u64,
+    ) -> Result<u64> {
+        // Trim any trailing 4-byte zeroes from the end (padding)
+        while next_address >= symbol.address + 4
+            && let Some(data) = section.data_range(next_address - 4, 4)
+            && data == [0u8; 4]
+        {
+            next_address -= 4;
+        }
+        Ok(next_address.saturating_sub(symbol.address))
+    }
 }
 
 impl ArchPpc {
@@ -657,7 +675,7 @@ fn make_symbol_ref(symbol: &object::Symbol) -> Result<ExtabSymbolRef> {
 #[derive(Debug)]
 struct PoolReference {
     addr_src_gpr: powerpc::GPR,
-    addr_offset: i16,
+    addr_offset: i64,
     addr_dst_gpr: Option<powerpc::GPR>,
 }
 
@@ -665,16 +683,20 @@ struct PoolReference {
 // If so, return information pertaining to where the instruction is getting that address from and
 // what it's doing with the address (e.g. copying it into another register, adding an offset, etc).
 fn get_pool_reference_for_inst(
-    opcode: powerpc::Opcode,
+    ins: powerpc::Ins,
     simplified: &powerpc::ParsedIns,
 ) -> Option<PoolReference> {
     use powerpc::{Argument, Opcode};
     let args = &simplified.args;
-    if flow_analysis::guess_data_type_from_load_store_inst_op(opcode).is_some() {
+    if flow_analysis::guess_data_type_from_load_store_inst_op(ins.op).is_some() {
         match (args[1], args[2]) {
             (Argument::Offset(offset), Argument::GPR(addr_src_gpr)) => {
                 // e.g. lwz. Immediate offset.
-                Some(PoolReference { addr_src_gpr, addr_offset: offset.0, addr_dst_gpr: None })
+                Some(PoolReference {
+                    addr_src_gpr,
+                    addr_offset: offset.0 as i64,
+                    addr_dst_gpr: None,
+                })
             }
             (Argument::GPR(addr_src_gpr), Argument::GPR(_offset_gpr)) => {
                 // e.g. lwzx. The offset is in a register and was likely calculated from an index.
@@ -694,17 +716,51 @@ fn get_pool_reference_for_inst(
         // If either of these match, we also want to return the destination register that the
         // address is being copied into so that we can detect any future references to that new
         // register as well.
-        match (opcode, args[0], args[1], args[2]) {
+        match (ins.op, args[0], args[1], args[2]) {
             (
+                // `addi` or `subi`
                 Opcode::Addi,
                 Argument::GPR(addr_dst_gpr),
                 Argument::GPR(addr_src_gpr),
                 Argument::Simm(simm),
-            ) => Some(PoolReference {
-                addr_src_gpr,
-                addr_offset: simm.0,
-                addr_dst_gpr: Some(addr_dst_gpr),
-            }),
+            ) => {
+                let offset = if simplified.mnemonic == "addi" { simm.0 } else { -simm.0 };
+                Some(PoolReference {
+                    addr_src_gpr,
+                    addr_offset: offset as i64,
+                    addr_dst_gpr: Some(addr_dst_gpr),
+                })
+            }
+            (
+                // `addis`
+                Opcode::Addis,
+                Argument::GPR(addr_dst_gpr),
+                Argument::GPR(addr_src_gpr),
+                Argument::Uimm(uimm), // Note: `addis` uses UIMM, unlike `addi`, `subi`, and `subis`
+            ) => {
+                assert_eq!(simplified.mnemonic, "addis");
+                let offset = (uimm.0 as i64) << 16;
+                Some(PoolReference {
+                    addr_src_gpr,
+                    addr_offset: offset,
+                    addr_dst_gpr: Some(addr_dst_gpr),
+                })
+            }
+            (
+                // `subis`
+                Opcode::Addis,
+                Argument::GPR(addr_dst_gpr),
+                Argument::GPR(addr_src_gpr),
+                Argument::Simm(simm),
+            ) => {
+                assert_eq!(simplified.mnemonic, "subis");
+                let offset = (simm.0 as i64) << 16;
+                Some(PoolReference {
+                    addr_src_gpr,
+                    addr_offset: offset,
+                    addr_dst_gpr: Some(addr_dst_gpr),
+                })
+            }
             (
                 // `mr` or `mr.`
                 Opcode::Or,
@@ -759,13 +815,13 @@ fn clear_overwritten_gprs(ins: powerpc::Ins, gpr_pool_relocs: &mut BTreeMap<u8,
 // Also, if this instruction is accessing the middle of a symbol instead of the start, we add an
 // addend to indicate that.
 fn make_fake_pool_reloc(
-    offset: i16,
+    offset: i64,
     cur_addr: u32,
     pool_reloc: &Relocation,
     symbols: &[Symbol],
 ) -> Option<Relocation> {
     let pool_reloc = resolve_relocation(symbols, pool_reloc);
-    let offset_from_pool = pool_reloc.relocation.addend + offset as i64;
+    let offset_from_pool = pool_reloc.relocation.addend + offset;
     let target_address = pool_reloc.symbol.address.checked_add_signed(offset_from_pool)?;
     let target_symbol;
     let addend;
@@ -850,44 +906,43 @@ fn generate_fake_pool_relocations_for_function(
                     break;
                 }
             }
-            if let Some(branch_dest) = branch_dest {
-                if branch_dest >= func_address as u32
-                    && (branch_dest - func_address as u32) < code.len() as u32
-                {
-                    let dest_offset_into_func = branch_dest - func_address as u32;
-                    let dest_code_slice = &code[dest_offset_into_func as usize..];
-                    match ins.op {
-                        Opcode::Bc => {
-                            // Conditional branch.
-                            // Add the branch destination to the queue to do later.
+            if let Some(branch_dest) = branch_dest
+                && branch_dest >= func_address as u32
+                && (branch_dest - func_address as u32) < code.len() as u32
+            {
+                let dest_offset_into_func = branch_dest - func_address as u32;
+                let dest_code_slice = &code[dest_offset_into_func as usize..];
+                match ins.op {
+                    Opcode::Bc => {
+                        // Conditional branch.
+                        // Add the branch destination to the queue to do later.
+                        ins_iters_with_gpr_state.push((
+                            InsIter::new(dest_code_slice, branch_dest, extensions),
+                            gpr_pool_relocs.clone(),
+                        ));
+                        // Then continue on with the current iterator.
+                    }
+                    Opcode::B => {
+                        if simplified.mnemonic != "bl" {
+                            // Unconditional branch.
+                            // Add the branch destination to the queue.
                             ins_iters_with_gpr_state.push((
                                 InsIter::new(dest_code_slice, branch_dest, extensions),
                                 gpr_pool_relocs.clone(),
                             ));
-                            // Then continue on with the current iterator.
+                            // Break out of the current iterator so we can do the newly added one.
+                            break;
                         }
-                        Opcode::B => {
-                            if simplified.mnemonic != "bl" {
-                                // Unconditional branch.
-                                // Add the branch destination to the queue.
-                                ins_iters_with_gpr_state.push((
-                                    InsIter::new(dest_code_slice, branch_dest, extensions),
-                                    gpr_pool_relocs.clone(),
-                                ));
-                                // Break out of the current iterator so we can do the newly added one.
-                                break;
-                            }
-                        }
-                        _ => unreachable!(),
                     }
+                    _ => unreachable!(),
                 }
             }
-            if let Opcode::Bcctr = ins.op {
-                if simplified.mnemonic == "bctr" {
-                    // Unconditional branch to count register.
-                    // Likely a jump table.
-                    gpr_state_at_bctr.insert(cur_addr, gpr_pool_relocs.clone());
-                }
+            if let Opcode::Bcctr = ins.op
+                && simplified.mnemonic == "bctr"
+            {
+                // Unconditional branch to count register.
+                // Likely a jump table.
+                gpr_state_at_bctr.insert(cur_addr, gpr_pool_relocs.clone());
             }
 
             // Then handle keeping track of which GPR contains which pool relocation.
@@ -929,7 +984,7 @@ fn generate_fake_pool_relocations_for_function(
                         clear_overwritten_gprs(ins, &mut gpr_pool_relocs);
                     }
                 }
-            } else if let Some(pool_ref) = get_pool_reference_for_inst(ins.op, &simplified) {
+            } else if let Some(pool_ref) = get_pool_reference_for_inst(ins, &simplified) {
                 // This instruction doesn't have a real relocation, so it may be a reference to one of
                 // the already-loaded pools.
                 if let Some(pool_reloc) = gpr_pool_relocs.get(&pool_ref.addr_src_gpr.0) {
@@ -948,7 +1003,7 @@ fn generate_fake_pool_relocations_for_function(
                         // with the offset within the .data section of an array variable into r21.
                         // Then the body of the loop will `lwzx` one of the array elements from r21.
                         let mut new_reloc = pool_reloc.clone();
-                        new_reloc.addend += pool_ref.addr_offset as i64;
+                        new_reloc.addend += pool_ref.addr_offset;
                         gpr_pool_relocs.insert(addr_dst_gpr.0, new_reloc);
                     } else {
                         clear_overwritten_gprs(ins, &mut gpr_pool_relocs);

objdiff-core/src/arch/x86.rs

@@ -11,7 +11,7 @@ use object::{Endian as _, Object as _, ObjectSection as _, elf, pe};
 use crate::{
     arch::{Arch, RelocationOverride, RelocationOverrideTarget},
     diff::{DiffObjConfig, X86Formatter, display::InstructionPart},
-    obj::{InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef},
+    obj::{InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef, Section, Symbol},
 };
 
 #[derive(Debug)]
@@ -303,6 +303,52 @@ impl Arch for ArchX86 {
     fn data_reloc_size(&self, flags: RelocationFlags) -> usize {
         self.reloc_size(flags).unwrap_or(1)
     }
+
+    fn infer_function_size(
+        &self,
+        symbol: &Symbol,
+        section: &Section,
+        next_address: u64,
+    ) -> Result<u64> {
+        let Ok(size) = (next_address - symbol.address).try_into() else {
+            return Ok(next_address.saturating_sub(symbol.address));
+        };
+        let Some(code) = section.data_range(symbol.address, size) else {
+            return Ok(0);
+        };
+        // Decode instructions to find the last non-NOP instruction
+        let mut decoder = self.decoder(code, symbol.address);
+        let mut instruction = Instruction::default();
+        let mut new_address = 0;
+        let mut reloc_iter = section.relocations.iter().peekable();
+        'outer: while decoder.can_decode() {
+            let address = decoder.ip();
+            while let Some(reloc) = reloc_iter.peek() {
+                match reloc.address.cmp(&address) {
+                    Ordering::Less => {
+                        reloc_iter.next();
+                    }
+                    Ordering::Equal => {
+                        // If the instruction starts at a relocation, it's inline data
+                        let reloc_size = self.reloc_size(reloc.flags).with_context(|| {
+                            format!("Unsupported inline x86 relocation {:?}", reloc.flags)
+                        })?;
+                        if decoder.set_position(decoder.position() + reloc_size).is_ok() {
+                            new_address = address + reloc_size as u64;
+                            decoder.set_ip(new_address);
+                            continue 'outer;
+                        }
+                    }
+                    Ordering::Greater => break,
+                }
+            }
+            decoder.decode_out(&mut instruction);
+            if instruction.mnemonic() != iced_x86::Mnemonic::Nop {
+                new_address = instruction.next_ip();
+            }
+        }
+        Ok(new_address.saturating_sub(symbol.address))
+    }
 }
 
 struct InstructionFormatterOutput<'a> {