ttyd-tools
ttyd-tools copied to clipboard
PistonMiner
elf2rel: Add support for linking against other rels
Symbols for other rel modules can now be added to lst files with the syntax ~~addr:symbolName?moduleId,sectionId (see here for an example of it in use)~~
Edit: syntax is now module,section,offset:name (see here for an example) (the old syntax for dol symbols is still supported also)
Thanks for your contribution! Sorry for the delay in getting back to you.
The biggest concern I have here is that this doesn't seem to account for fixed link. The only relocations that may be removed during a fixed link are relocations against the module itself and against the DOL, since these are the only components guaranteed to be fixed. Relocations against other modules must not be removed, since these modules may be loaded at a later time and must still be resolved. Right now, these would be removed along with all others.
The way to enable this is to order the relocations such that relocations against the module itself and the DOL are placed last in the relocation section, and then set fixedDataSize to the border between these relocations and relocations against other modules such that during a fixed link, only the information no longer required is removed. Since there are users relying on fixed link functionality in order to reduce memory usage and this will likely be introduced into the rel template as well, I'd like to be compatible with this.
The other thing I'm not so sure about is the symbol map syntax for symbols in other modules. I'm not sure I'm a fan of postfixing the name with additional location information, I think this should all be in one group instead for readability and simplicity reasons. Also not sure about using three different symbols to delimit section and module ID, this seems unnecessary. Perhaps something like <module>,<section>,<offset>:name?
I have some stylistic concerns about some of the code in its current form, however given that, should you choose to continue to work on this, the code will likely undergo non-trivial changes, I think it makes sense to wait for those before a more in-depth review.
Ah, the issue with fixed link is something I hadn't considered at all, since SPM only has the one rel and never unlinks it, that's definitely an issue yeah. The syntax is something I've grown to not really like myself either... no clue why I chose what I did, honestly, I'd definitely be open to changing it to your way.
As for actually making these changes, I've already got a lot I'm working on at the minute and I'm also about to go away on holiday soon, so it's probably going to be a while before I actually get around to it, apologies for that, but thanks for reviewing :)
I'd like to pull this into my project as well. One critique is I think this ought to be backwards compatible with the old format as well. Also, it'd be nice to have proper error handling when the format isn't right and doesn't parse.
As in the ? format? I thought the only project using it was a single version of spm practice codes, if it's being used elsewhere then it could be worth supporting
Nah I meant the original <RAM addr>:<name> format. By the way, in this new format, how would you specify a relocation against e.g. 0xE0000000? Maybe 0,0,E0000000:label?
Oh right, that syntax is actually still supported (although you could write it with the 0,0, in front too if you wanted)
I get this error using the old format:
terminate called after throwing an instance of 'std::out_of_range'
what(): basic_string::substr: __pos (which is 22) > this->size() (which is 8)
Also wouldn't 0,0,<addr>:name be different than <addr>:name since addr is an absolute address vs. offset depending on the syntax? I could subtract 0x80000000 when in the 0xE0000000 range I guess but it's a little weird
Rels treat relocations against module 0 as absolute addresses. Could you send the lst that's causing that error? I'll look into it later
And I see you just use the old syntax for DOL locations anyway, got it
Ah, I think I see the issue. Despite a comment saying the comma search is limited to before the symbol name, I used the wrong variable so that doesn't actually happen lol. I'll try fix it now
Seems to be working for me, could you try with this build when you get the chance? If it works I'll do a new release on my fork for now
I get this error now with that change (can't test exe since I build in Linux):
terminate called after throwing an instance of 'std::invalid_argument'
what(): stoul
I attempted to improve the robustness/readability of the parsing code: https://github.com/SeekyCt/spm-rel-loader/pull/1/files
Looks good, thank you :) Guessing that fixed all the problems you were experiencing?
Np! And actually, not quite :( It seems like all symbols are being treated as absolute, judging from some errors I'm getting in Dolphin. I'll have to look at it later.
Huh, rel symbols were still working for me Edit: just realised this was never mentioned here, we figured out the issue there, the REL sections were wrong in the LST
Brought back the 0x functionality for the section id and module id that I hadn't realised were removed in ComplexPlane's PR, as well as going back to 'manual' parsing (but hopefully more robust than last time) due to PistonMiner being against the use of regex for readability.
This should be compatible with LSTs that worked with the last release on my fork (unless you were using leading zeros for alignment of module/section id columns since that'll now be considered octal), but potentially not with ones that worked with manual builds made after I accepted the PR (if it depended on the modulle/section id being in hex with no 0x prefix).
To be clear about how the fields behave:
- Section id and module id are in decimal by default. They can be in hex with the 0x prefix, or octal with the 0 prefix (with the octal part being new behaviour from using base 0 in stoul instead of a manual 0x check)
- Address/offset is always in hex, and can be optionally prefixed with 0x