splat icon indicating copy to clipboard operation
splat copied to clipboard

Linker file issues with `shiftable: True`

Open PartyPlanner64 opened this issue 4 years ago • 4 comments

With the BSS updates, I've been able to get a lot closer to being able to edit certain parts of the ROM and support shiftability. I'm running into a couple issues with setting shiftable: True in the splat.yaml though.

  1. The .ld file gives an error undefined symbol __romPos referenced in expression immediately, because the first line is . = __romPos; and __romPos has never been initialized. It could be initialized to zero at the top to fix this.
  2. When non-16 byte alignment is used in a section, the linker_entry sometimes will do _end_block + _begin_segment and create multiple segments. However, it doesn't do __romPos += SIZEOF(.the_previous_segment); between each of these. This leads to an invalid __romPos and some data getting overwritten.

I also don't think the . = __romPos; lines are necessary with shiftable: True, but that doesn't seem to be a functional issue.

PartyPlanner64 avatar Sep 11 '21 03:09 PartyPlanner64

Here is an example of issue 2, for reference:

This chunk of the Mario Party splat.yaml uses subalign: 8.

  - type: code
    start: 0x8FCF0
    vram: 0x8008F0F0
    subalign: 8
    subsegments:
    - [0x8FCF0, bin]
    - [0x96EF0, .data, "BCA0"]
    - [0x98260, bin]
    - [0x98310, bin]
    - [0xA7590, .rodata, "decode"]
    - [0xA75A8, bin]

The last file ends up in a separate section, but there is no SIZEOF added to __romPos in between.

    . = __romPos;
    _8FCF0_ROM_START = __romPos;
    _8FCF0_VRAM = ADDR(._8FCF0);
    ._8FCF0 0x8008F0F0 : AT(_8FCF0_ROM_START) SUBALIGN(8)
    {
        _8FCF0_TEXT_START = .;
        _8FCF0_TEXT_END = .;
        _8FCF0_DATA_START = .;
        _8FCF0_bin = .;
        build/assets/8FCF0.bin.o(.data);
        BCA0_c = .;
        build/src/BCA0.c.o(.data);
        _98260_bin = .;
        build/assets/98260.bin.o(.data);
        _98310_bin = .;
        build/assets/98310.bin.o(.data);
        build/src/decode.c.o(.rodata);
    }
// Need to add SIZEOF(._8FCF0) here.
    . = __romPos;
    _8FCF0_A75A8bin_ROM_START = __romPos;
    _8FCF0_A75A8bin_VRAM = ADDR(._8FCF0_A75A8bin);
    ._8FCF0_A75A8bin 0x800A69A8 : AT(_8FCF0_A75A8bin_ROM_START) SUBALIGN(8)
    {
        A75A8_bin = .;
        build/assets/A75A8.bin.o(.data);
        _8FCF0_DATA_END = .;
        _8FCF0_BSS_START = .;
        _8FCF0_BSS_END = .;
    }
    __romPos += SIZEOF(._8FCF0);     // Note that this SIZEOF is referring to the first section.
    _8FCF0_ROM_END = __romPos;

PartyPlanner64 avatar Sep 11 '21 03:09 PartyPlanner64

A third thing I'm running into while using shiftable: True as I dig further into this:

  1. BSS data ends up being emitted to the ROM when shiftable: True is enabled.

When shiftable is not enabled, the linker file uses hardcoded positions from the yaml, and this causes any BSS to typically be excluded. When shiftable is enabled, the .bss is treated like any other part of a section and included, and I don't know of a way to exclude it.

Here is an example of one of my sections. I included a subtraction manually by hand in this example to fix it locally:

    overlays_intro_ROM_START = __romPos;
    overlays_intro_VRAM = ADDR(.overlays_intro);
    .overlays_intro 0x801059A0 : AT(overlays_intro_ROM_START) SUBALIGN(16)
    {
        overlays_intro_TEXT_START = .;
        build/src/overlays/intro/intro_main.c.o(.text);
        build/src/overlays/intro/intro.c.o(.text);
        overlays_intro_TEXT_END = .;
        overlays_intro_DATA_START = .;
        overlays_intro_intro_main_c = .;
        build/src/overlays/intro/intro_main.c.o(.data);
        overlays_intro_intro_c = .;
        build/src/overlays/intro/intro.c.o(.data);
        overlays_intro_DATA_END = .;
        overlays_intro_BSS_START = .;
        build/src/overlays/intro/intro_main.c.o(.bss);
        build/src/overlays/intro/intro.c.o(.bss);
        overlays_intro_BSS_END = .;
    }
    __romPos += SIZEOF(.overlays_intro);
    __romPos -= overlays_intro_BSS_END - overlays_intro_BSS_START; // splat doesn't do this currently.
    overlays_intro_ROM_END = __romPos;

Might also be some way to use DISCARD and a separate section, but that might get complicated. I do want the .bss to be considered as following the data, for the purposes of calculating all its addresses, but Mario Party doesn't have .bss in the ROM. It just needs to know what the size of the .bss is.

This issue also might not matter too much, since once things are shiftable, it shouldn't matter if these extra zeroes are present or not.

PartyPlanner64 avatar Sep 11 '21 16:09 PartyPlanner64

I'm gonna have to defer to alex on this one unfortunately, because I don't know anything about the shiftable switch and I've never tested it. Does wua use it?

ethteck avatar Sep 11 '21 17:09 ethteck

Update: this may be fixed when #124 is fixed, which I'm working on now

ethteck avatar May 09 '22 15:05 ethteck

@PartyPlanner64 all 3 of these should be fixed. Closing for now, but lmk if you encounter further issues.

ethteck avatar Sep 12 '22 18:09 ethteck