gbdev / rgbds

Rednex Game Boy Development System - An assembly toolchain for the Nintendo Game Boy and Game Boy Color
https://rgbds.gbdev.io
MIT License
1.34k stars 172 forks source link

Add support for indeterminate section type when linking with SDCC object files #1466

Closed RubenZwietering closed 2 months ago

RubenZwietering commented 2 months ago

Most sections created by SDCC are indeterminate, which causes linking to fail at https://github.com/gbdev/rgbds/blob/master/src/link/section.cpp#L231 . See https://github.com/gbdev/rgbds/blob/master/src/link/sdas_obj.cpp#L333 .

ISSOtm commented 2 months ago

The intended solution is to use a linker script to give the sections a type; this is what the FLOATING modifier has been added for.

Please let me know if this is a problem for your use case?

Rangi42 commented 2 months ago

We could have a more informative error message for indeterminate sections then.

RubenZwietering commented 2 months ago

I see, thank you ISSOtm. I am not that familiar with SDCC's build process. I have tried looking through the SDCC Compiler User Guide (sdccman.pdf) for a solution, but could not find one. Perhaps I have made this issue a bit too hastily.

ISSOtm commented 2 months ago

I could hardly fault you, I don't think this is documented anywhere. (We haven't had anyone attempt to combine SDCC and RGBLINK thus far.) If you would like to walk us through your use case, we may be able to improve the situation further?

RubenZwietering commented 2 months ago

Of course, I made a very small test project here https://github.com/RubenZwietering/rgbds-sdcc-link-example

RubenZwietering commented 2 months ago

With rgbds 0.8.0 it runs into the linker bug that I created a pull request for, but with that patch the linker errors out with error: Section ... has an invalid type because no of the no linker script (this is the same behaviour as rgblink 0.7.0). Is there a way that sdcc can automatically generate linker scripts?

ISSOtm commented 2 months ago

No, because SDCC is not aware of RGBDS. I believe SDCC's area names are all hardcoded, though?

ISSOtm commented 2 months ago

It is however unclear what area name maps to what memory type, yep.

error: Section "_INITIALIZER" has an invalid type
error: Section "_GSFINAL" has an invalid type
error: Section "_GSINIT" has an invalid type
error: Section "_HOME" has an invalid type
error: Section "_INITIALIZED" has an invalid type
error: Section "_DATA" has an invalid type
error: Section "_CODE" has an invalid type
RubenZwietering commented 2 months ago

compiling main.c with --asm=rgbds transpiles into:

; File Created by SDCC : free open source ISO C Compiler 
; Version 4.4.0 #14620 (Linux)
;--------------------------------------------------------
        ; MODULE main
        ; Generated using the rgbds tokens.

;--------------------------------------------------------
; Public variables in this module
;--------------------------------------------------------
        EXPORT _main
;--------------------------------------------------------
; special function registers
;--------------------------------------------------------
;--------------------------------------------------------
; ram data
;--------------------------------------------------------
        SECTION FRAGMENT "main.c_DATA",WRAMX
;--------------------------------------------------------
; ram data
;--------------------------------------------------------
        SECTION FRAGMENT "_INITIALIZED",ROM0
;--------------------------------------------------------
; absolute external ram data
;--------------------------------------------------------
        SECTION FRAGMENT "_DABS (ABS)",ROM0
;--------------------------------------------------------
; global & static initialisations
;--------------------------------------------------------
        SECTION FRAGMENT "_HOME",ROM0
        SECTION FRAGMENT "_GSINIT",ROM0
        SECTION FRAGMENT "_GSFINAL",ROM0
        SECTION FRAGMENT "_GSINIT",ROM0
;--------------------------------------------------------
; Home
;--------------------------------------------------------
        SECTION FRAGMENT "main.c_HOME",ROM0
        SECTION FRAGMENT "main.c_HOME",ROM0
;--------------------------------------------------------
; code
;--------------------------------------------------------
        SECTION FRAGMENT "main.c_CODE",ROMX
;main.c:1: int main(void)
;       ---------------------------------
; Function main
; ---------------------------------
_main::
;main.c:4: return 1;
        ld      bc, $0001
.l00101:
;main.c:8: }
        ret
        SECTION FRAGMENT "main.c_CODE",ROMX
        SECTION FRAGMENT "_INITIALIZER",ROM0
        SECTION FRAGMENT "_CABS (ABS)",ROM0

So we can nicely see the section types here. The reason I am not using this option in my project is because this often does not compile because relative jumps (jr) are out of range and the generated z80 is sometimes completely botched. This does not seem to be the case when using SDCC's assembler (yet).

SelvinPL commented 2 months ago

well, there is a problem ... between _GSINIT and _GSFINAL there is crt initializer which copies data from _INITIALIZER to _INITIALIZED and have to be called before main There should be no _GSINIT nor _GSFINAL in normal assembly it's only part of C-runtime

_HOME is part of crt - should somewhere at the begining of bank 0 _DATA is just a allocated memory there can be also _DATA_X where X is a number of bank _CODE is just code there is also _CODE_X version

ISSOtm commented 2 months ago

Is there some documentation about this?

ISSOtm commented 2 months ago

(I'm also surprised that this was updated to make use of SECTION FRAGMENT.)

SelvinPL commented 2 months ago

I've just found it on gbdk https://github.com/gbdk-2020/gbdk-2020/blob/develop/docs/pages/04_coding_guidelines.md#segments--areas (well, kind of)

SelvinPL commented 2 months ago

https://github.com/gbdk-2020/gbdk-2020/blob/develop/gbdk-lib/libc/targets/sm83/gb/crt0.s#L442 example of GSINIT `lmeans length of area s_` means start of area

RubenZwietering commented 2 months ago

Interesting that data is copied to _INITIALIZED, yet the section is of type ROM0 in the example z80 code above, with a comment it is ram data...

SelvinPL commented 2 months ago

maybe you didn't use initializer so it shouldn't matter, try add

char some = 1; 

as global variable

RubenZwietering commented 2 months ago
;--------------------------------------------------------
; File Created by SDCC : free open source ISO C Compiler 
; Version 4.4.0 #14620 (Linux)
;--------------------------------------------------------
        ; MODULE main
        ; Generated using the rgbds tokens.

;--------------------------------------------------------
; Public variables in this module
;--------------------------------------------------------
        EXPORT _main
        EXPORT _some
;--------------------------------------------------------
; special function registers
;--------------------------------------------------------
;--------------------------------------------------------
; ram data
;--------------------------------------------------------
        SECTION FRAGMENT "main.c_DATA",WRAMX
;--------------------------------------------------------
; ram data
;--------------------------------------------------------
        SECTION FRAGMENT "_INITIALIZED",ROM0
_some:
        DS 1
;--------------------------------------------------------
; absolute external ram data
;--------------------------------------------------------
        SECTION FRAGMENT "_DABS (ABS)",ROM0
;--------------------------------------------------------
; global & static initialisations
;--------------------------------------------------------
        SECTION FRAGMENT "_HOME",ROM0
        SECTION FRAGMENT "_GSINIT",ROM0
        SECTION FRAGMENT "_GSFINAL",ROM0
        SECTION FRAGMENT "_GSINIT",ROM0
;--------------------------------------------------------
; Home
;--------------------------------------------------------
        SECTION FRAGMENT "main.c_HOME",ROM0
        SECTION FRAGMENT "main.c_HOME",ROM0
;--------------------------------------------------------
; code
;--------------------------------------------------------
        SECTION FRAGMENT "main.c_CODE",ROMX
;main.c:3: int main(void)
;       ---------------------------------
; Function main
; ---------------------------------
_main::
;main.c:6: return 1;
        ld      bc, $0001
.l00101:
;main.c:10: }
        ret
        SECTION FRAGMENT "main.c_CODE",ROMX
        SECTION FRAGMENT "_INITIALIZER",ROM0
__xinit__some:
        DB $01  ; 1
        SECTION FRAGMENT "_CABS (ABS)",ROM0

still ROM0...

ISSOtm commented 2 months ago

Looks like a codegen bug, then; this is quite clearly meant to be RAM. I also got a segfault trying to map _INITIALIZED to ROM, so I'll be fixing that as well. [EDIT]: Done in https://github.com/gbdev/rgbds/pull/1465/commits/5214ae8fb53885f277e43d0b454b5c5dc4168883

ISSOtm commented 2 months ago

We could have a more informative error message for indeterminate sections then.

Fixed by https://github.com/gbdev/rgbds/pull/1465/commits/336ebf1f7af9693a9168e3701fbfcf653e76ee40

RubenZwietering commented 2 months ago

Great work, with these commits it now works flawlessly in my project as well!

ISSOtm commented 2 months ago

I've actually written a little linker script to go with that:

ROM0
    FLOATING
    "_HOME" OPTIONAL
    "_BASE" OPTIONAL
    "_CODE_0" OPTIONAL
    "_LIT_0" OPTIONAL
    "_INITIALIZER" OPTIONAL ; Initializer of `_INITIALIZED`.
    "_GSINIT" OPTIONAL ; Reportedly internal to the crt0... there's some trickery here, TODO: investigate.
    "_GSFINAL" OPTIONAL

ROMX FLOATING ; TODO
    FLOATING
    "_CODE" OPTIONAL
    "_LIT" OPTIONAL

WRAM0
    FLOATING
    "_DATA" OPTIONAL ; Uninit'd RAM.
    "_BSS" OPTIONAL ; Zero-init'd RAM.
    "_INITIALIZED" OPTIONAL ; Initialised by `_INITIALIZER`.
    "_DABS (ABS)" OPTIONAL ; TODO: what is this?

By default, code goes into ROMX (any bank!! :D), unless you #pragma bank 0, then it goes into ROM0.

You could add

ROMX 1
    "_CODE_1"

to allow #pragma bank 1 to work, etc.

I probably missed some other things, but I've asked a friend who probably has more info.

ISSOtm commented 2 months ago

(The last commit I just pushed to that branch is what enables ROMX FLOATING, which should nicely round up the linker script's feature set. cc @Rangi42!)

RubenZwietering commented 2 months ago

Great addition, very handy. One thing though, i think there might be a bug in rgblink where it causes some sort of infinite loop if the linker file does not end in a newline.

ISSOtm commented 2 months ago

Yep, should be fixed by this commit fresh off the presses!

RubenZwietering commented 2 months ago

You are amazing!

RubenZwietering commented 2 months ago

Hey I'm back for another potential issue. I updated https://github.com/RubenZwietering/rgbds-sdcc-link-example to link 2 functions in separate files, but as you can see it does not quite link correctly. Both _function0 and _function1 link to 0x4000 even though the _function1 code is at 0x4007.

Since both _function0 and _function1 use section _CODE though i think that should be considered an error. And instead support for the FRAGMENT and UNION section modifiers could be added in linker script.

ISSOtm commented 2 months ago

It shouldn't be considered an error, instead both are to be treated like SECTION FRAGMENT (this is, reportedly, how SDLD handles it). The data was correctly appended, but there was a bug where the resulting offset was NOT correctly applied to the contained symbols.

I just pushed a commit to your PR's branch that fixes that. Thank you for reporting it!

RubenZwietering commented 2 months ago

Oh I see, thank you for the clarification and the fix!