From cbdba65218877303680bdf8852031a671301c37c Mon Sep 17 00:00:00 2001 From: Martin Griffin Date: Fri, 13 Oct 2023 19:29:30 +0100 Subject: [PATCH] Move battle tests off the heap (#3413) * Catch OOM in tests * Alias gBattleTestRunnerState with sBackupMapData Reverts #3398 "Fix Broken Move Anims in Tests" which moved the AI logs to statically-allocated EWRAM_DATA instead of being part of gBattleTestRunnerState. --- gflib/malloc.c | 23 +++++++++++++++++++++ include/fieldmap.h | 1 + include/test/battle.h | 3 ++- src/fieldmap.c | 2 +- test/test_runner_battle.c | 42 +++++++++++++++------------------------ 5 files changed, 43 insertions(+), 28 deletions(-) diff --git a/gflib/malloc.c b/gflib/malloc.c index dcfac6ee81..b3191e30b6 100644 --- a/gflib/malloc.c +++ b/gflib/malloc.c @@ -1,5 +1,8 @@ #include "global.h" #include "malloc.h" +#if TESTING +#include "test/test.h" +#endif static void *sHeapStart; static u32 sHeapSize; @@ -71,7 +74,27 @@ void *AllocInternal(void *heapStart, u32 size, const char *location) } if (pos->next == head) + { +#if TESTING + const struct MemBlock *head = HeapHead(); + const struct MemBlock *block = head; + do + { + if (block->allocated) + { + const char *location = MemBlockLocation(block); + if (location) + MgbaPrintf_("%s: %d bytes allocated", location, block->size); + else + MgbaPrintf_(": %d bytes allocated", block->size); + } + block = block->next; + } + while (block != head); + Test_ExitWithResult(TEST_RESULT_ERROR, "%s: OOM allocating %d bytes", location, size); +#endif return NULL; + } pos = pos->next; } diff --git a/include/fieldmap.h b/include/fieldmap.h index 47072bd1be..ecb1e49c4d 100644 --- a/include/fieldmap.h +++ b/include/fieldmap.h @@ -22,6 +22,7 @@ #include "main.h" extern struct BackupMapLayout gBackupMapLayout; +extern u16 ALIGNED(4) sBackupMapData[MAX_MAP_DATA_SIZE]; u32 MapGridGetMetatileIdAt(int, int); u32 MapGridGetMetatileBehaviorAt(int, int); diff --git a/include/test/battle.h b/include/test/battle.h index 818b1e5546..3fc49e96bb 100644 --- a/include/test/battle.h +++ b/include/test/battle.h @@ -679,6 +679,7 @@ struct BattleTestData u8 aiActionsPlayed[MAX_BATTLERS_COUNT]; struct ExpectedAIAction expectedAiActions[MAX_BATTLERS_COUNT][MAX_EXPECTED_ACTIONS]; struct ExpectedAiScore expectedAiScores[MAX_BATTLERS_COUNT][MAX_TURNS][MAX_AI_SCORE_COMPARISION_PER_TURN]; // Max 4 comparisions per turn + struct AILogLine aiLogLines[MAX_BATTLERS_COUNT][MAX_MON_MOVES][MAX_AI_LOG_LINES]; u8 aiLogPrintedForMove[MAX_BATTLERS_COUNT]; // Marks ai score log as printed for move, so the same log isn't displayed multiple times. }; @@ -711,7 +712,7 @@ struct BattleTestRunnerState }; extern const struct TestRunner gBattleTestRunner; -extern struct BattleTestRunnerState *gBattleTestRunnerState; +extern struct BattleTestRunnerState *const gBattleTestRunnerState; #define MEMBERS(...) VARARG_8(MEMBERS_, __VA_ARGS__) #define MEMBERS_0() diff --git a/src/fieldmap.c b/src/fieldmap.c index 5ef51a15f8..5e8ffec4aa 100644 --- a/src/fieldmap.c +++ b/src/fieldmap.c @@ -25,7 +25,7 @@ struct ConnectionFlags u8 east:1; }; -EWRAM_DATA static u16 ALIGNED(4) sBackupMapData[MAX_MAP_DATA_SIZE] = {0}; +EWRAM_DATA u16 ALIGNED(4) sBackupMapData[MAX_MAP_DATA_SIZE] = {0}; EWRAM_DATA struct MapHeader gMapHeader = {0}; EWRAM_DATA struct Camera gCamera = {0}; EWRAM_DATA static struct ConnectionFlags sMapConnectionFlags = {0}; diff --git a/test/test_runner_battle.c b/test/test_runner_battle.c index c5a6e60525..8482ff243c 100644 --- a/test/test_runner_battle.c +++ b/test/test_runner_battle.c @@ -4,6 +4,7 @@ #include "battle_anim.h" #include "battle_controllers.h" #include "characters.h" +#include "fieldmap.h" #include "item_menu.h" #include "main.h" #include "malloc.h" @@ -34,8 +35,9 @@ #undef Q_4_12 #define Q_4_12(n) (s32)((n) * 4096) -static EWRAM_DATA struct AILogLine sAILogLines[MAX_BATTLERS_COUNT][MAX_MON_MOVES][MAX_AI_LOG_LINES] = {0}; -EWRAM_DATA struct BattleTestRunnerState *gBattleTestRunnerState = NULL; +// Alias sBackupMapData to avoid using heap. +struct BattleTestRunnerState *const gBattleTestRunnerState = (void *)sBackupMapData; +STATIC_ASSERT(sizeof(struct BattleTestRunnerState) <= sizeof(sBackupMapData), sBackupMapDataSpace); static void CB2_BattleTest_NextParameter(void); static void CB2_BattleTest_NextTrial(void); @@ -147,9 +149,6 @@ static u32 BattleTest_EstimateCost(void *data) { u32 cost; const struct BattleTest *test = data; - STATE = AllocZeroed(sizeof(*STATE)); - if (!STATE) - return 0; STATE->runRandomly = TRUE; InvokeTestFunction(test); cost = 1; @@ -159,23 +158,21 @@ static u32 BattleTest_EstimateCost(void *data) cost *= 3; else if (STATE->trials > 1) cost *= STATE->trials; - FREE_AND_SET_NULL(STATE); return cost; } static void BattleTest_SetUp(void *data) { const struct BattleTest *test = data; - STATE = AllocZeroed(sizeof(*STATE)); - if (!STATE) - Test_ExitWithResult(TEST_RESULT_ERROR, "OOM: STATE = AllocZerod(%d)", sizeof(*STATE)); + memset(STATE, 0, sizeof(*STATE)); InvokeTestFunction(test); STATE->parameters = STATE->parametersCount; if (STATE->parametersCount == 0 && test->resultsSize > 0) Test_ExitWithResult(TEST_RESULT_INVALID, "results without PARAMETRIZE"); - STATE->results = AllocZeroed(test->resultsSize * STATE->parameters); - if (!STATE->results) - Test_ExitWithResult(TEST_RESULT_ERROR, "OOM: STATE->results = AllocZerod(%d)", sizeof(test->resultsSize * STATE->parameters)); + if (sizeof(*STATE) + test->resultsSize * STATE->parameters > sizeof(sBackupMapData)) + Test_ExitWithResult(TEST_RESULT_ERROR, "OOM: STATE (%d) + STATE->results (%d) too big for sBackupMapData (%d)", sizeof(*STATE), test->resultsSize * STATE->parameters, sizeof(sBackupMapData)); + STATE->results = (void *)((char *)sBackupMapData + sizeof(struct BattleTestRunnerState)); + memset(STATE->results, 0, test->resultsSize * STATE->parameters); switch (test->type) { case BATTLE_TEST_SINGLES: @@ -939,7 +936,7 @@ static void PrintAiMoveLog(u32 battlerId, u32 moveSlot, u32 moveId, s32 totalSco MgbaPrintf_("Score Log for move %S:\n", gMoveNames[moveId]); for (i = 0; i < MAX_AI_LOG_LINES; i++) { - struct AILogLine *log = &sAILogLines[battlerId][moveSlot][i]; + struct AILogLine *log = &DATA.aiLogLines[battlerId][moveSlot][i]; if (log->file) { if (log->set) @@ -975,7 +972,7 @@ static void ClearAiLog(u32 battlerId) u32 i, j; for (i = 0; i < MAX_MON_MOVES; i++) { - struct AILogLine *logs = sAILogLines[battlerId][i]; + struct AILogLine *logs = DATA.aiLogLines[battlerId][i]; for (j = 0; j < MAX_AI_LOG_LINES; j++) memset(&logs[j], 0, sizeof(struct AILogLine)); } @@ -1388,15 +1385,10 @@ static void CB2_BattleTest_NextTrial(void) static void BattleTest_TearDown(void *data) { - if (STATE) - { - // Free resources that aren't cleaned up when the battle was - // aborted unexpectedly. - if (STATE->tearDownBattle) - TearDownBattle(); - FREE_AND_SET_NULL(STATE->results); - FREE_AND_SET_NULL(STATE); - } + // Free resources that aren't cleaned up when the battle was + // aborted unexpectedly. + if (STATE->tearDownBattle) + TearDownBattle(); } static bool32 BattleTest_CheckProgress(void *data) @@ -2532,8 +2524,6 @@ void QueueStatus(u32 sourceLine, struct BattlePokemon *battler, struct StatusEve void ValidateFinally(u32 sourceLine) { // Defer this error until after estimating the cost. - if (STATE->results == NULL) - return; INVALID_IF(STATE->parametersCount == 0, "FINALLY without PARAMETRIZE"); } @@ -2550,7 +2540,7 @@ struct AILogLine *GetLogLine(u32 battlerId, u32 moveIndex) for (i = 0; i < MAX_AI_LOG_LINES; i++) { - struct AILogLine *log = &sAILogLines[battlerId][moveIndex][i]; + struct AILogLine *log = &DATA.aiLogLines[battlerId][moveIndex][i]; if (log->file == NULL) { return log;