Skip to content

Make GetMonData and GetBoxMonData ABI-safe#2315

Open
AZero13 wants to merge 1 commit into
pret:masterfrom
AZero13:getmondata2
Open

Make GetMonData and GetBoxMonData ABI-safe#2315
AZero13 wants to merge 1 commit into
pret:masterfrom
AZero13:getmondata2

Conversation

@AZero13

@AZero13 AZero13 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Clang and LLVM hate this, so instead I am doing the manual work of using macros to do the equivalent of a default parameter in C++ to avoid work downstream.

@AZero13 AZero13 force-pushed the getmondata2 branch 3 times, most recently from 264adfb to 6f9c1a4 Compare June 16, 2026 18:34
Comment thread include/pokemon.h

@mrgriffin mrgriffin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need all these changes? Couldn't it have been more like this in include/pokemon.h?

 u32 GetMonData3(struct Pokemon *mon, s32 field, u8 *data);
-u32 GetMonData2(struct Pokemon *mon, s32 field);
 u32 GetBoxMonData3(struct BoxPokemon *boxMon, s32 field, u8 *data);
-u32 GetBoxMonData2(struct BoxPokemon *boxMon, s32 field);
+#if UBFIX
+#define GetMonData2(mon, field) GetMonData3(mon, field, NULL)
+#define GetBoxMonData2(boxMon, field) GetBoxMonData3(boMon, field, NULL)
+#else
 u32 GetMonData2(struct Pokemon *mon, s32 field);
 u32 GetBoxMonData2(struct BoxPokemon *boxMon, s32 field);
+#endif

And then just the conditional declaration of aliases in src/pokemon.c, i.e. without renaming GetMonData3/GetBoxMonData3.

Also, have you picked UBFIX because on a different (non-ARM) ABI the behavior would be undefined? I don't think it could cause problems on AArch32 unless the functions took 5+ parameters?

Comment thread src/pokemon.c Outdated
Comment on lines +51 to +54
#ifndef UBFIX
u32 GetMonData2(struct Pokemon *mon, s32 field) __attribute__((alias("GetMonData3")));
u32 GetBoxMonData2(struct BoxPokemon *boxMon, s32 field) __attribute__((alias("GetBoxMonData3")));
#endif

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How does this work? Aren't these aliasing symbols that aren't defined?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How does this work? Aren't these aliasing symbols that aren't defined?

An alias does not create a wrapper function. It doesn't create new assembly code that calls the old function. It is a zero-cost linker trick.

This is for the linker.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Without it the compilation will fail on not UBFIX. But because this is not UBFIX and it compiles matxhing on not ubfix anyway, it shouldn't matter.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Right, but the GetMonData3 and GetBoxMonData3 symbols did not appear to be defined on the version of the code I was commenting on because you'd unconditionally renamed them to GetMonData and GetBoxMonData.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In pokemon.h, there is a macro: #define GetMonData(...) ... which automatically adds a 3 to the end of the name if 3 arguments are passed.

In pokemon.c, I define your function as: u32 GetMonData(struct Pokemon *mon, s32 field, u8 *data)

Because I passed 3 arguments, the C preprocessor instantly rewrites your function definition to be: u32 GetMonData3(struct Pokemon *mon, s32 field, u8 *data).

When agbcc compiles the file, the symbol that gets saved into the object file is GetMonData3.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see! That's awful.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's the best I could do. It sucks but

Clang and LLVM hate this, so instead I am doing the manual work of using macros to do the equivalent of a default parameter in C++ to avoid work downstream.
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.

3 participants