Skip to content

Commit

Permalink
Match Functions with Same Name in dups (Xeeynamo#1449)
Browse files Browse the repository at this point in the history
The duplicates report has several false negatives when a function has
the same name as another function which has not been decompiled. This
affects overlays which share many of the same function names (e.g.
`EntityWeaponAttack`).

The duplicates tool now parses the `INCLUDE_ASM` macro to extract the
path and ASM file. It then uses that when determining whether or not
each ASM file is actually included or not. Previously, only the function
name was checked, so if any `INCLUDE_ASM` file had the target function
name, it would be marked as not decompiled.

Before:

% | Decomp? | Name | Asm Path

-----|-------|-------------------------------|--------------------------------
| 1.00 | false | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_008/EntityWeaponAttack.s
| 1.00 | false | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_009/EntityWeaponAttack.s
| 0.91 | false | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_010/EntityWeaponAttack.s
| 1.00 | false | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_011/EntityWeaponAttack.s
| 0.98 | false | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_025/EntityWeaponAttack.s
| 1.00 | false | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_026/EntityWeaponAttack.s
| 1.00 | false | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_027/EntityWeaponAttack.s
| 1.00 | false | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_028/EntityWeaponAttack.s
| 0.97 | false | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_058/EntityWeaponAttack.s

After:

% | Decomp? | Name | Asm Path

-----|-------|-------------------------------|--------------------------------
1.00 | true | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_008/EntityWeaponAttack.s
1.00 | true | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_009/EntityWeaponAttack.s
0.91 | false | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_010/EntityWeaponAttack.s
1.00 | true | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_011/EntityWeaponAttack.s
0.98 | false | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_025/EntityWeaponAttack.s
1.00 | true | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_026/EntityWeaponAttack.s
1.00 | true | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_027/EntityWeaponAttack.s
1.00 | true | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_028/EntityWeaponAttack.s
0.97 | true | EntityWeaponAttack |
../../asm/us/weapon/nonmatchings/w_058/EntityWeaponAttack.s

(note: `w_008`, `w_009`, and `w_011` are decompiled in my workspace, but
not in GH)

Co-authored-by: Jonathan Hohle <[email protected]>
  • Loading branch information
hohle and hohle authored Jul 28, 2024
1 parent 93c51db commit 85cbb97
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 3 deletions.
45 changes: 45 additions & 0 deletions tools/dups/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions tools/dups/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ edition = "2021"

[dependencies]
clap = { version = "4.3.19", features = ["derive"] }
regex = "1.10.5"

10 changes: 8 additions & 2 deletions tools/dups/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use regex::Regex;
use std::env::*;
use std::fs;
use std::fs::File;
Expand Down Expand Up @@ -151,11 +152,13 @@ struct Args {
pub struct IncludeAsmEntry {
pub line: String,
pub path: String,
pub asm_path: String,
}

fn process_directory_for_include_asm(dir: &str) -> Vec<IncludeAsmEntry> {
let entries = std::fs::read_dir(dir).expect("Unable to read directory");

let re = Regex::new("INCLUDE_ASM\\(\"([^\"]*)\", ([^)]*)\\)").unwrap();
let mut output = Vec::new();

entries.for_each(|entry| {
Expand All @@ -170,9 +173,12 @@ fn process_directory_for_include_asm(dir: &str) -> Vec<IncludeAsmEntry> {
let line_str = line.unwrap();

if line_str.contains("INCLUDE_ASM") {
let (full, [asm_dir, asm_file]) = re.captures(&line_str).unwrap().extract();

output.push(IncludeAsmEntry {
line: line_str.clone(),
path: item_path.to_string_lossy().to_string(),
asm_path: format!("../../asm/us/{}/{}.s", asm_dir, asm_file),
});
}
}
Expand Down Expand Up @@ -304,7 +310,7 @@ fn do_dups_report(output_file: Option<String>, threshold: f64) {
SrcAsmPair {
asm_dir: String::from("../../asm/us/weapon/nonmatchings/"),
src_dir: String::from("../../src/weapon/"),
overlay_name: String::from("WEAPON"),
overlay_name: String::from("WEAPON0"),
include_asm: get_all_include_asm("../../src/weapon/"),
path_matcher: "/weapon/".to_string(),
},
Expand Down Expand Up @@ -386,7 +392,7 @@ fn do_dups_report(output_file: Option<String>, threshold: f64) {
for pair in &pairs.clone() {
if function.file.contains(&pair.path_matcher) {
for inc in &pair.include_asm {
if inc.line.contains(&function.name) {
if function.file == inc.asm_path && inc.line.contains(&function.name) {
decompiled = false;
}
}
Expand Down
25 changes: 24 additions & 1 deletion tools/tools.mk
Original file line number Diff line number Diff line change
@@ -1,2 +1,25 @@
test:
python3 tools/symbols_test.py
$(PYTHON) tools/symbols_test.py

function-finder:
# TODO: make sure graphviz is installed
$(MAKE) force_symbols
$(MAKE) force_extract
$(PYTHON) tools/analyze_calls.py
git clean -fdx asm/
git checkout config/
rm -f build/us/main.ld
rm -rf build/us/weapon.ld
$(MAKE) -j extract
$(PYTHON) tools/function_finder/function_finder_psx.py --use-call-trees > gh-duplicates/functions.md
rm -rf gh-duplicates/function_calls || true
mv function_calls gh-duplicates/
mv function_graphs.md gh-duplicates/

duplicates-report:
$(MAKE) force_symbols
$(MAKE) force_extract
cd tools/dups; \
cargo run --release -- \
--threshold .90 \
--output-file ../../gh-duplicates/duplicates.txt

0 comments on commit 85cbb97

Please sign in to comment.