From 220f48a98bf44365b8d45cdfb5d48e0080fe92d6 Mon Sep 17 00:00:00 2001 From: Clownacy Date: Mon, 6 Jul 2020 15:10:01 +0100 Subject: [PATCH] Backport some undefined-behaviour fixes --- src/Bullet.cpp | 7 +++- src/Caret.cpp | 97 +++++++++++++++++++++++++++++++++-------------- src/MycParam.cpp | 10 ++++- src/NpcAct120.cpp | 31 +++++++++++---- src/NpcAct140.cpp | 4 +- src/NpcAct180.cpp | 6 ++- 6 files changed, 114 insertions(+), 41 deletions(-) diff --git a/src/Bullet.cpp b/src/Bullet.cpp index c6d3fb4d..fde280c0 100644 --- a/src/Bullet.cpp +++ b/src/Bullet.cpp @@ -1641,7 +1641,12 @@ void ActBullet_Edge(BULLET *bul) bul->damage = 1; if (bul->ani_no > 4) + { bul->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rcLeft' and 'rcRight', even though it's now too high + #endif + } break; } @@ -1662,8 +1667,6 @@ void ActBullet_Edge(BULLET *bul) {96, 88, 120, 112}, }; - // Note that 'bul->ani_no' can exceed the size of 'rcLeft' and 'rcRight' - if (bul->direct == 0) bul->rect = rcLeft[bul->ani_no]; else diff --git a/src/Caret.cpp b/src/Caret.cpp index 8d44da4e..e34a89ec 100644 --- a/src/Caret.cpp +++ b/src/Caret.cpp @@ -71,11 +71,15 @@ void ActCaret01(CARET *crt) if (++crt->ani_wait > 5) { crt->ani_wait = 0; - if (++crt->ani_no > 3) - crt->cond = 0; - } - // Note that 'crt->ani_no' can exceed the size of 'rcLeft' and 'rcRight' + if (++crt->ani_no > 3) + { + crt->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rcLeft' and 'rcRight', even though it's now too high + #endif + } + } if (crt->direct == 0) crt->rect = rcLeft[crt->ani_no]; @@ -118,9 +122,12 @@ void ActCaret02(CARET *crt) } if (crt->ani_no > 3) + { crt->cond = 0; - - // Note that 'crt->ani_no' can exceed the size of 'rect_left' + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rect_left', even though it's now too high + #endif + } crt->rect = rect_left[crt->ani_no]; break; @@ -133,9 +140,12 @@ void ActCaret02(CARET *crt) } if (crt->ani_no > 3) + { crt->cond = 0; - - // Note that 'crt->ani_no' can exceed the size of 'rect_right' + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rect_right', even though it's now too high + #endif + } crt->rect = rect_right[crt->ani_no]; break; @@ -162,11 +172,15 @@ void ActCaret03(CARET *crt) if (++crt->ani_wait > 2) { crt->ani_wait = 0; - if (++crt->ani_no > 3) - crt->cond = 0; - } - // Note that 'crt->ani_no' can exceed the size of 'rect' + if (++crt->ani_no > 3) + { + crt->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rect', even though it's now too high + #endif + } + } crt->rect = rect[crt->ani_no]; } @@ -177,9 +191,11 @@ void ActCaret04(CARET *crt) {64, 32, 80, 48}, {80, 32, 96, 48}, {96, 32, 112, 48}, + {64, 48, 80, 64}, {80, 48, 96, 64}, {96, 48, 112, 64}, + {64, 64, 80, 80}, {80, 64, 96, 80}, {96, 64, 112, 80}, @@ -190,7 +206,12 @@ void ActCaret04(CARET *crt) crt->ani_wait = 0; if (++crt->ani_no > 2) + { crt->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rect', even though it's now too high + #endif + } } crt->rect = rect[(crt->direct * 3) + crt->ani_no]; @@ -215,13 +236,16 @@ void ActCaret05(CARET *crt) } if (crt->ani_no > 6) + { crt->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rect', even though it's now too high + #endif + } crt->x += 0x80; crt->y -= 0x80; - // Note that 'crt->ani_no' can exceed the size of 'rect' - crt->rect = rect[crt->ani_no]; } @@ -242,11 +266,14 @@ void ActCaret07(CARET *crt) crt->ani_wait = 0; if (++crt->ani_no > 6) + { crt->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rcLeft', even though it's now too high + #endif + } } - // Note that 'crt->ani_no' can exceed the size of rcLeft - crt->rect = rcLeft[crt->ani_no]; switch (crt->direct) @@ -359,11 +386,15 @@ void ActCaret11(CARET *crt) if (++crt->ani_wait > 2) { crt->ani_wait = 0; - if (++crt->ani_no > 6) - crt->cond = 0; - } - // Note that 'crt->ani_no' can exceed the size of 'rcRight' + if (++crt->ani_no > 6) + { + crt->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rcRight', even though it's now too high + #endif + } + } crt->rect = rcRight[crt->ani_no]; } @@ -378,11 +409,15 @@ void ActCaret12(CARET *crt) if (++crt->ani_wait > 2) { crt->ani_wait = 0; - if (++crt->ani_no > 1) - crt->cond = 0; - } - // Note that 'crt->ani_no' can exceed the size of 'rcLeft' + if (++crt->ani_no > 1) + { + crt->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rcLeft', even though it's now too high + #endif + } + } crt->rect = rcLeft[crt->ani_no]; } @@ -446,11 +481,14 @@ void ActCaret14(CARET *crt) crt->ani_wait = 0; if (++crt->ani_no > 4) + { crt->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rect', even though it's now too high + #endif + } } - // Note that 'crt->ani_no' can exceed the size of 'rect' - crt->rect = rect[crt->ani_no]; } @@ -468,11 +506,14 @@ void ActCaret15(CARET *crt) crt->ani_wait = 0; if (++crt->ani_no > 3) + { crt->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rcLeft', even though it's now too high + #endif + } } - // Note that 'crt->ani_no' can exceed the size of 'rcLeft' - crt->rect = rcLeft[crt->ani_no]; } diff --git a/src/MycParam.cpp b/src/MycParam.cpp index 76f8f55a..04cfe6f7 100644 --- a/src/MycParam.cpp +++ b/src/MycParam.cpp @@ -270,8 +270,16 @@ void PutArmsEnergy(BOOL flash) RECT rcExpMax = {40, 72, 80, 80}; RECT rcExpFlash = {40, 80, 80, 88}; - // Note that this can result in '-1', causing the following array accesses to be out-of-bounds int lv = gArmsData[gSelectedArms].level - 1; + +#ifdef FIX_BUGS + // When the player has no weapons, the default level is 0, which becomes -1. + // Catch it, and set it to 0 instead, so the following array-accesses aren't + // out-of-bounds. + if (lv < 0) + lv = 0; +#endif + int arms_code = gArmsData[gSelectedArms].code; int exp_now = gArmsData[gSelectedArms].exp; int exp_next = gArmsLevelTable[arms_code].exp[lv]; diff --git a/src/NpcAct120.cpp b/src/NpcAct120.cpp index df019784..801cbf0b 100644 --- a/src/NpcAct120.cpp +++ b/src/NpcAct120.cpp @@ -585,11 +585,15 @@ void ActNpc127(NPCHAR *npc) if (++npc->ani_wait > 0) { npc->ani_wait = 0; - if (++npc->ani_no > 2) - npc->cond = 0; - } - // Note that 'npc->ani_no' can exceed the size of 'rcH' and 'rcV' + if (++npc->ani_no > 2) + { + npc->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rcH' and 'rcV', even though it's now too high + #endif + } + } if (npc->direct == 0) npc->rect = rcH[npc->ani_no]; @@ -649,9 +653,12 @@ void ActNpc128(NPCHAR *npc) } if (++npc->ani_no > 4) + { npc->cond = 0; - - // Note that 'npc->ani_no' can exceed the bounds of 'rcLeft', 'rcUp', 'rcRight' and 'rcDown' + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rcLeft' and co., even though it's now too high + #endif + } switch (npc->direct) { @@ -680,18 +687,23 @@ void ActNpc129(NPCHAR *npc) {0x80, 0x30, 0x90, 0x40}, {0x90, 0x30, 0xA0, 0x40}, {0xA0, 0x30, 0xB0, 0x40}, + {0x80, 0x40, 0x90, 0x50}, {0x90, 0x40, 0xA0, 0x50}, {0xA0, 0x40, 0xB0, 0x50}, + {0x80, 0x50, 0x90, 0x60}, {0x90, 0x50, 0xA0, 0x60}, {0xA0, 0x50, 0xB0, 0x60}, + {0xB0, 0x30, 0xC0, 0x40}, {0xC0, 0x30, 0xD0, 0x40}, {0xD0, 0x30, 0xE0, 0x40}, + {0xB0, 0x40, 0xC0, 0x50}, {0xC0, 0x40, 0xD0, 0x50}, {0xD0, 0x40, 0xE0, 0x50}, + {0xB0, 0x50, 0xC0, 0x60}, {0xC0, 0x50, 0xD0, 0x60}, {0xD0, 0x50, 0xE0, 0x60}, @@ -702,13 +714,16 @@ void ActNpc129(NPCHAR *npc) npc->ani_wait = 0; if (++npc->ani_no > 2) + { npc->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rect', even though it's now too high + #endif + } } npc->y += npc->ym; - // Note that '(npc->direct * 3) + npc->ani_no' can exceed the size of 'rect' - npc->rect = rect[(npc->direct * 3) + npc->ani_no]; } diff --git a/src/NpcAct140.cpp b/src/NpcAct140.cpp index 9d05b289..1aae6938 100644 --- a/src/NpcAct140.cpp +++ b/src/NpcAct140.cpp @@ -753,12 +753,14 @@ void ActNpc146(NPCHAR *npc) { SetDestroyNpChar(npc->x, npc->y, 0x1000, 8); npc->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rect', even though it's now too high + #endif } break; } - // Note that 'npc->ani_no' can exceed the size of 'rect' npc->rect = rect[npc->ani_no]; } diff --git a/src/NpcAct180.cpp b/src/NpcAct180.cpp index 372b3563..cfb426ab 100644 --- a/src/NpcAct180.cpp +++ b/src/NpcAct180.cpp @@ -1426,11 +1426,15 @@ void ActNpc199(NPCHAR *npc) } if (npc->ani_no > 4) + { npc->cond = 0; + #ifdef FIX_BUGS + return; // The code below will use 'ani_no' to access 'rect', even though it's now too high + #endif + } npc->x += npc->xm; npc->y += npc->ym; - // Note that 'npc->ani_no' can exceed the size of 'rect' npc->rect = rect[npc->ani_no]; }