diff --git a/.github/workflows/release.yml b/.github/workflows/build.yml similarity index 71% rename from .github/workflows/release.yml rename to .github/workflows/build.yml index 354bece..c1d894f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/build.yml @@ -1,10 +1,48 @@ -name: Release dsd +name: Build on: + pull_request: workflow_dispatch: +env: + RUSTFLAGS: -D warnings + jobs: - build: + clippy: + name: Clippy + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Rust + uses: dtolnay/rust-toolchain@stable + with: + components: clippy + + - name: Cache Rust workspace + uses: Swatinem/rust-cache@v2 + + - name: Clippy + run: cargo clippy + + fmt: + name: Format + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Rust + uses: dtolnay/rust-toolchain@nightly + with: + components: rustfmt + + - name: Format + run: cargo fmt --all --check + + build-cli: + name: Build dsd strategy: matrix: include: @@ -18,7 +56,7 @@ jobs: target: aarch64-apple-darwin file: dsd-macos-arm64 - - os: macos-13 + - os: macos-latest name: macos-x86_64 target: x86_64-apple-darwin file: dsd-macos-x86_64 @@ -27,7 +65,6 @@ jobs: name: linux-x86_64 target: x86_64-unknown-linux-gnu file: dsd-linux-x86_64 - runs-on: ${{ matrix.os }} steps: - name: Checkout code @@ -64,9 +101,11 @@ jobs: name: dsd-${{ matrix.name }} path: ${{ matrix.file }} if-no-files-found: error - + release: - needs: build + name: Release + needs: build-cli + if: startsWith(github.ref, 'refs/tags/') runs-on: ubuntu-latest permissions: contents: write diff --git a/cli/src/analysis/data.rs b/cli/src/analysis/data.rs index b6c8178..0af4b16 100644 --- a/cli/src/analysis/data.rs +++ b/cli/src/analysis/data.rs @@ -135,7 +135,8 @@ fn add_function_calls_as_relocations( if called_function.address != symbol.addr { log::warn!( "Local function call from {:#010x} in {} to {:#010x} goes to middle of function '{}' at {:#010x}, adding an external label symbol", - address, module_kind, + address, + module_kind, called_function.address, symbol.name, symbol.addr diff --git a/cli/src/analysis/functions.rs b/cli/src/analysis/functions.rs index 7432356..202fa22 100644 --- a/cli/src/analysis/functions.rs +++ b/cli/src/analysis/functions.rs @@ -1,6 +1,6 @@ use std::io; -use anyhow::{bail, Result}; +use anyhow::{Result, bail}; use ds_decomp::analysis::functions::Function; use unarm::{ArmVersion, DisplayOptions, Endian, ParseFlags, ParseMode, Parser, RegNames}; diff --git a/cli/src/analysis/overlay_groups.rs b/cli/src/analysis/overlay_groups.rs index dc35c2e..6e75b80 100644 --- a/cli/src/analysis/overlay_groups.rs +++ b/cli/src/analysis/overlay_groups.rs @@ -1,4 +1,4 @@ -use anyhow::{bail, Result}; +use anyhow::{Result, bail}; use ds_rom::rom::Overlay; pub struct OverlayGroups { diff --git a/cli/src/cmd/check/mod.rs b/cli/src/cmd/check/mod.rs index f212a54..5a40825 100644 --- a/cli/src/cmd/check/mod.rs +++ b/cli/src/cmd/check/mod.rs @@ -1,11 +1,10 @@ mod modules; mod symbols; -pub use modules::*; -pub use symbols::*; - use anyhow::Result; use clap::{Args, Subcommand}; +pub use modules::*; +pub use symbols::*; /// Subcommands for checking/verifying build output. #[derive(Args)] diff --git a/cli/src/cmd/check/modules.rs b/cli/src/cmd/check/modules.rs index 73b05b6..e37ba6e 100644 --- a/cli/src/cmd/check/modules.rs +++ b/cli/src/cmd/check/modules.rs @@ -3,14 +3,14 @@ use std::{ path::{Path, PathBuf}, }; -use anyhow::{bail, Context, Result}; +use anyhow::{Context, Result, bail}; use clap::Args; use ds_decomp::config::{ config::{Config, ConfigModule}, module::ModuleKind, }; -use crate::util::io::{read_file, FileError}; +use crate::util::io::{FileError, read_file}; /// Verifies that built modules are matching the base ROM. #[derive(Args)] diff --git a/cli/src/cmd/delink.rs b/cli/src/cmd/delink.rs index 82e79cd..b25a5e1 100644 --- a/cli/src/cmd/delink.rs +++ b/cli/src/cmd/delink.rs @@ -17,6 +17,7 @@ use ds_decomp::config::{ use ds_rom::rom::{Rom, RomLoadOptions, raw::AutoloadKind}; use object::{Architecture, BinaryFormat, Endianness, RelocationFlags}; +use super::Lcf; use crate::{ config::{ delinks::DelinksExt, @@ -27,8 +28,6 @@ use crate::{ util::io::{create_dir_all, create_file}, }; -use super::Lcf; - /// Delinks an extracted ROM into relocatable ELF files. #[derive(Args)] pub struct Delink { @@ -57,17 +56,14 @@ impl Delink { let config_path = self.config_path.parent().unwrap().to_path_buf(); let symbol_maps = SymbolMaps::from_config(&config_path, &config)?; - let rom = Rom::load( - config_path.join(&config.rom_config), - RomLoadOptions { - key: None, - compress: false, - encrypt: false, - load_files: false, - load_header: false, - load_banner: false, - }, - )?; + let rom = Rom::load(config_path.join(&config.rom_config), RomLoadOptions { + key: None, + compress: false, + encrypt: false, + load_files: false, + load_header: false, + load_banner: false, + })?; let dtcm_end = rom .arm9() .autoloads()? @@ -119,22 +115,21 @@ impl<'a> Delinker<'a> { (symbol_map, module_section.end_address()) }; - if let Some((symbol, size)) = symbol_map.get_symbol_containing(section.end_address() - 1, section_end)? { - if symbol.addr >= section.start_address() - && symbol.addr < section.end_address() - && symbol.addr + size > section.end_address() - { - bail!( - "Last symbol '{}' in section '{}' of file '{}' has the range {:#010x}..{:#010x} but is not contained within the file's section range ({:#010x}..{:#010x})", - symbol.name, - section.name(), - file.name, - symbol.addr, - symbol.addr + size, - section.start_address(), - section.end_address(), - ); - } + if let Some((symbol, size)) = symbol_map.get_symbol_containing(section.end_address() - 1, section_end)? + && symbol.addr >= section.start_address() + && symbol.addr < section.end_address() + && symbol.addr + size > section.end_address() + { + bail!( + "Last symbol '{}' in section '{}' of file '{}' has the range {:#010x}..{:#010x} but is not contained within the file's section range ({:#010x}..{:#010x})", + symbol.name, + section.name(), + file.name, + symbol.addr, + symbol.addr + size, + section.start_address(), + section.end_address(), + ); } } @@ -158,7 +153,12 @@ impl<'a> Delinker<'a> { Ok(()) } - fn delink(&self, symbol_maps: &SymbolMaps, module: &Module, delink_file: &DelinkFile) -> Result { + fn delink( + &self, + symbol_maps: &SymbolMaps, + module: &Module, + delink_file: &DelinkFile, + ) -> Result> { let symbol_map = symbol_maps.get(module.kind()).unwrap(); let dtcm_symbol_map = symbol_maps.get(ModuleKind::Autoload(AutoloadKind::Dtcm)).unwrap(); let mut object = object::write::Object::new(BinaryFormat::Elf, Architecture::Arm, Endianness::Little); @@ -196,7 +196,7 @@ impl<'a> Delinker<'a> { name, // same name as section value: 0, size: 0, - kind: object::SymbolKind::Label, + kind: object::SymbolKind::Section, scope: object::SymbolScope::Compilation, weak: false, section: object::write::SymbolSection::Section(obj_section_id), @@ -365,15 +365,12 @@ impl<'a> Delinker<'a> { // Create relocation let r_type = relocation.kind().as_elf_relocation_type(); let addend = relocation.addend(); - object.add_relocation( - obj_section_id, - object::write::Relocation { - offset: offset as u64, - symbol: symbol_id, - addend, - flags: RelocationFlags::Elf { r_type }, - }, - )?; + object.add_relocation(obj_section_id, object::write::Relocation { + offset: offset as u64, + symbol: symbol_id, + addend, + flags: RelocationFlags::Elf { r_type }, + })?; } } diff --git a/cli/src/cmd/dis.rs b/cli/src/cmd/dis.rs index 05d7603..96f0263 100644 --- a/cli/src/cmd/dis.rs +++ b/cli/src/cmd/dis.rs @@ -46,17 +46,14 @@ impl Disassemble { let config_path = self.config_path.parent().unwrap(); let rom_config_path = config_path.join(&config.rom_config); - let rom = Rom::load( - &rom_config_path, - RomLoadOptions { - key: None, - compress: false, - encrypt: false, - load_files: false, - load_header: false, - load_banner: false, - }, - )?; + let rom = Rom::load(&rom_config_path, RomLoadOptions { + key: None, + compress: false, + encrypt: false, + load_files: false, + load_header: false, + load_banner: false, + })?; let mut symbol_maps = SymbolMaps::from_config(config_path, &config)?; diff --git a/cli/src/cmd/dump/mod.rs b/cli/src/cmd/dump/mod.rs index 8933d10..ef4bee0 100644 --- a/cli/src/cmd/dump/mod.rs +++ b/cli/src/cmd/dump/mod.rs @@ -2,9 +2,8 @@ mod ambig_relocs; mod elf_symbols; use ambig_relocs::*; -use elf_symbols::*; - use clap::{Args, Subcommand}; +use elf_symbols::*; /// Subcommands for dumping information from a dsd project. #[derive(Args)] diff --git a/cli/src/cmd/fix/ctor_symbols.rs b/cli/src/cmd/fix/ctor_symbols.rs index 11eb719..f9c9860 100644 --- a/cli/src/cmd/fix/ctor_symbols.rs +++ b/cli/src/cmd/fix/ctor_symbols.rs @@ -1,4 +1,3 @@ -use crate::util::bytes::FromSlice; use std::path::PathBuf; use anyhow::{Result, bail}; @@ -14,6 +13,8 @@ use ds_decomp::{ }; use ds_rom::rom::{Rom, RomLoadOptions}; +use crate::util::bytes::FromSlice; + /// Adds missing symbols in the .init and .ctor sections. #[derive(Args, Clone)] pub struct FixCtorSymbols { @@ -33,17 +34,14 @@ impl FixCtorSymbols { let mut symbol_maps = SymbolMaps::from_config(config_path, &config)?; - let rom = Rom::load( - config_path.join(&config.rom_config), - RomLoadOptions { - key: None, - compress: false, - encrypt: false, - load_files: false, - load_header: false, - load_banner: false, - }, - )?; + let rom = Rom::load(config_path.join(&config.rom_config), RomLoadOptions { + key: None, + compress: false, + encrypt: false, + load_files: false, + load_header: false, + load_banner: false, + })?; self.fix_module(&config.main_module, ModuleKind::Arm9, &mut symbol_maps, &rom)?; for autoload in &config.autoloads { diff --git a/cli/src/cmd/fix/mod.rs b/cli/src/cmd/fix/mod.rs index 85ca071..d7115b7 100644 --- a/cli/src/cmd/fix/mod.rs +++ b/cli/src/cmd/fix/mod.rs @@ -1,11 +1,10 @@ mod ctor_symbols; mod thumb_nop; +use clap::{Args, Subcommand}; use ctor_symbols::*; use thumb_nop::*; -use clap::{Args, Subcommand}; - /// Subcommands for retroactively fixing already initialized dsd projects. #[derive(Args)] pub struct FixArgs { diff --git a/cli/src/cmd/fix/thumb_nop.rs b/cli/src/cmd/fix/thumb_nop.rs index 7baef41..71a53bf 100644 --- a/cli/src/cmd/fix/thumb_nop.rs +++ b/cli/src/cmd/fix/thumb_nop.rs @@ -31,17 +31,14 @@ impl FixThumbNop { let mut symbol_maps = SymbolMaps::from_config(config_path, &config)?; - let rom = Rom::load( - config_path.join(&config.rom_config), - RomLoadOptions { - key: None, - compress: false, - encrypt: false, - load_files: false, - load_header: false, - load_banner: false, - }, - )?; + let rom = Rom::load(config_path.join(&config.rom_config), RomLoadOptions { + key: None, + compress: false, + encrypt: false, + load_files: false, + load_header: false, + load_banner: false, + })?; let mut num_changes = 0; num_changes += self.fix_module(&config, ModuleKind::Arm9, &rom, &mut symbol_maps)?; diff --git a/cli/src/cmd/import/mod.rs b/cli/src/cmd/import/mod.rs index 22669da..b209920 100644 --- a/cli/src/cmd/import/mod.rs +++ b/cli/src/cmd/import/mod.rs @@ -1,9 +1,8 @@ mod symbols; -use symbols::*; - use anyhow::Result; use clap::{Args, Subcommand}; +use symbols::*; /// Subcommands for importing config data from existing builds. #[derive(Args)] diff --git a/cli/src/cmd/init.rs b/cli/src/cmd/init.rs index 525b5a7..0ff06b2 100644 --- a/cli/src/cmd/init.rs +++ b/cli/src/cmd/init.rs @@ -52,10 +52,12 @@ pub struct Init { impl Init { pub fn run(&self) -> Result<()> { - let rom = Rom::load( - &self.rom_config, - RomLoadOptions { compress: false, encrypt: false, load_files: false, ..Default::default() }, - )?; + let rom = Rom::load(&self.rom_config, RomLoadOptions { + compress: false, + encrypt: false, + load_files: false, + ..Default::default() + })?; let arm9_output_path = self.output_path.join("arm9"); let arm9_overlays_output_path = arm9_output_path.join("overlays"); diff --git a/cli/src/cmd/json/mod.rs b/cli/src/cmd/json/mod.rs index 5d3cdb5..c54e784 100644 --- a/cli/src/cmd/json/mod.rs +++ b/cli/src/cmd/json/mod.rs @@ -1,8 +1,7 @@ mod delinks; -pub use delinks::*; - use clap::{Args, Subcommand}; +pub use delinks::*; #[derive(Args)] pub struct JsonArgs { diff --git a/cli/src/cmd/lcf.rs b/cli/src/cmd/lcf.rs index 0293443..8fbc91f 100644 --- a/cli/src/cmd/lcf.rs +++ b/cli/src/cmd/lcf.rs @@ -82,17 +82,14 @@ impl Lcf { self.validate_all_file_names(&config)?; let config_dir = self.config_path.parent().unwrap(); - let rom = Rom::load( - config_dir.join(&config.rom_config), - RomLoadOptions { - key: None, - compress: false, - encrypt: false, - load_files: false, - load_header: false, - load_banner: false, - }, - )?; + let rom = Rom::load(config_dir.join(&config.rom_config), RomLoadOptions { + key: None, + compress: false, + encrypt: false, + load_files: false, + load_header: false, + load_banner: false, + })?; let build_path = config_dir.join(&config.build_path).clean(); diff --git a/cli/src/cmd/objdiff.rs b/cli/src/cmd/objdiff.rs index feec579..093b546 100644 --- a/cli/src/cmd/objdiff.rs +++ b/cli/src/cmd/objdiff.rs @@ -7,11 +7,11 @@ use anyhow::Result; use clap::Args; use ds_decomp::config::{ config::{Config, ConfigModule}, - delinks::Delinks, + delinks::{Categories, Delinks}, module::ModuleKind, }; use globset::Glob; -use objdiff_core::config::ProjectObject; +use objdiff_core::config::{ProjectObject, ProjectProgressCategory}; use crate::{ config::delinks::DelinksExt, @@ -65,36 +65,30 @@ impl Objdiff { let abs_output_path = std::path::absolute(&output_path)?; let mut existing_units: HashMap = HashMap::new(); - if let Some((Ok(project_config), _)) = objdiff_core::config::try_project_config(&output_path) { - if let Some(units) = project_config.units { - for unit in units { - let Some(name) = unit.name.clone() else { - continue; - }; - existing_units.insert(name, unit); - } + if let Some((Ok(project_config), _)) = objdiff_core::config::try_project_config(&output_path) + && let Some(units) = project_config.units + { + for unit in units { + let Some(name) = unit.name.clone() else { + continue; + }; + existing_units.insert(name, unit); } } - let mut units = vec![]; - units.extend(self.get_units(&config.main_module, ModuleKind::Arm9, config_path, &config, &abs_output_path)?); + let (mut units, mut categories) = + self.get_units(&config.main_module, ModuleKind::Arm9, config_path, &config, &abs_output_path)?; for autoload in &config.autoloads { - units.extend(self.get_units( - &autoload.module, - ModuleKind::Autoload(autoload.kind), - config_path, - &config, - &abs_output_path, - )?); + let (new_units, new_categories) = + self.get_units(&autoload.module, ModuleKind::Autoload(autoload.kind), config_path, &config, &abs_output_path)?; + units.extend(new_units); + categories.extend(new_categories); } for overlay in &config.overlays { - units.extend(self.get_units( - &overlay.module, - ModuleKind::Overlay(overlay.id), - config_path, - &config, - &abs_output_path, - )?); + let (new_units, new_categories) = + self.get_units(&overlay.module, ModuleKind::Overlay(overlay.id), config_path, &config, &abs_output_path)?; + units.extend(new_units); + categories.extend(new_categories); } for unit in &mut units { @@ -138,14 +132,24 @@ impl Objdiff { Glob::new("*.json")?, ]), units: Some(units), - progress_categories: None, + progress_categories: if categories.categories.is_empty() { + None + } else { + Some( + categories + .categories + .iter() + .map(|category| ProjectProgressCategory { id: category.clone(), name: category.clone() }) + .collect(), + ) + }, }; create_dir_all(&output_path)?; - objdiff_core::config::save_project_config( - &project_config, - &objdiff_core::config::ProjectConfigInfo { path: output_path.join("objdiff.json"), timestamp: None }, - )?; + objdiff_core::config::save_project_config(&project_config, &objdiff_core::config::ProjectConfigInfo { + path: output_path.join("objdiff.json"), + timestamp: None, + })?; Ok(()) } @@ -157,9 +161,10 @@ impl Objdiff { config_path: &Path, config: &Config, abs_output_path: &Path, - ) -> Result> { + ) -> Result<(Vec, Categories)> { let delinks: Delinks = Delinks::from_file_and_generate_gaps(config_path.join(&module.delinks), module_kind)?; - delinks + let mut all_categories = delinks.global_categories.clone(); + let units = delinks .files .iter() .map(|file| { @@ -215,6 +220,11 @@ impl Objdiff { None }; + all_categories.extend(file.categories.clone()); + + let mut categories = file.categories.clone(); + categories.extend(delinks.global_categories.clone()); + Ok(objdiff_core::config::ProjectObject { name: Some(file_path.to_string()), path: None, @@ -225,12 +235,17 @@ impl Objdiff { complete: Some(file.complete), reverse_fn_order: Some(false), source_path, - progress_categories: None, + progress_categories: if categories.categories.is_empty() { + None + } else { + Some(categories.categories.clone()) + }, auto_generated: Some(file.gap()), }), ..Default::default() }) }) - .collect::>>() + .collect::>>()?; + Ok((units, all_categories)) } } diff --git a/cli/src/cmd/rom/build.rs b/cli/src/cmd/rom/build.rs index 0b23ad0..169574a 100644 --- a/cli/src/cmd/rom/build.rs +++ b/cli/src/cmd/rom/build.rs @@ -1,6 +1,6 @@ use std::path::PathBuf; -use anyhow::{bail, Result}; +use anyhow::{Result, bail}; use clap::Args; use ds_rom::{ crypto::blowfish::BlowfishKey, diff --git a/cli/src/cmd/rom/config.rs b/cli/src/cmd/rom/config.rs index edfcc8f..abefb9f 100644 --- a/cli/src/cmd/rom/config.rs +++ b/cli/src/cmd/rom/config.rs @@ -11,7 +11,7 @@ use ds_decomp::config::{ module::ModuleKind, section::{Section, Sections}, }; -use ds_rom::rom::{raw::AutoloadKind, OverlayConfig, OverlayTableConfig, Rom, RomConfig, RomLoadOptions}; +use ds_rom::rom::{OverlayConfig, OverlayTableConfig, Rom, RomConfig, RomLoadOptions, raw::AutoloadKind}; use object::{Object, ObjectSection, ObjectSymbol}; use path_slash::PathExt; use pathdiff::diff_paths; @@ -41,17 +41,14 @@ impl ConfigRom { let old_rom_paths_path = config_path.join(&config.rom_config); let rom_extract_dir = old_rom_paths_path.parent().unwrap(); - let rom = Rom::load( - &old_rom_paths_path, - RomLoadOptions { - key: None, - compress: false, - encrypt: false, - load_files: false, - load_header: false, - load_banner: false, - }, - )?; + let rom = Rom::load(&old_rom_paths_path, RomLoadOptions { + key: None, + compress: false, + encrypt: false, + load_files: false, + load_header: false, + load_banner: false, + })?; let mut rom_paths = rom.config().clone(); let main_module_path = config_path.join(&config.main_module.object); diff --git a/cli/src/cmd/rom/extract.rs b/cli/src/cmd/rom/extract.rs index 1e97e93..989df8a 100644 --- a/cli/src/cmd/rom/extract.rs +++ b/cli/src/cmd/rom/extract.rs @@ -1,11 +1,10 @@ use std::path::PathBuf; -use anyhow::{bail, Result}; - +use anyhow::{Result, bail}; use clap::Args; use ds_rom::{ crypto::blowfish::BlowfishKey, - rom::{raw, Rom, RomSaveError}, + rom::{Rom, RomSaveError, raw}, }; /// Extracts a ROM to a given path. diff --git a/cli/src/cmd/rom/mod.rs b/cli/src/cmd/rom/mod.rs index e61a59b..b2bd22f 100644 --- a/cli/src/cmd/rom/mod.rs +++ b/cli/src/cmd/rom/mod.rs @@ -2,13 +2,12 @@ mod build; mod config; mod extract; +use anyhow::Result; use build::*; +use clap::{Args, Subcommand}; pub use config::*; use extract::*; -use anyhow::Result; -use clap::{Args, Subcommand}; - /// Subcommands for extracting/building a ROM. #[derive(Args)] pub struct RomArgs { diff --git a/cli/src/cmd/sig/mod.rs b/cli/src/cmd/sig/mod.rs index e9f0a02..f27f57f 100644 --- a/cli/src/cmd/sig/mod.rs +++ b/cli/src/cmd/sig/mod.rs @@ -2,9 +2,8 @@ mod apply; mod list; mod new; -use clap::{Args, Subcommand}; - pub use apply::*; +use clap::{Args, Subcommand}; pub use list::*; pub use new::*; diff --git a/cli/src/config/delinks.rs b/cli/src/config/delinks.rs index ff51785..3907baf 100644 --- a/cli/src/config/delinks.rs +++ b/cli/src/config/delinks.rs @@ -3,7 +3,7 @@ use std::{cmp::Ordering, collections::HashMap, path::Path}; use anyhow::{Context, Result, bail}; use ds_decomp::config::{ config::{Config, ConfigModule}, - delinks::{DelinkFile, Delinks}, + delinks::{Categories, DelinkFile, Delinks}, module::ModuleKind, section::{DTCM_SECTION, Section, Sections}, }; @@ -260,6 +260,7 @@ impl DelinksPrivExt for Delinks { dtcm_section.end_address(), )?])?, complete: delink_file.complete, + categories: delink_file.categories.clone(), gap: false, }); } @@ -287,6 +288,6 @@ impl DelinkFileExt for DelinkFile { }, }; - Ok(Self { name, sections: Sections::new(), complete: false, gap: true }) + Ok(Self { name, sections: Sections::new(), complete: false, categories: Categories::new(), gap: true }) } } diff --git a/cli/src/config/symbol.rs b/cli/src/config/symbol.rs index e62bc64..89c34a6 100644 --- a/cli/src/config/symbol.rs +++ b/cli/src/config/symbol.rs @@ -10,9 +10,8 @@ use ds_rom::rom::raw::AutoloadKind; use object::{Object, ObjectSection, ObjectSymbol}; use unarm::LookupSymbol; -use crate::util::bytes::FromSlice; - use super::relocation::RelocationModuleExt; +use crate::util::bytes::FromSlice; pub struct LookupSymbolMap(SymbolMap); @@ -159,11 +158,11 @@ pub trait SymDataExt { impl SymDataExt for SymData { fn write_assembly(&self, w: &mut W, symbol: &Symbol, bytes: &[u8], symbols: &SymbolLookup) -> Result<()> { - if let Some(size) = self.size() { - if bytes.len() < size as usize { - log::error!("Not enough bytes to write raw data directive"); - bail!("Not enough bytes to write raw data directive"); - } + if let Some(size) = self.size() + && bytes.len() < size as usize + { + log::error!("Not enough bytes to write raw data directive"); + bail!("Not enough bytes to write raw data directive"); } let mut offset = 0; diff --git a/cli/tests/test_roundtrip.rs b/cli/tests/test_roundtrip.rs index 41a84db..f76f7a4 100644 --- a/cli/tests/test_roundtrip.rs +++ b/cli/tests/test_roundtrip.rs @@ -17,7 +17,7 @@ use ds_decomp_cli::{ }; use ds_rom::{ crypto::blowfish::BlowfishKey, - rom::{raw, Rom}, + rom::{Rom, raw}, }; use log::LevelFilter; use zip::ZipArchive; diff --git a/docs/delinks.md b/docs/delinks.md index adbd730..6ed21d8 100644 --- a/docs/delinks.md +++ b/docs/delinks.md @@ -4,6 +4,7 @@ This document describes how a `delinks.txt` file is structured. ## Contents - [Format](#format) - [Module](#module) + - [Module options](#module-options) - [Section kinds](#section-kinds) - [Files](#files) - [File options](#file-options) @@ -26,18 +27,25 @@ Example: Goes at the top of the file. These are the sections of the entire module. ``` + OPTION + OPTION + ... SECTION start:START end:END kind:KIND align:ALIGN SECTION start:START end:END kind:KIND align:ALIGN ... ``` Notice the indentation on the lines above! +- [`OPTION`] - `SECTION`: The section's name, such as `.text`, `.data` or `.bss`. - `START`: Any 32-bit address aligned to `ALIGN`. - `END`: Any 32-bit address greater than `START`. - [`KIND`](#section-kinds) - `ALIGN`: Any power of two. +#### Module options +- `categories: CATEGORY, CATEGORY, ...`: Comma-separated list of objdiff progress categories to apply to this file. + #### Section kinds - `code`: Contains mostly code and some data - `data`: Contains only data @@ -69,10 +77,12 @@ PATH: The files may appear in any order, `dsd lcf` will handle the link order automatically. #### File options +- `categories: CATEGORY, CATEGORY, ...`: Comma-separated list of objdiff progress categories to apply to this file. - `complete`: This file has been fully decompiled. `dsd lcf` will pass this decompiled file to the linker instead of the delinked file. ## Example ``` + categories: core .text start:0x020773c0 end:0x020d8770 kind:code align:32 .rodata start:0x020d8770 end:0x020df338 kind:rodata align:4 .init start:0x020df338 end:0x020e1e88 kind:code align:4 @@ -81,16 +91,19 @@ The files may appear in any order, `dsd lcf` will handle the link order automati .bss start:0x020e9320 end:0x020eed40 kind:bss align:32 src/00_Core/Actor/Actor.cpp: + categories: actor .text start:0x020c1500 end:0x020c3348 .rodata start:0x020dd370 end:0x020dd3f8 .data start:0x020e71a0 end:0x020e72a8 src/00_Core/Actor/ActorManager.cpp: + categories: actor, manager complete .text start:0x020c33d4 end:0x020c3e54 .data start:0x020e72a8 end:0x020e72f4 src/00_Core/Item/Item.cpp: + categories: item .text start:0x020ad020 end:0x020ad090 .rodata start:0x020dc574 end:0x020dc6c4 ``` diff --git a/lib/src/analysis/ctor.rs b/lib/src/analysis/ctor.rs index 7b46eff..43382f6 100644 --- a/lib/src/analysis/ctor.rs +++ b/lib/src/analysis/ctor.rs @@ -4,9 +4,8 @@ use ds_rom::rom::{Arm9, Autoload, raw::RawBuildInfoError}; use snafu::Snafu; use unarm::args::Argument; -use crate::config::module::ModuleKind; - -use super::functions::{Function, FunctionAnalysisError, FunctionParseOptions, ParseFunctionResult}; +use super::functions::{Function, FunctionAnalysisError, FunctionParseOptions, ParseFunctionError}; +use crate::{analysis::functions::IntoFunctionError, config::module::ModuleKind}; #[derive(Debug)] pub struct CtorRange { @@ -21,11 +20,11 @@ pub enum CtorRangeError { #[snafu(transparent)] FunctionAnalysis { source: FunctionAnalysisError }, #[snafu(display("failed to analyze entrypoint function: {parse_result}\n{backtrace}"))] - EntryAnalysisFailed { parse_result: ParseFunctionResult, backtrace: Backtrace }, + EntryAnalysisFailed { parse_result: ParseFunctionError, backtrace: Backtrace }, #[snafu(display("no function calls in entrypoint:\n{backtrace}"))] NoEntryFunctionCalls { backtrace: Backtrace }, #[snafu(display("failed to parse static initializer function: {parse_result}\n{backtrace}"))] - InitFunctionAnalysisFailed { parse_result: ParseFunctionResult, backtrace: Backtrace }, + InitFunctionAnalysisFailed { parse_result: ParseFunctionError, backtrace: Backtrace }, #[snafu(display("no pool constants found in static initializer function:\n{backtrace}"))] NoInitPoolConstants { backtrace: Backtrace }, #[snafu(display(".ctor runner at {address:#010x} not in range of module '{module_kind}:\n{backtrace}'"))] @@ -63,13 +62,16 @@ impl CtorRange { module_end_address: arm9.end_address()?, parse_options: Default::default(), ..Default::default() - })?; + }); let entry_func = match parse_result { - ParseFunctionResult::Found(ref function) => function, - _ => return EntryAnalysisFailedSnafu { parse_result }.fail(), + Ok(function) => function, + Err(FunctionAnalysisError::IntoFunction { source: IntoFunctionError::ParseFunction { source } }) => { + return EntryAnalysisFailedSnafu { parse_result: source }.fail(); + } + Err(e) => return Err(e.into()), }; - let run_inits_addr = Self::find_last_function_call(entry_func, entry_code, entry_addr) + let run_inits_addr = Self::find_last_function_call(&entry_func, entry_code, entry_addr) .ok_or_else(|| NoEntryFunctionCallsSnafu.build())?; let run_inits_offset = (run_inits_addr - arm9.base_address()) as usize; @@ -100,10 +102,13 @@ impl CtorRange { module_end_address: run_inits_module_end, parse_options: Default::default(), ..Default::default() - })?; + }); let run_inits_func = match parse_result { - ParseFunctionResult::Found(function) => function, - _ => return InitFunctionAnalysisFailedSnafu { parse_result }.fail(), + Ok(function) => function, + Err(FunctionAnalysisError::IntoFunction { source: IntoFunctionError::ParseFunction { source } }) => { + return InitFunctionAnalysisFailedSnafu { parse_result: source }.fail(); + } + Err(e) => return Err(e.into()), }; let p_ctor_start = run_inits_func.pool_constants().first().ok_or_else(|| NoInitPoolConstantsSnafu.build())?; diff --git a/lib/src/analysis/data.rs b/lib/src/analysis/data.rs index a2aad96..9437fd2 100644 --- a/lib/src/analysis/data.rs +++ b/lib/src/analysis/data.rs @@ -1,5 +1,7 @@ use std::ops::Range; +use snafu::Snafu; + use crate::{ analysis::functions::Function, config::{ @@ -10,7 +12,6 @@ use crate::{ }, function, }; -use snafu::Snafu; pub struct FindLocalDataOptions<'a> { pub sections: &'a Sections, @@ -47,9 +48,10 @@ pub fn find_local_data_from_pools( continue; }; let function = symbol_map.get_function(pointer & !1)?; - if section.kind() == SectionKind::Code && function.is_some() { + if section.kind() == SectionKind::Code + && let Some((function, _)) = function + { let thumb = (pointer & 1) != 0; - let (function, _) = function.unwrap(); if function.mode.into_thumb() != Some(thumb) { // Instruction mode must match continue; diff --git a/lib/src/analysis/exception.rs b/lib/src/analysis/exception.rs index 101cbc0..db1e59b 100644 --- a/lib/src/analysis/exception.rs +++ b/lib/src/analysis/exception.rs @@ -1,7 +1,7 @@ use std::backtrace::Backtrace; use bytemuck::{Pod, Zeroable}; -use ds_rom::rom::{raw::RawBuildInfoError, Arm9, Autoload}; +use ds_rom::rom::{Arm9, Autoload, raw::RawBuildInfoError}; use snafu::Snafu; use crate::{config::section::SectionCodeError, util::bytes::FromSlice}; diff --git a/lib/src/analysis/function_start.rs b/lib/src/analysis/function_start.rs index efb50ba..540aabd 100644 --- a/lib/src/analysis/function_start.rs +++ b/lib/src/analysis/function_start.rs @@ -1,6 +1,7 @@ use unarm::{ + Ins, ParsedIns, args::{Argument, OffsetReg, Reg, Register}, - arm, thumb, Ins, ParsedIns, + arm, thumb, }; pub fn is_valid_function_start_arm(_address: u32, ins: arm::Ins, parsed_ins: &ParsedIns) -> bool { @@ -32,12 +33,12 @@ pub fn is_valid_function_start_thumb(_address: u32, ins: thumb::Ins, parsed_ins: let args = &parsed_ins.args; - if ins.is_data_operation() { - if let Argument::Reg(Reg { reg, .. }) = args[1] { - // Data operand must be an argument register, SP or PC - if !matches!(reg, Register::R0 | Register::R1 | Register::R2 | Register::R3 | Register::Sp | Register::Pc) { - return false; - } + if ins.is_data_operation() + && let Argument::Reg(Reg { reg, .. }) = args[1] + { + // Data operand must be an argument register, SP or PC + if !matches!(reg, Register::R0 | Register::R1 | Register::R2 | Register::R3 | Register::Sp | Register::Pc) { + return false; } } diff --git a/lib/src/analysis/functions.rs b/lib/src/analysis/functions.rs index 33ef777..25c8b9c 100644 --- a/lib/src/analysis/functions.rs +++ b/lib/src/analysis/functions.rs @@ -11,11 +11,6 @@ use unarm::{ arm, thumb, }; -use crate::{ - config::symbol::{SymbolMap, SymbolMapError}, - util::bytes::FromSlice, -}; - use super::{ function_branch::FunctionBranchState, function_start::is_valid_function_start, @@ -24,6 +19,10 @@ use super::{ jump_table::{JumpTable, JumpTableState}, secure_area::SecureAreaState, }; +use crate::{ + config::symbol::{SymbolMap, SymbolMapError}, + util::bytes::FromSlice, +}; // All keys in the types below are instruction addresses pub type Labels = BTreeSet; @@ -55,6 +54,8 @@ pub enum FunctionAnalysisError { SymbolMap { source: SymbolMapError }, } +const PARSE_FLAGS: ParseFlags = ParseFlags { version: ArmVersion::V5Te, ual: false }; + impl Function { pub fn size(&self) -> u32 { self.end_address - self.first_instruction_address @@ -146,68 +147,56 @@ impl Function { } } - fn function_parser_loop( - mut parser: Parser<'_>, - options: FunctionParseOptions, - ) -> Result { + fn function_parser_loop(mut parser: Parser<'_>, options: FunctionParseOptions) -> Result { let thumb = parser.mode == ParseMode::Thumb; let mut context = ParseFunctionContext::new(thumb, options); - let Some((address, ins, parsed_ins)) = parser.next() else { return Ok(ParseFunctionResult::NoEpilogue) }; + let Some((address, ins, parsed_ins)) = parser.next() else { + return Err(FunctionAnalysisError::IntoFunction { source: NoEpilogueSnafu.build().into() }); + }; if !is_valid_function_start(address, ins, &parsed_ins) { - return Ok(ParseFunctionResult::InvalidStart { address, ins, parsed_ins }); + return Err(FunctionAnalysisError::IntoFunction { source: InvalidStartSnafu { address, ins }.build().into() }); } let state = context.handle_ins(&mut parser, address, ins, parsed_ins); - let result = if state.ended() { + let mut function = if state.ended() { return Ok(context.into_function(state)?); } else { loop { let Some((address, ins, parsed_ins)) = parser.next() else { - break context.into_function(ParseFunctionState::Done); + break context.into_function(ParseFunctionState::Done)?; }; let state = context.handle_ins(&mut parser, address, ins, parsed_ins); if state.ended() { - break context.into_function(state); + break context.into_function(state)?; } } }; - let result = result?; - let ParseFunctionResult::Found(mut function) = result else { - return Ok(result); - }; - - if let Some(first_pool_address) = function.pool_constants.first() { - if *first_pool_address < function.start_address { - log::info!( - "Function at {:#010x} was adjusted to include pre-code constant pool at {:#010x}", - function.start_address, - first_pool_address - ); + if let Some(first_pool_address) = function.pool_constants.first() + && *first_pool_address < function.start_address + { + log::info!( + "Function at {:#010x} was adjusted to include pre-code constant pool at {:#010x}", + function.start_address, + first_pool_address + ); - function.first_instruction_address = function.start_address; - function.start_address = *first_pool_address; - } + function.first_instruction_address = function.start_address; + function.start_address = *first_pool_address; } - Ok(ParseFunctionResult::Found(function)) + Ok(function) } - pub fn parse_function(options: FunctionParseOptions) -> Result { + pub fn parse_function(options: FunctionParseOptions) -> Result { let FunctionParseOptions { start_address, base_address, module_code, parse_options, .. } = &options; let thumb = parse_options.thumb.unwrap_or(Function::is_thumb_function(*start_address, module_code)); let parse_mode = if thumb { ParseMode::Thumb } else { ParseMode::Arm }; let start = (start_address - base_address) as usize; let function_code = &module_code[start..]; - let parser = Parser::new( - parse_mode, - *start_address, - Endian::Little, - ParseFlags { version: ArmVersion::V5Te, ual: false }, - function_code, - ); + let parser = Parser::new(parse_mode, *start_address, Endian::Little, PARSE_FLAGS, function_code); Self::function_parser_loop(parser, options) } @@ -247,13 +236,7 @@ impl Function { let thumb = Function::is_thumb_function(address, function_code); let parse_mode = if thumb { ParseMode::Thumb } else { ParseMode::Arm }; - let parser = Parser::new( - parse_mode, - address, - Endian::Little, - ParseFlags { version: ArmVersion::V5Te, ual: false }, - function_code, - ); + let parser = Parser::new(parse_mode, address, Endian::Little, PARSE_FLAGS, function_code); let (name, new) = if let Some((_, symbol)) = symbol_map.by_address(address)? { (symbol.name.clone(), false) @@ -261,91 +244,85 @@ impl Function { (format!("{default_name_prefix}{address:08x}"), true) }; - let function_result = Function::function_parser_loop( - parser, - FunctionParseOptions { - name, - start_address: address, - base_address, - module_code, - known_end_address: None, - module_start_address, - module_end_address, - existing_functions: search_options.existing_functions, - check_defs_uses: search_options.check_defs_uses, - parse_options: Default::default(), - }, - )?; + let function_result = Function::function_parser_loop(parser, FunctionParseOptions { + name, + start_address: address, + base_address, + module_code, + known_end_address: None, + module_start_address, + module_end_address, + existing_functions: search_options.existing_functions, + check_defs_uses: search_options.check_defs_uses, + parse_options: Default::default(), + }); let function = match function_result { - ParseFunctionResult::Found(function) => function, - ParseFunctionResult::IllegalIns { address: illegal_address, ins, .. } => { - let search_limit = prev_valid_address.saturating_add(search_options.max_function_start_search_distance); - let limit_reached = address >= search_limit; - - if !limit_reached { - // It's possible that we've attempted to analyze pool constants as code, which can happen if the - // function has a constant pool ahead of its code. - let mut next_address = (address + 1).next_multiple_of(4); - if let Some(function_addresses) = search_options.function_addresses.as_ref() { - if let Some(&next_function) = function_addresses.range(address + 1..).next() { - next_address = next_function; + Ok(function) => function, + Err(FunctionAnalysisError::IntoFunction { source: IntoFunctionError::ParseFunction { source } }) => { + match source { + ParseFunctionError::IllegalIns { address: illegal_address, ins, .. } => { + let search_limit = + prev_valid_address.saturating_add(search_options.max_function_start_search_distance); + let limit_reached = address >= search_limit; + + if !limit_reached { + // It's possible that we've attempted to analyze pool constants as code, which can happen if the + // function has a constant pool ahead of its code. + let mut next_address = (address + 1).next_multiple_of(4); + if let Some(function_addresses) = search_options.function_addresses.as_ref() + && let Some(&next_function) = function_addresses.range(address + 1..).next() + { + next_address = next_function; + } + address = next_address; + function_code = &module_code[(address - base_address) as usize..]; + continue; + } else { + if thumb { + log::debug!( + "Terminating function analysis due to illegal instruction at {:#010x}: {:04x}", + illegal_address, + ins.code() + ); + } else { + log::debug!( + "Terminating function analysis due to illegal instruction at {:#010x}: {:08x}", + illegal_address, + ins.code() + ); + } + break; } } - address = next_address; - function_code = &module_code[(address - base_address) as usize..]; - continue; - } else { - if thumb { - log::debug!( - "Terminating function analysis due to illegal instruction at {:#010x}: {:04x}", - illegal_address, - ins.code() - ); - } else { + ParseFunctionError::NoEpilogue => { log::debug!( - "Terminating function analysis due to illegal instruction at {:#010x}: {:08x}", - illegal_address, - ins.code() + "Terminating function analysis due to no epilogue in function starting from {:#010x}", + address ); + break; } - break; - } - } - ParseFunctionResult::NoEpilogue => { - log::debug!( - "Terminating function analysis due to no epilogue in function starting from {:#010x}", - address - ); - break; - } - ParseFunctionResult::InvalidStart { address: start_address, ins, parsed_ins } => { - let search_limit = prev_valid_address.saturating_add(search_options.max_function_start_search_distance); - let limit_reached = address >= search_limit; - - if !limit_reached { - let ins_size = parse_mode.instruction_size(0); - address += ins_size as u32; - function_code = &function_code[ins_size..]; - continue; - } else { - if thumb { - log::debug!( - "Terminating function analysis due to invalid function start at {:#010x}: {:04x} {}", - start_address, - ins.code(), - parsed_ins.display(Default::default()) - ); - } else { - log::debug!( - "Terminating function analysis due to invalid function start at {:#010x}: {:08x} {}", - start_address, - ins.code(), - parsed_ins.display(Default::default()) - ); + ParseFunctionError::InvalidStart { address: start_address, ins } => { + let search_limit = + prev_valid_address.saturating_add(search_options.max_function_start_search_distance); + let limit_reached = address >= search_limit; + + if !limit_reached { + let ins_size = parse_mode.instruction_size(0); + address += ins_size as u32; + function_code = &function_code[ins_size..]; + continue; + } else { + log::debug!( + "Terminating function analysis due to invalid function start at {:#010x}: {}", + start_address, + ins + ); + break; + } } - break; } } + Err(e) => return Err(e), }; // A function was found @@ -390,7 +367,7 @@ impl Function { if thumb { ParseMode::Thumb } else { ParseMode::Arm }, pointer_value, Endian::Little, - ParseFlags { ual: false, version: ArmVersion::V5Te }, + PARSE_FLAGS, &module_code[offset..], ); let (address, ins, parsed_ins) = parser.next().unwrap(); @@ -437,14 +414,12 @@ impl Function { ) -> BTreeMap { let mut functions = BTreeMap::new(); - let parse_flags = ParseFlags { ual: false, version: ArmVersion::V5Te }; - let mut address = base_addr; let mut state = SecureAreaState::default(); for ins_code in module_code.chunks_exact(2) { let ins_code = u16::from_le_slice(ins_code); - let ins = thumb::Ins::new(ins_code as u32, &parse_flags); - let parsed_ins = ins.parse(&parse_flags); + let ins = thumb::Ins::new(ins_code as u32, &PARSE_FLAGS); + let parsed_ins = ins.parse(&PARSE_FLAGS); state = state.handle(address, &parsed_ins); if let Some(function) = state.get_function() { @@ -475,7 +450,7 @@ impl Function { if self.thumb { ParseMode::Thumb } else { ParseMode::Arm }, self.start_address, Endian::Little, - ParseFlags { ual: false, version: ArmVersion::V5Te }, + PARSE_FLAGS, self.code(module_code, base_address), ) } @@ -617,6 +592,8 @@ struct ParseFunctionContext<'a> { pub enum IntoFunctionError { #[snafu(display("Cannot turn parse context into function before parsing is done"))] NotDone { backtrace: Backtrace }, + #[snafu(transparent)] + ParseFunction { source: ParseFunctionError }, } impl<'a> ParseFunctionContext<'a> { @@ -712,7 +689,7 @@ impl<'a> ParseFunctionContext<'a> { 4 } else { // Not combined - return ParseFunctionState::IllegalIns { address, ins, parsed_ins: parsed_ins.clone() }; + return ParseFunctionState::IllegalIns { address, ins }; } } else { // ARM instruction @@ -721,7 +698,7 @@ impl<'a> ParseFunctionContext<'a> { self.illegal_code_state = self.illegal_code_state.handle(ins, parsed_ins); if self.illegal_code_state.is_illegal() { - return ParseFunctionState::IllegalIns { address, ins, parsed_ins: parsed_ins.clone() }; + return ParseFunctionState::IllegalIns { address, ins }; } let in_conditional_block = Some(address) < self.last_conditional_destination; @@ -816,13 +793,14 @@ impl<'a> ParseFunctionContext<'a> { for usage in uses { let legal = match usage { Argument::Reg(reg) => { - if let Ins::Arm(ins) = ins { - if ins.op == arm::Opcode::Str && ins.field_rn_deref().reg == Register::Sp { - // There are instance of `str Rd, [sp, #imm]` where Rd is not defined. - // Potential UB bug in mwccarm. - self.defined_registers.insert(reg.reg); - continue; - } + if let Ins::Arm(ins) = ins + && ins.op == arm::Opcode::Str + && ins.field_rn_deref().reg == Register::Sp + { + // There are instance of `str Rd, [sp, #imm]` where Rd is not defined. + // Potential UB bug in mwccarm. + self.defined_registers.insert(reg.reg); + continue; } self.defined_registers.contains(®.reg) @@ -833,7 +811,7 @@ impl<'a> ParseFunctionContext<'a> { _ => continue, }; if !legal { - return ParseFunctionState::IllegalIns { address, ins, parsed_ins: parsed_ins.clone() }; + return ParseFunctionState::IllegalIns { address, ins }; } } if !is_return { @@ -916,27 +894,27 @@ impl<'a> ParseFunctionContext<'a> { None } - fn into_function(self, state: ParseFunctionState) -> Result { + fn into_function(self, state: ParseFunctionState) -> Result { match state { ParseFunctionState::Continue => { return NotDoneSnafu.fail(); } - ParseFunctionState::IllegalIns { address, ins, parsed_ins } => { - return Ok(ParseFunctionResult::IllegalIns { address, ins, parsed_ins }); + ParseFunctionState::IllegalIns { address, ins } => { + return IllegalInsSnafu { address, ins }.fail()?; } ParseFunctionState::Done => {} }; let Some(end_address) = self.end_address else { - return Ok(ParseFunctionResult::NoEpilogue); + return NoEpilogueSnafu.fail()?; }; let end_address = self.known_end_address.unwrap_or(end_address.max(self.last_pool_address.map(|a| a + 4).unwrap_or(0))); if end_address > self.module_end_address { - return Ok(ParseFunctionResult::NoEpilogue); + return NoEpilogueSnafu.fail()?; } - Ok(ParseFunctionResult::Found(Function { + Ok(Function { name: self.name, start_address: self.start_address, end_address, @@ -947,7 +925,7 @@ impl<'a> ParseFunctionContext<'a> { jump_tables: self.jump_tables, inline_tables: self.inline_tables, function_calls: self.function_calls, - })) + }) } fn is_return( @@ -1023,7 +1001,7 @@ pub struct ParseFunctionOptions { enum ParseFunctionState { Continue, - IllegalIns { address: u32, ins: Ins, parsed_ins: ParsedIns }, + IllegalIns { address: u32, ins: Ins }, Done, } @@ -1036,26 +1014,33 @@ impl ParseFunctionState { } } -#[derive(Debug)] -pub enum ParseFunctionResult { - Found(Function), - IllegalIns { address: u32, ins: Ins, parsed_ins: ParsedIns }, +#[derive(Debug, Snafu)] +pub enum ParseFunctionError { + #[snafu(display("Illegal instruction at {address:#010x}: {ins:?}"))] + IllegalIns { address: u32, ins: Ins }, + #[snafu(display("No epilogue found"))] NoEpilogue, - InvalidStart { address: u32, ins: Ins, parsed_ins: ParsedIns }, + #[snafu(display("Illegal function start at {address:#010x}: {ins}"))] + InvalidStart { address: u32, ins: DisplayIns }, } -impl Display for ParseFunctionResult { +#[derive(Debug)] +pub struct DisplayIns(Ins); + +impl From for DisplayIns { + fn from(value: Ins) -> Self { + Self(value) + } +} + +impl Display for DisplayIns { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match self { - Self::Found(function) => write!(f, "Found function: {}", function.name()), - Self::IllegalIns { address, parsed_ins, .. } => { - write!(f, "Illegal instruction at {:#010x}: {}", address, parsed_ins.display(Default::default())) - } - Self::NoEpilogue => write!(f, "No epilogue found"), - Self::InvalidStart { address, parsed_ins, .. } => { - write!(f, "Invalid function start at {:#010x}: {}", address, parsed_ins.display(Default::default())) - } - } + let parsed_ins = match self.0 { + Ins::Arm(ins) => ins.parse(&PARSE_FLAGS), + Ins::Thumb(ins) => ins.parse(&PARSE_FLAGS), + Ins::Data => return write!(f, ""), + }; + write!(f, "{}", parsed_ins.display(Default::default())) } } diff --git a/lib/src/analysis/illegal_code.rs b/lib/src/analysis/illegal_code.rs index aaab2fb..68b7d38 100644 --- a/lib/src/analysis/illegal_code.rs +++ b/lib/src/analysis/illegal_code.rs @@ -1,6 +1,6 @@ use unarm::{ - args::{Argument as Arg, OffsetReg, Reg, Register}, Ins, ParsedIns, + args::{Argument as Arg, OffsetReg, Reg, Register}, }; /// Detects illegal code sequences that never appears in any game. diff --git a/lib/src/analysis/inline_table.rs b/lib/src/analysis/inline_table.rs index 121acf3..9e1ac18 100644 --- a/lib/src/analysis/inline_table.rs +++ b/lib/src/analysis/inline_table.rs @@ -1,6 +1,6 @@ use unarm::{ - args::{Argument, Reg, Register, Shift, ShiftImm}, ParsedIns, + args::{Argument, Reg, Register, Shift, ShiftImm}, }; use crate::config::symbol::SymData; diff --git a/lib/src/analysis/jump_table.rs b/lib/src/analysis/jump_table.rs index 7cf89f9..ef51421 100644 --- a/lib/src/analysis/jump_table.rs +++ b/lib/src/analysis/jump_table.rs @@ -1,6 +1,6 @@ use unarm::{ - args::{Argument, OffsetImm, Reg, Register, Shift, ShiftImm}, Ins, ParsedIns, + args::{Argument, OffsetImm, Reg, Register, Shift, ShiftImm}, }; use super::functions::JumpTables; @@ -143,11 +143,7 @@ impl JumpTableStateArm { }, Self::ValidJumpTable { table_address, limit } => { let end = table_address + limit * 4; - if address > end { - Self::default() - } else { - self - } + if address > end { Self::default() } else { self } } } } @@ -339,11 +335,7 @@ impl JumpTableStateThumb { }, Self::ValidJumpTable { table_address, limit } => { let end = table_address + limit * 2; - if address > end { - Self::default() - } else { - self - } + if address > end { Self::default() } else { self } } } } diff --git a/lib/src/analysis/main.rs b/lib/src/analysis/main.rs index 774d613..569f7a1 100644 --- a/lib/src/analysis/main.rs +++ b/lib/src/analysis/main.rs @@ -1,10 +1,11 @@ use std::backtrace::Backtrace; -use ds_rom::rom::{raw::RawBuildInfoError, Arm9}; +use ds_rom::rom::{Arm9, raw::RawBuildInfoError}; use snafu::Snafu; use unarm::args::{Argument, OffsetImm, Reg, Register}; -use super::functions::{Function, FunctionAnalysisError, FunctionParseOptions, ParseFunctionResult}; +use super::functions::{Function, FunctionAnalysisError, FunctionParseOptions, ParseFunctionError}; +use crate::analysis::functions::IntoFunctionError; #[derive(Clone, Copy)] pub struct MainFunction { @@ -18,7 +19,7 @@ pub enum MainFunctionError { #[snafu(transparent)] FunctionAnalysis { source: FunctionAnalysisError }, #[snafu(display("failed to analyze entrypoint function: {parse_result:x?}:\n{backtrace}"))] - MainAnalysisFailed { parse_result: ParseFunctionResult, backtrace: Backtrace }, + MainAnalysisFailed { parse_result: ParseFunctionError, backtrace: Backtrace }, #[snafu(display("Expected entry function to contain pool constants:\n{backtrace}"))] NoPoolConstants { backtrace: Backtrace }, #[snafu(display("Expected last instruction of entry function to be 'bx ':\n{backtrace}"))] @@ -84,10 +85,13 @@ impl MainFunction { module_end_address: arm9.end_address()?, parse_options: Default::default(), ..Default::default() - })?; + }); let entry_func = match parse_result { - ParseFunctionResult::Found(function) => function, - _ => return MainAnalysisFailedSnafu { parse_result }.fail(), + Ok(function) => function, + Err(FunctionAnalysisError::IntoFunction { source: IntoFunctionError::ParseFunction { source } }) => { + return MainAnalysisFailedSnafu { parse_result: source }.fail(); + } + Err(e) => return Err(e.into()), }; let main = Self::find_tail_call(entry_func, entry_code, entry_addr)?; diff --git a/lib/src/analysis/secure_area.rs b/lib/src/analysis/secure_area.rs index 98bcd66..cf29572 100644 --- a/lib/src/analysis/secure_area.rs +++ b/lib/src/analysis/secure_area.rs @@ -1,7 +1,7 @@ use snafu::Snafu; use unarm::{ - args::{Argument, Reg, Register}, ParsedIns, + args::{Argument, Reg, Register}, }; #[derive(Clone, Copy, Default, Debug)] diff --git a/lib/src/config/config.rs b/lib/src/config/config.rs index e3e3428..d2fe771 100644 --- a/lib/src/config/config.rs +++ b/lib/src/config/config.rs @@ -83,34 +83,28 @@ impl Config { let delinks = Delinks::from_file(config_path.join(&module_config.delinks), module_kind)?; let code = rom.get_code(module_kind)?; - let module = Module::new( - symbol_map, - ModuleOptions { - kind: module_kind, - name: module_config.name.clone(), - relocations, - sections: delinks.sections, - code: &code, - signed: false, - }, - )?; + let module = Module::new(symbol_map, ModuleOptions { + kind: module_kind, + name: module_config.name.clone(), + relocations, + sections: delinks.sections, + code: &code, + signed: false, + })?; Ok(module) } - pub fn load_rom>(&self, config_path: P) -> Result { + pub fn load_rom>(&self, config_path: P) -> Result, RomSaveError> { let config_path = config_path.as_ref(); - Rom::load( - config_path.join(&self.rom_config), - RomLoadOptions { - key: None, - compress: false, - encrypt: false, - load_files: false, - load_header: false, - load_banner: false, - }, - ) + Rom::load(config_path.join(&self.rom_config), RomLoadOptions { + key: None, + compress: false, + encrypt: false, + load_files: false, + load_header: false, + load_banner: false, + }) } } diff --git a/lib/src/config/delinks.rs b/lib/src/config/delinks.rs index 2451285..ba5dec0 100644 --- a/lib/src/config/delinks.rs +++ b/lib/src/config/delinks.rs @@ -8,18 +8,18 @@ use std::{ use snafu::Snafu; -use crate::util::io::{FileError, create_file, open_file}; - use super::{ ParseContext, module::ModuleKind, section::{Section, SectionInheritParseError, SectionParseError, Sections, SectionsError}, }; +use crate::util::io::{FileError, create_file, open_file}; pub struct Delinks { pub sections: Sections, - pub files: Vec, + pub global_categories: Categories, module_kind: ModuleKind, + pub files: Vec, } #[derive(Debug, Snafu)] @@ -46,7 +46,7 @@ pub enum DelinksWriteError { impl Delinks { pub fn new(sections: Sections, files: Vec, module_kind: ModuleKind) -> Self { - Self { sections, files, module_kind } + Self { sections, global_categories: Categories::new(), files, module_kind } } pub fn from_file>(path: P, module_kind: ModuleKind) -> Result { @@ -58,6 +58,7 @@ impl Delinks { let mut sections: Sections = Sections::new(); let mut files = vec![]; + let mut global_categories = Categories::new(); let mut lines = reader.lines(); while let Some(line) = lines.next() { @@ -70,6 +71,10 @@ impl Delinks { if Self::try_parse_delink_file(line, &mut lines, &mut context, &mut files, §ions)? { break; } + if let Some(new_categories) = Categories::try_parse(line) { + global_categories.extend(new_categories); + continue; + }; let Some(section) = Section::parse(line, &context)? else { continue; }; @@ -86,7 +91,7 @@ impl Delinks { Self::try_parse_delink_file(line, &mut lines, &mut context, &mut files, §ions)?; } - Ok(Self { sections, files, module_kind }) + Ok(Self { sections, global_categories, files, module_kind }) } fn try_parse_delink_file( @@ -116,7 +121,7 @@ impl Delinks { Ok(()) } - pub fn display(&self) -> DisplayDelinks { + pub fn display(&self) -> DisplayDelinks<'_> { DisplayDelinks { sections: &self.sections, files: &self.files } } @@ -146,6 +151,7 @@ pub struct DelinkFile { pub name: String, pub sections: Sections, pub complete: bool, + pub categories: Categories, pub gap: bool, } @@ -163,7 +169,7 @@ pub enum DelinkFileParseError { impl DelinkFile { pub fn new(name: String, sections: Sections, complete: bool) -> Self { - Self { name, sections, complete, gap: false } + Self { name, sections, complete, categories: Categories::new(), gap: false } } pub fn parse( @@ -180,6 +186,8 @@ impl DelinkFile { let mut complete = false; let mut sections = Sections::new(); + let mut categories = Categories::new(); + for line in lines.by_ref() { context.row += 1; let line = line?; @@ -191,11 +199,15 @@ impl DelinkFile { complete = true; continue; } + if let Some(new_categories) = Categories::try_parse(line) { + categories.extend(new_categories); + continue; + }; let section = Section::parse_inherit(line, context, inherit_sections)?.unwrap(); sections.add(section)?; } - Ok(DelinkFile { name, sections, complete, gap: false }) + Ok(DelinkFile { name, sections, complete, categories, gap: false }) } pub fn split_file_ext(&self) -> (&str, &str) { @@ -216,3 +228,38 @@ impl Display for DelinkFile { Ok(()) } } + +#[derive(Clone)] +pub struct Categories { + pub categories: Vec, +} + +impl Categories { + pub fn new() -> Self { + Self { categories: Vec::new() } + } + + pub fn try_parse(line: &str) -> Option { + let list = line.trim().strip_prefix("categories:")?; + let categories = list.trim().split(',').map(|category| category.trim().to_string()).collect(); + Some(Self { categories }) + } + + pub fn extend(&mut self, other: Categories) { + self.categories.extend(other.categories); + self.categories.sort_unstable(); + self.categories.dedup(); + } +} + +impl Default for Categories { + fn default() -> Self { + Self::new() + } +} + +impl From> for Categories { + fn from(value: Vec) -> Self { + Self { categories: value } + } +} diff --git a/lib/src/config/module.rs b/lib/src/config/module.rs index 46519bd..9e83c84 100644 --- a/lib/src/config/module.rs +++ b/lib/src/config/module.rs @@ -10,6 +10,12 @@ use ds_rom::rom::{ }; use snafu::Snafu; +use self::data::FindLocalDataError; +use super::{ + relocations::Relocations, + section::{Section, SectionCodeError, SectionError, SectionKind, SectionOptions, Sections, SectionsError}, + symbol::{SymData, SymbolKind, SymbolMap, SymbolMapError, SymbolMaps}, +}; use crate::{ analysis::{ ctor::{CtorRange, CtorRangeError}, @@ -17,21 +23,13 @@ use crate::{ exception::{ExceptionData, ExceptionDataError}, functions::{ FindFunctionsOptions, Function, FunctionAnalysisError, FunctionParseOptions, FunctionSearchOptions, - ParseFunctionOptions, ParseFunctionResult, + IntoFunctionError, ParseFunctionError, ParseFunctionOptions, }, main::{MainFunction, MainFunctionError}, }, config::symbol::Symbol, }; -use self::data::FindLocalDataError; - -use super::{ - relocations::Relocations, - section::{Section, SectionCodeError, SectionError, SectionKind, SectionOptions, Sections, SectionsError}, - symbol::{SymData, SymbolKind, SymbolMap, SymbolMapError, SymbolMaps}, -}; - pub struct Module { name: String, kind: ModuleKind, @@ -61,7 +59,7 @@ pub enum ModuleError { #[snafu(transparent)] FunctionAnalysis { source: FunctionAnalysisError }, #[snafu(display("function {name} could not be analyzed: {parse_result:x?}:\n{backtrace}"))] - FunctionAnalysisFailed { name: String, parse_result: ParseFunctionResult, backtrace: Backtrace }, + FunctionAnalysisFailed { name: String, parse_result: ParseFunctionError, backtrace: Backtrace }, #[snafu(transparent)] Section { source: SectionError }, #[snafu(transparent)] @@ -391,10 +389,13 @@ impl Module { module_end_address: end_address, parse_options: ParseFunctionOptions { thumb: sym_function.mode.into_thumb() }, ..Default::default() - })?; + }); let function = match parse_result { - ParseFunctionResult::Found(function) => function, - _ => return FunctionAnalysisFailedSnafu { name: symbol.name, parse_result }.fail(), + Ok(function) => function, + Err(FunctionAnalysisError::IntoFunction { source: IntoFunctionError::ParseFunction { source } }) => { + return FunctionAnalysisFailedSnafu { name: symbol.name, parse_result: source }.fail(); + } + Err(e) => return Err(e.into()), }; function.add_local_symbols_to_map(symbol_map)?; sections.add_function(function); @@ -659,10 +660,13 @@ impl Module { parse_options: Default::default(), check_defs_uses: true, existing_functions: Some(&functions), - })?; + }); let autoload_function = match parse_result { - ParseFunctionResult::Found(function) => function, - _ => return FunctionAnalysisFailedSnafu { name, parse_result }.fail(), + Ok(function) => function, + Err(FunctionAnalysisError::IntoFunction { source: IntoFunctionError::ParseFunction { source } }) => { + return FunctionAnalysisFailedSnafu { name, parse_result: source }.fail(); + } + Err(e) => return Err(e.into()), }; symbol_map.add_function(&autoload_function); functions.insert(autoload_function.first_instruction_address(), autoload_function); @@ -744,15 +748,15 @@ impl Module { self.add_bss_section(bss_start)?; let section_after_text = self.sections.get_section_after(text_end); - if let Some(section_after_text) = section_after_text { - if text_end != section_after_text.start_address() { - log::warn!( - "Expected .text to end ({:#010x}) where {} starts ({:#010x})", - text_end, - section_after_text.name(), - section_after_text.start_address() - ); - } + if let Some(section_after_text) = section_after_text + && text_end != section_after_text.start_address() + { + log::warn!( + "Expected .text to end ({:#010x}) where {} starts ({:#010x})", + text_end, + section_after_text.name(), + section_after_text.start_address() + ); } Ok(()) diff --git a/lib/src/config/relocations.rs b/lib/src/config/relocations.rs index b5f28c8..e1f903e 100644 --- a/lib/src/config/relocations.rs +++ b/lib/src/config/relocations.rs @@ -13,6 +13,10 @@ use ds_rom::rom::raw::AutoloadKind; use serde::{Deserialize, Serialize}; use snafu::Snafu; +use super::{ + ParseContext, iter_attributes, + module::{Module, ModuleKind}, +}; use crate::{ config::symbol::{Symbol, SymbolMap}, util::{ @@ -21,11 +25,6 @@ use crate::{ }, }; -use super::{ - ParseContext, iter_attributes, - module::{Module, ModuleKind}, -}; - pub struct Relocations { relocations: BTreeMap, } diff --git a/lib/src/config/section.rs b/lib/src/config/section.rs index a280792..399ef73 100644 --- a/lib/src/config/section.rs +++ b/lib/src/config/section.rs @@ -9,13 +9,12 @@ use std::{ use serde::Serialize; use snafu::Snafu; +use super::{ParseContext, iter_attributes, module::Module}; use crate::{ analysis::functions::Function, util::{bytes::FromSlice, parse::parse_u32}, }; -use super::{iter_attributes, module::Module, ParseContext}; - pub const DTCM_SECTION: &str = ".dtcm"; #[derive(Clone, Copy)] @@ -39,7 +38,9 @@ pub enum SectionError { EndBeforeStart { name: String, start_address: u32, end_address: u32, backtrace: Backtrace }, #[snafu(display("Section {name} aligment ({alignment}) must be a power of two:\n{backtrace}"))] AlignmentPowerOfTwo { name: String, alignment: u32, backtrace: Backtrace }, - #[snafu(display("Section {name} starts at a misaligned address {start_address:#010x}; the provided alignment was {alignment}:\n{backtrace}"))] + #[snafu(display( + "Section {name} starts at a misaligned address {start_address:#010x}; the provided alignment was {alignment}:\n{backtrace}" + ))] MisalignedStart { name: String, start_address: u32, alignment: u32, backtrace: Backtrace }, } @@ -53,14 +54,12 @@ pub enum SectionParseError { ParseEndAddress { context: ParseContext, value: String, error: ParseIntError, backtrace: Backtrace }, #[snafu(display("{context}: failed to parse alignment '{value}': {error}\n{backtrace}"))] ParseAlignment { context: ParseContext, value: String, error: ParseIntError, backtrace: Backtrace }, - #[snafu(display( - "{context}: expected section attribute 'kind', 'start', 'end' or 'align' but got '{key}':\n{backtrace}" - ))] + #[snafu(display("{context}: expected section attribute 'kind', 'start', 'end' or 'align' but got '{key}':\n{backtrace}"))] UnknownAttribute { context: ParseContext, key: String, backtrace: Backtrace }, #[snafu(display("{context}: missing '{attribute}' attribute:\n{backtrace}"))] MissingAttribute { context: ParseContext, attribute: String, backtrace: Backtrace }, #[snafu(display("{context}: {error}"))] - Section { context: ParseContext, error: SectionError }, + Section { context: ParseContext, error: Box }, } #[derive(Debug, Snafu)] @@ -519,9 +518,8 @@ impl Sections { } impl IntoIterator for Sections { - type Item = Section; - type IntoIter = as IntoIterator>::IntoIter; + type Item = Section; fn into_iter(self) -> Self::IntoIter { self.sections.into_iter() diff --git a/lib/src/config/symbol.rs b/lib/src/config/symbol.rs index 33d4b65..5157313 100644 --- a/lib/src/config/symbol.rs +++ b/lib/src/config/symbol.rs @@ -11,6 +11,7 @@ use std::{ use snafu::{Snafu, ensure}; +use super::{ParseContext, config::Config, iter_attributes, module::ModuleKind}; use crate::{ analysis::{functions::Function, jump_table::JumpTable}, util::{ @@ -19,8 +20,6 @@ use crate::{ }, }; -use super::{ParseContext, config::Config, iter_attributes, module::ModuleKind}; - pub struct SymbolMaps { symbol_maps: BTreeMap, } @@ -240,7 +239,7 @@ impl SymbolMap { Ok(Some((index, symbol))) } - pub fn iter_by_address(&self, range: Range) -> SymbolIterator { + pub fn iter_by_address(&self, range: Range) -> SymbolIterator<'_> { SymbolIterator { symbols_by_address: self.symbols_by_address.range(range), indices: [].iter(), symbols: &self.symbols } } diff --git a/lib/src/util/parse.rs b/lib/src/util/parse.rs index 1bf7003..f3bfeb2 100644 --- a/lib/src/util/parse.rs +++ b/lib/src/util/parse.rs @@ -23,9 +23,5 @@ pub fn parse_i32(text: &str) -> Result { } else { i32::from_str_radix(value, 10)? }; - if negative { - Ok(-abs_value) - } else { - Ok(abs_value) - } + if negative { Ok(-abs_value) } else { Ok(abs_value) } }