From 889ae950fef08ec468ce10e05733be56f4ed2606 Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Tue, 29 Aug 2023 10:16:52 +1200 Subject: [PATCH 1/2] Fix a few cases of UB incl. OoB Behaviour should be mostly the same but there are some OoB accesses in the menu code where I've had to assume things. Tests for each OoB case: - Accessing of layer -1: Select a game file in the "Choose Your Reality" screen. - Accessing of menu for the 5th player: Die in a mission. --- src/game/camera.c | 9 +++++++++ src/game/gfxreplace.c | 4 ++++ src/game/menu.c | 21 +++++++++++++++++++++ src/lib/mp3/decoder.c | 4 ++-- 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/game/camera.c b/src/game/camera.c index 92996ae26..c47490563 100644 --- a/src/game/camera.c +++ b/src/game/camera.c @@ -1,4 +1,5 @@ #include +#include #include "constants.h" #include "game/atan2f.h" #include "game/camera.h" @@ -600,6 +601,9 @@ bool camIsPosInFovAndVisibleRoom(RoomNum *rooms, struct coord *pos, f32 arg2) RoomNum room; bool hasdata = false; struct drawslot *thisthing; +#ifdef AVOID_UB + static struct drawslot dslot; +#endif struct screenbox box; for (i = 0, room = rooms[i]; room != -1; i++, room = rooms[i]) { @@ -637,5 +641,10 @@ bool camIsPosInFovAndVisibleRoom(RoomNum *rooms, struct coord *pos, f32 arg2) return false; } +#ifdef AVOID_UB + memcpy(&dslot.box, &box, sizeof(box)); + return camIsPosInScreenBox(pos, arg2, &dslot); +#else return camIsPosInScreenBox(pos, arg2, (struct drawslot *) &(((u8 *) &box)[-((uintptr_t) &(((struct drawslot *)0)->box))])); +#endif } diff --git a/src/game/gfxreplace.c b/src/game/gfxreplace.c index 73174e085..2d03b8b4b 100644 --- a/src/game/gfxreplace.c +++ b/src/game/gfxreplace.c @@ -329,7 +329,11 @@ void gfxReplaceGbiCommands(Gfx *startgdl, Gfx *endgdl, s32 type) void gfxReplaceGbiCommandsRecursively(struct roomblock *block, s32 type) { +#ifndef AVOID_UB + // Sometimes block is NULL when this is called. + // If UBSan is being used, this will crash in this instance. if (block->type == ROOMBLOCKTYPE_PARENT); +#endif while (true) { if (!block) { diff --git a/src/game/menu.c b/src/game/menu.c index cee8361a4..464def658 100644 --- a/src/game/menu.c +++ b/src/game/menu.c @@ -3413,8 +3413,14 @@ void menuClose(void) void func0f0f8120(void) { +#ifdef AVOID_UB + u32 mpindex = g_MpPlayerNum % MAX_PLAYERS; + struct menudialog *prev = g_Menus[mpindex].curdialog; + s32 i; +#else struct menudialog *prev = g_Menus[g_MpPlayerNum].curdialog; s32 i; +#endif if (g_MenuData.unk66e > 0) { for (i = g_MenuData.unk66e; i >= 0; i--) { @@ -3422,11 +3428,22 @@ void func0f0f8120(void) } } +#ifdef AVOID_UB + mpindex = g_MpPlayerNum; + if (mpindex >= MAX_PLAYERS) + mpindex -= MAX_PLAYERS; + if (g_Menus[mpindex].curdialog == prev) { + while (g_Menus[mpindex].depth > 0) { + menuPopDialog(); + } + } +#else if (g_Menus[g_MpPlayerNum].curdialog == prev) { while (g_Menus[g_MpPlayerNum].depth > 0) { menuPopDialog(); } } +#endif } void func0f0f820c(struct menudialogdef *dialogdef, s32 root) @@ -4340,7 +4357,11 @@ void dialogTick(struct menudialog *dialog, struct menuinputs *inputs, u32 tickfl } // Apply default navigational behaviour if requested +#ifdef AVOID_UB + if (usedefaultbehaviour && (tickflags & MENUTICKFLAG_DIALOGISCURRENT) && !dialog->dimmed && g_Menus[g_MpPlayerNum].depth >= 1) { +#else if (usedefaultbehaviour && (tickflags & MENUTICKFLAG_DIALOGISCURRENT) && !dialog->dimmed) { +#endif struct menulayer *layer = &g_Menus[g_MpPlayerNum].layers[g_Menus[g_MpPlayerNum].depth - 1]; if (layer->numsiblings <= 1) { diff --git a/src/lib/mp3/decoder.c b/src/lib/mp3/decoder.c index 5751abbb0..11383b68d 100644 --- a/src/lib/mp3/decoder.c +++ b/src/lib/mp3/decoder.c @@ -2736,8 +2736,8 @@ bool mp3dec000427d8(struct asistream *stream, u32 gr) f32 *sp0c = stream->unk4664[0][0].unk00; s32 i = 0; - if (stream->window_switching[gr][0] && stream->window_switching[gr][2] == 2) { - if (stream->window_switching[gr][4]) { + if (stream->window_switching[gr][0] && stream->block_type[gr][0] == 2) { + if (stream->mixed_block_flag[gr][0]) { while (i++ < 36) { *(sp10++) = *(sp0c++); } From 43a91b2b4b5147291e2108ce0e62fbaa5617601c Mon Sep 17 00:00:00 2001 From: GreaseMonkey Date: Tue, 29 Aug 2023 13:11:58 +1200 Subject: [PATCH 2/2] Fix "mpindex=4" UB cases for various view gadgets --- src/game/bondview.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/game/bondview.c b/src/game/bondview.c index 2b24a6516..2ced8b44f 100644 --- a/src/game/bondview.c +++ b/src/game/bondview.c @@ -1952,6 +1952,9 @@ Gfx *bviewDrawNvLens(Gfx *gdl) s32 viewbottom = viewtop + viewheight; s32 brightness; s32 y; +#ifdef AVOID_UB + u32 mpindex = g_Vars.currentplayerstats->mpindex % MAX_PLAYERS; +#endif var8007f840++; @@ -1973,9 +1976,15 @@ Gfx *bviewDrawNvLens(Gfx *gdl) skySetOverexposure(brightness, brightness, brightness); } +#ifdef AVOID_UB + if (g_Menus[mpindex].curdialog == NULL) { + gdl = bviewDrawMotionBlur(gdl, 0x00ff0000, 0x60); + } +#else if (g_Menus[g_Vars.currentplayerstats->mpindex].curdialog == NULL) { gdl = bviewDrawMotionBlur(gdl, 0x00ff0000, 0x60); } +#endif gDPPipeSync(gdl++); @@ -2034,6 +2043,9 @@ Gfx *bviewDrawIrLens(Gfx *gdl) s32 viewcentrey; f32 viewheightf; s32 a0; +#ifdef AVOID_UB + u32 mpindex = g_Vars.currentplayerstats->mpindex % MAX_PLAYERS; +#endif #if VERSION < VERSION_NTSC_1_0 static s32 fsscanline = 0; @@ -2198,9 +2210,15 @@ Gfx *bviewDrawIrLens(Gfx *gdl) #endif } +#ifdef AVOID_UB + if (g_Menus[mpindex].curdialog == NULL) { + gdl = bviewDrawMotionBlur(gdl, 0xff000000, 0x40); + } +#else if (g_Menus[g_Vars.currentplayerstats->mpindex].curdialog == NULL) { gdl = bviewDrawMotionBlur(gdl, 0xff000000, 0x40); } +#endif return gdl; }