ethteck / splat

A binary splitting tool to assist with decompilation and modding projects
MIT License
159 stars 42 forks source link

EEGCC output's bad asm / asm alignment. #364

Closed weirdbeardgame closed 5 months ago

weirdbeardgame commented 5 months ago

When running through Fatal Frame 2 with splat's EEGCC compiler option enabled, I noticed I started to get function already defined errors.

In the ASM of these functions, the align is set to 3

.align 3
glabel InitGPhaseSys__Fv
/* B09A8 001AF9A8 F0FFBD27 */  addiu      $29, $29, -0x10
/* B09AC 001AF9AC 4B00023C */  lui        $2, %hi(D_4AF6E0)
/* B09B0 001AF9B0 E0F64224 */  addiu      $2, $2, %lo(D_4AF6E0)
/* B09B4 001AF9B4 0000BFFF */  sd         $31, 0x0($29)
/* B09B8 001AF9B8 FFFF0424 */  addiu      $4, $0, -0x1
/* B09BC 001AF9BC 05000324 */  addiu      $3, $0, 0x5
/* B09C0 001AF9C0 14004224 */  addiu      $2, $2, 0x14
/* B09C4 001AF9C4 00000000 */  nop

Which causes the issue. Manually removing the .align statements in these files will fix the compiler issue.

weirdbeardgame commented 5 months ago

This issue does not occur on the normal GCC compiler.

ethteck commented 5 months ago

thanks for the report @weirdbeardgame - can you link a repro branch?

weirdbeardgame commented 5 months ago

https://github.com/weirdbeardgame/Fatal-Frame-2 - Uncomment Gphase.c and the file right below it then try and run it

AngheloAlf commented 5 months ago

This behavior is expected due to how the INCLUDE_ASM macro is defined. What is happening is both your INCLUDE_ASM macro and the function asm file are defining the same symbol (InitGPhaseSys__Fv in this case) by the "\t.globl\t" #NAME "\n" and the glabel InitGPhaseSys__Fv respectively.

The processed asm ends up looking something like this:

.section .text
    .align    3
    .globl    InitGPhaseSys__Fv
    .ent    InitGPhaseSys__Fv
InitGPhaseSys__Fv:
    .set noreorder
    .set noat
.align 3
glabel InitGPhaseSys__Fv
/* B09A8 001AF9A8 F0FFBD27 */  addiu      $29, $29, -0x10
# ...

Notice how the symbol is defined twice. This usually doesn't matter, because in normal circumstances there's nothing that may change the alignment of the symbol between the first and second declaration, but here there's an alignment directive that (in the eyes of the assembler) may change the address of the symbol, then the assembler decides to bark at you.

You can rewrite the INCLUDE_ASM macro to something like this to fix it:

#define INCLUDE_ASM(FOLDER, NAME) \
    __asm__( \
        ".section .text\n" \
        "    .set noat\n" \
        "    .set noreorder\n" \
        "    .include \""FOLDER"/"#NAME".s\"\n" \
        "    .set reorder\n" \
        "    .set at\n" \
        "    .globl    " #NAME ".NON_MATCHING\n" \
        "    " #NAME ".NON_MATCHING" " = " #NAME "\n" \
    )

The important part is the macro is not defining the symbol again, it is letting the asm file to do that instead. I know it works because that macro is what I'm using at hit_and_run

btw, why is your INCLUDE_ASM macro so cursed? why do you even need something like INCLUDE_ASM_INTERNAL?

weirdbeardgame commented 5 months ago

That seemed to fix this issue. Thanks!