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

Changes to use unpatched binutils for linking IRX #253

Merged
merged 2 commits into from
Aug 17, 2022

Conversation

uyjulian
Copy link
Member

@uyjulian uyjulian commented Mar 15, 2022

Tested: OPL works

@rickgaiser
Copy link
Member

@uyjulian what's the status of this draft PR?

@uyjulian
Copy link
Member Author

Still a work in progress

@rickgaiser
Copy link
Member

Perhaps instead of elftypechanger we can use iopfixup, the name sce used for this tool?

@fjtrujy
Copy link
Member

fjtrujy commented Jun 24, 2022

Perhaps instead of elftypechanger we can use iopfixup, the name sce used for this tool?

Why not iopelf2irx

@rickgaiser
Copy link
Member

Also note that @frno7 made some great iop tools here:
https://github.com/frno7/iopmod

I'm already using the iopmod-info tool a lot, but there's also an iopmod-link tool that does something similar to what the proposed elftypechanger will do. Do we need our own tools, or can we use the iopmod repository from @frno7 ?

@frno7
Copy link

frno7 commented Jun 24, 2022

The iopmod tools are intended to be compatible with PS2SDK, in the sense that its symbols such as QueryIntrContext are recognised and displayed (given the --alias option to iopmod-info). The idea is that the same IOP modules can, in principle, be used with both PS2SDK and Linux, and anything else people might do. The tools were also made to be compatible with the latest, standard, GCC and -march=r3000, without any need for additional GCC patches for IOP module compilation.

@rickgaiser
Copy link
Member

Since we now have 2 toolchains in the CI for the IOP (an irx and an elf version), I would like to move forward with this change as well so we can remove the irx toolchain once completed.

I've tested this PR with the following findings:

  • I see link errors/warnings, can they be solved?
  • There's extra link flags -q -n --no-check-sections, do we really need them?
  • Perhaps related to the above: there are many sections discarded in the linkfile, shouldn't we do this with strip instead?
  • From the samples makefile: $(PS2SDK)/iop/startup/linkfile does not exist

On a side note: I would like to have https://github.com/frno7/iopmod/ tools available in ps2sdk, and the CI. So we can use iopmod-info and perhaps also iopmod-link. @fjtrujy will you make the PR for adding these tools?

I've been testing with this patch and both elf-to-irx (iopmod-link and elftypechanger) tools, but so far I've only managed to produce IRX files that don't run.

@fjtrujy
Copy link
Member

fjtrujy commented Jun 28, 2022

Since we now have 2 toolchains in the CI for the IOP (an irx and an elf version), I would like to move forward with this change as well so we can remove the irx toolchain once completed.

I've tested this PR with the following findings:

  • I see link errors/warnings, can they be solved?
  • There's extra link flags -q -n --no-check-sections, do we really need them?
  • Perhaps related to the above: there are many sections discarded in the linkfile, shouldn't we do this with strip instead?
  • From the samples makefile: $(PS2SDK)/iop/startup/linkfile does not exist

On a side note: I would like to have https://github.com/frno7/iopmod/ tools available in ps2sdk, and the CI. So we can use iopmod-info and perhaps also iopmod-link. @fjtrujy will you make the PR for adding these tools?

I've been testing with this patch and both elf-to-irx (iopmod-link and elftypechanger) tools, but so far I've only managed to produce IRX files that don't run.

Yes, I will try to do it today. I will add the tools to the toolchain-iop repo

@uyjulian
Copy link
Member Author

uyjulian commented Jun 28, 2022

elftypechanger does what it is supposed to do.

The remaining issue is figuring out why the IOP linker (its code in loadcore, udnl) does not accept the file. The reason why the linker script strips sections is because it is mandatory that those sections not be there or the IOP linker does not accept the file.

@uyjulian uyjulian force-pushed the e_type_changer branch 5 times, most recently from 978087b to a972a66 Compare June 29, 2022 03:55
@uyjulian
Copy link
Member Author

uyjulian commented Jun 29, 2022

I believe the remaining issue is that OPL black screens. wLE works fine.

I'll debug this more later

@fjtrujy
Copy link
Member

fjtrujy commented Aug 6, 2022

What’s the status of this? Would be really nice to use mainstream tools for IOP

@uyjulian
Copy link
Member Author

uyjulian commented Aug 6, 2022

OPL is currently broken.
I plan to debug this later

@uyjulian uyjulian marked this pull request as ready for review August 15, 2022 05:51
@uyjulian uyjulian changed the title [Don't merge] Changes to use unpatched binutils for linking IRX Changes to use unpatched binutils for linking IRX Aug 15, 2022
@uyjulian
Copy link
Member Author

OPL works now.

I've unmarked this as draft

iop/Rules.make Outdated Show resolved Hide resolved
@fjtrujy
Copy link
Member

fjtrujy commented Aug 15, 2022

Could you make a comparison between binaries sizes and/or performance?
Have you tried to use latest GCC and Binutils?
GCC 12 did some refactors, maybe we have some improvements from there.

Cheers

@uyjulian
Copy link
Member Author

I removed the max-page-size=4096 as it works without it.

I did a size comparison.

Before PR for mcman: 76,696 bytes
After PR for mcman: 76,454 bytes

Other than the section reordering, there is no change in code generation.
This is with the current version of binutils and GCC.

@fjtrujy
Copy link
Member

fjtrujy commented Aug 15, 2022

I removed the max-page-size=4096 as it works without it.

I did a size comparison.

Before PR for mcman: 76,696 bytes After PR for mcman: 76,454 bytes

Other than the section reordering, there is no change in code generation. This is with the current version of binutils and GCC.

Could you try to set the same max page size than EE? if I remember properly it was 128. Default value in newer GCCs was 4096

@uyjulian
Copy link
Member Author

Yes, it works.

The size after PR is still 76,454 bytes

@fjtrujy
Copy link
Member

fjtrujy commented Aug 15, 2022

This PR looks great for me, just for curiosity, I suppose that now you have some more bytes available in OPL, I hope this will help you to increase game compatibility

Copy link
Member

@rickgaiser rickgaiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great progress!

The new tool (ps2-irxgen) looks very complex compared to the one from @frno7 (iopmod-link), yet they seem to do the same (convert elf to irx)?

Are they compatible or does your tool do more things?
One difference I see is the "relocations". The linker flags do --emit-relocs, and then ps2-irxgen seems to use these. What does this do?

@fjtrujy did you manage to get the iopmod tools into ps2dev?

IOP_LDFLAGS := -nostdlib -s $(IOP_LDFLAGS)
IOP_LDFLAGS := -Wl,--gpsize=0 -Wl,-G0 -Wl,--nmagic -Wl,--orphan-handling=error -Wl,--discard-all -Wl,--gc-sections -Wl,--emit-relocs -nostdlib -Wl,-z,max-page-size=128 -Wl,--no-relax $(IOP_LDFLAGS)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain all these flags and why the are needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--gpsize=0
Set the maximum size of objects to be optimized using the GP register to size. This is only meaningful for object file formats such as MIPS ELF that support putting large and small objects into different sections. This is ignored for other object file formats.

-G0
Set the maximum size of objects to be optimized using the GP register to size. This is only meaningful for object file formats such as MIPS ELF that support putting large and small objects into different sections. This is ignored for other object file formats.

--nmagic
Turn off page alignment of sections, and disable linking against shared libraries. If the output format supports Unix style magic numbers, mark the output as "NMAGIC".

--orphan-handling=error
Control how orphan sections are handled. An orphan section is one not specifically mentioned in a linker script.
The linker will exit with an error if any orphan section is found.

--discard-all
Delete all local symbols.

--gc-sections
Enable garbage collection of unused input sections. It is ignored on targets that do not support this option. The default behaviour (of not performing this garbage collection) can be restored by specifying --no-gc-sections on the command line. Note that garbage collection for COFF and PE format targets is supported, but the implementation is currently considered to be experimental.
--gc-sections decides which input sections are used by examining symbols and relocations. The section containing the entry symbol and all sections containing symbols undefined on the command-line will be kept, as will sections containing symbols referenced by dynamic objects. Note that when building shared libraries, the linker must assume that any visible symbol is referenced. Once this initial set of sections has been determined, the linker recursively marks as used any section referenced by their relocations. See --entry, --undefined, and --gc-keep-exported.

This option can be set when doing a partial link (enabled with option -r). In this case the root of symbols kept must be explicitly specified either by one of the options --entry, --undefined, or --gc-keep-exported or by a "ENTRY" command in the linker script.

As a GNU extension, ELF input sections marked with the "SHF_GNU_RETAIN" flag will not be garbage collected.

--emit-relocs
Leave relocation sections and contents in fully linked executables. Post link analysis and optimization tools may need this information in order to perform correct modifications of executables. This results in larger executables.
This option is currently only supported on ELF platforms.

-z,max-page-size=128
The recognized keywords are:
Set the maximum memory page size supported to 128.

--no-relax
An option with machine dependent effects. This option is only supported on a few targets.
On some platforms the --relax option performs target specific, global optimizations that become possible when the linker resolves addressing in the program, such as relaxing address modes, synthesizing new instructions, selecting shorter version of current instructions, and combining constant values.

On some platforms these link time global optimizations may make symbolic debugging of the resulting executable impossible. This is known to be the case for the Matsushita MN10200 and MN10300 family of processors.

On platforms where the feature is supported, the option --no-relax will disable it.

On platforms where the feature is not supported, both --relax and --no-relax are accepted, but ignored.

@uyjulian
Copy link
Member Author

uyjulian commented Aug 15, 2022

ps2-irxgen removes non-loadable sections like symbols and debug info.
It also changes executable type and .iopmod section type.
It also moves the .iopmod section contents near the start of the ELF file.

It probably does different stuff than iopmod-link does.

Relocations are necessary so that LOADCORE can move the .irx contents anywhere in memory and still have it running. However, the symbols associated with those relocations are not necessary, so ps2-irxgen removes those.
Unpatched ld won't allow emitting relocations without emitting the associated symbols.

The reason to remove the symbols is because of RAM limitations. Unlike loading EE ELF files where only the necessary sections are read, MODLOAD will read the whole IRX file into memory.

@rickgaiser
Copy link
Member

ps2-irxgen removes non-loadable sections like symbols and debug info.

Can't we use strip for this? Or add parameters to LD for stripping these sections?
If possible I rather use standard tools than custom ones.

It also changes executable type and .iopmod section type.

This is the only thing iopmod-link does I think.

It also moves the .iopmod section contents near the start of the ELF file.

I think the linkfile can do this (or already does this?).

Relocations are necessary so that LOADCORE can move the .irx contents anywhere in memory and still have it running. However, the symbols associated with those relocations are not necessary, so ps2-irxgen removes those.
Unpatched ld won't allow emitting relocations without emitting the associated symbols.

The reason to remove the symbols is because of RAM limitations. Unlike loading EE ELF files where only the necessary sections are read, MODLOAD will read the whole IRX file into memory.

So this is size optimization, and an improvement over iopmod-link?
strip can also not strip these symbols?

@uyjulian
Copy link
Member Author

ps2-irxgen removes non-loadable sections like symbols and debug info.

Can't we use strip for this? Or add parameters to LD for stripping these sections? If possible I rather use standard tools than custom ones.

ld already removes these sections in the linker script.
ps2-irxgen "removes" these sections by nature of how it parses the ELF file: It looks for the .iopmod section, the PROGBITS sections, and the REL sections. Any other section is ignored for output.

It also changes executable type and .iopmod section type.

This is the only thing iopmod-link does I think.

I believe that is the case.

It also moves the .iopmod section contents near the start of the ELF file.

I think the linkfile can do this (or already does this?).

It moves the section definition, but not the section contents. For some reason the section contents are still put near the end of the file when using ld.

Relocations are necessary so that LOADCORE can move the .irx contents anywhere in memory and still have it running. However, the symbols associated with those relocations are not necessary, so ps2-irxgen removes those.
Unpatched ld won't allow emitting relocations without emitting the associated symbols.
The reason to remove the symbols is because of RAM limitations. Unlike loading EE ELF files where only the necessary sections are read, MODLOAD will read the whole IRX file into memory.

So this is size optimization, and an improvement over iopmod-link? strip can also not strip these symbols?

Correct, strip cannot remove these symbols.

@frno7
Copy link

frno7 commented Aug 16, 2022

It moves the section definition, but not the section contents. For some reason the section contents are still put near the end of the file when using ld.

Rearranging the arguments to ld can handle this, as I recall it, by having the wanted section first in the library, and linking the library as the last argument.

So this is size optimization, and an improvement over iopmod-link? strip can also not strip these symbols?

Correct, strip cannot remove these symbols.

Isn’t GNU objcopy designed to manipulate these, and many other things?

@uyjulian
Copy link
Member Author

So this is size optimization, and an improvement over iopmod-link? strip can also not strip these symbols?

Correct, strip cannot remove these symbols.

Isn’t GNU objcopy designed to manipulate these, and many other things?

I tried using GNU objcopy to delete the .strtab and .symtab sections. It didn't delete those sections, so it won't remove those symbols.

Also I tried using the --strip-all option of GNU objcopy. It did delete the .strtab and .symtab sections, but it also deleted the .rel.* sections, which I don't want to delete. Using additional command line arguments to attempt to avoid to delete the .rel.* sections did not do anything useful. It just remained deleted.

@frno7
Copy link

frno7 commented Aug 16, 2022

I tried using GNU objcopy to delete the .strtab and .symtab sections. It didn't delete those sections, so it won't remove those symbols.

I assume .strtab must be in the ELF, if other sections such as .rel.* depend on it, but it could be made to only have the empty string. So renaming all symbols to the NUL string, conveniently string index 0, should allow objcopy, or some other tool, to remove strings that aren’t used any more. Likewise with the .symtab section. I haven’t tested this myself, but it looks plausible to me. :-)

@frno7
Copy link

frno7 commented Aug 16, 2022

In fact, it does work. :-) As an experiment I added

static void strip_module(struct file f)
{
        Elf_Ehdr *ehdr = f.data;
        Elf_Shdr *shdr;
        Elf_Sym *sym;

        elf_for_each_sym (sym, shdr, ehdr)
                sym->st_name = 0;
}

to iopmod-link, activated with the --strip option. See the strip branch for details. Then objcopy can do its thing with

$(MODULE_IRX): %.irx : %.iop
	$(QUIET_LINK)$(IOPMOD_LINK) --strip -o $@ $<
	$(QUIET_LINK)$(CROSS_COMPILE)objcopy $@ $@
	$(QUIET_LINK)$(IOPMOD_LINK) -o $@ $@

and with readelf one can observe that .strtab now has size 1,

[Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
[ 8] .strtab           STRTAB          00000000 002300 000001 00      0   0  1

as opposed to the previous size of 0x2cc,

[Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
[ 8] .strtab           STRTAB          00000000 002320 0002cc 00      0   0  1

saving in total 744 bytes for this particular IRX. It works with the PlayStation 2 hardware too. :-)

@rickgaiser
Copy link
Member

With ps2-irxgen it's already much better than what we currently have (custom toolchain), so I think we can merge this PR.

However I would prefer to use iopmod-link instead of ps2-irxgen becouse the approach seems to be more simple and using more standard tools. The result is the same with the --strip option, right? @frno7 do you plan to add it to your master/main branch?

@uyjulian what do you think about using iopmod-link instead of ps2-irxgen?

@uyjulian
Copy link
Member Author

Personally I find using a single tool ps2-irxgen to be more straightforward than using objcopy, iopmod-link, then objcopy again.

So in that case I would prefer to use ps2-irxgen instead of iopmod-link.

In the future I may improve on the import/export table handling/generation, but that's all I'm going to do on this PR for now since I spent quite a bit of time to get this in a working state.

@rickgaiser
Copy link
Member

Fair enough, ready for merge then?

@fjtrujy
Copy link
Member

fjtrujy commented Aug 17, 2022

Fair enough, ready for merge then?

Yes, merging

@fjtrujy fjtrujy merged commit da00e09 into ps2dev:master Aug 17, 2022
@frno7
Copy link

frno7 commented Aug 18, 2022

@rickgaiser, yes, eventually. See frno7/iopmod#11. I’ll consider ‘compacting’ sections too. If it’s simple enough (shifting subsequent memory and adjusting section offsets accordingly whilst honouring alignment restrictions), objcopy wouldn’t be necessary. Doesn’t the IOP discard sections such as strings it doesn’t need when loading an IRX? I should take a look at IOP memory and figure out what it actually does with it.

@uyjulian, the main benefit, in my opinion, for retaining .strtab (as a 1 byte section having only the empty string) is that the ELF remains structurally valid, so standard tools may still examine and manipulate it reliably.

@rickgaiser
Copy link
Member

When comparing an IRX file generated by the old toolchain to an IRX file generated by the new toolchain I find something very interesting:

First few lines output of iopmod-info, old toolchain:

iopmod id addr		0x4d5c
iopmod id name		cdvd_driver
iopmod id version	0x0101
iopmod entry addr	0xe24
iopmod unknown addr	0xce60
iopmod text size	18688
iopmod data size	1376
iopmod bss size		20000
iopmod version		0x0101
iopmod name		cdvd_driver

First few lines output of iopmod-info, new toolchain:

iopmod id addr		0x4cfc
iopmod id name		cdvd_driver
iopmod id version	0x0101
iopmod entry addr	0x10e0
iopmod unknown addr	0x9be0
iopmod text size	18616
iopmod data size	764
iopmod bss size		19968
iopmod version		0x6463
iopmod name		vd_driver

Most noticable difference is the iopmod version and name. They are wrong and seem to be off by 1 byte.

Looks like an alignment issue in either ps2-irxgen or iopmod-info ?

@uyjulian
Copy link
Member Author

May be either alignment or off by one issue. I will check it out later

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 this pull request may close these issues.

4 participants