Added compatibility for decomp-toolkit macros #275

dbalatoni13 · 2024-05-28T17:07:53Z

encounter/decomp-toolkit outputs assembly files that are currently not fully compatible with m2c.
One problem is that labels have the format

.obj name, local
    .float 0
.endobj name

This makes m2c not detect any data, bss, rodata or jump table labels.
Another problem is that jump tables have the prefix "jumptable_".
Ultimately, strings use the type ".string".

simonlindholm · 2024-05-28T17:12:37Z

m2c/asm_file.py

+                        ".asciz",
+                        ".ascii",
+                        ".asciiz",
+                        "string",


This won't work -- this section is guarded by directive.startswith(".")

My bad, it should actually be ".string"

simonlindholm · 2024-05-28T17:13:08Z

m2c/asm_file.py

@@ -339,7 +339,9 @@ def re_comment_replacer(match: Match[str]) -> str:
    re_whitespace_or_string = re.compile(r'\s+|"(?:\\.|[^\\"])*"')
    re_local_glabel = re.compile("L(_.*_)?[0-9A-F]{8}")
    re_local_label = re.compile("loc_|locret_|def_|lbl_|LAB_|jump_")
-    re_label = re.compile(r'(?:([a-zA-Z0-9_.$]+)|"([a-zA-Z0-9_.$<>@,-]+)"):')
+    re_label = re.compile(
+        r'(?:(?:\.obj )?([a-zA-Z0-9_.$]+)|"([a-zA-Z0-9_.$<>@,-]+)")[:,]'


Instead of complicating this regex, can you add a case near where jlabel/glabel/etc are handled?

I will give it a try.

simonlindholm · 2024-05-28T17:16:30Z

Thanks, compatibility with more formats is always welcome. (Sorry for not responding in time to #272, I've been a bit strapped for time.)

dbalatoni13 · 2024-05-28T17:20:56Z

No problem!

dbalatoni13 · 2024-05-28T18:33:07Z

Does it look alright? I also made sure to set z to True for .string.

dbalatoni13 · 2024-05-28T18:37:45Z

Sorry, my previous edit deleted the jump table change, I'm not really experienced with git yet. I added it back now. Why does black change the formatting for something that was already in the repo?

simonlindholm · 2024-05-28T19:19:04Z

m2c/asm_file.py

@@ -548,6 +554,14 @@ def process_label(label: str, *, kind: LabelKind) -> None:
                        data = parse_incbin(args, options, warnings)
                        if data is not None:
                            asm_file.new_data_bytes(data)
+                    elif directive == ".obj":  # decomp-toolkit label format
+                        parts = line.split()


can you make use of args instead, which is the arguments split by comma? we don't want to make unnecessary assumptions about whitespace here

simonlindholm · 2024-05-28T19:20:32Z

m2c/asm_file.py

+                        parts = line.split()
+                        if len(parts) >= 3:
+                            try:
+                                kind = LabelKind[parts[2].upper()]


LabelKind[...] exposes internal details in an ugly way and isn't searchable. Can you use some manual if-else switch instead?

Is it fine if I set it to global by default and change it to local if the string equals local? Or rather not do anything if the value is not one of those?
I did some testing and my variables get marked as static not depending on the LabelKind at all, is that normal?

Sure, that's fine. A global label here means one that starts a function; for data it doesn't make a difference whether the label is global or not. And yeah I think m2c will happily mark things as static regardless of this, it's basing that decision on the context file instead.

simonlindholm · 2024-05-28T19:21:14Z

m2c/flow_graph.py

@@ -862,7 +860,7 @@ def find_block_by_label(label: str) -> Optional[Block]:
                            and isinstance(arg.argument, AsmGlobalSymbol)
                            and any(
                                arg.argument.symbol_name.startswith(prefix)
-                                for prefix in ("jtbl", "jpt_", "lbl_")
+                                for prefix in ("jtbl", "jpt_", "lbl_", "_jumptable")


just double checking: _jumptable vs jumptable_?

You are right.

simonlindholm · 2024-05-28T20:10:14Z

m2c/asm_file.py

@@ -548,6 +554,12 @@ def process_label(label: str, *, kind: LabelKind) -> None:
                        data = parse_incbin(args, options, warnings)
                        if data is not None:
                            asm_file.new_data_bytes(data)
+                    elif directive == ".obj":  # decomp-toolkit label format
+                        if len(args) == 2:


assert len(args) == 2 instead to have some minimum of error handling. Or raise DecompFailure("...") in an else branch but that feels like effort.

Okay, I will use assert

simonlindholm · 2024-05-28T20:10:38Z

m2c/asm_file.py

+                            kind = LabelKind.GLOBAL
+                            if args[1] == "local":
+                                kind = LabelKind.LOCAL


Suggested change

kind = LabelKind.GLOBAL

if args[1] == "local":

kind = LabelKind.LOCAL

kind = LabelKind.LOCAL if args[1] == "local" else LabelKind.GLOBAL

if you want

sure, black adds brackets btw

simonlindholm · 2024-05-28T20:11:15Z

m2c/asm_file.py

@@ -548,6 +554,12 @@ def process_label(label: str, *, kind: LabelKind) -> None:
                        data = parse_incbin(args, options, warnings)
                        if data is not None:
                            asm_file.new_data_bytes(data)
+                    elif directive == ".obj":  # decomp-toolkit label format


btw, maybe move this up next to the handling for .rel which is also related to decomp-toolkit?

dbalatoni13 · 2024-05-28T21:19:25Z

I ran black and it was strange that it changed the formatting in other sections of this file and also multiple other files. I didn't commit those other changes, what do you think the issue probably is?

simonlindholm · 2024-05-28T21:20:01Z

Test failure due to too old black version? Because I don't get reformatting in flow_graph.py locally.

simonlindholm · 2024-05-28T21:24:00Z

23.12.1 is the version used in CI btw (mentioned in pyproject.toml), and 24.4.2 seems to be behave the same for me. You can probably just python3 -m pip install --upgrade black

simonlindholm · 2024-05-28T21:40:30Z

Actually, nvm, 23.12.1 and 24.4.2 do not behave the same, I just had parallel install of black somehow. (depot_tools??? this is a new way of screwing up python installs that I heretofore was not aware of.) I'll push an update to master to make things simple.

simonlindholm · 2024-05-28T21:43:31Z

so just rebase or merge with master and you should be good

…ction cleaner

dbalatoni13 · 2024-05-28T22:00:22Z

Oh, okay. I did a rebase.

simonlindholm reviewed May 28, 2024

View reviewed changes

dbalatoni13 force-pushed the dtk-compatibility branch from 26fcb86 to dbebf75 Compare May 28, 2024 18:17

simonlindholm reviewed May 28, 2024

View reviewed changes

dbalatoni13 force-pushed the dtk-compatibility branch from 86dc22f to 7043db7 Compare May 28, 2024 20:07

simonlindholm approved these changes May 28, 2024

View reviewed changes

dbalatoni13 force-pushed the dtk-compatibility branch from 7043db7 to 492b9f3 Compare May 28, 2024 20:18

dbalatoni13 added 2 commits May 28, 2024 23:58

Added compatibility for decomp-toolkit macros

9781ee7

Added an alternative jump table prefix and made the label format dete…

38db511

…ction cleaner

dbalatoni13 force-pushed the dtk-compatibility branch from 492b9f3 to 38db511 Compare May 28, 2024 21:58

simonlindholm merged commit 81b1748 into matt-kempster:master May 29, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added compatibility for decomp-toolkit macros #275

Added compatibility for decomp-toolkit macros #275

dbalatoni13 commented May 28, 2024 •

edited

Loading

simonlindholm May 28, 2024

dbalatoni13 May 28, 2024

simonlindholm May 28, 2024

dbalatoni13 May 28, 2024

simonlindholm commented May 28, 2024

dbalatoni13 commented May 28, 2024

dbalatoni13 commented May 28, 2024

dbalatoni13 commented May 28, 2024 •

edited

Loading

simonlindholm May 28, 2024

dbalatoni13 May 28, 2024

simonlindholm May 28, 2024

dbalatoni13 May 28, 2024 •

edited

Loading

simonlindholm May 28, 2024

dbalatoni13 May 28, 2024

simonlindholm May 28, 2024

dbalatoni13 May 28, 2024

simonlindholm May 28, 2024

dbalatoni13 May 28, 2024

simonlindholm May 28, 2024

dbalatoni13 May 28, 2024

simonlindholm May 28, 2024

dbalatoni13 commented May 28, 2024

simonlindholm commented May 28, 2024

simonlindholm commented May 28, 2024

simonlindholm commented May 28, 2024

simonlindholm commented May 28, 2024

dbalatoni13 commented May 28, 2024

Added compatibility for decomp-toolkit macros #275

Added compatibility for decomp-toolkit macros #275

Conversation

dbalatoni13 commented May 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonlindholm commented May 28, 2024

dbalatoni13 commented May 28, 2024

dbalatoni13 commented May 28, 2024

dbalatoni13 commented May 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbalatoni13 May 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbalatoni13 commented May 28, 2024

simonlindholm commented May 28, 2024

simonlindholm commented May 28, 2024

simonlindholm commented May 28, 2024

simonlindholm commented May 28, 2024

dbalatoni13 commented May 28, 2024

dbalatoni13 commented May 28, 2024 •

edited

Loading

dbalatoni13 commented May 28, 2024 •

edited

Loading

dbalatoni13 May 28, 2024 •

edited

Loading