Improve local branch relocation handling

Reworks the local-branch handling logic to be more
unified: scan_instructions does all the work up front,
and process_instruction / display_instruction can
simply use the calculated branch destination instead
of performing their own is-relocation-target-
function-local checks.

(Hopefully) Fixes #192

.editorconfig

@@ -0,0 +1,2 @@
+[*.md]
+trim_trailing_whitespace = false

.github/workflows/build.yaml

@@ -4,10 +4,14 @@ on:
   pull_request:
   push:
     paths-ignore:
-      - '*.md'
-      - 'LICENSE*'
+      - "*.md"
+      - "LICENSE*"
   workflow_dispatch:
 
+permissions:
+  # For npm publish provenance
+  id-token: write
+
 env:
   BUILD_PROFILE: release-lto
   CARGO_INCREMENTAL: 0
@@ -32,9 +36,9 @@ jobs:
       - name: Cache Rust workspace
         uses: Swatinem/rust-cache@v2
       - name: Cargo check
-        run: cargo check --features all
+        run: cargo check --all-targets --all-features
       - name: Cargo clippy
-        run: cargo clippy --features all
+        run: cargo clippy --all-targets --all-features
 
   fmt:
     name: Format
@@ -159,7 +163,7 @@ jobs:
           key: ${{ matrix.target }}
       - name: Cargo build
         run: >
-          cargo ${{ matrix.build }} --profile ${{ env.BUILD_PROFILE }} --target ${{ matrix.target }} 
+          cargo ${{ matrix.build }} --profile ${{ env.BUILD_PROFILE }} --target ${{ matrix.target }}
           --bin ${{ env.CARGO_BIN_NAME }} --features ${{ matrix.features }}
       - name: Upload artifacts
         uses: actions/upload-artifact@v4
@@ -226,7 +230,7 @@ jobs:
           key: ${{ matrix.target }}
       - name: Cargo build
         run: >
-          cargo ${{ matrix.build }} --profile ${{ env.BUILD_PROFILE }} --target ${{ matrix.target }} 
+          cargo ${{ matrix.build }} --profile ${{ env.BUILD_PROFILE }} --target ${{ matrix.target }}
           --bin ${{ env.CARGO_BIN_NAME }} --features ${{ matrix.features }}
       - name: Upload artifacts
         uses: actions/upload-artifact@v4
@@ -249,22 +253,30 @@ jobs:
           components: rust-src
       - name: Cache Rust workspace
         uses: Swatinem/rust-cache@v2
+      - name: Setup Node
+        uses: actions/setup-node@v4
+        with:
+          node-version: lts/*
       - name: Install dependencies
-        run: npm -C objdiff-wasm install
+        run: npm -C objdiff-wasm ci
       - name: Build
         run: npm -C objdiff-wasm run build
+      - name: Upload artifacts
+        uses: actions/upload-artifact@v4
+        with:
+          name: wasm
+          path: objdiff-wasm/dist/
+          if-no-files-found: error
 
-  release:
-    name: Release
+  check-version:
+    name: Check package versions
     if: startsWith(github.ref, 'refs/tags/')
     runs-on: ubuntu-latest
-    needs: [ build-cli, build-gui ]
-    permissions:
-      contents: write
+    needs: [ build-cli, build-gui, build-wasm ]
     steps:
       - name: Checkout
         uses: actions/checkout@v4
-      - name: Check git tag against Cargo version
+      - name: Check git tag against package versions
         shell: bash
         run: |
           set -eou pipefail
@@ -276,9 +288,24 @@ jobs:
             echo "::error::Git tag doesn't match the Cargo version! ($tag != $version)"
             exit 1
           fi
+          version="v$(jq -r .version objdiff-wasm/package.json)"
+          if [ "$tag" != "$version" ]; then
+            echo "::error::Git tag doesn't match the npm version! ($tag != $version)"
+            exit 1
+          fi
+
+  release-github:
+    name: Release (GitHub)
+    if: startsWith(github.ref, 'refs/tags/')
+    runs-on: ubuntu-latest
+    needs: [ check-version ]
+    permissions:
+      contents: write
+    steps:
       - name: Download artifacts
         uses: actions/download-artifact@v4
         with:
+          pattern: objdiff-*
           path: artifacts
       - name: Rename artifacts
         working-directory: artifacts
@@ -308,3 +335,53 @@ jobs:
           files: out/*
           draft: true
           generate_release_notes: true
+
+  release-cargo:
+    name: Release (Cargo)
+    if: 'false' # TODO re-enable when all dependencies are published
+    runs-on: ubuntu-latest
+    needs: [ check-version ]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+      - name: Setup Rust toolchain
+        uses: dtolnay/rust-toolchain@stable
+      - name: Publish
+        env:
+          CARGO_REGISTRY_TOKEN: ${{ secrets.CARGO_REGISTRY_TOKEN }}
+        run: cargo publish -p objdiff-core
+
+  release-npm:
+    name: Release (npm)
+    if: startsWith(github.ref, 'refs/tags/')
+    runs-on: ubuntu-latest
+    needs: [ check-version ]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+      - name: Setup Node
+        uses: actions/setup-node@v4
+        with:
+          node-version: lts/*
+          registry-url: 'https://registry.npmjs.org'
+      - name: Download artifacts
+        uses: actions/download-artifact@v4
+        with:
+          name: wasm
+          path: objdiff-wasm/dist
+      - name: Publish
+        working-directory: objdiff-wasm
+        env:
+          NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
+        run: |
+          set -euo pipefail
+          version=$(jq -r '.version' package.json)
+          tag="latest"
+          # Check for prerelease by looking for a dash
+          case "$version" in
+            *-*)
+              tag=$(echo "$version" | sed -e 's/^[^-]*-//' -e 's/\..*$//')
+              ;;
+          esac
+          echo "Publishing version $version with tag '$tag'..."
+          npm publish --provenance --access public --tag "$tag"

.pre-commit-config.yaml

@@ -0,0 +1,36 @@
+# See https://pre-commit.com for more information
+# See https://pre-commit.com/hooks.html for more hooks
+repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v5.0.0
+    hooks:
+      - id: trailing-whitespace
+        args: [--markdown-linebreak-ext=md]
+      - id: end-of-file-fixer
+      - id: fix-byte-order-marker
+      - id: check-yaml
+      - id: check-added-large-files
+  - repo: local
+    hooks:
+      - id: cargo-fmt
+        name: cargo fmt
+        description: Run cargo fmt on all project files.
+        language: system
+        entry: cargo
+        args: ["+nightly", "fmt", "--all"]
+        pass_filenames: false
+      - id: cargo clippy
+        name: cargo clippy
+        description: Run cargo clippy on all project files.
+        language: system
+        entry: cargo
+        args: ["+nightly", "clippy", "--all-targets", "--all-features"]
+        pass_filenames: false
+      - id: cargo-deny
+        name: cargo deny
+        description: Run cargo deny on all project files.
+        language: system
+        entry: cargo
+        args: ["deny", "check"]
+        pass_filenames: false
+        always_run: true

Cargo.toml

@@ -14,7 +14,7 @@ strip = "debuginfo"
 codegen-units = 1
 
 [workspace.package]
-version = "3.0.0-beta.3"
+version = "3.0.0-beta.7"
 authors = ["Luke Street <luke@street.dev>"]
 edition = "2024"
 license = "MIT OR Apache-2.0"

README.md

@@ -16,11 +16,12 @@ Features:
 
 Supports:
 
-- PowerPC 750CL (GameCube, Wii)
-- MIPS (N64, PS1, PS2, PSP)
-- x86 (COFF only at the moment)
 - ARM (GBA, DS, 3DS)
-- ARM64 (Switch, experimental)
+- ARM64 (Switch)
+- MIPS (N64, PS1, PS2, PSP)
+- PowerPC (GameCube, Wii)
+- SuperH (Saturn, Dreamcast)
+- x86 (COFF only)
 
 See [Usage](#usage) for more information.
 
@@ -143,15 +144,15 @@ 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.
-> 
+>
 > `target_path`: Path to the "target" or "expected" object from the project root.  
 > This object is the **intended result** of the match.
-> 
+>
 > `base_path`: Path to the "base" or "actual" object from the project root.  
 > This object is built from the **current source code**.
-> 
+>
 > `metadata.auto_generated` _(optional)_: Hides the object from the object list, but still includes it in reports.
-> 
+>
 > `metadata.complete` _(optional)_: Marks the object as "complete" (or "linked") in the object list.  
 > This is useful for marking objects that are fully decompiled. A value of `false` will mark the object as "incomplete".
 
@@ -173,6 +174,21 @@ $ cargo install --locked --git https://github.com/encounter/objdiff.git objdiff-
 
 The binaries will be installed to `~/.cargo/bin` as `objdiff` and `objdiff-cli`.
 
+## Installing `pre-commit`
+
+When contributing, it's recommended to install `pre-commit` to automatically run the linter and formatter before a commit.
+
+[`uv`](https://github.com/astral-sh/uv#installation) is recommended to manage Python version and tools.
+
+Rust nightly is required for `cargo +nightly fmt` and `cargo +nightly clippy`.
+
+```shell
+$ cargo install --locked cargo-deny
+$ rustup toolchain install nightly
+$ uv tool install pre-commit
+$ pre-commit install
+```
+
 ## License
 
 Licensed under either of

deny.toml

@@ -103,6 +103,7 @@ allow = [
     "0BSD",
     "OFL-1.1",
     "Ubuntu-font-1.0",
+    "CDLA-Permissive-2.0",
 ]
 # The confidence threshold for detecting a license from license text.
 # The higher the value, the more closely the license text must be to the
@@ -240,8 +241,6 @@ allow-git = []
 [sources.allow-org]
 # github.com organizations to allow git sources for
 github = [
-    "CelestialAmber", # cwextab, rlwinmdec
-    "Decompollaborate", # rabbitizer
     "enarx", # flagset
     "encounter",
 ]

objdiff-cli/Cargo.toml

@@ -28,7 +28,7 @@ supports-color = "3.0"
 time = { version = "0.3", features = ["formatting", "local-offset"] }
 tracing = "0.1"
 tracing-subscriber = { version = "0.3", features = ["env-filter"] }
-typed-path = "0.10"
+typed-path = "0.11"
 
 [target.'cfg(target_env = "musl")'.dependencies]
 mimalloc = "0.1"

objdiff-cli/src/cmd/diff.rs

@@ -1,7 +1,6 @@
 use std::{
     io::stdout,
     mem,
-    str::FromStr,
     sync::{
         Arc,
         atomic::{AtomicBool, Ordering},
@@ -29,10 +28,7 @@ use objdiff_core::{
         ProjectConfig, ProjectObject, ProjectObjectMetadata, build_globset,
         path::{check_path_buf, platform_path, platform_path_serde_option},
     },
-    diff::{
-        self, ConfigEnum, ConfigPropertyId, ConfigPropertyKind, DiffObjConfig, MappingConfig,
-        ObjectDiff,
-    },
+    diff::{self, DiffObjConfig, MappingConfig, ObjectDiff},
     jobs::{
         Job, JobQueue, JobResult,
         objdiff::{ObjDiffConfig, start_build},
@@ -43,6 +39,7 @@ use ratatui::prelude::*;
 use typed_path::{Utf8PlatformPath, Utf8PlatformPathBuf};
 
 use crate::{
+    cmd::apply_config_args,
     util::{
         output::{OutputFormat, write_output},
         term::crossterm_panic_handler,
@@ -183,28 +180,7 @@ pub fn run(args: Args) -> Result<()> {
 
 fn build_config_from_args(args: &Args) -> Result<(DiffObjConfig, MappingConfig)> {
     let mut diff_config = DiffObjConfig::default();
-    for config in &args.config {
-        let (key, value) = config.split_once('=').context("--config expects \"key=value\"")?;
-        let property_id = ConfigPropertyId::from_str(key)
-            .map_err(|()| anyhow!("Invalid configuration property: {}", key))?;
-        diff_config.set_property_value_str(property_id, value).map_err(|()| {
-            let mut options = String::new();
-            match property_id.kind() {
-                ConfigPropertyKind::Boolean => {
-                    options = "true, false".to_string();
-                }
-                ConfigPropertyKind::Choice(variants) => {
-                    for (i, variant) in variants.iter().enumerate() {
-                        if i > 0 {
-                            options.push_str(", ");
-                        }
-                        options.push_str(variant.value);
-                    }
-                }
-            }
-            anyhow!("Invalid value for {}. Expected one of: {}", property_id.name(), options)
-        })?;
-    }
+    apply_config_args(&mut diff_config, &args.config)?;
     let mut mapping_config = MappingConfig {
         mappings: Default::default(),
         selecting_left: args.selecting_left.clone(),

objdiff-cli/src/cmd/mod.rs

@@ -1,2 +1,33 @@
 pub mod diff;
 pub mod report;
+
+use std::str::FromStr;
+
+use anyhow::{Context, Result, anyhow};
+use objdiff_core::diff::{ConfigEnum, ConfigPropertyId, ConfigPropertyKind, DiffObjConfig};
+
+pub fn apply_config_args(diff_config: &mut DiffObjConfig, args: &[String]) -> Result<()> {
+    for config in args {
+        let (key, value) = config.split_once('=').context("--config expects \"key=value\"")?;
+        let property_id = ConfigPropertyId::from_str(key)
+            .map_err(|()| anyhow!("Invalid configuration property: {}", key))?;
+        diff_config.set_property_value_str(property_id, value).map_err(|()| {
+            let mut options = String::new();
+            match property_id.kind() {
+                ConfigPropertyKind::Boolean => {
+                    options = "true, false".to_string();
+                }
+                ConfigPropertyKind::Choice(variants) => {
+                    for (i, variant) in variants.iter().enumerate() {
+                        if i > 0 {
+                            options.push_str(", ");
+                        }
+                        options.push_str(variant.value);
+                    }
+                }
+            }
+            anyhow!("Invalid value for {}. Expected one of: {}", property_id.name(), options)
+        })?;
+    }
+    Ok(())
+}

objdiff-cli/src/cmd/report.rs

@@ -17,7 +17,7 @@ use tracing::{info, warn};
 use typed_path::{Utf8PlatformPath, Utf8PlatformPathBuf};
 
 use crate::{
-    cmd::diff::ObjectConfig,
+    cmd::{apply_config_args, diff::ObjectConfig},
     util::output::{OutputFormat, write_output},
 };
 
@@ -52,6 +52,9 @@ pub struct GenerateArgs {
     #[argp(option, short = 'f')]
     /// Output format (json, json-pretty, proto) (default: json)
     format: Option<String>,
+    #[argp(option, short = 'c')]
+    /// Configuration property (key=value)
+    config: Vec<String>,
 }
 
 #[derive(FromArgs, PartialEq, Debug)]
@@ -80,6 +83,15 @@ pub fn run(args: Args) -> Result<()> {
 }
 
 fn generate(args: GenerateArgs) -> Result<()> {
+    let mut diff_config = diff::DiffObjConfig {
+        function_reloc_diffs: diff::FunctionRelocDiffs::None,
+        combine_data_sections: true,
+        combine_text_sections: true,
+        ppc_calculate_pool_relocations: false,
+        ..Default::default()
+    };
+    apply_config_args(&mut diff_config, &args.config)?;
+
     let output_format = OutputFormat::from_option(args.format.as_deref())?;
     let project_dir = args.project.as_deref().unwrap_or_else(|| Utf8PlatformPath::new("."));
     info!("Loading project {}", project_dir);
@@ -114,14 +126,15 @@ fn generate(args: GenerateArgs) -> Result<()> {
     if args.deduplicate {
         // If deduplicating, we need to run single-threaded
         for object in &objects {
-            if let Some(unit) = report_object(object, Some(&mut existing_functions))? {
+            if let Some(unit) = report_object(object, &diff_config, Some(&mut existing_functions))?
+            {
                 units.push(unit);
             }
         }
     } else {
         let vec = objects
             .par_iter()
-            .map(|object| report_object(object, None))
+            .map(|object| report_object(object, &diff_config, None))
             .collect::<Result<Vec<Option<ReportUnit>>>>()?;
         units = vec.into_iter().flatten().collect();
     }
@@ -145,6 +158,7 @@ fn generate(args: GenerateArgs) -> Result<()> {
 
 fn report_object(
     object: &ObjectConfig,
+    diff_config: &diff::DiffObjConfig,
     mut existing_functions: Option<&mut HashSet<String>>,
 ) -> Result<Option<ReportUnit>> {
     match (&object.target_path, &object.base_path) {
@@ -158,16 +172,12 @@ fn report_object(
         }
         _ => {}
     }
-    let diff_config = diff::DiffObjConfig {
-        function_reloc_diffs: diff::FunctionRelocDiffs::None,
-        ..Default::default()
-    };
     let mapping_config = diff::MappingConfig::default();
     let target = object
         .target_path
         .as_ref()
         .map(|p| {
-            obj::read::read(p.as_ref(), &diff_config)
+            obj::read::read(p.as_ref(), diff_config)
                 .with_context(|| format!("Failed to open {}", p))
         })
         .transpose()?;
@@ -175,12 +185,12 @@ fn report_object(
         .base_path
         .as_ref()
         .map(|p| {
-            obj::read::read(p.as_ref(), &diff_config)
+            obj::read::read(p.as_ref(), diff_config)
                 .with_context(|| format!("Failed to open {}", p))
         })
         .transpose()?;
     let result =
-        diff::diff_objs(target.as_ref(), base.as_ref(), None, &diff_config, &mapping_config)?;
+        diff::diff_objs(target.as_ref(), base.as_ref(), None, diff_config, &mapping_config)?;
 
     let metadata = ReportUnitMetadata {
         complete: object.metadata.complete,
@@ -202,6 +212,9 @@ fn report_object(
     for ((section_idx, section), section_diff) in
         obj.sections.iter().enumerate().zip(&obj_diff.sections)
     {
+        if section.kind == SectionKind::Unknown {
+            continue;
+        }
         let section_match_percent = section_diff.match_percent.unwrap_or_else(|| {
             // Support cases where we don't have a target object,
             // assume complete means 100% match
@@ -215,6 +228,7 @@ fn report_object(
                 demangled_name: None,
                 virtual_address: section.virtual_address,
             }),
+            address: None,
         });
 
         match section.kind {
@@ -231,14 +245,12 @@ fn report_object(
         for (symbol, symbol_diff) in obj.symbols.iter().zip(&obj_diff.symbols) {
             if symbol.section != Some(section_idx)
                 || symbol.size == 0
-                || symbol.flags.contains(SymbolFlag::Hidden)
-                || symbol.flags.contains(SymbolFlag::Ignored)
+                || symbol.flags.contains(SymbolFlag::Hidden | SymbolFlag::Ignored)
             {
                 continue;
             }
             if let Some(existing_functions) = &mut existing_functions {
-                if (symbol.flags.contains(SymbolFlag::Global)
-                    || symbol.flags.contains(SymbolFlag::Weak))
+                if symbol.flags.contains(SymbolFlag::Global | SymbolFlag::Weak)
                     && !existing_functions.insert(symbol.name.clone())
                 {
                     continue;
@@ -262,6 +274,7 @@ fn report_object(
                     demangled_name: symbol.demangled_name.clone(),
                     virtual_address: symbol.virtual_address,
                 }),
+                address: symbol.address.checked_sub(section.address),
             });
             if match_percent == 100.0 {
                 measures.matched_functions += 1;
@@ -269,6 +282,16 @@ fn report_object(
             measures.total_functions += 1;
         }
     }
+    sections.sort_by(|a, b| a.name.cmp(&b.name));
+    let reverse_fn_order = object.metadata.reverse_fn_order.unwrap_or(false);
+    functions.sort_by(|a, b| {
+        if reverse_fn_order {
+            b.address.unwrap_or(0).cmp(&a.address.unwrap_or(0))
+        } else {
+            a.address.unwrap_or(u64::MAX).cmp(&b.address.unwrap_or(u64::MAX))
+        }
+        .then_with(|| a.size.cmp(&b.size))
+    });
     if metadata.complete.unwrap_or(false) {
         measures.complete_code = measures.total_code;
         measures.complete_data = measures.total_data;

objdiff-core/Cargo.toml

@@ -27,6 +27,7 @@ all = [
     "mips",
     "ppc",
     "x86",
+    "superh"
 ]
 # Implicit, used to check if any arch is enabled
 any-arch = [
@@ -75,12 +76,15 @@ std = [
     "object/std",
     "prost?/std",
     "serde?/std",
+    "similar?/std",
     "typed-path?/std",
     "dep:filetime",
     "dep:memmap2",
 ]
 mips = [
     "any-arch",
+    "dep:cpp_demangle",
+    "dep:cwdemangle",
     "dep:rabbitizer",
 ]
 ppc = [
@@ -108,6 +112,9 @@ arm64 = [
     "dep:yaxpeax-arch",
     "dep:yaxpeax-arm",
 ]
+superh = [
+    "any-arch",
+]
 
 [package.metadata.docs.rs]
 features = ["all"]
@@ -120,13 +127,13 @@ 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 = { version = "0.36", default-features = false, features = ["read_core", "elf", "pe"] }
+object = { git = "https://github.com/gimli-rs/object", rev = "a74579249e21ab8fcd3a86be588de336f18297cb", default-features = false, features = ["read_core", "elf", "pe"] }
 pbjson = { version = "0.7", default-features = false, optional = true }
 prost = { version = "0.13", default-features = false, features = ["prost-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, optional = true, git = "https://github.com/encounter/similar.git", branch = "no_std" }
-typed-path = { version = "0.10", default-features = false, optional = true }
+similar = { version = "2.7", default-features = false, features = ["hashbrown"], optional = true, git = "https://github.com/encounter/similar.git", branch = "no_std" }
+typed-path = { version = "0.11", default-features = false, optional = true }
 
 # config
 globset = { version = "0.4", default-features = false, optional = true }
@@ -138,12 +145,12 @@ gimli = { version = "0.31", default-features = false, features = ["read"], optio
 
 # ppc
 cwdemangle = { version = "1.0", optional = true }
-cwextab = { version = "1.0", optional = true, git = "https://github.com/CelestialAmber/cwextab.git" }
+cwextab = { version = "1.0", optional = true }
 ppc750cl = { version = "0.3", optional = true }
-rlwinmdec = { version = "1.1", optional = true, git = "https://github.com/CelestialAmber/rlwinmdec.git" }
+rlwinmdec = { version = "1.1", optional = true }
 
 # mips
-rabbitizer = { git = "https://github.com/Decompollaborate/rabbitizer.git", branch = "🦀", default-features = false, features = ["all_extensions"], optional = true }
+rabbitizer = { version = "2.0.0-alpha.1", default-features = false, features = ["all_extensions"], optional = true }
 
 # x86
 cpp_demangle = { version = "0.4", default-features = false, features = ["alloc"], optional = true }
@@ -151,7 +158,7 @@ iced-x86 = { version = "1.21", default-features = false, features = ["decoder",
 msvc-demangler = { version = "0.11", optional = true }
 
 # arm
-unarm = { version = "1.7", optional = true }
+unarm = { version = "1.8", optional = true }
 arm-attr = { version = "0.2", optional = true }
 
 # arm64
@@ -162,8 +169,9 @@ yaxpeax-arm = { version = "0.3", default-features = false, optional = true }
 notify = { version = "8.0.0", optional = true }
 notify-debouncer-full = { version = "0.5.0", optional = true }
 shell-escape = { version = "0.1", optional = true }
-tempfile = { version = "3.17", optional = true }
+tempfile = { version = "3.19", optional = true }
 time = { version = "0.3", optional = true }
+encoding_rs = "0.8.35"
 
 [target.'cfg(windows)'.dependencies]
 winapi = { version = "0.3", optional = true }
@@ -192,4 +200,4 @@ syn = { version = "2.0", optional = true }
 [dev-dependencies]
 # Enable all features for tests
 objdiff-core = { path = ".", features = ["all"] }
-insta = "1.42"
+insta = "1.43"

objdiff-core/README.md

@@ -5,11 +5,12 @@ objdiff-core contains the core functionality of [objdiff](https://github.com/enc
 ## Crate feature flags
 
 - **`all`**: Enables all main features.
+- **`bindings`**: Enables serialization and deserialization of objdiff data structures.
 - **`config`**: Enables objdiff configuration file support.
 - **`dwarf`**: Enables extraction of line number information from DWARF debug sections.
-- **`mips`**: Enables the MIPS backend powered by [rabbitizer](https://github.com/Decompollaborate/rabbitizer). (Note: C library with Rust bindings)
-- **`ppc`**: Enables the PowerPC backend powered by [ppc750cl](https://github.com/encounter/ppc750cl).
-- **`x86`**: Enables the x86 backend powered by [iced-x86](https://crates.io/crates/iced-x86).
-- **`arm`**: Enables the ARM backend powered by [unarm](https://github.com/AetiasHax/unarm).
 - **`arm64`**: Enables the ARM64 backend powered by [yaxpeax-arm](https://github.com/iximeow/yaxpeax-arm).
-- **`bindings`**: Enables serialization and deserialization of objdiff data structures.
+- **`arm`**: Enables the ARM backend powered by [unarm](https://github.com/AetiasHax/unarm).
+- **`mips`**: Enables the MIPS backend powered by [rabbitizer](https://github.com/Decompollaborate/rabbitizer).
+- **`ppc`**: Enables the PowerPC backend powered by [ppc750cl](https://github.com/encounter/ppc750cl).
+- **`superh`**: Enables the SuperH backend powered by an included disassembler.
+- **`x86`**: Enables the x86 backend powered by [iced-x86](https://crates.io/crates/iced-x86).

objdiff-core/config-schema.json

@@ -275,4 +275,4 @@
       ]
     }
   ]
-}
+}

objdiff-core/protos/report.proto

@@ -99,6 +99,8 @@ message ReportItem {
   float fuzzy_match_percent = 3;
   // Extra metadata for this item
   optional ReportItemMetadata metadata = 4;
+  // Address of the item (section-relative offset)
+  optional uint64 address = 5;
 }
 
 // Extra metadata for an item

objdiff-core/src/arch/arm.rs

@@ -14,8 +14,8 @@ use crate::{
     arch::Arch,
     diff::{ArmArchVersion, ArmR9Usage, DiffObjConfig, display::InstructionPart},
     obj::{
-        InstructionRef, RelocationFlags, ResolvedInstructionRef, ResolvedRelocation,
-        ScannedInstruction, SymbolFlag, SymbolFlagSet, SymbolKind,
+        InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef, ResolvedRelocation,
+        Section, SectionKind, Symbol, SymbolFlag, SymbolFlagSet, SymbolKind,
     },
 };
 
@@ -32,7 +32,8 @@ impl ArchArm {
         let endianness = file.endianness();
         match file {
             object::File::Elf32(_) => {
-                let disasm_modes = Self::elf_get_mapping_symbols(file);
+                // The disasm_modes mapping is populated later in the post_init step so that we have access to merged sections.
+                let disasm_modes = BTreeMap::new();
                 let detected_version = Self::elf_detect_arm_version(file)?;
                 Ok(Self { disasm_modes, detected_version, endianness })
             }
@@ -73,29 +74,26 @@ impl ArchArm {
         Ok(None)
     }
 
-    fn elf_get_mapping_symbols(file: &object::File) -> BTreeMap<usize, Vec<DisasmMode>> {
-        file.sections()
-            .filter(|s| s.kind() == object::SectionKind::Text)
-            .map(|s| {
-                let index = s.index();
-                let mut mapping_symbols: Vec<_> = file
-                    .symbols()
-                    .filter(|s| s.section_index().map(|i| i == index).unwrap_or(false))
-                    .filter_map(|s| DisasmMode::from_symbol(&s))
+    fn get_mapping_symbols(
+        sections: &[Section],
+        symbols: &[Symbol],
+    ) -> BTreeMap<usize, Vec<DisasmMode>> {
+        sections
+            .iter()
+            .enumerate()
+            .filter(|(_, section)| section.kind == SectionKind::Code)
+            .map(|(index, _)| {
+                let mut mapping_symbols: Vec<_> = symbols
+                    .iter()
+                    .filter(|s| s.section.map(|i| i == index).unwrap_or(false))
+                    .filter_map(DisasmMode::from_symbol)
                     .collect();
                 mapping_symbols.sort_unstable_by_key(|x| x.address);
-                (s.index().0, mapping_symbols)
+                (index, mapping_symbols)
             })
             .collect()
     }
 
-    fn endian(&self) -> unarm::Endian {
-        match self.endianness {
-            object::Endianness::Little => unarm::Endian::Little,
-            object::Endianness::Big => unarm::Endian::Big,
-        }
-    }
-
     fn parse_flags(&self, diff_config: &DiffObjConfig) -> unarm::ParseFlags {
         unarm::ParseFlags {
             ual: diff_config.arm_unified_syntax,
@@ -130,6 +128,31 @@ impl ArchArm {
         code: &[u8],
         diff_config: &DiffObjConfig,
     ) -> Result<(unarm::Ins, unarm::ParsedIns)> {
+        if ins_ref.opcode == thumb::Opcode::BlH as u16 && ins_ref.size == 4 {
+            // Special case: combined thumb BL instruction
+            let parse_flags = self.parse_flags(diff_config);
+            let first_ins = thumb::Ins {
+                code: match self.endianness {
+                    object::Endianness::Little => u16::from_le_bytes([code[0], code[1]]),
+                    object::Endianness::Big => u16::from_be_bytes([code[0], code[1]]),
+                } as u32,
+                op: thumb::Opcode::BlH,
+            };
+            let second_ins = thumb::Ins::new(
+                match self.endianness {
+                    object::Endianness::Little => u16::from_le_bytes([code[2], code[3]]),
+                    object::Endianness::Big => u16::from_be_bytes([code[2], code[3]]),
+                } as u32,
+                &parse_flags,
+            );
+            let first_parsed = first_ins.parse(&parse_flags);
+            let second_parsed = second_ins.parse(&parse_flags);
+            return Ok((
+                unarm::Ins::Thumb(first_ins),
+                first_parsed.combine_thumb_bl(&second_parsed),
+            ));
+        }
+
         let code = match (self.endianness, ins_ref.size) {
             (object::Endianness::Little, 2) => u16::from_le_bytes([code[0], code[1]]) as u32,
             (object::Endianness::Little, 4) => {
@@ -153,24 +176,25 @@ impl ArchArm {
         } else {
             let ins = thumb::Ins { code, op: thumb::Opcode::from(ins_ref.opcode as u8) };
             let parsed = ins.parse(&self.parse_flags(diff_config));
-            if ins.is_half_bl() {
-                todo!("Combine thumb BL instructions");
-            } else {
-                (unarm::Ins::Thumb(ins), parsed)
-            }
+            (unarm::Ins::Thumb(ins), parsed)
         };
         Ok((ins, parsed_ins))
     }
 }
 
 impl Arch for ArchArm {
-    fn scan_instructions(
+    fn post_init(&mut self, sections: &[Section], symbols: &[Symbol]) {
+        self.disasm_modes = Self::get_mapping_symbols(sections, symbols);
+    }
+
+    fn scan_instructions_internal(
         &self,
         address: u64,
         code: &[u8],
         section_index: usize,
+        _relocations: &[Relocation],
         diff_config: &DiffObjConfig,
-    ) -> Result<Vec<ScannedInstruction>> {
+    ) -> Result<Vec<InstructionRef>> {
         let start_addr = address as u32;
         let end_addr = start_addr + code.len() as u32;
 
@@ -184,61 +208,126 @@ impl Arch for ArchArm {
             .unwrap_or(&fallback_mappings);
         let first_mapping_idx = mapping_symbols
             .binary_search_by_key(&start_addr, |x| x.address)
-            .unwrap_or_else(|idx| idx - 1);
-        let first_mapping = mapping_symbols[first_mapping_idx].mapping;
+            .unwrap_or_else(|idx| idx.saturating_sub(1));
+        let mut mode = mapping_symbols[first_mapping_idx].mapping;
 
-        let mut mappings_iter =
-            mapping_symbols.iter().skip(first_mapping_idx + 1).take_while(|x| x.address < end_addr);
+        let mut mappings_iter = mapping_symbols
+            .iter()
+            .copied()
+            .skip(first_mapping_idx + 1)
+            .take_while(|x| x.address < end_addr);
         let mut next_mapping = mappings_iter.next();
 
-        let ins_count = code.len() / first_mapping.instruction_size(start_addr);
-        let mut ops = Vec::<ScannedInstruction>::with_capacity(ins_count);
+        let ins_count = code.len() / mode.instruction_size(start_addr);
+        let mut ops = Vec::<InstructionRef>::with_capacity(ins_count);
 
-        let endian = self.endian();
         let parse_flags = self.parse_flags(diff_config);
-        let mut parser = unarm::Parser::new(first_mapping, start_addr, endian, parse_flags, code);
 
-        while let Some((address, ins, _parsed_ins)) = parser.next() {
-            let size = parser.mode.instruction_size(address);
-            if let Some(next) = next_mapping {
-                let next_address = parser.address;
-                if next_address >= next.address {
-                    // Change mapping
-                    parser.mode = next.mapping;
-                    next_mapping = mappings_iter.next();
-                }
+        let mut address = start_addr;
+        while address < end_addr {
+            while let Some(next) = next_mapping.take_if(|x| address >= x.address) {
+                // Change mapping
+                mode = next.mapping;
+                next_mapping = mappings_iter.next();
             }
-            let (opcode, branch_dest) = match ins {
-                unarm::Ins::Arm(x) => {
-                    let opcode = x.op as u16 | (1 << 15);
-                    let branch_dest = match x.op {
+
+            let mut ins_size = mode.instruction_size(address);
+            let data = &code[(address - start_addr) as usize..];
+            if data.len() < ins_size {
+                // Push the remainder as data
+                ops.push(InstructionRef {
+                    address: address as u64,
+                    size: data.len() as u8,
+                    opcode: u16::MAX,
+                    branch_dest: None,
+                });
+                break;
+            }
+            let code = match (self.endianness, ins_size) {
+                (object::Endianness::Little, 2) => u16::from_le_bytes([data[0], data[1]]) as u32,
+                (object::Endianness::Little, 4) => {
+                    u32::from_le_bytes([data[0], data[1], data[2], data[3]])
+                }
+                (object::Endianness::Big, 2) => u16::from_be_bytes([data[0], data[1]]) as u32,
+                (object::Endianness::Big, 4) => {
+                    u32::from_be_bytes([data[0], data[1], data[2], data[3]])
+                }
+                _ => {
+                    // Invalid instruction size
+                    ops.push(InstructionRef {
+                        address: address as u64,
+                        size: ins_size as u8,
+                        opcode: u16::MAX,
+                        branch_dest: None,
+                    });
+                    address += ins_size as u32;
+                    continue;
+                }
+            };
+
+            let (opcode, branch_dest) = match mode {
+                unarm::ParseMode::Arm => {
+                    let ins = arm::Ins::new(code, &parse_flags);
+                    let opcode = ins.op as u16 | (1 << 15);
+                    let branch_dest = match ins.op {
                         arm::Opcode::B | arm::Opcode::Bl => {
-                            address.checked_add_signed(x.field_branch_offset())
+                            address.checked_add_signed(ins.field_branch_offset())
                         }
-                        arm::Opcode::BlxI => address.checked_add_signed(x.field_blx_offset()),
+                        arm::Opcode::BlxI => address.checked_add_signed(ins.field_blx_offset()),
                         _ => None,
                     };
                     (opcode, branch_dest)
                 }
-                unarm::Ins::Thumb(x) => {
-                    let opcode = x.op as u16;
-                    let branch_dest = match x.op {
+                unarm::ParseMode::Thumb => {
+                    let ins = thumb::Ins::new(code, &parse_flags);
+                    let opcode = ins.op as u16;
+                    let branch_dest = match ins.op {
                         thumb::Opcode::B | thumb::Opcode::Bl => {
-                            address.checked_add_signed(x.field_branch_offset_8())
+                            address.checked_add_signed(ins.field_branch_offset_8())
+                        }
+                        thumb::Opcode::BlH if data.len() >= 4 => {
+                            // Combine BL instructions
+                            let second_ins = thumb::Ins::new(
+                                match self.endianness {
+                                    object::Endianness::Little => {
+                                        u16::from_le_bytes([data[2], data[3]]) as u32
+                                    }
+                                    object::Endianness::Big => {
+                                        u16::from_be_bytes([data[2], data[3]]) as u32
+                                    }
+                                },
+                                &parse_flags,
+                            );
+                            if let Some(low) = match second_ins.op {
+                                thumb::Opcode::Bl => Some(second_ins.field_low_branch_offset_11()),
+                                thumb::Opcode::BlxI => Some(second_ins.field_low_blx_offset_11()),
+                                _ => None,
+                            } {
+                                ins_size = 4;
+                                address.checked_add_signed(
+                                    (ins.field_high_branch_offset_11() + (low as i32)) << 9 >> 9,
+                                )
+                            } else {
+                                None
+                            }
                         }
                         thumb::Opcode::BLong => {
-                            address.checked_add_signed(x.field_branch_offset_11())
+                            address.checked_add_signed(ins.field_branch_offset_11())
                         }
                         _ => None,
                     };
                     (opcode, branch_dest)
                 }
-                unarm::Ins::Data => (u16::MAX, None),
+                unarm::ParseMode::Data => (u16::MAX, None),
             };
-            ops.push(ScannedInstruction {
-                ins_ref: InstructionRef { address: address as u64, size: size as u8, opcode },
+
+            ops.push(InstructionRef {
+                address: address as u64,
+                size: ins_size as u8,
+                opcode,
                 branch_dest: branch_dest.map(|x| x as u64),
             });
+            address += ins_size as u32;
         }
 
         Ok(ops)
@@ -256,6 +345,7 @@ impl Arch for ArchArm {
             cb(InstructionPart::reloc())?;
         } else {
             push_args(
+                ins,
                 &parsed_ins,
                 resolved.relocation,
                 resolved.ins_ref.address as u32,
@@ -359,7 +449,7 @@ impl Arch for ArchArm {
 
     fn extra_symbol_flags(&self, symbol: &object::Symbol) -> SymbolFlagSet {
         let mut flags = SymbolFlagSet::default();
-        if DisasmMode::from_symbol(symbol).is_some() {
+        if DisasmMode::from_object_symbol(symbol).is_some() {
             flags |= SymbolFlag::Hidden;
         }
         flags
@@ -373,15 +463,21 @@ struct DisasmMode {
 }
 
 impl DisasmMode {
-    fn from_symbol<'a>(sym: &object::Symbol<'a, '_, &'a [u8]>) -> Option<Self> {
+    fn from_object_symbol<'a>(sym: &object::Symbol<'a, '_, &'a [u8]>) -> Option<Self> {
         sym.name()
             .ok()
             .and_then(unarm::ParseMode::from_mapping_symbol)
             .map(|mapping| DisasmMode { address: sym.address() as u32, mapping })
     }
+
+    fn from_symbol(sym: &Symbol) -> Option<Self> {
+        unarm::ParseMode::from_mapping_symbol(&sym.name)
+            .map(|mapping| DisasmMode { address: sym.address as u32, mapping })
+    }
 }
 
 fn push_args(
+    ins: unarm::Ins,
     parsed_ins: &unarm::ParsedIns,
     relocation: Option<ResolvedRelocation>,
     cur_addr: u32,
@@ -391,7 +487,7 @@ fn push_args(
     let reloc_arg = find_reloc_arg(parsed_ins, relocation);
     let mut writeback = false;
     let mut deref = false;
-    for (i, arg) in parsed_ins.args_iter().enumerate() {
+    for (i, &arg) in parsed_ins.args_iter().enumerate() {
         // Emit punctuation before separator
         if deref {
             match arg {
@@ -463,19 +559,19 @@ fn push_args(
                 | args::Argument::CoOpcode(value)
                 | args::Argument::SatImm(value) => {
                     arg_cb(InstructionPart::basic("#"))?;
-                    arg_cb(InstructionPart::unsigned(*value))?;
+                    arg_cb(InstructionPart::unsigned(value))?;
                 }
                 args::Argument::SImm(value)
                 | args::Argument::OffsetImm(args::OffsetImm { post_indexed: _, value }) => {
                     arg_cb(InstructionPart::basic("#"))?;
-                    arg_cb(InstructionPart::signed(*value))?;
+                    arg_cb(InstructionPart::signed(value))?;
                 }
                 args::Argument::BranchDest(value) => {
-                    arg_cb(InstructionPart::branch_dest(cur_addr.wrapping_add_signed(*value)))?;
+                    arg_cb(InstructionPart::branch_dest(cur_addr.wrapping_add_signed(value)))?;
                 }
                 args::Argument::CoOption(value) => {
                     arg_cb(InstructionPart::basic("{"))?;
-                    arg_cb(InstructionPart::unsigned(*value))?;
+                    arg_cb(InstructionPart::unsigned(value))?;
                     arg_cb(InstructionPart::basic("}"))?;
                 }
                 args::Argument::CoprocNum(value) => {
@@ -527,6 +623,14 @@ fn push_args(
             arg_cb(InstructionPart::opaque("!"))?;
         }
     }
+
+    let branch_dest = get_pc_relative_load_address(ins, cur_addr);
+    if let Some(branch_dest) = branch_dest {
+        arg_cb(InstructionPart::basic(" (->"))?;
+        arg_cb(InstructionPart::branch_dest(branch_dest))?;
+        arg_cb(InstructionPart::basic(")"))?;
+    }
+
     Ok(())
 }
 
@@ -554,3 +658,21 @@ fn find_reloc_arg(
         None
     }
 }
+
+fn get_pc_relative_load_address(ins: unarm::Ins, address: u32) -> Option<u32> {
+    match ins {
+        unarm::Ins::Arm(ins)
+            if ins.op == arm::Opcode::Ldr
+                && ins.modifier_addr_ldr_str() == arm::AddrLdrStr::Imm
+                && ins.field_rn_deref().reg == args::Register::Pc =>
+        {
+            let offset = ins.field_offset_12().value;
+            Some(address.wrapping_add_signed(offset + 8))
+        }
+        unarm::Ins::Thumb(ins) if ins.op == thumb::Opcode::LdrPc => {
+            let offset = ins.field_rel_immed_8().value;
+            Some((address & !3).wrapping_add_signed(offset + 4))
+        }
+        _ => None,
+    }
+}

objdiff-core/src/arch/arm64.rs

@@ -17,8 +17,7 @@ use crate::{
     arch::Arch,
     diff::{DiffObjConfig, display::InstructionPart},
     obj::{
-        InstructionRef, RelocationFlags, ResolvedInstructionRef, ResolvedRelocation,
-        ScannedInstruction,
+        InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef, ResolvedRelocation,
     },
 };
 
@@ -30,15 +29,16 @@ impl ArchArm64 {
 }
 
 impl Arch for ArchArm64 {
-    fn scan_instructions(
+    fn scan_instructions_internal(
         &self,
         address: u64,
         code: &[u8],
         _section_index: usize,
+        _relocations: &[Relocation],
         _diff_config: &DiffObjConfig,
-    ) -> Result<Vec<ScannedInstruction>> {
+    ) -> Result<Vec<InstructionRef>> {
         let start_address = address;
-        let mut ops = Vec::<ScannedInstruction>::with_capacity(code.len() / 4);
+        let mut ops = Vec::<InstructionRef>::with_capacity(code.len() / 4);
 
         let mut reader = U8Reader::new(code);
         let decoder = InstDecoder::default();
@@ -57,8 +57,10 @@ impl Arch for ArchArm64 {
                     DecodeError::InvalidOpcode
                     | DecodeError::InvalidOperand
                     | DecodeError::IncompleteDecoder => {
-                        ops.push(ScannedInstruction {
-                            ins_ref: InstructionRef { address, size: 4, opcode: u16::MAX },
+                        ops.push(InstructionRef {
+                            address,
+                            size: 4,
+                            opcode: u16::MAX,
                             branch_dest: None,
                         });
                         continue;
@@ -67,9 +69,9 @@ impl Arch for ArchArm64 {
             }
 
             let opcode = opcode_to_u16(ins.opcode);
-            let ins_ref = InstructionRef { address, size: 4, opcode };
-            let branch_dest = branch_dest(ins_ref, &code[offset as usize..offset as usize + 4]);
-            ops.push(ScannedInstruction { ins_ref, branch_dest });
+            let branch_dest =
+                branch_dest(opcode, address, &code[offset as usize..offset as usize + 4]);
+            ops.push(InstructionRef { address, size: 4, opcode, branch_dest });
         }
 
         Ok(ops)
@@ -162,7 +164,7 @@ impl Arch for ArchArm64 {
     }
 }
 
-fn branch_dest(ins_ref: InstructionRef, code: &[u8]) -> Option<u64> {
+fn branch_dest(opcode: u16, address: u64, code: &[u8]) -> Option<u64> {
     const OPCODE_B: u16 = opcode_to_u16(Opcode::B);
     const OPCODE_BL: u16 = opcode_to_u16(Opcode::BL);
     const OPCODE_BCC: u16 = opcode_to_u16(Opcode::Bcc(0));
@@ -172,21 +174,21 @@ fn branch_dest(ins_ref: InstructionRef, code: &[u8]) -> Option<u64> {
     const OPCODE_TBNZ: u16 = opcode_to_u16(Opcode::TBNZ);
 
     let word = u32::from_le_bytes(code.try_into().ok()?);
-    match ins_ref.opcode {
+    match opcode {
         OPCODE_B | OPCODE_BL => {
             let offset = ((word & 0x03ff_ffff) << 2) as i32;
             let extended_offset = (offset << 4) >> 4;
-            ins_ref.address.checked_add_signed(extended_offset as i64)
+            address.checked_add_signed(extended_offset as i64)
         }
         OPCODE_BCC | OPCODE_CBZ | OPCODE_CBNZ => {
             let offset = (word as i32 & 0x00ff_ffe0) >> 3;
             let extended_offset = (offset << 11) >> 11;
-            ins_ref.address.checked_add_signed(extended_offset as i64)
+            address.checked_add_signed(extended_offset as i64)
         }
         OPCODE_TBZ | OPCODE_TBNZ => {
             let offset = (word as i32 & 0x0007_ffe0) >> 3;
             let extended_offset = (offset << 16) >> 16;
-            ins_ref.address.checked_add_signed(extended_offset as i64)
+            address.checked_add_signed(extended_offset as i64)
         }
         _ => None,
     }

objdiff-core/src/arch/mips.rs

@@ -1,9 +1,8 @@
 use alloc::{
     collections::{BTreeMap, BTreeSet},
-    string::ToString,
+    string::{String, ToString},
     vec::Vec,
 };
-use core::ops::Range;
 
 use anyhow::{Result, bail};
 use object::{Endian as _, Object as _, ObjectSection as _, ObjectSymbol as _, elf};
@@ -19,7 +18,7 @@ use crate::{
     diff::{DiffObjConfig, MipsAbi, MipsInstrCategory, display::InstructionPart},
     obj::{
         InstructionArg, InstructionArgValue, InstructionRef, Relocation, RelocationFlags,
-        ResolvedInstructionRef, ResolvedRelocation, ScannedInstruction, SymbolFlag, SymbolFlagSet,
+        ResolvedInstructionRef, ResolvedRelocation, SymbolFlag, SymbolFlagSet,
     },
 };
 
@@ -189,15 +188,16 @@ impl ArchMips {
 }
 
 impl Arch for ArchMips {
-    fn scan_instructions(
+    fn scan_instructions_internal(
         &self,
         address: u64,
         code: &[u8],
         _section_index: usize,
+        _relocations: &[Relocation],
         diff_config: &DiffObjConfig,
-    ) -> Result<Vec<ScannedInstruction>> {
+    ) -> Result<Vec<InstructionRef>> {
         let instruction_flags = self.instruction_flags(diff_config);
-        let mut ops = Vec::<ScannedInstruction>::with_capacity(code.len() / 4);
+        let mut ops = Vec::<InstructionRef>::with_capacity(code.len() / 4);
         let mut cur_addr = address as u32;
         for chunk in code.chunks_exact(4) {
             let code = self.endianness.read_u32_bytes(chunk.try_into()?);
@@ -205,10 +205,7 @@ impl Arch for ArchMips {
                 rabbitizer::Instruction::new(code, Vram::new(cur_addr), instruction_flags);
             let opcode = instruction.opcode() as u16;
             let branch_dest = instruction.get_branch_vram_generic().map(|v| v.inner() as u64);
-            ops.push(ScannedInstruction {
-                ins_ref: InstructionRef { address: cur_addr as u64, size: 4, opcode },
-                branch_dest,
-            });
+            ops.push(InstructionRef { address: cur_addr as u64, size: 4, opcode, branch_dest });
             cur_addr += 4;
         }
         Ok(ops)
@@ -224,16 +221,7 @@ impl Arch for ArchMips {
         let display_flags = self.instruction_display_flags(diff_config);
         let opcode = instruction.opcode();
         cb(InstructionPart::opcode(opcode.name(), opcode as u16))?;
-        let start_address = resolved.symbol.address;
-        let function_range = start_address..start_address + resolved.symbol.size;
-        push_args(
-            &instruction,
-            resolved.relocation,
-            function_range,
-            resolved.section_index,
-            &display_flags,
-            cb,
-        )?;
+        push_args(&instruction, resolved.relocation, &display_flags, cb)?;
         Ok(())
     }
 
@@ -286,6 +274,13 @@ impl Arch for ArchMips {
         })
     }
 
+    fn demangle(&self, name: &str) -> Option<String> {
+        cpp_demangle::Symbol::new(name)
+            .ok()
+            .and_then(|s| s.demangle(&cpp_demangle::DemangleOptions::default()).ok())
+            .or_else(|| cwdemangle::demangle(name, &cwdemangle::DemangleOptions::default()))
+    }
+
     fn reloc_name(&self, flags: RelocationFlags) -> Option<&'static str> {
         match flags {
             RelocationFlags::Elf(r_type) => match r_type {
@@ -330,8 +325,6 @@ impl Arch for ArchMips {
 fn push_args(
     instruction: &rabbitizer::Instruction,
     relocation: Option<ResolvedRelocation>,
-    function_range: Range<u64>,
-    section_index: usize,
     display_flags: &rabbitizer::InstructionDisplayFlags,
     mut arg_cb: impl FnMut(InstructionPart) -> Result<()>,
 ) -> Result<()> {
@@ -354,23 +347,7 @@ fn push_args(
             }
             ValuedOperand::core_label(..) | ValuedOperand::core_branch_target_label(..) => {
                 if let Some(resolved) = relocation {
-                    // If the relocation target is within the current function, we can
-                    // convert it into a relative branch target. Note that we check
-                    // target_address > start_address instead of >= so that recursive
-                    // tail calls are not considered branch targets.
-                    let target_address =
-                        resolved.symbol.address.checked_add_signed(resolved.relocation.addend);
-                    if resolved.symbol.section == Some(section_index)
-                        && target_address.is_some_and(|addr| {
-                            addr > function_range.start && addr < function_range.end
-                        })
-                    {
-                        // TODO move this logic up a level
-                        let target_address = target_address.unwrap();
-                        arg_cb(InstructionPart::branch_dest(target_address))?;
-                    } else {
-                        push_reloc(resolved.relocation, &mut arg_cb)?;
-                    }
+                    push_reloc(resolved.relocation, &mut arg_cb)?;
                 } else if let Some(branch_dest) = instruction
                     .get_branch_offset_generic()
                     .map(|o| (instruction.vram() + o).inner() as u64)

objdiff-core/src/arch/mod.rs

@@ -2,6 +2,7 @@ use alloc::{borrow::Cow, boxed::Box, format, string::String, vec::Vec};
 use core::{ffi::CStr, fmt, fmt::Debug};
 
 use anyhow::{Result, bail};
+use encoding_rs::SHIFT_JIS;
 use object::Endian as _;
 
 use crate::{
@@ -10,8 +11,8 @@ use crate::{
         display::{ContextItem, HoverItem, InstructionPart},
     },
     obj::{
-        InstructionArg, Object, ParsedInstruction, Relocation, RelocationFlags,
-        ResolvedInstructionRef, ScannedInstruction, Symbol, SymbolFlagSet, SymbolKind,
+        InstructionArg, InstructionRef, Object, ParsedInstruction, Relocation, RelocationFlags,
+        ResolvedInstructionRef, ResolvedSymbol, Section, Symbol, SymbolFlagSet, SymbolKind,
     },
     util::ReallySigned,
 };
@@ -24,6 +25,8 @@ pub mod arm64;
 pub mod mips;
 #[cfg(feature = "ppc")]
 pub mod ppc;
+#[cfg(feature = "superh")]
+pub mod superh;
 #[cfg(feature = "x86")]
 pub mod x86;
 
@@ -57,13 +60,18 @@ impl fmt::Display for DataType {
 impl DataType {
     pub fn display_labels(&self, endian: object::Endianness, bytes: &[u8]) -> Vec<String> {
         let mut strs = Vec::new();
-        for literal in self.display_literals(endian, bytes) {
-            strs.push(format!("{}: {}", self, literal))
+        for (literal, label_override) in self.display_literals(endian, bytes) {
+            let label = label_override.unwrap_or_else(|| format!("{}", self));
+            strs.push(format!("{}: {}", label, literal))
         }
         strs
     }
 
-    pub fn display_literals(&self, endian: object::Endianness, bytes: &[u8]) -> Vec<String> {
+    pub fn display_literals(
+        &self,
+        endian: object::Endianness,
+        bytes: &[u8],
+    ) -> Vec<(String, Option<String>)> {
         let mut strs = Vec::new();
         if self.required_len().is_some_and(|l| bytes.len() < l) {
             log::warn!(
@@ -87,56 +95,72 @@ impl DataType {
         match self {
             DataType::Int8 => {
                 let i = i8::from_ne_bytes(bytes.try_into().unwrap());
-                strs.push(format!("{:#x}", i));
+                strs.push((format!("{:#x}", i), None));
 
                 if i < 0 {
-                    strs.push(format!("{:#x}", ReallySigned(i)));
+                    strs.push((format!("{:#x}", ReallySigned(i)), None));
                 }
             }
             DataType::Int16 => {
                 let i = endian.read_i16_bytes(bytes.try_into().unwrap());
-                strs.push(format!("{:#x}", i));
+                strs.push((format!("{:#x}", i), None));
 
                 if i < 0 {
-                    strs.push(format!("{:#x}", ReallySigned(i)));
+                    strs.push((format!("{:#x}", ReallySigned(i)), None));
                 }
             }
             DataType::Int32 => {
                 let i = endian.read_i32_bytes(bytes.try_into().unwrap());
-                strs.push(format!("{:#x}", i));
+                strs.push((format!("{:#x}", i), None));
 
                 if i < 0 {
-                    strs.push(format!("{:#x}", ReallySigned(i)));
+                    strs.push((format!("{:#x}", ReallySigned(i)), None));
                 }
             }
             DataType::Int64 => {
                 let i = endian.read_i64_bytes(bytes.try_into().unwrap());
-                strs.push(format!("{:#x}", i));
+                strs.push((format!("{:#x}", i), None));
 
                 if i < 0 {
-                    strs.push(format!("{:#x}", ReallySigned(i)));
+                    strs.push((format!("{:#x}", ReallySigned(i)), None));
                 }
             }
             DataType::Float => {
                 let bytes: [u8; 4] = bytes.try_into().unwrap();
-                strs.push(format!("{:?}f", match endian {
-                    object::Endianness::Little => f32::from_le_bytes(bytes),
-                    object::Endianness::Big => f32::from_be_bytes(bytes),
-                }));
+                strs.push((
+                    format!("{:?}f", match endian {
+                        object::Endianness::Little => f32::from_le_bytes(bytes),
+                        object::Endianness::Big => f32::from_be_bytes(bytes),
+                    }),
+                    None,
+                ));
             }
             DataType::Double => {
                 let bytes: [u8; 8] = bytes.try_into().unwrap();
-                strs.push(format!("{:?}", match endian {
-                    object::Endianness::Little => f64::from_le_bytes(bytes),
-                    object::Endianness::Big => f64::from_be_bytes(bytes),
-                }));
+                strs.push((
+                    format!("{:?}", match endian {
+                        object::Endianness::Little => f64::from_le_bytes(bytes),
+                        object::Endianness::Big => f64::from_be_bytes(bytes),
+                    }),
+                    None,
+                ));
             }
             DataType::Bytes => {
-                strs.push(format!("{:#?}", bytes));
+                strs.push((format!("{:#?}", bytes), None));
             }
             DataType::String => {
                 if let Ok(cstr) = CStr::from_bytes_until_nul(bytes) {
-                    strs.push(format!("{:?}", cstr));
+                    strs.push((format!("{:?}", cstr), None));
+                }
+                if let Some(nul_idx) = bytes.iter().position(|&c| c == b'\0') {
+                    let (cow, _, had_errors) = SHIFT_JIS.decode(&bytes[..nul_idx]);
+                    if !had_errors {
+                        let str = format!("{:?}", cow);
+                        // Only add the Shift JIS string if it's different from the ASCII string.
+                        if !strs.iter().any(|x| x.0 == str) {
+                            strs.push((str, Some("Shift JIS".into())));
+                        }
+                    }
                 }
             }
         }
@@ -158,42 +182,108 @@ impl DataType {
     }
 }
 
-pub trait Arch: Send + Sync + Debug {
+impl dyn Arch {
     /// Generate a list of instructions references (offset, size, opcode) from the given code.
     ///
-    /// The opcode IDs are used to generate the initial diff. Implementations should do as little
-    /// parsing as possible here: just enough to identify the base instruction opcode, size, and
-    /// possible branch destination (for visual representation). As needed, instructions are parsed
-    /// via `process_instruction` to compare their arguments.
-    fn scan_instructions(
+    /// See [`scan_instructions_internal`] for more details.
+    pub fn scan_instructions(
         &self,
-        address: u64,
-        code: &[u8],
-        section_index: usize,
+        resolved: ResolvedSymbol,
         diff_config: &DiffObjConfig,
-    ) -> Result<Vec<ScannedInstruction>>;
+    ) -> Result<Vec<InstructionRef>> {
+        let mut result = self.scan_instructions_internal(
+            resolved.symbol.address,
+            resolved.data,
+            resolved.section_index,
+            &resolved.section.relocations,
+            diff_config,
+        )?;
+
+        let function_start = resolved.symbol.address;
+        let function_end = function_start + resolved.symbol.size;
+
+        // 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;
+                }
+            }
+        }
+
+        // Resolve relocation targets within the same function to branch destinations
+        let mut ins_iter = result.iter_mut().peekable();
+        'outer: for reloc in resolved
+            .section
+            .relocations
+            .iter()
+            .skip_while(|r| r.address < function_start)
+            .take_while(|r| r.address < function_end)
+        {
+            let ins = loop {
+                let Some(ins) = ins_iter.peek_mut() else {
+                    break 'outer;
+                };
+                if reloc.address < ins.address {
+                    continue 'outer;
+                }
+                let ins = ins_iter.next().unwrap();
+                if reloc.address >= ins.address && reloc.address < ins.address + ins.size as u64 {
+                    break ins;
+                }
+            };
+            // Clear existing branch destination for instructions with relocations
+            ins.branch_dest = None;
+            let Some(target) = resolved.obj.symbols.get(reloc.target_symbol) else {
+                continue;
+            };
+            if target.section != Some(resolved.section_index) {
+                continue;
+            }
+            let Some(target_address) = target.address.checked_add_signed(reloc.addend) else {
+                continue;
+            };
+            // If the target address is within the function range, set it as a branch destination
+            if target_address >= function_start && target_address < function_end {
+                ins.branch_dest = Some(target_address);
+            }
+        }
+
+        Ok(result)
+    }
 
     /// Parse an instruction to gather its mnemonic and arguments for more detailed comparison.
     ///
     /// This is called only when we need to compare the arguments of an instruction.
-    fn process_instruction(
+    pub fn process_instruction(
         &self,
         resolved: ResolvedInstructionRef,
         diff_config: &DiffObjConfig,
     ) -> Result<ParsedInstruction> {
         let mut mnemonic = None;
         let mut args = Vec::with_capacity(8);
+        let mut relocation_emitted = false;
         self.display_instruction(resolved, diff_config, &mut |part| {
             match part {
                 InstructionPart::Opcode(m, _) => mnemonic = Some(Cow::Owned(m.into_owned())),
-                InstructionPart::Arg(arg) => args.push(arg.into_static()),
+                InstructionPart::Arg(arg) => {
+                    if arg == InstructionArg::Reloc {
+                        relocation_emitted = true;
+                        // If the relocation was resolved to a branch destination, emit that instead.
+                        if let Some(dest) = resolved.ins_ref.branch_dest {
+                            args.push(InstructionArg::BranchDest(dest));
+                            return Ok(());
+                        }
+                    }
+                    args.push(arg.into_static());
+                }
                 _ => {}
             }
             Ok(())
         })?;
         // If the instruction has a relocation, but we didn't format it in the display, add it to
         // the end of the arguments list.
-        if resolved.relocation.is_some() && !args.contains(&InstructionArg::Reloc) {
+        if resolved.relocation.is_some() && !relocation_emitted {
             args.push(InstructionArg::Reloc);
         }
         Ok(ParsedInstruction {
@@ -202,6 +292,26 @@ pub trait Arch: Send + Sync + Debug {
             args,
         })
     }
+}
+
+pub trait Arch: Send + Sync + Debug {
+    /// Finishes arch-specific initialization that must be done after sections have been combined.
+    fn post_init(&mut self, _sections: &[Section], _symbols: &[Symbol]) {}
+
+    /// Generate a list of instructions references (offset, size, opcode) from the given code.
+    ///
+    /// The opcode IDs are used to generate the initial diff. Implementations should do as little
+    /// parsing as possible here: just enough to identify the base instruction opcode, size, and
+    /// possible branch destination (for visual representation). As needed, instructions are parsed
+    /// via `process_instruction` to compare their arguments.
+    fn scan_instructions_internal(
+        &self,
+        address: u64,
+        code: &[u8],
+        section_index: usize,
+        relocations: &[Relocation],
+        diff_config: &DiffObjConfig,
+    ) -> Result<Vec<InstructionRef>>;
 
     /// Format an instruction for display.
     ///
@@ -246,7 +356,13 @@ pub trait Arch: Send + Sync + Debug {
         SymbolFlagSet::default()
     }
 
-    fn guess_data_type(&self, _resolved: ResolvedInstructionRef) -> Option<DataType> { None }
+    fn guess_data_type(
+        &self,
+        _resolved: ResolvedInstructionRef,
+        _bytes: &[u8],
+    ) -> Option<DataType> {
+        None
+    }
 
     fn symbol_hover(&self, _obj: &Object, _symbol_index: usize) -> Vec<HoverItem> { Vec::new() }
 
@@ -284,6 +400,8 @@ pub fn new_arch(object: &object::File) -> Result<Box<dyn Arch>> {
         object::Architecture::Arm => Box::new(arm::ArchArm::new(object)?),
         #[cfg(feature = "arm64")]
         object::Architecture::Aarch64 => Box::new(arm64::ArchArm64::new(object)?),
+        #[cfg(feature = "superh")]
+        object::Architecture::SuperH => Box::new(superh::ArchSuperH::new(object)?),
         arch => bail!("Unsupported architecture: {arch:?}"),
     })
 }
@@ -296,13 +414,14 @@ impl ArchDummy {
 }
 
 impl Arch for ArchDummy {
-    fn scan_instructions(
+    fn scan_instructions_internal(
         &self,
         _address: u64,
         _code: &[u8],
         _section_index: usize,
+        _relocations: &[Relocation],
         _diff_config: &DiffObjConfig,
-    ) -> Result<Vec<ScannedInstruction>> {
+    ) -> Result<Vec<InstructionRef>> {
         Ok(Vec::new())
     }