ps2dev / ps2toolchain

This program will automatically build and install a compiler and other tools used in the creation of homebrew software for the Sony PlayStation® 2 videogame system.
BSD 2-Clause "Simplified" License
239 stars 73 forks source link

Last bytes of ELF file are sometimes corrupted #57

Closed rickgaiser closed 4 years ago

rickgaiser commented 4 years ago

The following piece of code will produce invalid output:

#include <stdio.h>

extern const int i1, i2, i3, i4;

int main()
{
        printf("i1=%d\n", i1);
        printf("i2=%d\n", i2);
        printf("i3=%d\n", i3);
        printf("i4=%d\n", i4);

        return 0;
}

const int dum1=1;
const int dum2=2;
const int dum3=3;
//const int dum4=4;
//const int dum5=5;
//const int dum6=6;
const int i1=1111;
const int i2=2222;
const int i3=3333;
const int i4=4444;

When tried with a random number of dummy constants, the results will vary from: Good:

i1=1111
i2=2222
i3=3333
i4=4444

To bad:

i1=1111
i2=2222
i3=3333
i4=0
i1=1111
i2=2222
i3=0
i4=0
i1=1111
i2=0
i3=0
i4=0

Testcase:

ee-gcc -o hello.elf hello.c
ps2client -h 192.168.1.10 execee host:hello.elf

This bug causes size_ds34bt_irx and/or size_ds34usb_irx in OPL to be 0. Causing a random crash of OPL at boot (black screen). Adding a line of code at a random place in OPL, would cause this bug to appear and disappear.

The invalid values seem to be in the last 128bit of the ELF file, when viewed using: ee-objdump -D hello.elf

Values in the elf file are good, so it seems to go wrong somewhere while loading the elf.

fjtrujy commented 4 years ago

Amazing how you reached that conclusion

sp193 commented 4 years ago

If it seems like values within the ELF are good, why don't you think it is a ps2link problem? What if you used another method to load your ELF? I think it's possibly caused by ps2link not clearing memory. The sceSifLoadElf() function does not zero any memory, hence bad data may appear if memory is not erased before the ELF is loaded with that function.

rickgaiser commented 4 years ago

If it seems like values within the ELF are good, why don't you think it is a ps2link problem? What if you used another method to load your ELF?

Yes, the data in the ELF file seems good, so I guess the title of this issue is a little misleading. I'll try another loader.

I think it's possibly caused by ps2link not clearing memory. The sceSifLoadElf() function does not zero any memory, hence bad data may appear if memory is not erased before the ELF is loaded with that function.

The const int's in are in the .sdata (static data) section. I think the .sdata section is copied from the elf file to memory in units of 16bytes/128bits somewhere. Perhaps if the .sdata section is not a multiple of 128bits, the remaining data is not copied, leaving zeroes in it's place? Or perhaps the .sdata section is copied correctly, but another zeroed section is not aligned properly next to it? And the zeroeing happens after the .sdata is copied?

fjtrujy commented 4 years ago

Hello, In order to see if @sp193 was right and the issue was in the ps2link I have modified the example and now it uses the functions scr_prinf to see the output in the screen and be easily executed by ULaunchELF or loaded into PCSX2

The content of the file

#include <stdio.h>

void init_scr(void);
void scr_printf(const char *, ...) __attribute__((format(printf,1,2)));

extern const int i1, i2, i3, i4;

int main()
{
   init_scr();
   scr_printf("i1=%d\n", i1);
   scr_printf("i2=%d\n", i2);
   scr_printf("i3=%d\n", i3);
   scr_printf("i4=%d\n", i4);

   while(1) {}

   return 0;
}

const int dum1=1;
const int dum2=2;
const int dum3=3;
const int dum4=4;
const int dum5=5;
const int i1=1111;
const int i2=2222;
const int i3=3333;
const int i4=4444;

Compilation process

ee-gcc -o hello.elf hello.c -L$PS2SDK/ee/lib -ldebug

The output, in this case, using PCSX2

Screenshot 2020-01-15 at 09 47 54

As you can see the result is the same that @rickgaiser commented.

i1=1111
i2=2222
i3=3333
i4=0

Thanks

rickgaiser commented 4 years ago

ps2link uses the function SifLoadElf to load the elf file: https://github.com/ps2dev/ps2link/blob/1ec566c54bb8b567ee0a702982b392f37807b5e2/ee/cmdHandler.c#L138

from ps2sdk: https://github.com/ps2dev/ps2sdk/blob/5385a6ccd9db8c9fa54ecfd8af45322a5908bf6d/ee/kernel/src/loadfile.c#L129

That ends up calling an RPC function on the IOP. Part of LOADFILE.

Could this be a bug in LOADFILE? Or perhaps our toolchain doen not align the sections the way LOADFILE expects them? Could the sbv_patches fix this issue? I can see they patch LOADFILE, but do they patch anything related to this?

sp193 commented 4 years ago

The SBV patch for LOADFILE was to enable the RPC function for LoadModuleBuffer(). For some reason, that function isn't supported by the early version of LOADFILE. Sony patches it too, so it's clearly something that was overlooked.

rickgaiser commented 4 years ago

I found the bug:

crt0 clears the .bss section, in units of 16byte/128bit. It requires the .bss section to be aligned to at least 16byte/128bit. Previously this was guaranteed by the linkerfile in ps2sdk. Hewever, the default linkerscript in binutils (I added here: https://github.com/ps2dev/ps2toolchain/commit/7c494f217d379639aaca23d3c588e48986177a51) does not align the .bss section properly for crt0. Resulting in the last few values in .sdata to be cleared also.

I have created a fix for the builtin binutils linkerscript here. It will be part of the newlib PR.

asmblur commented 4 years ago

IIRC, you always want to make sure sections are aligned 128-bit.I believe it was something to do with DMA alignments or some such but not sure.

rickgaiser commented 4 years ago

This issue has been solved with the newlib PR's that have been recently merged, specifically this commit: https://github.com/ps2dev/ps2toolchain/commit/f5544b8d68fbdf611e277e9583714ae6627f3a61