Fixes known undefined behaviour from DmaMgr and Lib_Ptr taking u32 rather than void*

6 files changed, 53 insertions, 43 deletions

diff --git a/src/boot_O2_g3/z_std_dma.c b/src/boot_O2_g3/z_std_dma.c
index 692c44607..493041b6d 100644
--- a/src/boot_O2_g3/z_std_dma.c
+++ b/src/boot_O2_g3/z_std_dma.c
@@ -159,13 +159,13 @@ void Dmamgr_ThreadEntry(void* a0) {
     }
 }
 
-s32 DmaMgr_SendRequestImpl(DmaRequest* request, u32 vramStart, u32 vromStart, u32 size, UNK_TYPE4 unused, OSMesgQueue* callback, void* callbackMesg) {
+s32 DmaMgr_SendRequestImpl(DmaRequest* request, void* vramStart, u32 vromStart, u32 size, UNK_TYPE4 unused, OSMesgQueue* callback, void* callbackMesg) {
     if (gIrqMgrResetStatus >= 2) {
         return -2;
     }
 
     request->vromStart = vromStart;
-    request->dramAddr = (void*)vramStart;
+    request->dramAddr = vramStart;
     request->size = size;
     request->unk14 = 0;
     request->notifyQueue = callback;
@@ -176,7 +176,7 @@ s32 DmaMgr_SendRequestImpl(DmaRequest* request, u32 vramStart, u32 vromStart, u3
     return 0;
 }
 
-s32 DmaMgr_SendRequest0(u32 a0, u32 a1, u32 a2) {
+s32 DmaMgr_SendRequest0(void* vramStart, u32 vromStart, u32 size) {
 	DmaRequest sp48;
     OSMesgQueue sp30;
     OSMesg sp2C;
@@ -184,7 +184,7 @@ s32 DmaMgr_SendRequest0(u32 a0, u32 a1, u32 a2) {
 
     osCreateMesgQueue(&sp30, &sp2C, 1);
 
-	ret = DmaMgr_SendRequestImpl(&sp48, a0, a1, a2, 0, &sp30, 0);
+	ret = DmaMgr_SendRequestImpl(&sp48, vramStart, vromStart, size, 0, &sp30, 0);
 
 	if (ret == -1) {
 		return ret;
diff --git a/src/code/z_actor.c b/src/code/z_actor.c
index 569590e24..2a7abb458 100644
--- a/src/code/z_actor.c
+++ b/src/code/z_actor.c
@@ -215,7 +215,9 @@ void Actor_SetScale(Actor* actor, f32 scale) {
 }
 
 void Actor_SetObjectSegment(GlobalContext* ctxt, Actor* actor) {
-    gRspSegmentPhysAddrs[6] = (u32) ctxt->sceneContext.objects[actor->objBankIndex].vramAddr + 0x80000000;
+    // TODO: PHYSICAL_TO_VIRTUAL macro
+    // TODO: Segment number enum
+    gRspSegmentPhysAddrs[6] = (void*)((u32)ctxt->sceneContext.objects[actor->objBankIndex].vramAddr + 0x80000000);
 }
 
 #ifdef NON_MATCHING
diff --git a/src/code/z_kanfont.c b/src/code/z_kanfont.c
index c8c8621b7..1c3631498 100644
--- a/src/code/z_kanfont.c
+++ b/src/code/z_kanfont.c
@@ -4,26 +4,29 @@
 void Kanfont_Nop800F4F40(GlobalContext* ctxt, UNK_TYPE param_2, UNK_TYPE param_3) {}
 
 void Kanfont_LoadAsciiChar(GlobalContext* ctxt, u8 character, s32 iParm3) {
-    DmaMgr_SendRequest0((u32)ctxt->msgCtx.font.unk0[(ctxt->msgCtx).unk11EF0] + iParm3,
+    // UB to convert pointer to u32
+    DmaMgr_SendRequest0((void*)((u32)&ctxt->msgCtx.font.unk0[(ctxt->msgCtx).unk11EF0] + iParm3),
         (u32)&nes_font_static_vrom_start + character * 0x80 - 0x1000,
         0x80);
 }
 
 void Kanfont_LoadMessageBoxEnd(Font* font, u16 type) {
-    DmaMgr_SendRequest0((u32)font->unk7800, type * 0x80 + (u32)&message_static_vrom_start + 0x5000, 0x80);
+    // UB to convert pointer to u32
+    DmaMgr_SendRequest0(&font->unk7800[0][0], type * 0x80 + (u32)&message_static_vrom_start + 0x5000, 0x80);
 }
 
 void Kanfont_LoadOrderedFont(Font* font) {
     u32 loadOffset;
     s32 codePointIndex = 0;
-    u32 writeLocation;
+    void* writeLocation;
 
     while (1) {
-        writeLocation = (u32)&font->unk7800[codePointIndex + 1];
+        writeLocation = &font->unk7800[codePointIndex + 1];
         loadOffset = kanfontOrdering[codePointIndex] * 128;
         if (kanfontOrdering[codePointIndex] == 0) {
             loadOffset = 0;
         }
+        // UB to convert pointer to u32
         DmaMgr_SendRequest0(writeLocation, (u32)&nes_font_static_vrom_start + loadOffset, 0x80);
         if (kanfontOrdering[codePointIndex] == 140) break;
         codePointIndex++;
diff --git a/src/code/z_lib.c b/src/code/z_lib.c
index b30941ae5..f4209e98b 100644
--- a/src/code/z_lib.c
+++ b/src/code/z_lib.c
@@ -606,30 +606,37 @@ f32 Lib_PushAwayVec3f(Vec3f* start, Vec3f* pusher, f32 distanceToApproach) {
 
 void Lib_Nop801004FC(void) {}
 
-u32 Lib_PtrSegToVirt(void* ptr) {
-    return(gRspSegmentPhysAddrs[((u32)ptr << 4) >> 28] + ((u32)ptr & 0xFFFFFF)) + 0x80000000;
+void* Lib_PtrSegToVirt(void* ptr) {
+    // TODO: PHYSICAL_TO_VIRTUAL macro
+    // UB to cast the pointer to u32
+    return (void*)(((u32)gRspSegmentPhysAddrs[((u32)ptr << 4) >> 28] + ((u32)ptr & 0xFFFFFF)) + 0x80000000);
 }
 
-u32 Lib_PtrSegToVirtNull(void* ptr) {
+void* Lib_PtrSegToVirtNull(void* ptr) {
+    // UB to cast the pointer to u32 in order to bitshift.
     if (((u32)ptr >> 28) == 0) {
-        return (u32)ptr;
+        return ptr;
     }
 
-    return(gRspSegmentPhysAddrs[((u32)ptr << 4) >> 28] + ((u32)ptr & 0xFFFFFF)) + 0x80000000;
+    // TODO: PHYSICAL_TO_VIRTUAL macro
+    // UB to cast the pointer to u32
+    return (void*)(((u32)gRspSegmentPhysAddrs[((u32)ptr << 4) >> 28] + ((u32)ptr & 0xFFFFFF)) + 0x80000000);
 }
 
-u32 Lib_PtrSegToK0(void* ptr) {
+void* Lib_PtrSegToK0(void* ptr) {
     if (ptr == NULL) {
-        return 0;
+        return NULL;
     } else {
-        return (u32)ptr + 0x80000000;
+        // TODO: PHYSICAL_TO_VIRTUAL macro
+        return (void*)((u32)ptr + 0x80000000);
     }
 }
 
-u32 Lib_PtrSegToK0Null(void* ptr) {
+void* Lib_PtrSegToK0Null(void* ptr) {
     if (ptr == NULL) {
-        return 0;
+        return NULL;
     } else {
-        return (u32)ptr + 0x80000000;
+        // TODO: PHYSICAL_TO_VIRTUAL macro
+        return (void*)((u32)ptr + 0x80000000);
     }
 }
\ No newline at end of file
diff --git a/src/code/z_room.c b/src/code/z_room.c
index 7d3ddd171..5e8e6d42e 100644
--- a/src/code/z_room.c
+++ b/src/code/z_room.c
@@ -86,7 +86,7 @@ s32 Room_StartRoomTransition(GlobalContext* ctxt, RoomContext* roomCtxt, s32 ind
         roomCtxt->unk31 = 1;
 
         size = ctxt->roomAddrs[index].vromEnd - ctxt->roomAddrs[index].vromStart;
-        roomCtxt->activeRoomVram = ((s32)roomCtxt->roomMemPages[roomCtxt->activeMemPage] - (size + 8) * roomCtxt->activeMemPage + 8) & 0xfffffff0;
+        roomCtxt->activeRoomVram = (void*)((s32)roomCtxt->roomMemPages[roomCtxt->activeMemPage] - (size + 8) * roomCtxt->activeMemPage + 8) & 0xfffffff0;
 
         osCreateMesgQueue(&roomCtxt->loadQueue, roomCtxt->loadMsg, 1);
         DmaMgr_SendRequestImpl(&roomCtxt->dmaRequest, roomCtxt->activeRoomVram, ctxt->roomAddrs[index].vromStart, size,
@@ -108,8 +108,10 @@ s32 Room_HandleLoadCallbacks(GlobalContext* ctxt, RoomContext* roomCtxt) {
         if (!osRecvMesg(&roomCtxt->loadQueue, NULL, OS_MESG_NOBLOCK))
         {
             roomCtxt->unk31 = 0;
-            roomCtxt->currRoom.segment = (void*)(roomCtxt->activeRoomVram);
-            gRspSegmentPhysAddrs[3] = roomCtxt->activeRoomVram + 0x80000000;
+            roomCtxt->currRoom.segment = roomCtxt->activeRoomVram;
+            // TODO: PHYSICAL_TO_VIRTUAL macro
+            // TODO: Segment number enum
+            gRspSegmentPhysAddrs[3] = (void*)((u32)roomCtxt->activeRoomVram + 0x80000000);
 
             Scene_ProcessHeader(ctxt, (SceneCmd*)roomCtxt->currRoom.segment);
             func_80123140(ctxt, (ActorPlayer*)ctxt->actorCtx.actorList[2].first);
@@ -133,7 +135,9 @@ s32 Room_HandleLoadCallbacks(GlobalContext* ctxt, RoomContext* roomCtxt) {
 
 void Room_Draw(GlobalContext* ctxt, Room* room, u32 flags) {
     if (room->segment != NULL) {
-        gRspSegmentPhysAddrs[3] = (u32)room->segment + 0x80000000;
+        // TODO: PHYSICAL_TO_VIRTUAL macro
+        // TODO: Segment number enum
+        gRspSegmentPhysAddrs[3] = (void*)((u32)room->segment + 0x80000000);
         roomDrawFuncs[room->mesh->type0.type](ctxt, room, flags);
     }
     return;
diff --git a/src/code/z_scene.c b/src/code/z_scene.c
index b2e688e5b..79422d7ae 100644
--- a/src/code/z_scene.c
+++ b/src/code/z_scene.c
@@ -2,14 +2,13 @@
 #include <global.h>
 
 /* 
-TODO: There are a few issues left with this file, but rely on larger structural project changes.
+TODO:
+There are a few issues left with this file, but many rely on larger structural project changes.
 I am avoiding these in the mean time in order to not break the Ghidra project structures.
-We need a header file for just z_scene. Including OBJECT_EXCHANGE_BANK_MAX and relevant structs, Scene, and Object enums.
-We need a macro header file for ALIGN16, PHYSICAL_TO_VIRTUAL and other global macros.
-We need to convert a lot of u32 struct members to void* to avoid UB.
-u32 -> void*: gRspSegmentPhysAddrs, Lib_PtrSegToVirt, DmaMgr_SendRequest0, DmaMgr_SendRequestImpl
+We need a header file for just z_scene. Including relevant structs, Scene, and Object enums.
+The .data, .bss, and .rodata sections are not migrated to this file yet.
 
-Additionally, the .data, .bss, and .rodata sections are not migrated to this file yet.
+Additionally we need a macro header file for ALIGN16, PHYSICAL_TO_VIRTUAL, OBJECT_EXCHANGE_BANK_MAX and other global macros.
 */
 
 s32 Scene_LoadObject(SceneContext* sceneCtxt, s16 id) {
@@ -21,9 +20,7 @@ s32 Scene_LoadObject(SceneContext* sceneCtxt, s16 id) {
     if (sceneCtxt) {}
     
     if (size) {
-        // TODO: UB to convert vramAddr to u32
-        DmaMgr_SendRequest0((u32)sceneCtxt->objects[sceneCtxt->objectCount].vramAddr,
-                            objectFileTable[id].vromStart, size);
+        DmaMgr_SendRequest0(sceneCtxt->objects[sceneCtxt->objectCount].vramAddr, objectFileTable[id].vromStart, size);
     }
         
     // TODO: This 0x22 is OBJECT_EXCHANGE_BANK_MAX - 1 in OOT
@@ -71,7 +68,7 @@ void Scene_Init(GlobalContext* ctxt, SceneContext* sceneCtxt) {
     sceneCtxt->mainKeepIndex = Scene_LoadObject(sceneCtxt, 1);
     // TODO: PHYSICAL_TO_VIRTUAL macro
     // TODO: Segment number enum
-    gRspSegmentPhysAddrs[4] = (u32)sceneCtxt->objects[sceneCtxt->mainKeepIndex].vramAddr + 0x80000000;
+    gRspSegmentPhysAddrs[4] = (void*)((u32)sceneCtxt->objects[sceneCtxt->mainKeepIndex].vramAddr + 0x80000000);
 }
 
 void Scene_ReloadUnloadedObjects(SceneContext* sceneCtxt) {
@@ -92,8 +89,7 @@ void Scene_ReloadUnloadedObjects(SceneContext* sceneCtxt) {
                     status->id = 0;
                 } else {
                     osCreateMesgQueue(&status->loadQueue, &status->loadMsg, 1);
-                    // TODO: UB to cast pointer to u32
-                    DmaMgr_SendRequestImpl(&status->dmaReq, (u32)status->vramAddr, objectFile->vromStart,
+                    DmaMgr_SendRequestImpl(&status->dmaReq, status->vramAddr, objectFile->vromStart,
                                             size, 0, &status->loadQueue, NULL);
                 }
             } else if (!osRecvMesg(&status->loadQueue, NULL, OS_MESG_NOBLOCK)) {
@@ -135,8 +131,7 @@ void Scene_DmaAllObjects(SceneContext* sceneCtxt) {
             continue;
         }
         
-        // TODO: UB to cast void* to u32
-        DmaMgr_SendRequest0((u32)sceneCtxt->objects[i].vramAddr, objectFileTable[id].vromStart, vromSize);
+        DmaMgr_SendRequest0(sceneCtxt->objects[i].vramAddr, objectFileTable[id].vromStart, vromSize);
     }
 }
 
@@ -242,9 +237,9 @@ void Scene_HeaderCommand07(GlobalContext* ctxt, SceneCmd* entry) {
         ctxt->sceneContext.keepObjectId = Scene_LoadObject(&ctxt->sceneContext, 
                                                            entry->specialFiles.keepObjectId);
         // TODO: PHYSICAL_TO_VIRTUAL macro
-        // TODO: Segment number enum
-        gRspSegmentPhysAddrs[5] = (u32)(ctxt->sceneContext.objects[ctxt->sceneContext.keepObjectId].vramAddr)
-                                    + 0x80000000;
+        // TODO: Segment number enum 
+        gRspSegmentPhysAddrs[5] = (void*)((u32)(ctxt->sceneContext.objects[ctxt->sceneContext.keepObjectId].vramAddr)
+                                    + 0x80000000);
     }
 
     if (entry->specialFiles.cUpElfMsgNum != 0) {
@@ -358,8 +353,7 @@ s32 func_8012FF10(GlobalContext* ctxt, s32 fileIndex) {
     
     if (fileSize) {
         ctxt->roomContext.unk74 = GameStateHeap_AllocFromEnd(&ctxt->state.heap, fileSize);
-        // TODO: UB to cast pointer to u32
-        return DmaMgr_SendRequest0((u32)ctxt->roomContext.unk74, vromStart, fileSize);
+        return DmaMgr_SendRequest0(ctxt->roomContext.unk74, vromStart, fileSize);
     }
     
     // UB: Undefined behaviour to not have a return statement here, but it breaks matching to add one.