Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix MAD funcs not appearing in function report #2259

Merged
merged 2 commits into from
Feb 26, 2025

Conversation

JoshSchreuder
Copy link
Contributor

@JoshSchreuder JoshSchreuder commented Feb 25, 2025

EDIT: Approach updated, below is no longer applicable. See comments for more details.

This restores two MAD functions to the function report which went missing

Output from local:
https://gist.github.com/JoshSchreuder/c146abedc057b18d588b44446b00b596

| mad    | func_8018D8C8                     |      250 |         45 |        |                                                                                                    |       |     |
| mad    | func_80197B94                     |      491 |         44 | Yes    |                                                                                                    |       |     |

This was because since #2248 added MAD to the force_symbols extract, it was mapping 5 labels in these functions as symbols which was causing invalid splat disasm after extracting with these symbols

.L80197ED4 = 0x80197ED4; // allow_duplicated:True
.L80197F8C = 0x80197F8C; // allow_duplicated:True
.L8019803C = 0x8019803C; // allow_duplicated:True
.L801980DC = 0x801980DC; // allow_duplicated:True
.L80198198 = 0x80198198; // allow_duplicated:True

I have added a simple exclude for .L. This does not appear to have any impact on any other overlays or functions. I am not clear why these .L symbols were being added to the MAD elf in the first place, so if anyone has a better solution please let me know.

@ProjectOblivion
Copy link
Contributor

ProjectOblivion commented Feb 25, 2025

Wouldn't it be better to revert just the single change that is causing the issue, rather than a quick workaround that we're unsure whether there are consequences?

@JoshSchreuder
Copy link
Contributor Author

Yeah that is another option, it would mean losing MAD from the duplicates report though which was just recently fixed, so ideally we could fix both but I'm fine with whatever @sozud and @hohle are happy witth

Copy link
Owner

@Xeeynamo Xeeynamo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. I am not concerned at all to lose MAD since it is mostly decompiled anyway

@ProjectOblivion
Copy link
Contributor

This fixes the problem symbols from being added to the map in the first place and therefore won't be added with make force_symbols.

--- a/config/splat.us.stmad.yaml
+++ b/config/splat.us.stmad.yaml
@@ -20 +20 @@ options:
-  migrate_rodata_to_functions: no
+  migrate_rodata_to_functions: yes
@@ -79 +79 @@ segments:
-      - [0xD794, rodata]
+      - [0xD794, .rodata, D8C8]
@@ -84 +84 @@ segments:
-      - [0xD8AC, rodata]
+      - [0xD8AC, .rodata, 17B94]

@hohle
Copy link
Contributor

hohle commented Feb 25, 2025

I have a local change that I haven't pushed yet to ignore anything but `.text` sections. This filters out these vars without string matching on labels:
// parse .s file to get instructions and function name
 // parse .s file to get instructions and function name
 fn parse_instructions(input: &str, dir: &str, file: &str) -> Function {
     let mut instructions = Vec::new();
     let mut func_name = "";
+    let mut section = ".text";

     for line in input.lines() {
         let parts: Vec<&str> = line.split_whitespace().collect();

         // find the function name
         if parts.len() == 2 {
-            if parts[0] == "glabel" {
-                func_name = parts[1];
+            match parts[0] {
+                "glabel" => func_name = parts[1],
+                ".section" => section = parts[1],
+                _ => (),
             }
         }

+        if section != ".text" {
+            // ignore non-code sections
+            continue;
+        }
+

It was early, I was looking at the wrong thing. Ignore.

@sozud
Copy link
Collaborator

sozud commented Feb 25, 2025

Based on the discord convo it sounds like importing the rodata is the cleanest fix?

@JoshSchreuder
Copy link
Contributor Author

@sozud agree, have updated PR with the above approach from @ProjectOblivion

Works for me locally, here's a diff on function report with those changes
https://gist.github.com/JoshSchreuder/20597fec1f965802e3dfc3715d859ef4

@JoshSchreuder JoshSchreuder changed the title Exclude labels from force_symbols extract to fix MAD Fix MAD funcs not appearing in function report Feb 26, 2025
@sozud sozud merged commit 56b2469 into Xeeynamo:master Feb 26, 2025
9 checks passed
@JoshSchreuder JoshSchreuder deleted the fix/mad_func branch February 26, 2025 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants