Change tooling for better elf correctness (#1632)

* Add addresses for makerom and boot segments

* Mark entrypoint as the entry of the elf

* Add extra labels to macro.inc

* Use dlabel, jlabel and endlabel in disassembly

* Make jlabel global

* git subrepo pull tools/asm-processor

subrepo:
  subdir:   "tools/asm-processor"
  merged:   "92b9ec72f"
upstream:
  origin:   "git@github.com:simonlindholm/asm-processor.git"
  branch:   "main"
  commit:   "92b9ec72f"
git-subrepo:
  version:  "0.4.3"
  origin:   "https://github.com/ingydotnet/git-subrepo.git"
  commit:   "2f68596"

* Use `dlabel`s for non-functions in .text

* Add linker asserts

* Move framebuffer_lo to before the makerom segment to avoid `dot moved backwards` warnings

* fix

* Improve assert message

* fix

* Third fix

* Fix four

* Move undefined_syms to linker_script folder

* Make second linker script file to avoid abusing undefined_syms

* Update Makefile

Co-authored-by: Derek Hensley <hensley.derek58@gmail.com>

* Rename to extra.ld

* Use entrypoint symbol on ROM header

---------

Co-authored-by: Derek Hensley <hensley.derek58@gmail.com>

Makefile

@@ -120,7 +120,7 @@ OBJCOPY := $(MIPS_BINUTILS_PREFIX)objcopy
 OBJDUMP := $(MIPS_BINUTILS_PREFIX)objdump
 
 ASM_PROC   := $(PYTHON) tools/asm-processor/build.py
-ASM_PROC_FLAGS := --input-enc=utf-8 --output-enc=euc-jp --convert-statics=global-with-filename
+ASM_PROC_FLAGS := --input-enc=utf-8 --output-enc=euc-jp --convert-statics=global-with-filename --encode-cutscene-data-floats
 
 ifneq ($(ASM_PROC_FORCE), 0)
   ASM_PROC_FLAGS += --force
@@ -241,7 +241,9 @@ SCHEDULE_INC_FILES := $(foreach f,$(SCHEDULE_FILES:.schl=.schl.inc),$(BUILD_DIR)
 DEP_FILES := $(O_FILES:.o=.asmproc.d) $(OVL_RELOC_FILES:.o=.d)
 
 # create build directories
-$(shell mkdir -p $(BUILD_DIR)/baserom $(foreach dir,$(SRC_DIRS) $(ASM_DIRS) $(ASSET_BIN_DIRS) $(ASSET_BIN_DIRS_C_FILES),$(BUILD_DIR)/$(dir)))
+OTHER_DIRS := baserom linker_scripts
+
+$(shell mkdir -p $(foreach dir,$(SRC_DIRS) $(ASM_DIRS) $(ASSET_BIN_DIRS) $(ASSET_BIN_DIRS_C_FILES) $(OTHER_DIRS),$(BUILD_DIR)/$(dir)))
 
 # directory flags
 $(BUILD_DIR)/src/libultra/os/%.o: OPTFLAGS := -O1
@@ -319,8 +321,8 @@ $(ROMC): $(ROM) $(ELF) $(BUILD_DIR)/compress_ranges.txt
 	$(PYTHON) tools/buildtools/compress.py --in $(ROM) --out $@ --dma-start `tools/buildtools/dmadata_start.sh $(NM) $(ELF)` --compress `cat $(BUILD_DIR)/compress_ranges.txt` --threads $(N_THREADS)
 	$(PYTHON) -m ipl3checksum sum --cic 6105 --update $@
 
-$(ELF): $(TEXTURE_FILES_OUT) $(ASSET_FILES_OUT) $(O_FILES) $(OVL_RELOC_FILES) $(LDSCRIPT) $(BUILD_DIR)/undefined_syms.txt
+$(ELF): $(TEXTURE_FILES_OUT) $(ASSET_FILES_OUT) $(O_FILES) $(OVL_RELOC_FILES) $(LDSCRIPT) $(BUILD_DIR)/linker_scripts/undefined_syms.ld $(BUILD_DIR)/linker_scripts/extra.ld
-	$(LD) -T $(LDSCRIPT) -T $(BUILD_DIR)/undefined_syms.txt --no-check-sections --accept-unknown-input-arch --emit-relocs -Map $(MAP) -o $@
+	$(LD) -T $(LDSCRIPT) -T $(BUILD_DIR)/linker_scripts/undefined_syms.ld -T $(BUILD_DIR)/linker_scripts/extra.ld --no-check-sections --accept-unknown-input-arch --emit-relocs -Map $(MAP) -o $@
 
 ## Order-only prerequisites 
 # These ensure e.g. the O_FILES are built before the OVL_RELOC_FILES.
@@ -400,8 +402,8 @@ all: rom compress
 
 #### Various Recipes ####
 
-$(BUILD_DIR)/undefined_syms.txt: undefined_syms.txt
+$(BUILD_DIR)/%.ld: %.ld
-	$(CPP) $(CPPFLAGS) $< > $(BUILD_DIR)/undefined_syms.txt
+	$(CPP) $(CPPFLAGS) $< > $@
 
 $(BUILD_DIR)/$(SPEC): $(SPEC)
 	$(CPP) $(CPPFLAGS) $< | $(SPEC_REPLACE_VARS) > $@

include/macro.inc

@@ -1,4 +1,21 @@
 .macro glabel label
+    .global \label
+    .ent \label
+    .type \label, @function
+    \label:
+.endm
+
+.macro endlabel label
+    .end \label
+    .size \label, . - \label
+.endm
+
+.macro dlabel label
+    .global \label
+    \label:
+.endm
+
+.macro jlabel label
     .global \label
     \label:
 .endm

linker_scripts/extra.ld

@@ -0,0 +1,7 @@
+ENTRY(entrypoint);
+
+ASSERT ((_bootSegmentRomEnd <= 0x101000), "Error: _bootSegmentRomEnd is larger than 1 MB");
+
+// TODO: This should be checking for an address a lot earlier than this one.
+ASSERT ((_system_heapSegmentStart < 0x80780000), "Error: The game is overflowing the RAM limit for non-overlay segments.");
+ASSERT ((_framebuffer_hiSegmentStart == 0x80780000), "Error: framebuffer_hi shifted?");

spec

@@ -2,13 +2,6 @@
  * ROM spec file
  */
 
-beginseg
-    name "makerom"
-    include "$(BUILD_DIR)/asm/makerom/rom_header.o"
-    include "$(BUILD_DIR)/asm/makerom/ipl3.o"
-    include "$(BUILD_DIR)/asm/makerom/entry.o"
-endseg
-
 beginseg
     name "framebuffer_lo"
     address 0x80000500
@@ -16,8 +9,17 @@ beginseg
     include "$(BUILD_DIR)/src/buffers/framebuffer_lo.o"
 endseg
 
+beginseg
+    name "makerom"
+    address 0x8007F000
+    include "$(BUILD_DIR)/asm/makerom/rom_header.o"
+    include "$(BUILD_DIR)/asm/makerom/ipl3.o"
+    include "$(BUILD_DIR)/asm/makerom/entry.o"
+endseg
+
 beginseg
     name "boot"
+    address 0x80080060
     include "$(BUILD_DIR)/src/boot/boot_main.o"
     include "$(BUILD_DIR)/data/boot/rspboot.data.o"
     include "$(BUILD_DIR)/src/boot/idle.o"

tools/asm-processor/.gitrepo

@@ -6,7 +6,7 @@
 [subrepo]
 	remote = git@github.com:simonlindholm/asm-processor.git
 	branch = main
-	commit = fed1e3ddb9967bd427f4599b1b8ededa296bd9f9
+	commit = 92b9ec72f40fde329bd91a83046b6c74bdc00199
-	parent = 4fa13e4132cf8adb2f07bb34927e62e525c8cc79
+	parent = 1f6e7c5006bcc603714310b9fb891f4fa546c896
 	method = merge
-	cmdver = 0.4.6
+	cmdver = 0.4.3

tools/asm-processor/asm_processor.py

@@ -306,17 +306,17 @@ class Section:
 
         assert hdrr_magic == 0x7009, "Invalid magic value for .mdebug symbolic header"
 
-        hdrr_cbLineOffset += shift_by
+        if hdrr_cbLine: hdrr_cbLineOffset += shift_by
-        hdrr_cbDnOffset += shift_by
+        if hdrr_idnMax: hdrr_cbDnOffset += shift_by
-        hdrr_cbPdOffset += shift_by
+        if hdrr_ipdMax: hdrr_cbPdOffset += shift_by
-        hdrr_cbSymOffset += shift_by
+        if hdrr_isymMax: hdrr_cbSymOffset += shift_by
-        hdrr_cbOptOffset += shift_by
+        if hdrr_ioptMax: hdrr_cbOptOffset += shift_by
-        hdrr_cbAuxOffset += shift_by
+        if hdrr_iauxMax: hdrr_cbAuxOffset += shift_by
-        hdrr_cbSsOffset += shift_by
+        if hdrr_issMax: hdrr_cbSsOffset += shift_by
-        hdrr_cbSsExtOffset += shift_by
+        if hdrr_issExtMax: hdrr_cbSsExtOffset += shift_by
-        hdrr_cbFdOffset += shift_by
+        if hdrr_ifdMax: hdrr_cbFdOffset += shift_by
-        hdrr_cbRfdOffset += shift_by
+        if hdrr_crfd: hdrr_cbRfdOffset += shift_by
-        hdrr_cbExtOffset += shift_by
+        if hdrr_iextMax: hdrr_cbExtOffset += shift_by
 
         new_data[0:0x60] = self.fmt.pack("HHIIIIIIIIIIIIIIIIIIIIIII", hdrr_magic, hdrr_vstamp, hdrr_ilineMax, hdrr_cbLine, \
             hdrr_cbLineOffset, hdrr_idnMax, hdrr_cbDnOffset, hdrr_ipdMax, \
@@ -883,7 +883,7 @@ float_regexpr = re.compile(r"[-+]?[0-9]*\.?[0-9]+([eE][-+]?[0-9]+)?f")
 def repl_float_hex(m):
     return str(struct.unpack(">I", struct.pack(">f", float(m.group(0).strip().rstrip("f"))))[0])
 
-Opts = namedtuple('Opts', ['opt', 'framepointer', 'mips1', 'kpic', 'pascal', 'input_enc', 'output_enc'])
+Opts = namedtuple('Opts', ['opt', 'framepointer', 'mips1', 'kpic', 'pascal', 'input_enc', 'output_enc', 'enable_cutscene_data_float_encoding'])
 
 def parse_source(f, opts, out_dependencies, print_source=None):
     if opts.opt in ['O1', 'O2']:
@@ -996,6 +996,7 @@ def parse_source(f, opts, out_dependencies, print_source=None):
             output_lines[-1] = include_src.getvalue()
             include_src.close()
         else:
+            if opts.enable_cutscene_data_float_encoding:
                 # This is a hack to replace all floating-point numbers in an array of a particular type
                 # (in this case CutsceneData) with their corresponding IEEE-754 hexadecimal representation
                 if cutscene_data_regexpr.search(line) is not None:
@@ -1365,7 +1366,9 @@ def fixup_objfile(objfile_name, functions, asm_prelude, assembler, output_enc, d
                 if not existing:
                     name_to_sym[s.name] = s
                     newer_syms.append(s)
-                elif s.st_shndx != SHN_UNDEF:
+                elif s.st_shndx != SHN_UNDEF and not (
+                    existing.st_shndx == s.st_shndx and existing.st_value == s.st_value
+                ):
                     raise Failure("symbol \"" + s.name + "\" defined twice")
                 else:
                     s.replace_by = existing
@@ -1455,6 +1458,7 @@ def run_wrapped(argv, outfile, functions):
     parser.add_argument('--drop-mdebug-gptab', dest='drop_mdebug_gptab', action='store_true', help="drop mdebug and gptab sections")
     parser.add_argument('--convert-statics', dest='convert_statics', choices=["no", "local", "global", "global-with-filename"], default="local", help="change static symbol visibility (default: %(default)s)")
     parser.add_argument('--force', dest='force', action='store_true', help="force processing of files without GLOBAL_ASM blocks")
+    parser.add_argument('--encode-cutscene-data-floats', dest='enable_cutscene_data_float_encoding', action='store_true', default=False, help="Replace floats with their encoded hexadecimal representation in CutsceneData data")
     parser.add_argument('-framepointer', dest='framepointer', action='store_true')
     parser.add_argument('-mips1', dest='mips1', action='store_true')
     parser.add_argument('-g3', dest='g3', action='store_true')
@@ -1475,7 +1479,7 @@ def run_wrapped(argv, outfile, functions):
         raise Failure("-mips1 is only supported together with -O1 or -O2")
     if pascal and opt not in ('O1', 'O2', 'g3'):
         raise Failure("Pascal is only supported together with -O1, -O2 or -O2 -g3")
-    opts = Opts(opt, args.framepointer, args.mips1, args.kpic, pascal, args.input_enc, args.output_enc)
+    opts = Opts(opt, args.framepointer, args.mips1, args.kpic, pascal, args.input_enc, args.output_enc, args.enable_cutscene_data_float_encoding)
 
     if args.objfile is None:
         with open(args.filename, encoding=args.input_enc) as f:

tools/asm-processor/prelude.inc

@@ -16,6 +16,8 @@
     \label:
 .endm
 
+.macro endlabel label
+.endm
 
 # Float register aliases (o32 ABI, odd ones are rarely used)
 

tools/disasm/disasm.py

@@ -79,7 +79,6 @@ def discard_decomped_files(files_spec, include_files):
         seg_start = i
         new_files = {}
         included = False
-        saved_off = 0
         for offset, file in file_list.items():
             if file == "[PADDING]":
                 continue
@@ -1138,9 +1137,13 @@ def getImmOverride(insn: rabbitizer.Instruction):
 def getLabelForVaddr(vaddr: int, in_data: bool = False) -> str:
     label = ""
     if vaddr in functions:
-        label += f"\nglabel {proper_name(vaddr, in_data=in_data)}\n"
+        name = proper_name(vaddr, in_data=in_data)
+        if in_data:
+            label += f"\ndlabel {name}\n"
+        else:
+            label += f"\nglabel {name}\n"
     if vaddr in jtbl_labels:
-        label += f"glabel L{vaddr:08X}\n"
+        label += f"jlabel L{vaddr:08X}\n"
     if vaddr in branch_labels:
         label += f".L{vaddr:08X}:\n"
     return label
@@ -1164,12 +1167,25 @@ def fixup_text_symbols(data, vram, data_regions, info):
 
     delay_slot = False
     disasm_as_data = False
+    prev_func = ""
     for entry in file:
         insn = entry["insn"]
         in_data = entry["data"]
         comment = entry["comment"]
 
-        text.append(getLabelForVaddr(insn.vram, in_data))
+        cur_label = getLabelForVaddr(insn.vram, in_data)
+
+        # Handle adding endlabels to the previous function
+        if cur_label and ("glabel" in cur_label or "dlabel" in cur_label):
+            if prev_func:
+                text.append(f"endlabel {prev_func}\n")
+            if "glabel" in cur_label:
+                prev_func = cur_label.replace("glabel", "").strip().split('\n')[0]
+            else:
+                prev_func = ""
+
+        text.append(cur_label)
+
         if insn.vram in functions:
             # new function, needs to check this again
             disasm_as_data = False
@@ -1191,6 +1207,10 @@ def fixup_text_symbols(data, vram, data_regions, info):
 
         delay_slot = insn.hasDelaySlot()
 
+    # Add endlabel to last function
+    if prev_func:
+        text.append(f"endlabel {prev_func}\n")
+
     with open(f"{ASM_OUT}/{segment_dirname}/{info['name']}.text.s", "w") as outfile:
         outfile.write("".join(text))
 
@@ -1206,6 +1226,7 @@ def disassemble_text(data, vram, data_regions, info):
     os.makedirs(f"{ASM_OUT}/{segment_dirname}/", exist_ok=True)
 
     delay_slot = False
+    prev_func = ""
 
     for i, raw_insn in enumerate(raw_insns, 0):
         i *= 4
@@ -1241,7 +1262,19 @@ def disassemble_text(data, vram, data_regions, info):
             )
             continue
 
-        result += getLabelForVaddr(vaddr)
+        cur_label = getLabelForVaddr(vaddr)
+
+        # Handle adding endlabels to the previous function
+        if cur_label and ("glabel" in cur_label or "dlabel" in cur_label):
+            if prev_func:
+                result += f"endlabel {prev_func}\n"
+            if "glabel" in cur_label:
+                prev_func = cur_label.replace("glabel", "").strip().split('\n')[0]
+            else:
+                prev_func = ""
+
+        result += cur_label
+
 
         comment = f"/* {i:06X} {vaddr:08X} {raw_insn:08X} */"
         extraLJust = 0
@@ -1256,6 +1289,10 @@ def disassemble_text(data, vram, data_regions, info):
 
         delay_slot = insn.hasDelaySlot()
 
+    # Add endlabel to last function
+    if prev_func:
+        result += f"endlabel {prev_func}\n"
+
     with open(f"{ASM_OUT}/{segment_dirname}/{cur_file}.text.s", "w") as outfile:
         outfile.write(result)
 
@@ -1340,7 +1377,7 @@ def disassemble_data(data, vram, end, info):
             if data_offset == len(data):
                 continue
 
-            r = f"\nglabel {proper_name(symbol, True)}\n"
+            r = f"\ndlabel {proper_name(symbol, True)}\n"
 
             if symbol % 8 == 0 and data_size % 8 == 0 and symbol in doubles:
                 r += (
@@ -1525,7 +1562,7 @@ def disassemble_rodata(data, vram, end, info):
 
             force_ascii_str = symbol in [0x801D0708]
 
-            r = f"\nglabel {proper_name(symbol, True)}\n"
+            r = f"\ndlabel {proper_name(symbol, True)}\n"
 
             if symbol in strings:
                 string_data = data[data_offset : data_offset + data_size]
@@ -1695,7 +1732,7 @@ def disassemble_bss(vram, end, info):
             else:
                 next_symbol = end
 
-            result.append(f"\nglabel {proper_name(symbol, True)}\n")
+            result.append(f"\ndlabel {proper_name(symbol, True)}\n")
             result.append(
                 f"/* {symbol - vram:06X} {symbol:08X} */ .space 0x{next_symbol - symbol:X}\n"
             )
@@ -1820,7 +1857,7 @@ def disassemble_makerom(section):
 /* 0x00 */ ENDIAN_IDENTIFIER
 /* 0x01 */ PI_DOMAIN_1_CFG({pi_dom1_lat}, {pi_dom1_pwd}, {pi_dom1_reg & 0xF}, {(pi_dom1_reg >> 4) & 3})
 /* 0x04 */ SYSTEM_CLOCK_RATE_SETTING(0x{clockrate:X})
-/* 0x08 */ ENTRYPOINT(0x{entrypoint:08X})
+/* 0x08 */ ENTRYPOINT(entrypoint)
 /* 0x0C */ LIBULTRA_VERSION({hw_ver // 10}, {hw_ver % 10}, {chr(os_ver)})
 /* 0x10 */ CHECKSUM()
 /* 0x18 */ PADDING(8)
@@ -1864,7 +1901,7 @@ def disassemble_makerom(section):
         with open(f"{ASM_OUT}/makerom/entry.text.s") as infile:
             entry_asm = infile.read()
 
-        entry_asm = entry_asm.replace("0x63b0", "%lo(_bootSegmentBssSize)")
+        entry_asm = entry_asm.replace("0x63B0", "%lo(_bootSegmentBssSize)")
         with open(f"{ASM_OUT}/makerom/entry.s", "w") as outfile:
             outfile.write(entry_asm)
 
@@ -1891,7 +1928,7 @@ def disassemble_dmadata(section):
     .word 0xFFFFFFFF
 .endm
 
-glabel {variables_ast[0x8009F8B0][0]}
+dlabel {variables_ast[0x8009F8B0][0]}
 """
     filenames = []
     with open("tools/disasm/dma_filenames.txt", "r") as infile:
@@ -1944,7 +1981,7 @@ def disassemble_segment(section):
         segment_dirname = section[-1]["name"]
 
         result = asm_header(".rodata")
-        result += f"\nglabel {section[-1]['name']}_Reloc\n"
+        result += f"\ndlabel {section[-1]['name']}_Reloc\n"
 
         lines = [words[i * 8 : (i + 1) * 8] for i in range(0, (len(words) // 8) + 1)]
         for line in [line for line in lines if len(line) != 0]:
@@ -2016,7 +2053,7 @@ def rodata_syms(rodata):
 
 
 def rodata_blocks(rodata):
-    return ["glabel" + b for b in rodata.split("glabel")[1:]]
+    return ["dlabel" + b for b in rodata.split("dlabel")[1:]]
 
 
 def find_late_rodata_start(rodata):
@@ -2306,7 +2343,7 @@ with multiprocessing.get_context("fork").Pool(jobs) as p:
 print("Splitting text and migrating rodata")
 
 func_regex = re.compile(r"\n\nglabel \S+\n")
-rodata_symbols_regex = re.compile(r"(?<=\n)glabel (.+)(?=\n)")
+rodata_symbols_regex = re.compile(r"(?<=\n)dlabel (.+)(?=\n)")
 asm_symbols_regex = re.compile(r"%(?:lo|hi)\((.+?)\)")
 
 # Split files and migrate rodata that should be migrated