Misc fixes 1 (#1491)

* Some low-hanging UB

* document bug on z_file_nameset_NES.c

* ALIGNED on audio data

* document bug on decode

* document bugs on Message_Decode

* Format

* More ALIGNED

* Put the ALIGNED macro at the right like the other uses in the repo

* review

* fix Message Decode

(cherry picked from commit abfbc7068816756f7ea3fa4a36ee0fe664c54c72)

Co-authored-by: engineer124 <engineer124engineer124@gmail.com>

* remove comments

* Update src/overlays/gamestates/ovl_file_choose/z_file_choose_NES.c

Co-authored-by: engineer124 <47598039+engineer124@users.noreply.github.com>

* format

---------

Co-authored-by: engineer124 <engineer124engineer124@gmail.com>
Co-authored-by: engineer124 <47598039+engineer124@users.noreply.github.com>

include/z64environment.h

@@ -3,8 +3,10 @@
 
 #include "PR/ultratypes.h"
 #include "PR/os_message.h"
-#include "z64math.h"
+#include "color.h"
 #include "z64dma.h"
+#include "z64light.h"
+#include "z64math.h"
 #include "unk.h"
 
 struct GameOverContext;

src/audio/lib/data.c

@@ -1,6 +1,6 @@
 #include "global.h"
 
-s16 gLowPassFilterData[16 * 8] = {
+s16 gLowPassFilterData[16 * 8] ALIGNED(16) = {
     /* 0x0 */ 0,     0,     0,    32767, 0,    0,     0,     0,    // Identity filter (delta function)
     /* 0x1 */ 3854,  4188,  4398, 4469,  4398, 4188,  3854,  3416, // low-freq cutoff (more filtering)
     /* 0x2 */ 3415,  4314,  4915, 5126,  4915, 4314,  3415,  2351,
@@ -19,7 +19,7 @@ s16 gLowPassFilterData[16 * 8] = {
     /* 0xF */ 841,   -853,  863,  26829, 863,  -853,  841,   -820, // high-freq cutoff (less filtering)
 };
 
-s16 gHighPassFilterData[15 * 8] = {
+s16 gHighPassFilterData[15 * 8] ALIGNED(16) = {
     /* 0x0 */ -289,  -291,  -289,  30736, -289,  -291,  -289,  -290, // low-freq cutoff (less filtering)
     /* 0x1 */ -464,  -467,  -467,  29506, -467,  -467,  -464,  -463,
     /* 0x2 */ -662,  -670,  -672,  28101, -672,  -670,  -662,  -656,
@@ -38,7 +38,7 @@ s16 gHighPassFilterData[15 * 8] = {
 };
 
 // clang-format off
-s16 gBandStopFilterData[105 * 8] = {
+s16 gBandStopFilterData[105 * 8] ALIGNED(16) = {
     // Block 0
     /* 0x00 */ -43,   -716,  -1205, 28210, -1205, -716,  -43,   629,
     /* 0x01 */ 249,   -919,  -1819, 25489, -1819, -919,  249,   1301,
@@ -173,7 +173,7 @@ s16 gBandStopFilterData[105 * 8] = {
     /* 0x68 */ 434,   -453,  467,   29652, 467,   -453,  434,   -405,
 };
 
-s16 gBandPassFilterData[105 * 8] = {
+s16 gBandPassFilterData[105 * 8] ALIGNED(16) = {
     // Block 0
     /* 0x00 */ 687,   4058,  6599,  7544,  6599,  4058,  687,   -2532,
     /* 0x01 */ -1303, 2777,  6337,  7745,  6337,  2777,  -1303, -4185,
@@ -308,7 +308,7 @@ s16 gBandPassFilterData[105 * 8] = {
     /* 0x68 */ -3854, 4188,  -4398, 4469,  -4398, 4188,  -3854, 3415,
 };
 
-s16 gSawtoothWaveSample[] = {
+s16 gSawtoothWaveSample[] ALIGNED(16) = {
     // 1st Harmonic
     0,      1023,   2047,   3071,   4095,   5119,   6143,   7167,
     8191,   9215,   10239,  11263,  12287,  13311,  14335,  15359,
@@ -350,7 +350,7 @@ s16 gSawtoothWaveSample[] = {
     0,      8191,   16383,  24575,  -32767, -24575, -16383, -8191,
 };
 
-s16 gTriangleWaveSample[] = {
+s16 gTriangleWaveSample[] ALIGNED(16) = {
     // 1st Harmonic
     0,      2047,   4095,   6143,   8191,   10239,  12287,  14335,
     16383,  18431,  20479,  22527,  24575,  26623,  28671,  30719,
@@ -392,7 +392,7 @@ s16 gTriangleWaveSample[] = {
     0,      16383,  32767,  16383,  0,      -16383, -32767, -16383,
 };
 
-s16 gSineWaveSample[] = {
+s16 gSineWaveSample[] ALIGNED(16) = {
     // 1st Harmonic
     0,      3211,   6392,   9511,   12539,  15446,  18204,  20787,
     23169,  25329,  27244,  28897,  30272,  31356,  32137,  32609,
@@ -434,7 +434,7 @@ s16 gSineWaveSample[] = {
     0,      23169,  32767,  23169,  0,      -23169, -32767, -23169,
 };
 
-s16 gSquareWaveSample[] = {
+s16 gSquareWaveSample[] ALIGNED(16) = {
     // 1st Harmonic
     0,      0,      0,      0,      0,      0,      0,      0,
     0,      0,      0,      0,      0,      0,      0,      0,
@@ -476,7 +476,7 @@ s16 gSquareWaveSample[] = {
     0,      0,      32767,  32767,  0,      0,      -32767, -32767,
 };
 
-s16 gWhiteNoiseSample[] = {
+s16 gWhiteNoiseSample[] ALIGNED(16) = {
     // 1st Harmonic
     0,      -25689, -25791, 27803,  -27568, -21030, 22174,  6298,
     27071,  -18531, 28649,  2284,   3380,   6890,   -12682, -21114,
@@ -519,7 +519,7 @@ s16 gWhiteNoiseSample[] = {
 };
 
 // Sine White Noise?
-s16 D_801D4790[] = {
+s16 D_801D4790[] ALIGNED(16) = {
     // 1st Harmonic
     0,      16316,  20148,  20257,  27209,  -32657, 29264,  27259,
     -29394, -21494, -26410, 30770,  30033,  29130,  20206,  14129,
@@ -562,7 +562,7 @@ s16 D_801D4790[] = {
 };
 
 // Pulse Wave (duty cycle = 12.5%)
-s16 gEighthPulseWaveSample[] = {
+s16 gEighthPulseWaveSample[] ALIGNED(16) = {
     // 1st Harmonic
     0,      0,      0,      0,      0,      0, 0,      0,
     0,      0,      0,      0,      0,      0, 0,      0,
@@ -605,7 +605,7 @@ s16 gEighthPulseWaveSample[] = {
 };
 
 // Pulse Wave (duty cycle = 25%)
-s16 gQuarterPulseWaveSample[] = {
+s16 gQuarterPulseWaveSample[] ALIGNED(16) = {
     // 1st Harmonic
     0,      0,      0,      0,      0,      0,      0,      0,
     0,      0,      0,      0,      0,      0,      0,      0,

src/boot/yaz0.c

@@ -1,7 +1,7 @@
 #include "global.h"
 #include "fault.h"
 
-u8 sYaz0DataBuffer[0x400];
+u8 sYaz0DataBuffer[0x400] ALIGNED(16);
 u8* sYaz0CurDataEnd;
 uintptr_t sYaz0CurRomStart;
 u32 sYaz0CurSize;

src/code/z_common_data.c

@@ -1,6 +1,12 @@
-#include "global.h"
+#include "z64save.h"
 
-SaveContext gSaveContext;
+#include "alignment.h"
+#include "sequence.h"
+#include "libc/stdbool.h"
+#include "z64environment.h"
+#include "z64transition.h"
+
+SaveContext gSaveContext ALIGNED(16);
 
 void SaveContext_Init(void) {
     bzero(&gSaveContext, sizeof(SaveContext));

src/code/z_message.c

@@ -1921,7 +1921,7 @@ void func_8014CCB4(PlayState* play, s16* decodedBufPos, s32* offset, f32* arg3)
  * every digit will be added 0x824F to get an actual S-JIS
  * printable character.
  */
-void Message_GetTimerDigits(OSTime time, s16* digits) {
+void Message_GetTimerDigits(OSTime time, s16 digits[8]) {
     OSTime t = time;
 
     // 6 minutes
@@ -2160,20 +2160,20 @@ void Message_Decode(PlayState* play) {
     u32 timeToMoonCrash;
     s16 var_v0;
     s16 numLines;
-    u8* fontBuf;
+    s16 value;
-    s16 digits[4];
+    s16 digits[5];
     s16 spD2;
     f32 timeInSeconds;
     s32 charTexIndex;
-    f32 var_fs0;
+    u8* fontBuf;
     f32 spC0;
     s16 index;
-    s16 value;
     s16 playerNameLen;
+    s16 spAC[8];
+    f32 var_fs0;
     s16 i;
-    u16 index2 = 0;
-    s16 spAC[4];
     u16 curChar;
+    u8 index2 = 0;
 
     msgCtx->textDelayTimer = 0;
     msgCtx->textDelay = msgCtx->textDelayTimer;
@@ -2212,7 +2212,7 @@ void Message_Decode(PlayState* play) {
                         }
                     }
                 } else {
-                    s32 requiredScopeTemp;
+                    s8 requiredScopeTemp;
 
                     if ((msgCtx->textBoxType != TEXTBOX_TYPE_3) && (msgCtx->textBoxType != TEXTBOX_TYPE_4)) {
                         if (numLines == 0) {
@@ -2289,7 +2289,7 @@ void Message_Decode(PlayState* play) {
                 Message_GetTimerDigits(((void)0, gSaveContext.timerCurTimes[curChar - 0x204]), spAC);
 
                 loadChar = false;
-                for (i = 0; i < 5; i++) {
+                for (i = 0; i < ARRAY_COUNT(spAC) - 3; i++) {
                     if ((i == 1) || (spAC[i + 3] != 0)) {
                         loadChar = true;
                     }
@@ -2304,7 +2304,7 @@ void Message_Decode(PlayState* play) {
                 Message_GetTimerDigits(((void)0, gSaveContext.timerCurTimes[curChar - 0x204]), spAC);
 
                 loadChar = false;
-                for (i = 0; i < 8; i++) {
+                for (i = 0; i < ARRAY_COUNT(spAC); i++) {
                     if ((i == 4) || ((i != 2) && (i != 5) && (spAC[i] != '\0'))) {
                         loadChar = true;
                     }
@@ -2722,7 +2722,7 @@ void Message_Decode(PlayState* play) {
                 }
                 func_8014CCB4(play, &decodedBufPos, &charTexIndex, &spC0);
             } else if (curChar == 0x22F) {
-                for (i = 0; i < 5; i++) {
+                for (i = 0; i < ARRAY_COUNT(gSaveContext.save.saveInfo.bomberCode); i++) {
                     digits[i] = gSaveContext.save.saveInfo.bomberCode[i];
                     Font_LoadChar(play, digits[i] + 0x824F, charTexIndex);
                     charTexIndex += FONT_CHAR_TEX_SIZE;
@@ -2827,7 +2827,7 @@ void Message_Decode(PlayState* play) {
                 }
 
                 loadChar = false;
-                for (i = 0; i < 8; i++) {
+                for (i = 0; i < ARRAY_COUNT(spAC); i++) {
                     if ((i == 4) || ((i != 2) && (i != 5) && (spAC[i] != '\0'))) {
                         loadChar = true;
                     }

src/code/z_parameter.c

@@ -5441,7 +5441,7 @@ void Interface_StartMoonCrash(PlayState* play) {
     play->transitionType = TRANS_TYPE_FADE_WHITE;
 }
 
-void Interface_GetTimerDigits(OSTime time, s16* timerArr) {
+void Interface_GetTimerDigits(OSTime time, s16 timerArr[8]) {
     OSTime t = time;
 
     // 6 minutes

src/code/z_player_lib.c

@@ -2036,7 +2036,7 @@ s32 Player_OverrideLimbDrawGameplayCommon(PlayState* play, s32 limbIndex, Gfx**
         // Note: The increment would not be done for the root limb, even if it had a non-NULL `dList`.
         //       So if the root limb had a non-NULL `dList` (which is not the case in vanilla),
         //       an out-of-bounds write to `bodyPartsPos` would occur.
-        sPlayerCurBodyPartPos = &player->bodyPartsPos[-1];
+        sPlayerCurBodyPartPos = &player->bodyPartsPos[0] - 1;
 
         if (player->transformation != PLAYER_FORM_FIERCE_DEITY) {
             if (!(player->skelAnime.moveFlags & ANIM_FLAG_4) || (player->skelAnime.moveFlags & ANIM_FLAG_1)) {

src/libultra/io/motor.c

@@ -1,11 +1,12 @@
 #include "ultra64.h"
 #include "PR/os_motor.h"
 #include "PR/controller.h"
+#include "alignment.h"
 
 #define BANK_ADDR 0x400
 #define MOTOR_ID 0x80
 
-OSPifRam __MotorDataBuf[MAXCONTROLLERS];
+OSPifRam __MotorDataBuf[MAXCONTROLLERS] ALIGNED(16);
 
 s32 __osPfsSelectBank(OSPfs* pfs, u8 bank);
 

src/overlays/actors/ovl_En_Dnh/z_en_dnh.c

@@ -110,7 +110,7 @@ s32 func_80A50E40(EnDnh* this, PlayState* play) {
     return 1;
 }
 
-s32 func_80A50EC0(EnDnh* this) {
+void func_80A50EC0(EnDnh* this) {
     if (DECR(this->blinkTimer) == 0) {
         this->eyeTexIndex++;
         if (this->eyeTexIndex >= ARRAY_COUNT(sEyeTextures)) {

src/overlays/actors/ovl_En_Tanron2/z_en_tanron2.c

@@ -261,6 +261,8 @@ void func_80BB6BD8(EnTanron2* this, PlayState* play) {
                     }
                     break;
             }
+
+            //! @bug: sp32 may be used uninitialized
             Matrix_RotateYS(sp32, MTXMODE_NEW);
             Matrix_MultVecZ(this->actor.speed, &this->actor.velocity);
             this->actor.velocity.y = Rand_ZeroFloat(5.0f) + 12.0f;

src/overlays/actors/ovl_En_Test3/z_en_test3.c

@@ -274,7 +274,7 @@ s32 D_80A41D5C;
 s32 D_80A41D60;
 s32 D_80A41D64;
 s32 D_80A41D68;
-Vec3f* D_80A41D6C;
+Vec3f* sKafeiCurBodyPartPos;
 
 s32 func_80A3E7E0(EnTest3* this, EnTest3ActionFunc actionFunc) {
     if (actionFunc == this->unk_D94) {
@@ -1098,7 +1098,7 @@ s32 EnTest3_OverrideLimbDraw(PlayState* play, s32 limbIndex, Gfx** dList, Vec3f*
     EnTest3* this = THIS;
 
     if (limbIndex == KAFEI_LIMB_ROOT) {
-        D_80A41D6C = &this->player.bodyPartsPos[-1];
+        sKafeiCurBodyPartPos = &this->player.bodyPartsPos[0] - 1;
         if (!(this->player.skelAnime.moveFlags & ANIM_FLAG_4) || (this->player.skelAnime.moveFlags & ANIM_FLAG_1)) {
             pos->x *= this->player.ageProperties->unk_08;
             pos->z *= this->player.ageProperties->unk_08;
@@ -1118,7 +1118,7 @@ s32 EnTest3_OverrideLimbDraw(PlayState* play, s32 limbIndex, Gfx** dList, Vec3f*
         }
     } else {
         if (*dList != NULL) {
-            D_80A41D6C++;
+            sKafeiCurBodyPartPos++;
         }
         if (D_80A418C8) {
             *dList = NULL;
@@ -1153,13 +1153,13 @@ void EnTest3_PostLimbDraw(PlayState* play, s32 limbIndex, Gfx** dList1, Gfx** dL
     EnTest3* this = THIS;
 
     if (*dList2 != NULL) {
-        Matrix_MultZero(D_80A41D6C);
+        Matrix_MultZero(sKafeiCurBodyPartPos);
     }
     if (limbIndex == KAFEI_LIMB_LEFT_HAND) {
         MtxF curMtxF;
         Actor* leftHandActor;
 
-        Math_Vec3f_Copy(&this->player.leftHandWorld.pos, D_80A41D6C);
+        Math_Vec3f_Copy(&this->player.leftHandWorld.pos, sKafeiCurBodyPartPos);
         if (*dList1 != NULL) {
             func_80128640(play, &this->player, *dList1);
             if (this->player.stateFlags3 & PLAYER_STATE3_20000000) {

src/overlays/gamestates/ovl_file_choose/z_file_choose_NES.c

@@ -2452,9 +2452,9 @@ void FileSelect_InitContext(GameState* thisx) {
             this->nameBoxAlpha[2] = this->nameAlpha[0] = this->nameAlpha[1] = this->nameAlpha[2] =
                 this->connectorAlpha[0] = this->connectorAlpha[1] = this->connectorAlpha[2] = this->fileInfoAlpha[0] =
                     this->fileInfoAlpha[1] = this->fileInfoAlpha[2] = this->actionButtonAlpha[FS_BTN_ACTION_COPY] =
-                        this->actionButtonAlpha[FS_BTN_ACTION_ERASE] = this->actionButtonAlpha[2] =
+                        this->actionButtonAlpha[FS_BTN_ACTION_ERASE] = this->confirmButtonAlpha[FS_BTN_CONFIRM_YES] =
-                            this->actionButtonAlpha[3] = this->optionButtonAlpha = this->nameEntryBoxAlpha =
+                            this->confirmButtonAlpha[FS_BTN_CONFIRM_QUIT] = this->optionButtonAlpha =
-                                this->controlsAlpha = this->emptyFileTextAlpha = 0;
+                                this->nameEntryBoxAlpha = this->controlsAlpha = this->emptyFileTextAlpha = 0;
 
     this->windowPosX = 6;
     this->actionTimer = 4;

src/overlays/gamestates/ovl_file_choose/z_file_nameset_NES.c

@@ -1027,6 +1027,8 @@ void FileSelect_DrawOptionsImpl(GameState* thisx) {
             gDPSetEnvColor(POLY_OPA_DISP++, 0, 0, 0, 255);
         }
 
+        //! @bug the gOptionsMenuHeaders usage here will produce an OoB read for i == 5. It reads the first element of
+        //! `gOptionsMenuSettings`
         gDPLoadTextureBlock(POLY_OPA_DISP++, gOptionsMenuSettings[i].texture, G_IM_FMT_IA, G_IM_SIZ_8b,
                             gOptionsMenuSettings[i].width, gOptionsMenuHeaders[i].height, 0, G_TX_NOMIRROR | G_TX_WRAP,
                             G_TX_NOMIRROR | G_TX_WRAP, G_TX_NOMASK, G_TX_NOMASK, G_TX_NOLOD, G_TX_NOLOD);

tools/disasm/variables.txt

@@ -11302,7 +11302,7 @@
     0x80A41D60:("D_80A41D60","UNK_TYPE1","",0x1),
     0x80A41D64:("D_80A41D64","UNK_TYPE1","",0x1),
     0x80A41D68:("D_80A41D68","UNK_TYPE1","",0x1),
-    0x80A41D6C:("D_80A41D6C","UNK_TYPE1","",0x1),
+    0x80A41D6C:("sKafeiCurBodyPartPos","UNK_TYPE1","",0x1),
     0x80A43320:("En_Test4_InitVars","ActorInit","",0x20),
     0x80A43340:("sIsLoaded","s32","[3]",0x6),
     0x80A43342:("sNightMessages1","s16","[3]",0x6),