✨Mark bugs that involve genuine undefined behavior as UBFix

[AZero13]

The objEvent->localId != OBJ_EVENT_ID_CAMERA counts because this results in a null dereference, which is fine for vanilla I guess, but still wrong.

[mrgriffin]

I'm not sure why there's a distinction between BUGFIX and UBFIX, but assuming it's for a good reason* I've pointed out some cases where I'm not sure what the UB is.

* ig it's for people that want to compile a ROM which works almost exactly the same as vanilla, but without invoking any UB? If that's the case, then the src/battle_gfx_sfx_util.c/src/trainer_pokemon_sprites.c ones wouldn't qualify. Maybe it's "people want to compile a ROM that's altered but can't invoke any UB without them violating a reasonable precondition", "reasonable" because "you can't have non-transparent pixels in the first tile's top row" is obviously not reasonable.

[mrgriffin]

Why's this one UB?

[mrgriffin]

idk if this is really a fix for UB. Yes, it's UB if you ask for a tag that isn't in this list, but that's a precondition of the code; if "can invoke UB when you violate a precondition" is the bar for what's UB or not then every function that dereferences a pointer without a NULL check should grow UBFIX that checks for NULL and that's obviously silly.

[mrgriffin]

I haven't looked into it, but why's it UB for the camera to trigger ground effects?

[AZero13]

if the check objEvent->localId != OBJ_EVENT_ID_CAMERA is missing, the game will attempt to run the ground effects logic on the camera sprite

[AZero13]

During the OAM update/rendering loop, the engine checks sprite->subspriteTableNum and attempts to read from sprite->subspriteTables[4].

Because subspriteTables is NULL, this compiles to an offset reference of NULL + 4 * sizeof(void *), trying to read from memory address 0x00000010.

[mrgriffin]

afaict the only call to AddSubspritesToOamBuffer is on the else of if (!sprite->subspriteTables || sprite->subspriteMode == SUBSPRITES_OFF)? (i.e. only entered if subspriteTables != NULL && sprite->subspriteMode != SUBSPRITES_OFF)

[ketsuban]

I'm not sure why there's a distinction between BUGFIX and UBFIX

I thought the goal was to make something like the PC port easier to do by providing a compile-time flag to enable null-pointer checks Gamefreak didn't do because dereferencing a null pointer just reads the last instruction read from the firmware, and that happens not to crash the game in those instances in the vanilla game.

[AZero13]

Addressed!

[AZero13]

✨