From 2d7c9844d791386fe19cbebffb03dcc4f28cbdea Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Wed, 30 Sep 2026 17:25:27 -0400 Subject: [PATCH] dat: fix symbol ordering, format generated source in-place - No header code in base objects: jobj.h redeclared the static inline HSD_JObjRefThis without `inline`, which made clang emit it (and iref_INC, with its assert's file name) wherever the header is included. `samples report` lists anything a base object has outside .data. - The base object's .data is laid out in the target's order, so objdiff's section match agrees with the per-symbol one. clang lays variables out where an initializer first points to them, not where they are defined, so each unit is compiled with -fdata-sections and linked (ld.lld -r) with target/.ld, which `samples slice` writes. Every unit's .data bytes now equal the target's. - Formatting in place: codegen writes src/ and the format step formats it there, with a stamp in stamp/, so there is one set of generated sources. Also: - The build runs without the dev shell's environment, as objdiff runs it: NEWLIB_INCLUDE and AURORA_SRC are cached (from the environment when set; configuring without them fails), and the LLVM tools and cargo are found by absolute path. Configuring through a symlinked path fails: the build then names files by two paths, and ninja reran CMake and rebuilt everything on every build. - User-side switches in the build's cache: MELEE_DAT_SAMPLES_ALL (archive globs) samples every typed object of those archives, and MELEE_DAT_SAMPLES_EXCLUDE (type globs) keeps types out of the samples. - objdiff is pinned to v3.8.2, which no longer needs the Cargo.lock patch. 11237 of 11239 samples match, as before; the DOL still matches. Co-Authored-By: Claude Opus 5.5 --- .nix/duplicate-similar-dep.patch | 33 --------------- .nix/objdiff.nix | 11 +---- CMakePresets.json | 3 +- cmake/Dat.cmake | 71 +++++++++++++++++++++++++------- cmake/DatFormat.cmake | 18 ++++---- cmake/Dwarf.cmake | 4 ++ cmake/MeleeConfig.cmake | 11 ++++- cmake/ppc32-clang.cmake | 19 +++++++-- flake.lock | 8 ++-- flake.nix | 2 +- src/sysdolphin/baselib/jobj.h | 1 - tools/dat-cli/README.md | 17 ++++++-- tools/dat-cli/src/cmd/samples.rs | 70 +++++++++++++++++++++++++++++-- tools/dat-cli/src/samples.rs | 29 +++++++++++-- 14 files changed, 204 insertions(+), 93 deletions(-) delete mode 100644 .nix/duplicate-similar-dep.patch diff --git a/.nix/duplicate-similar-dep.patch b/.nix/duplicate-similar-dep.patch deleted file mode 100644 index 7eedf2830b..0000000000 --- a/.nix/duplicate-similar-dep.patch +++ /dev/null @@ -1,33 +0,0 @@ ---- a/Cargo.lock -+++ b/Cargo.lock -@@ -2520,7 +2520,7 @@ checksum = "154934ea70c58054b556dd430b99a98c2a7ff5309ac9891597e339b5c28f4371" - dependencies = [ - "console", - "once_cell", -- "similar 2.7.0 (registry+https://github.com/rust-lang/crates.io-index)", -+ "similar", - ] - - [[package]] -@@ -3501,7 +3501,7 @@ dependencies = [ - "serde", - "serde_json", - "shell-escape", -- "similar 2.7.0 (git+https://github.com/encounter/similar.git?branch=no_std)", -+ "similar", - "syn", - "tempfile", - "time", -@@ -4885,12 +4885,6 @@ version = "0.3.7" - source = "registry+https://github.com/rust-lang/crates.io-index" - checksum = "d66dc143e6b11c1eddc06d5c423cfc97062865baf299914ab64caa38182078fe" - --[[package]] --name = "similar" --version = "2.7.0" --source = "registry+https://github.com/rust-lang/crates.io-index" --checksum = "bbbb5d9659141646ae647b42fe094daf6c6192d1620870b449d9557f748b2daa" -- - [[package]] - name = "similar" - version = "2.7.0" diff --git a/.nix/objdiff.nix b/.nix/objdiff.nix index a50e39d5e9..b9b449f06a 100644 --- a/.nix/objdiff.nix +++ b/.nix/objdiff.nix @@ -1,10 +1,8 @@ { - stdenvNoCC, lib, fontconfig, pkg-config, rustPlatform, - srcOnly, src, }: @@ -12,14 +10,7 @@ rustPlatform.buildRustPackage (finalAttrs: { pname = "objdiff"; version = src.shortRev; - src = srcOnly { - name = "objdiff-patched"; - inherit src; - stdenv = stdenvNoCC; - patches = [ - ./duplicate-similar-dep.patch - ]; - }; + inherit src; cargoBuildFlags = [ "--workspace" diff --git a/CMakePresets.json b/CMakePresets.json index 28bc5d5850..d8edd00c0d 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -42,8 +42,7 @@ "MELEE_DWARF": { "type": "BOOL", "value": true - }, - "NEWLIB_INCLUDE": "$env{NEWLIB_INCLUDE}" + } } }, { diff --git a/cmake/Dat.cmake b/cmake/Dat.cmake index f4f35e91cc..b1f566f707 100644 --- a/cmake/Dat.cmake +++ b/cmake/Dat.cmake @@ -1,11 +1,19 @@ # Samples of the .dat archives' data, typed by the DWARF build and compared # with objdiff (see tools/dat-cli). Each archive is a unit, built in four -# steps: slice (archive → target/.o), codegen (→ gen/.c, which -# includes a header and source per root in gen//), format (→ src/) +# steps: slice (archive → target/.o), codegen (→ src/.c, which +# includes a header and source per root in src//), format (in place) # and compile (→ base/.o). The build directory is also the objdiff # project. include_guard(GLOBAL) +# Configured through a symlink, the build names the same files by two paths +# (the physical one CMake resolves, and the logical one from $PWD), and +# ninja then reruns CMake and rebuilds everything on every build +file(REAL_PATH "${CMAKE_SOURCE_DIR}" _dat_real_source) +if(NOT _dat_real_source STREQUAL CMAKE_SOURCE_DIR) + message(FATAL_ERROR "Configure from ${_dat_real_source}, not through a symlink (${CMAKE_SOURCE_DIR}): cd -P there first") +endif() + if(NOT MELEE_DWARF) message(FATAL_ERROR "MELEE_DAT_SAMPLES needs MELEE_DWARF: the samples are typed by its DWARF") endif() @@ -13,6 +21,23 @@ endif() set(MELEE_DAT "" CACHE FILEPATH "melee-dat binary; built with cargo if empty") set(MELEE_DAT_FILES "${CMAKE_SOURCE_DIR}/orig/${MELEE_VERSION}/files" CACHE PATH "The game's files, with its .dat archives") +# What to sample is the user's choice, not the project's +set(MELEE_DAT_SAMPLES_ALL "" CACHE STRING + "Archives (globs, e.g. PlFx.dat;Gr*.dat) to sample every typed object of, not one instance per type") +set(MELEE_DAT_SAMPLES_EXCLUDE "" CACHE STRING + "Types (globs on their names) never to sample, e.g. bulky vertex or image records") + +set(_dat_all_regexes) +foreach(_glob IN LISTS MELEE_DAT_SAMPLES_ALL) + string(REPLACE "." "\\." _regex "${_glob}") + string(REPLACE "*" ".*" _regex "${_regex}") + string(REPLACE "?" "." _regex "${_regex}") + list(APPEND _dat_all_regexes "^${_regex}$") +endforeach() +set(_dat_exclude) +foreach(_glob IN LISTS MELEE_DAT_SAMPLES_EXCLUDE) + list(APPEND _dat_exclude --exclude "${_glob}") +endforeach() set(_dat_config "${CMAKE_SOURCE_DIR}/config/${MELEE_VERSION}/dat.yml") set(_dat_symbols "${CMAKE_SOURCE_DIR}/config/${MELEE_VERSION}/dat_symbols.txt") @@ -27,9 +52,10 @@ if(MELEE_DAT) set(_dat_tool "${MELEE_DAT}") else() set(_dat_tool "${CMAKE_CURRENT_BINARY_DIR}/cargo/release/melee-dat") + find_program(MELEE_CARGO cargo REQUIRED) add_custom_command( OUTPUT "${_dat_tool}" - COMMAND cargo build --release -p melee-dat + COMMAND "${MELEE_CARGO}" build --release -p melee-dat --manifest-path "${CMAKE_SOURCE_DIR}/Cargo.toml" --target-dir "${CMAKE_CURRENT_BINARY_DIR}/cargo" DEPFILE "${_dat_tool}.d" @@ -69,46 +95,61 @@ foreach(_archive IN LISTS _dat_archives) get_filename_component(_unit "${_archive}" NAME_WE) set(_target "target/${_unit}.o") set(_sidecar "target/${_unit}.samples") - set(_generated "gen/${_unit}.c") + set(_layout "target/${_unit}.ld") + set(_object "obj/${_unit}.o") set(_source "src/${_unit}.c") + set(_formatted "stamp/${_unit}.formatted") set(_base "base/${_unit}.o") + set(_all) + foreach(_regex IN LISTS _dat_all_regexes) + if(_file MATCHES "${_regex}") + set(_all --all) + endif() + endforeach() add_custom_command( - OUTPUT "${_target}" "${_sidecar}" + OUTPUT "${_target}" "${_sidecar}" "${_layout}" COMMAND "${_dat_tool}" samples slice "${_file}" "${_dat_config}" -p "${CMAKE_SOURCE_DIR}" --types types.bin - --files "${MELEE_DAT_FILES}" -o "${_target}" + --files "${MELEE_DAT_FILES}" -o "${_target}" ${_all} ${_dat_exclude} DEPENDS "${_archive}" types.bin "${_dat_tool}" "${_dat_config}" "${_dat_symbols}" COMMENT "Slicing ${_file}" VERBATIM ) add_custom_command( - OUTPUT "${_generated}" + OUTPUT "${_source}" COMMAND "${_dat_tool}" samples codegen "${_target}" --types types.bin - -o "${_generated}" + -o "${_source}" DEPENDS "${_target}" "${_sidecar}" types.bin "${_dat_tool}" - COMMENT "Generating ${_generated}" + COMMENT "Generating ${_source}" VERBATIM ) add_custom_command( - OUTPUT "${_source}" + OUTPUT "${_formatted}" COMMAND "${CMAKE_COMMAND}" "-DCLANG_FORMAT=${MELEE_CLANG_FORMAT}" "-DSTYLE=${CMAKE_SOURCE_DIR}/.clang-format" - "-DGENERATED=${CMAKE_CURRENT_BINARY_DIR}/${_generated}" "-DSOURCE=${CMAKE_CURRENT_BINARY_DIR}/${_source}" + "-DSTAMP=${CMAKE_CURRENT_BINARY_DIR}/${_formatted}" -P "${CMAKE_SOURCE_DIR}/cmake/DatFormat.cmake" - DEPENDS "${_generated}" "${CMAKE_SOURCE_DIR}/.clang-format" + DEPENDS "${_source}" "${CMAKE_SOURCE_DIR}/.clang-format" "${CMAKE_SOURCE_DIR}/cmake/DatFormat.cmake" COMMENT "Formatting ${_source}" VERBATIM ) add_custom_command( OUTPUT "${_base}" - COMMAND "${CMAKE_C_COMPILER}" "@${_dat_flags}" -MD -MF "${_base}.d" - -c "${_source}" -o "${_base}" - DEPENDS "${_source}" "${_dat_flags}" + BYPRODUCTS "${_object}" + # Each sample in its own section, then linked into .data in the + # target's order: clang lays variables out where an initializer + # first points to them + COMMAND "${CMAKE_C_COMPILER}" "@${_dat_flags}" -fdata-sections + -MD -MF "${_base}.d" -MT "${_base}" + -c "${_source}" -o "${_object}" + COMMAND "${CMAKE_LINKER}" -r -T "${_layout}" "${_object}" + -o "${_base}" + DEPENDS "${_source}" "${_formatted}" "${_layout}" "${_dat_flags}" DEPFILE "${_base}.d" COMMENT "Compiling ${_source}" VERBATIM diff --git a/cmake/DatFormat.cmake b/cmake/DatFormat.cmake index 32ac54c7b3..22949362f6 100644 --- a/cmake/DatFormat.cmake +++ b/cmake/DatFormat.cmake @@ -1,21 +1,17 @@ -# Copies a unit's generated C (gen/.c and gen//) into src/ and -# formats it; run by Dat.cmake's format step with -P. +# Formats a unit's generated C (src/.c and src//) in place, then +# writes a stamp; run by Dat.cmake's format step with -P. # CLANG_FORMAT clang-format # STYLE the .clang-format file -# GENERATED gen/.c # SOURCE src/.c +# STAMP stamp/.formatted, written after formatting so that +# it is newer than the sources cmake_minimum_required(VERSION 3.20) -cmake_path(REMOVE_EXTENSION GENERATED LAST_ONLY OUTPUT_VARIABLE _gen_dir) -cmake_path(REMOVE_EXTENSION SOURCE LAST_ONLY OUTPUT_VARIABLE _src_dir) -cmake_path(GET SOURCE PARENT_PATH _src_parent) - -# The roots of the previous generation may be gone -file(REMOVE_RECURSE "${_src_dir}") -file(COPY "${_gen_dir}" "${GENERATED}" DESTINATION "${_src_parent}") -file(GLOB _files "${_src_dir}/*.[ch]") +cmake_path(REMOVE_EXTENSION SOURCE LAST_ONLY OUTPUT_VARIABLE _dir) +file(GLOB _files "${_dir}/*.[ch]") execute_process( COMMAND "${CLANG_FORMAT}" "--style=file:${STYLE}" -i "${SOURCE}" ${_files} COMMAND_ERROR_IS_FATAL ANY ) +file(TOUCH "${STAMP}") diff --git a/cmake/Dwarf.cmake b/cmake/Dwarf.cmake index 66c2579707..4424af21ae 100644 --- a/cmake/Dwarf.cmake +++ b/cmake/Dwarf.cmake @@ -14,6 +14,10 @@ if(MELEE_VERSION_NUM EQUAL -1) message(FATAL_ERROR "Unknown MELEE_VERSION ${MELEE_VERSION}; one of: ${MELEE_VERSIONS}") endif() +if(NOT NEWLIB_INCLUDE) + message(FATAL_ERROR "NEWLIB_INCLUDE is not set: configure from the dev shell") +endif() + target_compile_definitions(melee PRIVATE LINT DAT_ANNOTATIONS diff --git a/cmake/MeleeConfig.cmake b/cmake/MeleeConfig.cmake index 790b000f6f..c32554719a 100644 --- a/cmake/MeleeConfig.cmake +++ b/cmake/MeleeConfig.cmake @@ -3,9 +3,18 @@ include_guard(GLOBAL) get_filename_component(_melee_root "${CMAKE_CURRENT_LIST_DIR}/.." ABSOLUTE) +# From the dev shell's environment when it has it, else as cached: ninja +# re-runs CMake from wherever it is started (e.g. objdiff) +if(DEFINED ENV{AURORA_SRC}) + set(AURORA_SRC "$ENV{AURORA_SRC}" CACHE PATH "Aurora's source" FORCE) +endif() +if(NOT AURORA_SRC) + message(FATAL_ERROR "AURORA_SRC is not set: configure from the dev shell") +endif() + add_library(melee_game_headers INTERFACE) target_include_directories(melee_game_headers INTERFACE - $ENV{AURORA_SRC}/include + ${AURORA_SRC}/include ${_melee_root}/src ${_melee_root}/libs/doldecomp/include ) diff --git a/cmake/ppc32-clang.cmake b/cmake/ppc32-clang.cmake index ccf5c0e221..73ad9f171e 100644 --- a/cmake/ppc32-clang.cmake +++ b/cmake/ppc32-clang.cmake @@ -5,14 +5,25 @@ set(CMAKE_SYSTEM_PROCESSOR powerpc) set(CMAKE_C_COMPILER clang) set(CMAKE_C_COMPILER_TARGET ppc32-none-eabi) -set(CMAKE_AR llvm-ar) -set(CMAKE_RANLIB llvm-ranlib) -set(CMAKE_LINKER ld.lld) +# By absolute path, so that the build doesn't need the dev shell's PATH +find_program(MELEE_LLVM_AR llvm-ar REQUIRED) +find_program(MELEE_LLVM_RANLIB llvm-ranlib REQUIRED) +find_program(MELEE_LLD ld.lld REQUIRED) +set(CMAKE_AR "${MELEE_LLVM_AR}") +set(CMAKE_RANLIB "${MELEE_LLVM_RANLIB}") +set(CMAKE_LINKER "${MELEE_LLD}") # There is no C runtime to link a test executable against set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY) -set(NEWLIB_INCLUDE "" CACHE PATH "newlib include directory for powerpc-none-eabi") +# From the dev shell's environment when it has it, else as cached: ninja +# re-runs CMake from wherever it is started (e.g. objdiff) +set(_doc "newlib include directory for powerpc-none-eabi") +if(DEFINED ENV{NEWLIB_INCLUDE}) + set(NEWLIB_INCLUDE "$ENV{NEWLIB_INCLUDE}" CACHE PATH "${_doc}" FORCE) +else() + set(NEWLIB_INCLUDE "" CACHE PATH "${_doc}") +endif() if(NEWLIB_INCLUDE) set(CMAKE_C_STANDARD_INCLUDE_DIRECTORIES "${NEWLIB_INCLUDE}") endif() diff --git a/flake.lock b/flake.lock index 1c0db4da3f..ff7d1e16e8 100644 --- a/flake.lock +++ b/flake.lock @@ -66,16 +66,16 @@ "objdiff": { "flake": false, "locked": { - "lastModified": 1769751602, - "narHash": "sha256-h7+VIMB1lCgO/e24sowiDy/W4uvwiXODC5Lm+siF+Hg=", + "lastModified": 1790698507, + "narHash": "sha256-fM7fQv0TguxohDvc3wDM9NEmnI2BfYkS1kYlQl3MOTM=", "owner": "encounter", "repo": "objdiff", - "rev": "66c879a95d45c1170a0834071cab58655fd9773b", + "rev": "eed74b99c4e94dd154882259931201badc6fdbd1", "type": "github" }, "original": { "owner": "encounter", - "ref": "v3.6.1", + "ref": "v3.8.2", "repo": "objdiff", "type": "github" } diff --git a/flake.nix b/flake.nix index a5e8d0557e..cf98c60a9d 100644 --- a/flake.nix +++ b/flake.nix @@ -19,7 +19,7 @@ flake = false; }; objdiff = { - url = "github:encounter/objdiff/v3.6.1"; + url = "github:encounter/objdiff/v3.8.2"; flake = false; }; sjiswrap = { diff --git a/src/sysdolphin/baselib/jobj.h b/src/sysdolphin/baselib/jobj.h index b9f6bdbdbc..72d9e55d57 100644 --- a/src/sysdolphin/baselib/jobj.h +++ b/src/sysdolphin/baselib/jobj.h @@ -724,7 +724,6 @@ static inline void HSD_JObjRefThis(HSD_JObj* jobj) void HSD_JObjResolveRefs(HSD_JObj* jobj, HSD_Joint* joint); void HSD_JObjUnrefThis(HSD_JObj* jobj); -void HSD_JObjRefThis(HSD_JObj* jobj); void HSD_JObjMakeMatrix(HSD_JObj* jobj); void RecalcParentTrspBits(HSD_JObj* jobj); void HSD_JObjAddChild(HSD_JObj* jobj, HSD_JObj* child); diff --git a/tools/dat-cli/README.md b/tools/dat-cli/README.md index bc499fbe22..e6cff5666b 100644 --- a/tools/dat-cli/README.md +++ b/tools/dat-cli/README.md @@ -127,9 +127,13 @@ melee-dat samples report build/GALE01/dat the other data they point to, and designated initializers generated from the types; pointers into other roots include those roots' headers - `src/.c`: the unit, which includes every root's source; all of - `src` is generated into `gen` and formatted with the repository's - `.clang-format` -- `base/.o`: that C, compiled with the DWARF build's flags + it is formatted in place with the repository's `.clang-format` + (`stamp/.formatted` records that) +- `base/.o`: that C, compiled with the DWARF build's flags, one + section per variable (`obj/.o`), then linked with + `target/.ld` into one `.data` in the target's order (clang lays + variables out where they are first pointed to, not where they are + defined) `compile_commands.json` there gives clangd the same flags as the build. @@ -144,6 +148,13 @@ that doesn't round-trip. A union is written through the member its tag chose; a union object is declared as that member (`typeof(((union U *) 0)->member)`), since the archive only holds that member's bytes. +What to sample is up to you, in the build's cache: by default each archive +gives its best instance of each type. `MELEE_DAT_SAMPLES_ALL` takes archive +globs whose every typed object becomes a sample, e.g. +`cmake --preset dat -DMELEE_DAT_SAMPLES_ALL="PlFx.dat;Gr*.dat"`, and +`MELEE_DAT_SAMPLES_EXCLUDE` type globs never to sample (data they point to +stays bytes), for records too bulky to want in C. + Use `samples report` for the verdict: objdiff's own report measures data per section and misses relocation differences. To check a type change, edit the header and rebuild the preset. diff --git a/tools/dat-cli/src/cmd/samples.rs b/tools/dat-cli/src/cmd/samples.rs index 07b27b3a08..c755c457d7 100644 --- a/tools/dat-cli/src/cmd/samples.rs +++ b/tools/dat-cli/src/cmd/samples.rs @@ -11,6 +11,7 @@ use super::project::{Check, Project}; use anyhow::{Context, Result, bail}; +use globset::{Glob, GlobSetBuilder}; use melee_dat::{ dwarf::{TypeGraph, cache::TypesFile, canonical::Canonical}, hsd::Archive, @@ -18,7 +19,9 @@ use melee_dat::{ CWriter, Instance, Picker, SampleInfo, Source, root_of, target_object, }, }; -use object::{Object, ObjectSection, ObjectSymbol, RelocationTarget}; +use object::{ + Object, ObjectSection, ObjectSymbol, RelocationTarget, SectionKind, +}; use rayon::prelude::*; use serde::{Deserialize, Serialize}; use serde_json::json; @@ -60,6 +63,13 @@ struct Slice { /// The target object; its sidecar is written next to it as `.samples` #[arg(short, long)] output: PathBuf, + /// Every typed object, not just the best instance of each type + #[arg(long)] + all: bool, + /// Types never to sample, as globs on their names (e.g. + /// `HSD_VtxDescList`); data they point to stays bytes + #[arg(long)] + exclude: Vec, } #[derive(clap::Args)] @@ -133,7 +143,12 @@ fn slice(args: Slice) -> Result<()> { .with_context(|| format!("{}", path.display()))?; // This archive's best instance of each type and variant - let mut picker = Picker::new(&project.graph, &project.canonical); + let mut exclude = GlobSetBuilder::new(); + for glob in &args.exclude { + exclude.add(Glob::new(glob)?); + } + let mut picker = Picker::new(&project.graph, &project.canonical) + .select(args.all, exclude.build()?); let mut walks = BTreeMap::new(); for (at, archive) in &archives { let (_, walk) = project.walk(&args.archive, archive); @@ -182,6 +197,14 @@ fn slice(args: Slice) -> Result<()> { fs::create_dir_all(dir)?; } fs::write(&args.output, target_object(&pairs)?)?; + // The target's order, for linking the base object's `.data` the same: + // clang lays variables out where an initializer first points to them + let mut script = String::from("SECTIONS\n{\n .data : {\n"); + for info in &infos { + script += &format!(" *(.data.{})\n", info.symbol); + } + script += " *(.data .data.*)\n }\n}\n"; + fs::write(args.output.with_extension("ld"), script)?; let sidecar = Sidecar { archive: args.archive, samples: infos, @@ -247,8 +270,8 @@ fn codegen(args: Codegen) -> Result<()> { )?; // A directory of each root's header and source, next to the unit's - // file, which includes the sources: `gen/PlMr/ftDataMario.{h,c}`, - // `gen/PlMr.c` + // file, which includes the sources: `src/PlMr/ftDataMario.{h,c}`, + // `src/PlMr.c` let stem = args .output .file_stem() @@ -268,6 +291,9 @@ fn codegen(args: Codegen) -> Result<()> { ); for root in &roots { fs::write(dir.join(format!("{}.h", root.name)), &root.header)?; + unit += &format!("#include \"{stem}/{}.h\"\n", root.name); + } + for root in &roots { if let Some(source) = &root.source { fs::write(dir.join(format!("{}.c", root.name)), source)?; unit += &format!("#include \"{stem}/{}.c\"\n", root.name); @@ -369,6 +395,36 @@ struct UnitMatch { name: String, measures: MatchMeasures, samples: Vec, + /// The base object's contents outside `.data`, e.g. code or strings + /// from headers, which the target doesn't have. + extra: Vec, +} + +/// Allocated sections of a unit's base object other than `.data`, with +/// their sizes. +fn extra_sections(dir: &Path, unit: &str) -> Result> { + let path = dir.join(format!("base/{unit}.o")); + let data = + fs::read(&path).with_context(|| format!("{}", path.display()))?; + let obj = object::File::parse(&*data)?; + Ok(obj + .sections() + .filter(|s| s.size() > 0 && s.name() != Ok(".data")) + .filter(|s| { + matches!( + s.kind(), + SectionKind::Text + | SectionKind::Data + | SectionKind::ReadOnlyData + | SectionKind::ReadOnlyDataWithRel + | SectionKind::ReadOnlyString + | SectionKind::UninitializedData + ) + }) + .map(|s| { + format!("{} ({:#X} bytes)", s.name().unwrap_or("?"), s.size()) + }) + .collect()) } /// objdiff's diff of one unit's target and base objects: each sample's @@ -447,6 +503,7 @@ fn report(args: Report) -> Result<()> { name: name.clone(), measures, samples, + extra: extra_sections(&args.dir, name)?, }) }) .collect::>()?; @@ -477,6 +534,11 @@ fn report(args: Report) -> Result<()> { s.match_percent, s.name, s.size )?; } + for unit in &units { + for section in &unit.extra { + writeln!(out, " extra {section} in base/{}.o", unit.name)?; + } + } writeln!( out, "{}/{} samples match in {} units, {:.2}% of their bytes", diff --git a/tools/dat-cli/src/samples.rs b/tools/dat-cli/src/samples.rs index c86c104482..ec6c4b8d99 100644 --- a/tools/dat-cli/src/samples.rs +++ b/tools/dat-cli/src/samples.rs @@ -20,6 +20,7 @@ use crate::{ walk::Walk, }; use anyhow::{Context, Result, bail}; +use globset::GlobSet; use object::{ Architecture, BinaryFormat, Endianness, RelocationFlags, SectionKind, SymbolFlags, SymbolKind, SymbolScope, @@ -145,9 +146,13 @@ pub struct Picker<'a> { graph: &'a TypeGraph, canonical: &'a Canonical, renderer: Renderer<'a>, - /// By type and variant. - best: BTreeMap<(String, Vec), Sample>, + /// By type and variant, and in [`Picker::all`] mode by location too. + best: BTreeMap<(String, Vec, Option<(usize, u32)>), Sample>, skipped: BTreeMap, + /// Keep every instance, not the best of each type and variant. + all: bool, + /// Types never sampled, by name. + exclude: GlobSet, } impl<'a> Picker<'a> { @@ -158,9 +163,19 @@ impl<'a> Picker<'a> { renderer: Renderer::new(graph, canonical), best: BTreeMap::new(), skipped: BTreeMap::new(), + all: false, + exclude: GlobSet::empty(), } } + /// Keep every instance of every type, except the types `exclude` + /// matches. + pub fn select(mut self, all: bool, exclude: GlobSet) -> Self { + self.all = all; + self.exclude = exclude; + self + } + /// Consider every object one archive's walk typed. pub fn add( &mut self, @@ -239,6 +254,11 @@ impl<'a> Picker<'a> { member_name = Some(name.to_owned()); die = member_ty; } + if self.exclude.is_match(&key_name) + || self.exclude.is_match(&lookup) + { + continue; + } let Some(size) = self.member_size(die) else { continue; }; @@ -302,7 +322,8 @@ impl<'a> Picker<'a> { // A clean instance if there is one, so that a sample fails // only where its type is wrong everywhere; then the one that // exercises the most: pointers, then data - let key = (key_name, candidate.variant.clone()); + let at = self.all.then_some((archive_offset, offset)); + let key = (key_name, candidate.variant.clone(), at); let better = self.best.get(&key).is_none_or(|best| { (candidate.clean, candidate.relocs, candidate.nonzero) > (best.clean, best.relocs, best.nonzero) @@ -370,7 +391,7 @@ impl<'a> Picker<'a> { .skipped .into_iter() .filter(|(name, _)| { - !self.best.keys().any(|(t, _)| { + !self.best.keys().any(|(t, _, _)| { t == name || t.strip_prefix(name.as_str()) .is_some_and(|m| m.starts_with('.'))