Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature request] Specifying the order of sections explicitly #137

Closed
aaaaaa123456789 opened this issue Mar 15, 2017 · 14 comments · Fixed by #140
Closed

[Feature request] Specifying the order of sections explicitly #137

aaaaaa123456789 opened this issue Mar 15, 2017 · 14 comments · Fixed by #140

Comments

@aaaaaa123456789
Copy link
Member

Following the discussion from #136.

A way of stating blocks of sections to be linked together in a specific order would be considerably beneficial and helpful for projects like disassemblies and their modifications ("ROM hacks"), which were impacted quite significantly by the backwards-incompatible linker fix, as seen in the issue that sprouted this discussion.

A solution I proposed was the use of a manifest file, such as:

ROMX[1], $4000
  "1a" ; fixed
  "1b" ; immediately after
ROMX[2], $4000
  "2a"
  "2b"
ROMX
  ; no base address or bank
  "foo" ; truly floating

Of course, all sections not mentioned would follow the usual rules. (Also, it should be possible to incorporate alignment information somehow.)

Someone brought up sections that already define placement parameters (in the section header) defining different ones in the manifest — this could be handled as a linker error, since it is clearly an error in the source code or the manifest that has to be fixed.

Another proposed solution was to add a CONTINUES keyword to section headers, but that becomes a maintenance nightmare in large codebases, since all section information becomes spread out amongst all the section headers in the source code.

Summarizing, what is needed is a solution that allows specifying that some sections must be placed together when linking, one after another without gaps, in the stated order. It should also be possible to do so in a centralized manner, so as to not become a nightmare of section spaghetti; it also needs to be usable without knowing the exact sizes of the sections (otherwise fixing them to specific addresses would solve the issue).

@AntonioND
Copy link
Member

AntonioND commented Mar 15, 2017

My idea, as explained in #136, is to implement something like this:

SECTION "1A", ROMX, BANK[1]
label1: DB 1
SECTION "1B", CONTINUES "1A"
label2: DB 2

This is much easier to implement in the current linker and less of an overkill than adding a new placement system (with parser included). And it's basically what all the ROM disassemblies are already doing.

In short, this is a discussion of where the maintenance problem should be, in the linker or in the disassemblies.

Adding a new placement system creates problems in case you define conflicting attributes in both systems. Or should one of them be ignored? Or should the most restrictive one be enforced? It's not just a matter of adding the code, but of creating something that makes sense if we need to add more functionalities (this CONTINUE keyword could also make up for not having a PUSH/POP SECTION, for example).

@aaaaaa123456789
Copy link
Member Author

Adding a new placement system creates problems in case you define conflicting attributes in both systems. Or should one of them be ignored? Or should the most restrictive one be enforced?

That's clearly an error, and should be treated as an error. Prevent linking if that happens.

@AntonioND
Copy link
Member

But you need to define sections in the code somehow. The default way of doing it is by defining them as floating sections: SECTION "test", ROMX

If you do that, you already have 2 conflicting attributes, so you are implicitly saying that the manifiest has priority.

@aaaaaa123456789
Copy link
Member Author

As I see it, there are two solutions to that issue:

  • Allow the manifest to override floating sections only
  • Require all manifested sections to be declared as SECTION "foo" without any attributes

I have no position on which one is better.

@PikalaxALT
Copy link
Contributor

(this CONTINUE keyword could also make up for not having a PUSH/POP SECTION, for example).

You mean like PUSHS and POPS?

@AntonioND
Copy link
Member

Ok, yeah... That was stupid, sorry, I was trying to pay attention to too many things at the same time.

@AntonioND
Copy link
Member

In any case, implementing the linkerscript would take a long time. Right now I have no time to do something that big, but I could do the CONTINUES thing.

That doesn't mean I wouldn't accept a reasonably good pull request. Since both things are pretty much independent, the easy way could be implemented and the support for linkerscripts could be added later...

@AntonioND
Copy link
Member

Actually, it seems this is kind of supported.

SECTION "BANK 0",ROM0[0]
LABEL1:: DS 2
SECTION "BANK 1",ROMX
LABEL2:: DS 2
SECTION "BANK 0",ROM0[0]
LABEL3:: DS 2

If you generate the ROM like this:

rgbasm -o test.o test.asm
rgblink -o test.gb -n test.sym test.o

And the output of test.sym is:

;File generated by rgblink

00:0002 LABEL3
00:0000 LABEL1
01:4000 LABEL2

It only works for sections in the same object file, though.

@BenHetherington
Copy link
Contributor

Hmm… This feels like misusing another bug, to be honest. It's certainly no replacement for one of the new approaches (especially as this doesn't work between object files).

@AntonioND
Copy link
Member

Well, this is not a bug, as far as I see, this is the expected behaviour, the code explicitly checks if everything in both sections is the same and, if it is, it continues that section.

But, of course, it's no replacement for a proper way of doing it.

@BenHetherington
Copy link
Contributor

Whoops, I missed that it had the same section name! I assumed it was two distinct sections that both started at $0000, hence why that sounded like a bug to me.

@AntonioND
Copy link
Member

Ok, I've started to work on this. I've decided to go for a simple linkerscript with the following syntax, which is as simple as I could come up with. Are " quotes allowed in strings anyway? If so I will add an escape character like in C:

ROM0
  ORG $40 ; Set address for next section
  "Vectors" ; Interrupt vectors
  "Interrupt handlers" ; Goes after the interrupt vectors
  ALIGN 6 ; Set alignment for next section
  "Array" ; Goes after the interrupt handlers and it's aligned to a 6 bit boundary
  "More data" ; Goes after array

ROMX 1 ; This statement automatically sets the address to the base of the bank
  ; etc

In short, RGBLINK will allocate everything in the linkerscript first, then it will allocate the other sections defined in the code.

All sections that are used in the linkerscript must be floating and have the same type as the one assigned in the object file.

Floating sections won't appear in the linkerscript.

Any objections?

@aaaaaa123456789
Copy link
Member Author

aaaaaa123456789 commented Mar 18, 2017 via email

@AntonioND
Copy link
Member

AntonioND commented Mar 20, 2017

I've finished most of the code and now I need to integrate it and test it a bit, in case you're wondering what's going on with this. As we already have yacc as a dependency for rgbasm I've decided to use it for the linkerscript as well.

However, I've added flex as a dependency...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants