Internal rework for bss_size attribute. (#533)

* Internal rework for `bss_size` attribute.

Now internally every segment has a `bss_size` attribute, which is considered in most calculations now.

This fixes some usages where bss segments would report the wrong `vram_end`.

Now `bss_size` is known when constructing the `Segment` instance, replacing the old approach of letting the segment itself calculate the `bss_size` on its own when it needed it.

I also cleaned up some code a bit

* Allow zero sized bss segments

* format

Author: AngheloAlf

CHANGELOG.md

@@ -1,5 +1,12 @@
 # splat Release Notes
 
+### 0.40.1
+
+* Internal rework for `bss_size` attribute.
+  * Now internally every segment has a `bss_size` attribute, which is considered in most calculations now.
+  * This fixes some usages where bss segments would report the wrong `vram_end`.
+* Consider zero-sized bss segments as valid if they are the last subsegment of a code segment.
+
 ### 0.40.0
 
 * Append an `-include` directive to the generated elf dependency file.  

README.md

@@ -21,7 +21,7 @@ The brackets corresponds to the optional dependencies to install while installin
 If you use a `requirements.txt` file in your repository, then you can add this library with the following line:
 
 ```txt
-splat64[mips]>=0.40.0,<1.0.0
+splat64[mips]>=0.40.1,<1.0.0
 ```
 
 ### Optional dependencies

pyproject.toml

@@ -1,7 +1,7 @@
 [project]
 name = "splat64"
 # Should be synced with src/splat/__init__.py
-version = "0.40.0"
+version = "0.40.1"
 description = "A binary splitting tool to assist with decompilation and modding projects"
 readme = "README.md"
 license = {file = "LICENSE"}

src/splat/__init__.py

@@ -1,7 +1,7 @@
 __package_name__ = __name__
 
 # Should be synced with pyproject.toml
-__version__ = "0.40.0"
+__version__ = "0.40.1"
 __author__ = "ethteck"
 
 from . import util as util

src/splat/segtypes/common/bss.py

@@ -60,14 +60,9 @@ def disassemble_data(self, rom_bytes: bytes):
                 f"Segment '{self.name}' (type '{self.type}') requires a vram address. Got '{self.vram_start}'"
             )
 
-        next_subsegment = self.parent.get_next_subsegment_for_ram(
-            self.vram_start, self.index_within_group
-        )
-        if next_subsegment is None:
-            bss_end = self.get_most_parent().vram_end
-        else:
-            bss_end = next_subsegment.vram_start
-        assert isinstance(bss_end, int), f"{self.name} {bss_end}"
+        # Supposedly logic error, not user error
+        assert isinstance(self.bss_size, int), f"{self.name} {self.bss_size}"
+        bss_end = self.vram_start + self.bss_size
 
         self.spim_section = make_bss_section(
             self.rom_start,

src/splat/segtypes/common/code.py

@@ -1,5 +1,5 @@
 from collections import OrderedDict
-from typing import List, Optional, Type, Tuple
+from typing import List, Optional, Type, Tuple, Union
 
 from ...util import log, options, utils
 
@@ -21,9 +21,10 @@ def __init__(
         name: str,
         vram_start: Optional[int],
         args: list,
-        yaml,
+        yaml: Union[dict, list],
+        bss_size: Optional[int] = None,
     ):
-        self.bss_size: int = yaml.get("bss_size", 0) if isinstance(yaml, dict) else 0
+        self.bss_size = bss_size if bss_size is not None else self.parse_bss_size(yaml)
 
         super().__init__(
             rom_start,
@@ -33,6 +34,7 @@ def __init__(
             vram_start,
             args=args,
             yaml=yaml,
+            bss_size=self.bss_size,
         )
 
         self.reported_file_split = False
@@ -41,17 +43,6 @@ def __init__(
         if self.align is None:
             self.align = 0x10
 
-    @property
-    def needs_symbols(self) -> bool:
-        return True
-
-    @property
-    def vram_end(self) -> Optional[int]:
-        if self.vram_start is not None and self.size is not None:
-            return self.vram_start + self.size + self.bss_size
-        else:
-            return None
-
     # Generates a placeholder segment for the auto_link_sections option
     def _generate_segment_from_all(
         self,
@@ -62,13 +53,15 @@ def _generate_segment_from_all(
         rom_start: Optional[int] = None,
         rom_end: Optional[int] = None,
         vram_start: Optional[int] = None,
+        bss_size: Optional[int] = None,
     ) -> Segment:
         rep: Segment = replace_class(
             rom_start=rom_start,
             rom_end=rom_end,
             type=rep_type,
             name=base_name,
             vram_start=vram_start,
+            bss_size=bss_size,
             args=[],
             yaml={},
         )
@@ -146,19 +139,22 @@ def _insert_all_auto_sections(
         return ret
 
     def parse_subsegments(self, segment_yaml) -> List[Segment]:
+        ret: List[Segment] = []
+
         if "subsegments" not in segment_yaml:
             if not self.parent:
                 raise Exception(
                     f"No subsegments provided in top-level code segment {self.name}"
                 )
-            return []
+            return ret
+        yaml_subsegments = segment_yaml["subsegments"]
 
         base_segments: OrderedDict[str, Segment] = OrderedDict()
-        ret: List[Segment] = []
-        prev_start: Optional[int] = -1
-        prev_vram: Optional[int] = -1
+        prev_start: Optional[int] = None
+        prev_vram: Optional[int] = None
 
-        last_rom_end = None
+        # Start as the "start" address of the group.
+        last_rom_end = self.rom_start
 
         # Determine what comes first, either text or rodata/rdata
         readonly_before = False
@@ -172,39 +168,41 @@ def parse_subsegments(self, segment_yaml) -> List[Segment]:
                 if rdata_index is not None:
                     readonly_before = rdata_index < text_index
 
-        for i, subsegment_yaml in enumerate(segment_yaml["subsegments"]):
+        for i, subsegment_yaml in enumerate(yaml_subsegments):
             # endpos marker
             if isinstance(subsegment_yaml, list) and len(subsegment_yaml) == 1:
                 continue
 
+            next_subsegment_yaml = (
+                yaml_subsegments[i + 1] if i + 1 < len(yaml_subsegments) else None
+            )
+
             typ = Segment.parse_segment_type(subsegment_yaml)
             start, is_auto_segment = Segment.parse_segment_start(subsegment_yaml)
 
             segment_class = Segment.get_class_for_type(typ)
 
             if start is None:
-                # Attempt to infer the start address
-                if i == 0:
-                    # The start address of this segment is the start address of the group
-                    start = self.rom_start
-                else:
-                    # The start address is the end address of the previous segment
-                    start = last_rom_end
+                # Attempt to infer the start address.
+                # The start address is the end address of the previous segment.
+                # If this is the first subsegment then this value will fallback
+                # to the start address of the group.
+                start = last_rom_end
 
             # First, try to get the end address from the next segment's start address
             # Second, try to get the end address from the estimated size of this segment
             # Third, try to get the end address from the next segment with a start address
             end: Optional[int] = None
-            if i < len(segment_yaml["subsegments"]) - 1:
+            if next_subsegment_yaml is not None:
                 end, end_is_auto_segment = Segment.parse_segment_start(
-                    segment_yaml["subsegments"][i + 1]
+                    next_subsegment_yaml
                 )
             if start is not None and end is None:
                 est_size = segment_class.estimate_size(subsegment_yaml)
                 if est_size is not None:
                     end = start + est_size
             if end is None:
-                end = self.get_next_seg_start(i, segment_yaml["subsegments"])
+                end = self.get_next_seg_start(i, yaml_subsegments)
 
             if start is not None and prev_start is not None and start < prev_start:
                 log.error(
@@ -216,14 +214,25 @@ def parse_subsegments(self, segment_yaml) -> List[Segment]:
                 assert isinstance(start, int)
                 vram = self.get_most_parent().rom_to_ram(start)
 
-            if segment_class.is_noload() and last_rom_end is not None:
-                # Pretend bss's rom address is after the last actual rom segment
-                start = last_rom_end
-                # and it has a rom size of zero
-                end = last_rom_end
+            # noload (bss) segments need a bit of special calculation
+            start, end, vram, bss_size = self._calculate_noload_values(
+                segment_class,
+                subsegment_yaml,
+                next_subsegment_yaml,
+                start,
+                end,
+                vram,
+                last_rom_end,
+            )
 
-            segment: Segment = Segment.from_yaml(
-                segment_class, subsegment_yaml, start, end, self, vram
+            segment = Segment.from_yaml(
+                segment_class,
+                subsegment_yaml,
+                start,
+                end,
+                self,
+                vram,
+                bss_size,
             )
             segment.is_auto_segment = is_auto_segment
 
@@ -275,6 +284,10 @@ def parse_subsegments(self, segment_yaml) -> List[Segment]:
 
         return ret
 
+    @property
+    def needs_symbols(self) -> bool:
+        return True
+
     def scan(self, rom_bytes):
         # Always scan code first
         for sub in self.subsegments: