filename attribute for symbols (#358)

* Implement sym `filename` attribute

* cleanups

* black

* docs

* typo

CHANGELOG.md

@@ -1,7 +1,13 @@
 # splat Release Notes
 
 ### 0.22.4
-
+* splat now checks if symbol names can be valid filepaths and produce an error if not.
+  * This is checked because functions are written to their own files and the symbol name is used as the filepath.
+  * There are two checks in place:
+    * The resulting filename should not exceed 255 bytes, since most OSes impose that restriction.
+    * It should not contain any of the following characters: `"<", ">", ":", '"', "/", "\\", "|", "?", "*"`
+  * It is possible to specify a different filename and retain the symbol name by using the `filename` attribute for each symbol on the `symbol_addrs` file.
+    * Make sure that the new specified `filename` does fit the listed requirements.
 * Change `sbss` to properly work as a noload section.
   * To make it not behave as noload then turn off `ld_bss_is_noload`.
 * `ld_bss_is_noload` is now `False` by default for `psx` projects.

docs/Adding-Symbols.md

@@ -149,3 +149,17 @@ obj_fallCA1_tex_rgb_ia8 = 0x06013118; // allow_duplicated:True
 // ...
 obj_fallCA1_tex_rgb_ia8 = 0x060140A8; // allow_duplicated:True
 ```
+
+### `filename`
+
+Allows specifying a different filename than the default (the symbol's name) when writing the symbol to its own file.
+
+Useful when the symbol name has invalid characters for a filename or it exceeds the OS filename limit.
+
+**Example**
+
+```ini
+__opPCc__Q23std34_RefCountedPtr<c,Q23std9_Array<c>>CFv = 0x00202850; // filename:func_00202850
+```
+
+Gets written to `func_00202850.s`

src/splat/segtypes/common/c.py

@@ -134,7 +134,7 @@ def scan(self, rom_bytes: bytes):
         ):
             path = self.out_path()
             if path:
-                if options.opts.do_c_func_detection and os.path.exists(path):
+                if options.opts.do_c_func_detection and path.exists():
                     # TODO run cpp?
                     self.defined_funcs = self.get_funcs_defined_in_c(path)
                     self.global_asm_funcs = self.get_global_asm_funcs(path)
@@ -278,7 +278,7 @@ def create_c_asm_file(
         out_dir: Path,
         func_sym: Symbol,
     ):
-        outpath = out_dir / self.name / (func_sym.name + ".s")
+        outpath = out_dir / self.name / f"{func_sym.filename}.s"
 
         # Skip extraction if the file exists and the symbol is marked as extract=false
         if outpath.exists() and not func_sym.extract:
@@ -308,15 +308,15 @@ def create_c_asm_file(
                     func_rodata_entry.function, func_rodata_entry.lateRodataSyms
                 )
 
-        self.log(f"Disassembled {func_sym.name} to {outpath}")
+        self.log(f"Disassembled {func_sym.filename} to {outpath}")
 
     def create_unmigrated_rodata_file(
         self,
         spim_rodata_sym: spimdisasm.mips.symbols.SymbolBase,
         out_dir: Path,
         rodata_sym: Symbol,
     ):
-        outpath = out_dir / self.name / (rodata_sym.name + ".s")
+        outpath = out_dir / self.name / f"{rodata_sym.filename}.s"
 
         # Skip extraction if the file exists and the symbol is marked as extract=false
         if outpath.exists() and not rodata_sym.extract:
@@ -334,55 +334,52 @@ def create_unmigrated_rodata_file(
             f.write(f".section {rodata_sym.linker_section}\n\n")
             f.write(spim_rodata_sym.disassemble())
 
-        self.log(f"Disassembled {rodata_sym.name} to {outpath}")
+        self.log(f"Disassembled {rodata_sym.filename} to {outpath}")
 
     def get_c_line_include_macro(
         self,
-        spim_sym: spimdisasm.mips.symbols.SymbolBase,
+        sym: Symbol,
         asm_out_dir: Path,
         macro_name: str,
     ) -> str:
         if options.opts.compiler == IDO:
             # IDO uses the asm processor to embeed assembly, and it doesn't require a special directive to include symbols
-            asm_outpath = Path(
-                os.path.join(asm_out_dir, self.name, spim_sym.getName() + ".s")
-            )
+            asm_outpath = asm_out_dir / self.name / f"{sym.filename}.s"
             rel_asm_outpath = os.path.relpath(asm_outpath, options.opts.base_path)
             return f'#pragma GLOBAL_ASM("{rel_asm_outpath}")'
 
         if options.opts.use_legacy_include_asm:
             rel_asm_out_dir = asm_out_dir.relative_to(options.opts.nonmatchings_path)
-            return f'{macro_name}(const s32, "{rel_asm_out_dir / self.name}", {spim_sym.getName()});'
+            return f'{macro_name}(const s32, "{rel_asm_out_dir / self.name}", {sym.filename});'
 
-        return f'{macro_name}("{asm_out_dir / self.name}", {spim_sym.getName()});'
+        return f'{macro_name}("{asm_out_dir / self.name}", {sym.filename});'
 
     def get_c_lines_for_function(
-        self, func: spimdisasm.mips.symbols.SymbolFunction, asm_out_dir: Path
+        self,
+        sym: Symbol,
+        spim_sym: spimdisasm.mips.symbols.SymbolFunction,
+        asm_out_dir: Path,
     ) -> List[str]:
         c_lines = []
 
         # Terrible hack to "auto-decompile" empty functions
         if (
             options.opts.auto_decompile_empty_functions
-            and len(func.instructions) == 2
-            and func.instructions[0].isReturn()
-            and func.instructions[1].isNop()
+            and len(spim_sym.instructions) == 2
+            and spim_sym.instructions[0].isReturn()
+            and spim_sym.instructions[1].isNop()
         ):
-            c_lines.append("void " + func.getName() + "(void) {")
+            c_lines.append(f"void {spim_sym.getName()}(void) {{")
             c_lines.append("}")
         else:
             c_lines.append(
-                self.get_c_line_include_macro(func, asm_out_dir, "INCLUDE_ASM")
+                self.get_c_line_include_macro(sym, asm_out_dir, "INCLUDE_ASM")
             )
         c_lines.append("")
         return c_lines
 
-    def get_c_lines_for_rodata_sym(
-        self, rodata_sym: spimdisasm.mips.symbols.SymbolBase, asm_out_dir: Path
-    ):
-        c_lines = [
-            self.get_c_line_include_macro(rodata_sym, asm_out_dir, "INCLUDE_RODATA")
-        ]
+    def get_c_lines_for_rodata_sym(self, sym: Symbol, asm_out_dir: Path):
+        c_lines = [self.get_c_line_include_macro(sym, asm_out_dir, "INCLUDE_RODATA")]
         c_lines.append("")
         return c_lines
 
@@ -396,9 +393,24 @@ def create_c_file(
 
         for entry in symbols_entries:
             if entry.function is not None:
-                c_lines += self.get_c_lines_for_function(entry.function, asm_out_dir)
+                func_sym = self.get_symbol(
+                    entry.function.vram,
+                    in_segment=True,
+                    type="func",
+                    local_only=True,
+                )
+                assert func_sym is not None
+
+                c_lines += self.get_c_lines_for_function(
+                    func_sym, entry.function, asm_out_dir
+                )
             else:
-                for rodata_sym in entry.rodataSyms:
+                for spim_rodata_sym in entry.rodataSyms:
+                    rodata_sym = self.get_symbol(
+                        spim_rodata_sym.vram, in_segment=True, local_only=True
+                    )
+                    assert rodata_sym is not None
+
                     c_lines += self.get_c_lines_for_rodata_sym(rodata_sym, asm_out_dir)
 
         c_path.parent.mkdir(parents=True, exist_ok=True)

src/splat/util/symbols.py

@@ -28,6 +28,8 @@
 
 splat_sym_types = {"func", "jtbl", "jtbl_label", "label"}
 
+ILLEGAL_FILENAME_CHARS = ["<", ">", ":", '"', "/", "\\", "|", "?", "*"]
+
 
 def check_valid_type(typename: str) -> bool:
     if typename[0].isupper():
@@ -181,6 +183,9 @@ def get_seg_for_rom(rom: int) -> Optional["Segment"]:
                             if attr_name == "name_end":
                                 sym.given_name_end = attr_val
                                 continue
+                            if attr_name == "filename":
+                                sym.given_filename = attr_val
+                                continue
                         except:
                             log.parsing_error_preamble(path, line_num, line)
                             log.write(
@@ -267,6 +272,23 @@ def get_seg_for_rom(rom: int) -> Optional["Segment"]:
                                 f"Duplicate symbol detected! {sym.name} clashes with {item.name} defined at vram 0x{addr:08X}.\n  If this is intended, specify either a segment or a rom address for this symbol"
                             )
 
+            if len(sym.filename) > 253 or any(
+                c in ILLEGAL_FILENAME_CHARS for c in sym.filename
+            ):
+                log.parsing_error_preamble(path, line_num, line)
+                log.error(
+                    # sym.name is written on its own line so reading the error message is nicer because the sym name will be very long.
+                    # Other lines have two spaces to make identation nicer and consistent
+                    f"Ilegal symbol filename detected!\n"
+                    f"  The symbol\n"
+                    f"    {sym.name}\n"
+                    f"  exceeds the 255 bytes filename limit that most OS imposes or uses illegal characters,\n"
+                    f"  which will be a problem when writing the symbol to its own file.\n"
+                    f"  To fix this specify a `filename` for this symbol, like `filename:func_{sym.vram_start:08X}`.\n"
+                    f"  Make sure the filename does not exceed 253 bytes nor it contains any of the following characters:\n"
+                    f"    {ILLEGAL_FILENAME_CHARS}"
+                )
+
             seen_symbols[sym.name] = sym
 
             add_symbol(sym)
@@ -576,6 +598,8 @@ class Symbol:
 
     allow_duplicated: bool = False
 
+    given_filename: Optional[str] = None
+
     _generated_default_name: Optional[str] = None
     _last_type: Optional[str] = None
 
@@ -660,6 +684,12 @@ def size(self) -> int:
             return self.given_size
         return 4
 
+    @property
+    def filename(self) -> str:
+        if self.given_filename is not None:
+            return self.given_filename
+        return self.name
+
     def contains_vram(self, offset):
         return offset >= self.vram_start and offset < self.vram_end