From 801916920c8745b4e0abdb2a22d32dbbfd625f0b Mon Sep 17 00:00:00 2001 From: xxxsen Date: Fri, 9 Oct 2026 01:20:07 +0800 Subject: [PATCH 1/2] Preserve checkpoint variable IDs and dynamic name bindings --- .github/rpg-runtime/test-checkpoint.sh | 2 +- docs/retrom-checkpoint.md | 28 +++++ src/runner.c | 137 ++++++++++++++----------- tests/retrom_checkpoint_test.c | 113 +++++++++++++++++++- tests/test_retrom_web_host.py | 9 -- 5 files changed, 214 insertions(+), 75 deletions(-) create mode 100644 docs/retrom-checkpoint.md diff --git a/.github/rpg-runtime/test-checkpoint.sh b/.github/rpg-runtime/test-checkpoint.sh index 187fcc5c1..9ff8de412 100755 --- a/.github/rpg-runtime/test-checkpoint.sh +++ b/.github/rpg-runtime/test-checkpoint.sh @@ -7,7 +7,7 @@ trap 'rm -rf "$work"' EXIT cd "$root" ${CC:-cc} -std=gnu99 -DENABLE_WAD14 -DENABLE_WAD16 -DENABLE_WAD17 \ -ffunction-sections -fdata-sections -Isrc -Isrc/video -Ivendor/stb/ds \ - tests/retrom_checkpoint_test.c src/data_win.c src/gml_array.c src/gml_method.c \ + tests/retrom_checkpoint_test.c src/vm.c src/data_win.c src/gml_array.c src/gml_method.c \ src/instance.c src/int_rvalue_hashmap.c src/json_reader.c src/json_writer.c \ src/string_builder.c src/stb_ds.c src/log.c src/runner_gamepad.c -Wl,--gc-sections -lm -o "$work/test" "$work/test" diff --git a/docs/retrom-checkpoint.md b/docs/retrom-checkpoint.md new file mode 100644 index 000000000..e55d90dbd --- /dev/null +++ b/docs/retrom-checkpoint.md @@ -0,0 +1,28 @@ +# Retrom checkpoint variable identity + +The browser checkpoint keeps the `BSCP` envelope and +`butterscotch-checkpoint-v2` ABI. Its JSON stores global and instance variables +as arrays of `{ "id": number, "value": value }` entries. The integer ID is the +identity used by compiled GameMaker bytecode; a variable name is only a lookup +binding. Several compiled variables can have an empty name, and variables +created at runtime do not necessarily have entries in the compiled VARI chunk. + +`variableNames` captures the VM's complete name-to-ID map and `nextDynamicVarID` +captures its allocation cursor. Restore replaces the map created during cold +room initialization before restoring variables at their saved IDs. Subsequent +name lookups and allocations therefore address the same slots. Global and +instance scopes remain independent, and an explicitly undefined slot remains +present. + +The existing bounded value and nesting checks apply to the variable values. +Bindings and variable slots also count against the checkpoint cell budget. +Restore checks integral IDs, duplicate slots, the allocation cursor, and the +required joystick channel selections. It reads only this current contract; +there is no historical-format reader or conversion path. + +`tests/retrom_checkpoint_test.c` exercises multiple unnamed compiled IDs, +independent global and instance values, runtime-created names, a cold VM with +a different allocation order, allocation after restore, undefined slots, and +malformed or oversized variable collections. These deterministic fixtures do +not include game files or private save data. Product acceptance separately +verifies a new public save and its restoration in a fresh browser instance. diff --git a/src/runner.c b/src/runner.c index a3b7032a2..d9d1e14d3 100644 --- a/src/runner.c +++ b/src/runner.c @@ -4579,6 +4579,8 @@ static bool checkpointRValueSupported(RValue val, int32_t depth, int32_t* remain } static bool checkpointVariablesSupported(const IntRValueHashMap* variables, int32_t* remainingCells) { + if (variables->count > (uint32_t) *remainingCells) return false; + *remainingCells -= (int32_t) variables->count; for (uint32_t i = 0; i < variables->capacity; i++) { const IntRValueEntry* entry = &variables->entries[i]; if (entry->key == INT_RVALUE_HASHMAP_EMPTY_KEY) continue; @@ -4696,7 +4698,8 @@ RunnerCheckpointStatus Runner_checkpointStatus(Runner* runner) { runner->savedRoomStates[i].initialized) return RUNNER_CHECKPOINT_PERSISTENT_ROOM_STATE; } int32_t remainingCells = 100000; - if (!checkpointDataStructuresSupported(runner, &remainingCells)) return RUNNER_CHECKPOINT_VALUE_UNSUPPORTED; + if (!checkpointConsumeCells(shlen(runner->vmContext->varNameMap), &remainingCells) || + !checkpointDataStructuresSupported(runner, &remainingCells)) return RUNNER_CHECKPOINT_VALUE_UNSUPPORTED; if (!checkpointVariablesSupported(&runner->vmContext->globalScopeInstance->selfVars, &remainingCells)) return RUNNER_CHECKPOINT_VALUE_UNSUPPORTED; repeat(arrlen(runner->instances), i) { Instance* instance = runner->instances[i]; @@ -4862,6 +4865,32 @@ static void writeCheckpointJoysticks(JsonWriter* writer, const RunnerGamepadStat JsonWriter_endArray(writer); } +static void writeCheckpointVariables(JsonWriter* writer, const IntRValueHashMap* variables) { + // Names are not variable identities: compiled VARI entries can share an + // empty name, and runtime-created self variables need not appear in VARI. + JsonWriter_beginArray(writer); + for (uint32_t i = 0; i < variables->capacity; i++) { + const IntRValueEntry* entry = &variables->entries[i]; + if (entry->key == INT_RVALUE_HASHMAP_EMPTY_KEY) continue; + JsonWriter_beginObject(writer); + JsonWriter_propertyInt(writer, "id", entry->key); + JsonWriter_key(writer, "value"); + writeRValueJson(writer, entry->value); + JsonWriter_endObject(writer); + } + JsonWriter_endArray(writer); +} + +static void writeCheckpointVariableNames(JsonWriter* writer, const VMContext* vm) { + JsonWriter_propertyInt(writer, "nextDynamicVarID", vm->nextDynamicVarID); + JsonWriter_key(writer, "variableNames"); + JsonWriter_beginObject(writer); + repeat(shlen(vm->varNameMap), i) { + JsonWriter_propertyInt(writer, vm->varNameMap[i].key, vm->varNameMap[i].value); + } + JsonWriter_endObject(writer); +} + char* Runner_dumpStateJson(Runner* runner) { DataWin* dataWin = runner->dataWin; int32_t instanceCount = (int32_t) arrlen(runner->instances); @@ -4874,6 +4903,7 @@ char* Runner_dumpStateJson(Runner* runner) { JsonWriter_propertyInt(&w, "frame", runner->frameCount); writeCheckpointJoysticks(&w, runner->gamepads); + writeCheckpointVariableNames(&w, runner->vmContext); // Room info JsonWriter_key(&w, "room"); @@ -4969,30 +4999,8 @@ char* Runner_dumpStateJson(Runner* runner) { } JsonWriter_endObject(&w); - // Self variables (non-array, sparse hashmap) JsonWriter_key(&w, "selfVariables"); - JsonWriter_beginObject(&w); - repeat(inst->selfVars.capacity, svIdx) { - IntRValueEntry* entry = &inst->selfVars.entries[svIdx]; - if (entry->key == INT_RVALUE_HASHMAP_EMPTY_KEY) continue; - int32_t varID = entry->key; - RValue val = entry->value; - if (val.type == RVALUE_UNDEFINED) continue; - - // Resolve variable name from VARI chunk - const char* varName = "?"; - repeat(dataWin->vari.variableCount, varIdx) { - Variable* var = &dataWin->vari.variables[varIdx]; - if (var->instanceType == INSTANCE_SELF && var->varID == varID) { - varName = var->name; - break; - } - } - - JsonWriter_key(&w, varName); - writeRValueJson(&w, val); - } - JsonWriter_endObject(&w); + writeCheckpointVariables(&w, &inst->selfVars); JsonWriter_endObject(&w); } @@ -5033,25 +5041,8 @@ char* Runner_dumpStateJson(Runner* runner) { } JsonWriter_endArray(&w); - // Global variables (non-array) JsonWriter_key(&w, "globalVariables"); - JsonWriter_beginObject(&w); - - { - repeat(runner->vmContext->globalScopeInstance->selfVars.capacity, i) { - IntRValueEntry entryOnTheVarStruct = runner->vmContext->globalScopeInstance->selfVars.entries[i]; - RValue target = VM_structGetVariableByVarId(runner->vmContext->globalScopeInstance, entryOnTheVarStruct.key, -1); - - if (entryOnTheVarStruct.key != INT_RVALUE_HASHMAP_EMPTY_KEY) { - char* name = VM_getVariableNameByVarId(runner->vmContext, entryOnTheVarStruct.key); - - JsonWriter_key(&w, name); - writeRValueJson(&w, target); - } - } - } - - JsonWriter_endObject(&w); + writeCheckpointVariables(&w, &runner->vmContext->globalScopeInstance->selfVars); JsonWriter_endObject(&w); char* result = JsonWriter_copyOutput(&w); @@ -5307,16 +5298,46 @@ static bool restoreCheckpointDataStructures( return valid; } -static bool restoreCheckpointVariables(VMContext* vm, Instance* target, const JsonValue* object, int32_t* remainingCells) { - if (!JsonReader_isObject(object)) return false; - repeat(JsonReader_objectLength(object), i) { - const char* name = JsonReader_getJsonKeyByIndex(object, i); - // Some GameMaker VARI chunks contain an empty name for a real variable ID. - // Checkpoint v2 already writes that name as a valid empty JSON object key. - if (name == nullptr) return false; +static void clearCheckpointVariableNames(VMContext* vm) { + repeat(shlen(vm->varNameMap), i) free(vm->varNameMap[i].key); + shfree(vm->varNameMap); + vm->varNameMap = nullptr; +} + +static bool restoreCheckpointVariableNames(VMContext* vm, const JsonValue* root, int32_t* remainingCells) { + int32_t nextId; + JsonValue* names = checkpointField(root, "variableNames", JSON_OBJECT); + if (names == nullptr || !checkpointInteger(root, "nextDynamicVarID", &nextId) || + nextId <= 0 || nextId == INT32_MAX || + !checkpointConsumeCells(JsonReader_objectLength(names), remainingCells)) return false; + VMContext restored = {0}; + repeat(JsonReader_objectLength(names), i) { + const char* name = JsonReader_getJsonKeyByIndex(names, i); + int32_t id; + if (name == nullptr || !checkpointInteger(names, name, &id) || id < 0 || id >= nextId || + shgeti(restored.varNameMap, name) >= 0) { + clearCheckpointVariableNames(&restored); + return false; + } + shput(restored.varNameMap, safeStrdup(name), id); + } + clearCheckpointVariableNames(vm); + vm->varNameMap = restored.varNameMap; + vm->nextDynamicVarID = nextId; + return true; +} + +static bool restoreCheckpointVariables(VMContext* vm, Instance* target, const JsonValue* variables, int32_t* remainingCells) { + if (!JsonReader_isArray(variables) || + !checkpointConsumeCells(JsonReader_arrayLength(variables), remainingCells)) return false; + repeat(JsonReader_arrayLength(variables), i) { + JsonValue* entry = JsonReader_getArrayElement(variables, i); + int32_t id; + if (!checkpointInteger(entry, "id", &id) || id < 0 || id >= vm->nextDynamicVarID || + IntRValueHashMap_contains(&target->selfVars, id)) return false; RValue value = RValue_makeUndefined(); - if (!checkpointRValueFromJson(vm, JsonReader_getJsonValueByIndex(object, i), 0, remainingCells, &value)) return false; - Instance_setSelfVar(target, VM_getOrAllocateVarID(vm, name), value); + if (!checkpointRValueFromJson(vm, JsonReader_getJsonValueByKey(entry, "value"), 0, remainingCells, &value)) return false; + Instance_setSelfVar(target, id, value); RValue_free(&value); } return true; @@ -5343,7 +5364,7 @@ static bool restoreCheckpointInstance(Runner* runner, const JsonValue* value, in JsonValue* sprite = checkpointField(value, "sprite", JSON_OBJECT); JsonValue* scale = checkpointField(value, "scale", JSON_OBJECT); JsonValue* alarms = checkpointField(value, "alarms", JSON_OBJECT); - JsonValue* variables = checkpointField(value, "selfVariables", JSON_OBJECT); + JsonValue* variables = checkpointField(value, "selfVariables", JSON_ARRAY); int32_t spriteIndex, blend; double imageIndex, imageSpeed, imageXscale, imageYscale, imageAngle, imageAlpha; if (!checkpointInteger(value, "instanceId", &instanceId) || instanceId < 0 || @@ -5413,12 +5434,7 @@ static bool restoreCheckpointInstance(Runner* runner, const JsonValue* value, in static bool readCheckpointJoysticks(const JsonValue* root, int32_t devices[MAX_JOYSTICKS]) { JsonValue* values = JsonReader_getJsonValueByKey(root, "joystickDevices"); - // Older checkpoints used direct physical slots for joystick 1/2. - if (values == nullptr) { - for (int i = 0; i < MAX_JOYSTICKS; i++) devices[i] = i + 1; - return true; - } - if (values->type != JSON_ARRAY || JsonReader_arrayLength(values) != MAX_JOYSTICKS) return false; + if (values == nullptr || values->type != JSON_ARRAY || JsonReader_arrayLength(values) != MAX_JOYSTICKS) return false; int32_t parsed[MAX_JOYSTICKS]; for (int i = 0; i < MAX_JOYSTICKS; i++) { JsonValue* value = JsonReader_getArrayElement(values, i); @@ -5441,7 +5457,7 @@ bool Runner_restoreStateJson(Runner* runner, const char* json) { int32_t joystickDevices[MAX_JOYSTICKS]; JsonValue* room = checkpointField(root, "room", JSON_OBJECT); JsonValue* instances = checkpointField(root, "instances", JSON_ARRAY); - JsonValue* globals = checkpointField(root, "globalVariables", JSON_OBJECT); + JsonValue* globals = checkpointField(root, "globalVariables", JSON_ARRAY); JsonValue* dataStructures = checkpointField(root, "dataStructures", JSON_OBJECT); bool valid = checkpointInteger(root, "checkpointSchemaVersion", &schema) && schema == 2 && checkpointInteger(root, "frame", &frame) && frame >= 0 && checkpointInteger(room, "index", &roomIndex) && @@ -5460,7 +5476,8 @@ bool Runner_restoreStateJson(Runner* runner, const char* json) { IntRValueHashMap_freeAllValues(&runner->vmContext->globalScopeInstance->selfVars); clearCheckpointDataStructures(runner); int32_t remainingCells = 100000; - valid = restoreCheckpointDataStructures(runner, dataStructures, &remainingCells) && + valid = restoreCheckpointVariableNames(runner->vmContext, root, &remainingCells) && + restoreCheckpointDataStructures(runner, dataStructures, &remainingCells) && restoreCheckpointVariables(runner->vmContext, runner->vmContext->globalScopeInstance, globals, &remainingCells); for (int i = 0; valid && i < JsonReader_arrayLength(instances); i++) { valid = restoreCheckpointInstance(runner, JsonReader_getArrayElement(instances, i), &remainingCells); diff --git a/tests/retrom_checkpoint_test.c b/tests/retrom_checkpoint_test.c index 2ea385530..48b60885d 100644 --- a/tests/retrom_checkpoint_test.c +++ b/tests/retrom_checkpoint_test.c @@ -5,7 +5,114 @@ void platformLog(MAYBE_UNUSED logType type, const char* format, va_list args) { vfprintf(stderr, format, args); } +static void testCheckpointVariableIdentity(void) { + DataWin data = {0}; + data.gen8.wadVersion = 16; + Room room = {.name = "checkpoint fixture"}; + Instance globals = {.objectIndex = STRUCT_OBJECT_INDEX}; + VMContext vm = {.dataWin = &data, .globalScopeInstance = &globals, .nextDynamicVarID = 8}; + shput(vm.varNameMap, safeStrdup(""), 1); + int32_t firstDynamic = VM_getOrAllocateVarID(&vm, "runtime_first"); + int32_t secondDynamic = VM_getOrAllocateVarID(&vm, "runtime_second"); + Runner runner = {.dataWin = &data, .vmContext = &vm, .currentRoom = &room}; + Instance actor = {.objectIndex = -1, .active = true}; + arrput(runner.instances, &actor); + Instance_setSelfVar(&globals, 1, RValue_makeReal(11)); + Instance_setSelfVar(&globals, 2, RValue_makeString("player")); + Instance_setSelfVar(&globals, 3, RValue_makeReal(33)); + Instance_setSelfVar(&actor, 2, RValue_makeReal(22)); + Instance_setSelfVar(&actor, 6, RValue_makeUndefined()); + Instance_setSelfVar(&actor, firstDynamic, RValue_makeString("first")); + Instance_setSelfVar(&actor, secondDynamic, RValue_makeReal(99)); + char* encoded = Runner_dumpStateJson(&runner); + JsonValue* document = JsonReader_parse(encoded); + + VMContext fresh = {.dataWin = &data, .nextDynamicVarID = 8}; + shput(fresh.varNameMap, safeStrdup(""), 1); + // Cold room initialization can allocate variables in a different order. + assert(VM_getOrAllocateVarID(&fresh, "runtime_second") == firstDynamic); + assert(VM_getOrAllocateVarID(&fresh, "temporary_room_variable") == secondDynamic); + int32_t cells = 100; + assert(restoreCheckpointVariableNames(&fresh, document, &cells)); + Instance restoredGlobals = {.objectIndex = STRUCT_OBJECT_INDEX}; + assert(restoreCheckpointVariables(&fresh, &restoredGlobals, + JsonReader_getJsonValueByKey(document, "globalVariables"), &cells)); + assert(IntRValueHashMap_get(&restoredGlobals.selfVars, 1).real == 11); + RValue name = IntRValueHashMap_get(&restoredGlobals.selfVars, 2); + assert(name.type == RVALUE_STRING && strcmp(name.string, "player") == 0); + assert(IntRValueHashMap_get(&restoredGlobals.selfVars, 3).real == 33); + + JsonValue* instances = JsonReader_getJsonValueByKey(document, "instances"); + JsonValue* savedActor = JsonReader_getArrayElement(instances, 0); + Instance restoredActor = {0}; + assert(restoreCheckpointVariables(&fresh, &restoredActor, + JsonReader_getJsonValueByKey(savedActor, "selfVariables"), &cells)); + assert(IntRValueHashMap_get(&restoredActor.selfVars, 2).real == 22); + assert(IntRValueHashMap_contains(&restoredActor.selfVars, 6)); + assert(IntRValueHashMap_get(&restoredActor.selfVars, 6).type == RVALUE_UNDEFINED); + assert(VM_getOrAllocateVarID(&fresh, "runtime_first") == firstDynamic); + assert(VM_getOrAllocateVarID(&fresh, "runtime_second") == secondDynamic); + assert(strcmp(IntRValueHashMap_get(&restoredActor.selfVars, firstDynamic).string, "first") == 0); + assert(IntRValueHashMap_get(&restoredActor.selfVars, secondDynamic).real == 99); + assert(shgeti(fresh.varNameMap, "temporary_room_variable") < 0); + assert(VM_getOrAllocateVarID(&fresh, "after_restore") == secondDynamic + 1); + IntRValueHashMap_freeAllValues(&globals.selfVars); + IntRValueHashMap_freeAllValues(&actor.selfVars); + IntRValueHashMap_freeAllValues(&restoredGlobals.selfVars); + IntRValueHashMap_freeAllValues(&restoredActor.selfVars); + clearCheckpointVariableNames(&vm); + clearCheckpointVariableNames(&fresh); + arrfree(runner.instances); + JsonReader_free(document); + free(encoded); +} + +static void testCheckpointVariableValidation(void) { + VMContext vm = {.nextDynamicVarID = 10}; + const char* invalidVariables[] = { + "[{\"id\":-1,\"value\":1}]", "[{\"id\":0.5,\"value\":1}]", + "[{\"id\":2}]", "[{\"value\":1}]", + "[{\"id\":10,\"value\":1}]", + "[{\"id\":2,\"value\":1},{\"id\":2,\"value\":2}]", + }; + for (size_t i = 0; i < sizeof(invalidVariables) / sizeof(invalidVariables[0]); i++) { + JsonValue* document = JsonReader_parse(invalidVariables[i]); + Instance target = {0}; + int32_t cells = 100; + assert(!restoreCheckpointVariables(&vm, &target, document, &cells)); + IntRValueHashMap_freeAllValues(&target.selfVars); + JsonReader_free(document); + } + JsonValue* document = JsonReader_parse("[{\"id\":1,\"value\":11},{\"id\":2,\"value\":22}]"); + Instance target = {0}; + int32_t cells = 1; + assert(!restoreCheckpointVariables(&vm, &target, document, &cells)); + assert(target.selfVars.count == 0); + JsonReader_free(document); + + const char* invalidNames[] = { + "{\"variableNames\":{\"a\":1},\"nextDynamicVarID\":1}", + "{\"variableNames\":{\"a\":-1},\"nextDynamicVarID\":5}", + "{\"variableNames\":{\"a\":1.5},\"nextDynamicVarID\":5}", + "{\"variableNames\":{\"a\":1,\"a\":2},\"nextDynamicVarID\":5}", + "{\"variableNames\":{},\"nextDynamicVarID\":0}", + "{\"variableNames\":{},\"nextDynamicVarID\":2147483647}", + }; + shput(vm.varNameMap, safeStrdup("untouched"), 4); + for (size_t i = 0; i < sizeof(invalidNames) / sizeof(invalidNames[0]); i++) { + document = JsonReader_parse(invalidNames[i]); + cells = 100; + assert(!restoreCheckpointVariableNames(&vm, document, &cells)); + assert(vm.nextDynamicVarID == 10); + assert(shlen(vm.varNameMap) == 1 && shget(vm.varNameMap, "untouched") == 4); + JsonReader_free(document); + } + clearCheckpointVariableNames(&vm); +} + int main(void) { + testCheckpointVariableIdentity(); + testCheckpointVariableValidation(); // A late lower physical slot must not change the saved legacy selection // when a new runner restores with both devices already connected. RunnerGamepadState originalPads = {0}; @@ -31,17 +138,13 @@ int main(void) { const char* badPads[] = { "{\"joystickDevices\":[3,3]}", "{\"joystickDevices\":[-1,0]}", "{\"joystickDevices\":[17,0]}", "{\"joystickDevices\":[0.5,0]}", - "{\"joystickDevices\":[0]}", "{\"joystickDevices\":null}", + "{\"joystickDevices\":[0]}", "{\"joystickDevices\":null}", "{}", }; for (int i = 0; i < (int)(sizeof(badPads) / sizeof(badPads[0])); i++) { padDocument = JsonReader_parse(badPads[i]); assert(!readCheckpointJoysticks(padDocument, restoredPads.joystickDevices)); JsonReader_free(padDocument); } - padDocument = JsonReader_parse("{}"); - assert(readCheckpointJoysticks(padDocument, restoredPads.joystickDevices)); - assert(restoredPads.joystickDevices[0] == 1 && restoredPads.joystickDevices[1] == 2); - JsonReader_free(padDocument); DataWin data = {0}; data.gen8.wadVersion = 17; diff --git a/tests/test_retrom_web_host.py b/tests/test_retrom_web_host.py index 0d32515b6..24fa20f5d 100755 --- a/tests/test_retrom_web_host.py +++ b/tests/test_retrom_web_host.py @@ -87,15 +87,6 @@ def test_checkpoint_v2_preserves_supported_game_data_structures(self) -> None: self.assertIn("writeUint32LE(gCheckpointBytes + 4, 2);", self.source) self.assertIn("readUint32LE(bytes + 4) != 2", self.source) - def test_checkpoint_v2_restores_empty_game_variable_names(self) -> None: - runner = (ROOT / "src/runner.c").read_text(encoding="utf-8") - start = runner.index("static bool restoreCheckpointVariables") - end = runner.index("static void clearCheckpointInstances", start) - restore = runner[start:end] - self.assertIn("name == nullptr", restore) - self.assertNotIn("name[0] == '\\0'", restore) - self.assertIn("VM_getOrAllocateVarID(vm, name)", restore) - def test_closing_a_dirty_ini_clears_the_transient_checkpoint_blocker(self) -> None: start = self.builtins.index("static RValue builtin_ini_close") end = self.builtins.index("static RValue builtin_ini_read_string", start) From d8e52eb9f0b5df27f74b7c732bc07ac32e34c409 Mon Sep 17 00:00:00 2001 From: xxxsen Date: Fri, 9 Oct 2026 01:59:52 +0800 Subject: [PATCH 2/2] fix: bound variable allocation and exercise full checkpoint restore --- .github/rpg-runtime/test-checkpoint.sh | 18 +- docs/retrom-checkpoint.md | 19 +- src/event_table.c | 3 +- src/runner.c | 6 +- src/vm.c | 3 + tests/retrom_checkpoint_test.c | 236 ++++++++++++++++++++++++- 6 files changed, 274 insertions(+), 11 deletions(-) diff --git a/.github/rpg-runtime/test-checkpoint.sh b/.github/rpg-runtime/test-checkpoint.sh index 9ff8de412..6f460ee8d 100755 --- a/.github/rpg-runtime/test-checkpoint.sh +++ b/.github/rpg-runtime/test-checkpoint.sh @@ -5,9 +5,21 @@ mkdir -p "$root/.cache" work=$(mktemp -d "$root/.cache/checkpoint-test.XXXXXX") trap 'rm -rf "$work"' EXIT cd "$root" +sources=() +for source in src/*.c; do + # The test includes runner.c to cover its bounded-value helpers as well as + # the public restore entry point. No platform loop is needed headlessly. + case "$source" in src/runner.c|src/loop.c) continue ;; esac + sources+=("$source") +done +read -r -a cflags <<< "${CFLAGS:-}" +read -r -a ldflags <<< "${LDFLAGS:-}" ${CC:-cc} -std=gnu99 -DENABLE_WAD14 -DENABLE_WAD16 -DENABLE_WAD17 \ + -DENABLE_NOOP_RENDERER -DBUTTERSCOTCH_VIDEO_NULL "${cflags[@]}" \ -ffunction-sections -fdata-sections -Isrc -Isrc/video -Ivendor/stb/ds \ - tests/retrom_checkpoint_test.c src/vm.c src/data_win.c src/gml_array.c src/gml_method.c \ - src/instance.c src/int_rvalue_hashmap.c src/json_reader.c src/json_writer.c \ - src/string_builder.c src/stb_ds.c src/log.c src/runner_gamepad.c -Wl,--gc-sections -lm -o "$work/test" + -Ivendor/md5 -Ivendor/sha1 -Ivendor/base64 -Ivendor/bzip2 \ + tests/retrom_checkpoint_test.c "${sources[@]}" src/video/null_video.c \ + vendor/md5/md5.c vendor/sha1/sha1.c vendor/base64/base64.c \ + vendor/bzip2/{bz_internal_error,bzlib,crctable,decompress,huffman,randtable}.c \ + -Wl,--gc-sections "${ldflags[@]}" -lm -o "$work/test" "$work/test" diff --git a/docs/retrom-checkpoint.md b/docs/retrom-checkpoint.md index e55d90dbd..157467c9a 100644 --- a/docs/retrom-checkpoint.md +++ b/docs/retrom-checkpoint.md @@ -14,15 +14,28 @@ name lookups and allocations therefore address the same slots. Global and instance scopes remain independent, and an explicitly undefined slot remains present. +The allocation cursor may reach `INT32_MAX`, representing an exhausted ID +space. Existing name lookups still succeed, while allocating another name +fails explicitly before incrementing the cursor. Compiled IDs must also leave +room for a representable cursor when creating the VM. + The existing bounded value and nesting checks apply to the variable values. Bindings and variable slots also count against the checkpoint cell budget. Restore checks integral IDs, duplicate slots, the allocation cursor, and the required joystick channel selections. It reads only this current contract; there is no historical-format reader or conversion path. -`tests/retrom_checkpoint_test.c` exercises multiple unnamed compiled IDs, +The helper-level tests in `tests/retrom_checkpoint_test.c` exercise multiple unnamed compiled IDs, independent global and instance values, runtime-created names, a cold VM with a different allocation order, allocation after restore, undefined slots, and -malformed or oversized variable collections. These deterministic fixtures do -not include game files or private save data. Product acceptance separately +malformed or oversized variable collections. The public-entry regression uses +`VM_create`, `Runner_create`, and `Runner_restoreStateJson` with two synthetic +rooms, a legal object, and room creation bytecode that calls +`variable_global_set`. It covers reset and room initialization, a different +allocation order, a transition to the saved room, instance reconstruction, +and owned strings and nested arrays. A malformed value in the second instance +tests partial-state teardown and successful retry through the public entry +point. The native gate accepts `CFLAGS` and `LDFLAGS` for AddressSanitizer and +UndefinedBehaviorSanitizer checks, including leak detection. These fixtures +do not include game files or private save data. Product acceptance separately verifies a new public save and its restoration in a fresh browser instance. diff --git a/src/event_table.c b/src/event_table.c index 8a0ac5275..109d9c1e2 100644 --- a/src/event_table.c +++ b/src/event_table.c @@ -181,8 +181,7 @@ void ResolvedEventTable_build(ResolvedEventTable* outTable, DataWin* dw, const E } outTable->bySlot = (SlotResponderEntry *)safeMalloc((size_t) totalEntries * sizeof(SlotResponderEntry)); - uint32_t* slotCursor = (uint32_t *)safeMalloc((size_t) slotCount * sizeof(uint32_t)); - memset(slotCursor, 0, (size_t) slotCount * sizeof(uint32_t)); + uint32_t* slotCursor = (uint32_t *)safeCalloc((size_t) slotCount, sizeof(uint32_t)); { repeat(objectCount, oi) { diff --git a/src/runner.c b/src/runner.c index d9d1e14d3..05d1e3272 100644 --- a/src/runner.c +++ b/src/runner.c @@ -2290,7 +2290,9 @@ static void flattenCollisionEvents(Runner* runner) { depth++; } - qsort(dst->events, dst->eventCount, sizeof(FlattenedCollisionEvent), compareTargetObjectIndexAscending); + if (dst->eventCount > 1) { + qsort(dst->events, dst->eventCount, sizeof(FlattenedCollisionEvent), compareTargetObjectIndexAscending); + } } } @@ -5308,7 +5310,7 @@ static bool restoreCheckpointVariableNames(VMContext* vm, const JsonValue* root, int32_t nextId; JsonValue* names = checkpointField(root, "variableNames", JSON_OBJECT); if (names == nullptr || !checkpointInteger(root, "nextDynamicVarID", &nextId) || - nextId <= 0 || nextId == INT32_MAX || + nextId <= 0 || !checkpointConsumeCells(JsonReader_objectLength(names), remainingCells)) return false; VMContext restored = {0}; repeat(JsonReader_objectLength(names), i) { diff --git a/src/vm.c b/src/vm.c index 1bfae35f1..3f44dde3f 100644 --- a/src/vm.c +++ b/src/vm.c @@ -313,6 +313,8 @@ static GMLArray* VM_arraySetWithCoW(VMContext* ctx, RValue* slot, int32_t index, int32_t VM_getOrAllocateVarID(VMContext* ctx, const char* name) { ptrdiff_t slot = shgeti(ctx->varNameMap, name); if (slot >= 0) return ctx->varNameMap[slot].value; + requireMessage(ctx->nextDynamicVarID >= 0 && ctx->nextDynamicVarID < INT32_MAX, + "VM: Variable ID space exhausted"); int32_t id = ctx->nextDynamicVarID++; shput(ctx->varNameMap, safeStrdup(name), id); return id; @@ -3615,6 +3617,7 @@ VMContext* VM_create(DataWin* dataWin) { maxSelfVarID = variable->varID; } } + requireMessage(maxSelfVarID < INT32_MAX, "VM: Variable ID space exhausted"); ctx->nextDynamicVarID = maxSelfVarID + 1; // Build funcName -> codeIndex hash map from SCPT chunk diff --git a/tests/retrom_checkpoint_test.c b/tests/retrom_checkpoint_test.c index 48b60885d..654248263 100644 --- a/tests/retrom_checkpoint_test.c +++ b/tests/retrom_checkpoint_test.c @@ -1,5 +1,13 @@ #include +#include +#include +#include +#include #include "../src/runner.c" +#include "../src/noop_audio_system.h" +#include "../src/noop_file_system.h" +#include "../src/noop_renderer.h" +#include "../src/binary_utils.h" void platformLog(MAYBE_UNUSED logType type, const char* format, va_list args) { vfprintf(stderr, format, args); @@ -96,7 +104,7 @@ static void testCheckpointVariableValidation(void) { "{\"variableNames\":{\"a\":1.5},\"nextDynamicVarID\":5}", "{\"variableNames\":{\"a\":1,\"a\":2},\"nextDynamicVarID\":5}", "{\"variableNames\":{},\"nextDynamicVarID\":0}", - "{\"variableNames\":{},\"nextDynamicVarID\":2147483647}", + "{\"variableNames\":{},\"nextDynamicVarID\":2147483648}", }; shput(vm.varNameMap, safeStrdup("untouched"), 4); for (size_t i = 0; i < sizeof(invalidNames) / sizeof(invalidNames[0]); i++) { @@ -110,9 +118,235 @@ static void testCheckpointVariableValidation(void) { clearCheckpointVariableNames(&vm); } +static void testCheckpointVariableExhaustion(void) { + VMContext vm = {0}; + JsonValue* document = JsonReader_parse( + "{\"variableNames\":{\"existing\":1},\"nextDynamicVarID\":2147483646}"); + int32_t cells = 100; + assert(restoreCheckpointVariableNames(&vm, document, &cells)); + JsonReader_free(document); + assert(VM_getOrAllocateVarID(&vm, "last") == INT32_MAX - 1); + assert(vm.nextDynamicVarID == INT32_MAX); + // An exhausted cursor still preserves existing bindings on restore. + document = JsonReader_parse( + "{\"variableNames\":{\"existing\":1,\"last\":2147483646},\"nextDynamicVarID\":2147483647}"); + assert(restoreCheckpointVariableNames(&vm, document, &cells)); + JsonReader_free(document); + assert(VM_getOrAllocateVarID(&vm, "existing") == 1); + assert(VM_getOrAllocateVarID(&vm, "last") == INT32_MAX - 1); + + for (int compiled = 0; compiled < 2; compiled++) { + int diagnostics[2]; + assert(pipe(diagnostics) == 0); + pid_t child = fork(); + assert(child >= 0); + if (child == 0) { + struct rlimit noCoreDump = {0, 0}; + setrlimit(RLIMIT_CORE, &noCoreDump); + close(diagnostics[0]); + assert(dup2(diagnostics[1], STDERR_FILENO) >= 0); + close(diagnostics[1]); + if (compiled) { + DataWin data = {0}; + data.gen8.wadVersion = 16; + Variable variable = {.name = "compiled_max", .varID = INT32_MAX}; + data.vari.variableCount = 1; + data.vari.variables = &variable; + VM_free(VM_create(&data)); + } else { + VM_getOrAllocateVarID(&vm, "exhausted"); + } + _exit(0); + } + close(diagnostics[1]); + char message[1024] = {0}; + ssize_t bytes = read(diagnostics[0], message, sizeof(message) - 1); + assert(bytes > 0); + close(diagnostics[0]); + int status; + assert(waitpid(child, &status, 0) == child); + assert(WIFSIGNALED(status) && WTERMSIG(status) == SIGABRT); + assert(strstr(message, "VM: Variable ID space exhausted") != NULL); + } + clearCheckpointVariableNames(&vm); +} + +static Runner* createCheckpointRunner(DataWin* data) { + return Runner_create(data, VM_create(data), NoopRenderer_create(), + NoopFileSystem_create(), (AudioSystem*) NoopAudioSystem_create(), 1); +} + +static void freeCheckpointRunner(Runner* runner) { + VMContext* vm = runner->vmContext; + Renderer* renderer = runner->renderer; + AudioSystem* audio = runner->audioSystem; + FileSystem* files = runner->fileSystem; + Runner_free(runner); + VM_free(vm); + renderer->vtable->destroy(renderer); + audio->vtable->destroy(audio); + NoopFileSystem_destroy(files); +} + +static void testCheckpointRunnerRestore(void) { + // Synthetic WAD16 room creation bytecode executes the real builtin: + // variable_global_set("room_bootstrap", 73). No game files are needed. + uint32_t instructions[] = { + ((uint32_t) OP_PUSHI << 24) | (GML_TYPE_INT16 << 16) | 73, + ((uint32_t) OP_PUSH << 24) | (GML_TYPE_STRING << 16), 0, + ((uint32_t) OP_CALL << 24) | (GML_TYPE_VARIABLE << 16) | 2, 0, + (uint32_t) OP_POPZ << 24, (uint32_t) OP_EXIT << 24, + }; + uint8_t bytecode[sizeof(instructions)]; + for (size_t i = 0; i < sizeof(instructions) / sizeof(instructions[0]); i++) { + BinaryUtils_writeUint32(bytecode + i * 4, instructions[i]); + } + CodeEntry code = {.name = "checkpoint_room_init", .length = sizeof(bytecode), .present = true}; + Function function = {.name = "variable_global_set"}; + const char* strings[] = {"room_bootstrap"}; + Variable variables[] = { + {.name = "", .varID = 1}, {.name = "", .varID = 2}, + {.name = "", .varID = 3}, {.name = "compiled", .varID = 7}, + }; + GameObject object = {.name = "checkpoint_actor", .present = true, + .parentId = -1, .spriteId = -1, .textureMaskId = -1}; + RoomGameObject roomActor = {.objectDefinition = 0, .instanceID = 100001, + .creationCode = -1, .preCreateCode = -1, .scaleX = 1, .scaleY = 1, .color = 0xffffffff}; + RoomView views[MAX_VIEWS] = {0}; + RoomBackground backgrounds[8] = {0}; + Room rooms[2] = { + {.name = "checkpoint_entry", .present = true, .payloadLoaded = true, + .eagerlyLoaded = true, .views = views, .backgrounds = backgrounds, + .width = 320, .height = 240, .speed = 60, .creationCodeId = 0, + .gameObjectCount = 1, .gameObjects = &roomActor}, + {.name = "checkpoint_saved_room", .present = true, .payloadLoaded = true, + .eagerlyLoaded = true, .views = views, .backgrounds = backgrounds, + .width = 640, .height = 480, .speed = 30, .creationCodeId = -1}, + }; + uint32_t roomOrder[] = {0, 1}; + DataWin data = {0}; + data.gen8.wadVersion = 16; + data.gen8.roomOrderCount = 2; + data.gen8.roomOrder = roomOrder; + data.gen8.lastObj = 100010; + data.room.count = 2; + data.room.rooms = rooms; + data.objt.count = 1; + data.objt.objects = &object; + data.vari.variableCount = 4; + data.vari.variables = variables; + data.code.count = 1; + data.code.entries = &code; + data.func.functionCount = 1; + data.func.functions = &function; + data.strg.count = 1; + data.strg.strings = strings; + data.bytecodeBuffer = bytecode; + + Runner* original = createCheckpointRunner(&data); + VMContext* vm = original->vmContext; + int32_t first = VM_getOrAllocateVarID(vm, "runtime_first"); + Runner_initFirstRoom(original); + int32_t bootstrap = VM_getOrAllocateVarID(vm, "room_bootstrap"); + assert(first == 8 && bootstrap == 9); + assert(RValue_toReal(IntRValueHashMap_get(&vm->globalScopeInstance->selfVars, bootstrap)) == 73); + assert(arrlen(original->instances) == 1 && original->instances[0]->instanceId == 100001); + original->pendingRoom = 1; + Runner_handlePendingRoomChange(original); + int32_t second = VM_getOrAllocateVarID(vm, "runtime_second"); + assert(second == 10); + Instance_setSelfVar(vm->globalScopeInstance, 1, RValue_makeReal(11)); + Instance_setSelfVar(vm->globalScopeInstance, 2, RValue_makeString("player")); + Instance_setSelfVar(vm->globalScopeInstance, 3, RValue_makeReal(33)); + Instance* actor = Runner_createInstance(original, 42, 84, 0); + int32_t actorId = actor->instanceId; + Instance_setSelfVar(actor, 2, RValue_makeReal(22)); + Instance_setSelfVar(actor, 6, RValue_makeUndefined()); + Instance_setSelfVar(actor, first, RValue_makeString("first")); + Instance_setSelfVar(actor, second, RValue_makeReal(99)); + actor->alarm[0] = 19; + actor->activeAlarmMask = 1; + Instance* later = Runner_createInstance(original, 21, 63, 0); + int32_t laterId = later->instanceId; + Instance_setSelfVar(later, first, RValue_makeString("late-owned-string")); + Instance_setSelfVar(later, second, RValue_makeString("late-valid")); + JsonValue* lists = JsonReader_parse("[[[7,\"nested-owned-string\"]]]"); + int32_t cells = 100; + assert(restoreCheckpointLists(original, lists, &cells)); + JsonReader_free(lists); + original->frameCount = 123; + original->gamepads->joystickDevices[0] = 3; + assert(Runner_checkpointStatus(original) == RUNNER_CHECKPOINT_READY); + char* encoded = Runner_dumpStateJson(original); + freeCheckpointRunner(original); + + Runner* fresh = createCheckpointRunner(&data); + vm = fresh->vmContext; + // The fresh VM has a different allocation order and transient state which + // public restore must reset before rebuilding the saved room and instances. + assert(VM_getOrAllocateVarID(vm, "runtime_second") == first); + assert(VM_getOrAllocateVarID(vm, "cold_only") == bootstrap); + assert(Runner_restoreStateJson(fresh, encoded)); + assert(fresh->gameStartFired && fresh->currentRoomIndex == 1 && fresh->currentRoom == &rooms[1]); + assert(fresh->frameCount == 123 && fresh->gamepads->joystickDevices[0] == 3); + assert(arrlen(fresh->instances) == 2 && hmget(fresh->instancesById, roomActor.instanceID) == NULL); + assert(arrlen(fresh->instancesByObject[0]) == 2 && arrlen(fresh->instancesByExactObject[0]) == 2); + actor = hmget(fresh->instancesById, actorId); + assert(actor != NULL && actor->objectIndex == 0 && actor->createEventFired); + assert(actor->x == 42 && actor->y == 84 && actor->alarm[0] == 19 && actor->activeAlarmMask == 1); + assert(fresh->nextInstanceId == (uint32_t) laterId + 1); + assert(IntRValueHashMap_get(&vm->globalScopeInstance->selfVars, 1).real == 11); + assert(strcmp(IntRValueHashMap_get(&vm->globalScopeInstance->selfVars, 2).string, "player") == 0); + assert(IntRValueHashMap_get(&vm->globalScopeInstance->selfVars, 3).real == 33); + assert(IntRValueHashMap_get(&actor->selfVars, 2).real == 22); + assert(IntRValueHashMap_contains(&actor->selfVars, 6)); + assert(IntRValueHashMap_get(&actor->selfVars, 6).type == RVALUE_UNDEFINED); + assert(strcmp(IntRValueHashMap_get(&actor->selfVars, first).string, "first") == 0); + assert(IntRValueHashMap_get(&actor->selfVars, second).real == 99); + assert(VM_getOrAllocateVarID(vm, "runtime_first") == first); + assert(VM_getOrAllocateVarID(vm, "runtime_second") == second); + assert(VM_getOrAllocateVarID(vm, "room_bootstrap") == bootstrap); + assert(RValue_toReal(IntRValueHashMap_get(&vm->globalScopeInstance->selfVars, bootstrap)) == 73); + assert(shgeti(vm->varNameMap, "cold_only") < 0); + assert(VM_getOrAllocateVarID(vm, "after_restore") == second + 1); + assert(arrlen(fresh->dsListPool) == 1 && !fresh->dsListPool[0].freed); + RValue nested = fresh->dsListPool[0].items[0]; + assert(nested.type == RVALUE_ARRAY); + assert(RValue_toReal(*GMLArray_slot(nested.array, 0)) == 7); + assert(strcmp(GMLArray_slot(nested.array, 1)->string, "nested-owned-string") == 0); + freeCheckpointRunner(fresh); + + // Fail in the second instance after restoring DS values, globals, and the + // first instance. Both teardown and a later retry must release partial state. + char* malformed = safeStrdup(encoded); + char* badValue = strstr(malformed, "\"late-valid\""); + assert(badValue != NULL); + memcpy(badValue, "{\"bad\":true}", strlen("\"late-valid\"")); + for (int retry = 0; retry < 2; retry++) { + fresh = createCheckpointRunner(&data); + assert(!Runner_restoreStateJson(fresh, malformed)); + assert(arrlen(fresh->instances) == 2 && arrlen(fresh->dsListPool) == 1); + assert(strcmp(IntRValueHashMap_get(&fresh->vmContext->globalScopeInstance->selfVars, 2).string, "player") == 0); + assert(hmget(fresh->instancesById, actorId)->createEventFired); + assert(!hmget(fresh->instancesById, laterId)->createEventFired); + assert(strcmp(IntRValueHashMap_get(&hmget(fresh->instancesById, laterId)->selfVars, first).string, + "late-owned-string") == 0); + if (retry) { + assert(Runner_restoreStateJson(fresh, encoded)); + assert(arrlen(fresh->instances) == 2 && fresh->frameCount == 123); + assert(strcmp(IntRValueHashMap_get(&hmget(fresh->instancesById, laterId)->selfVars, second).string, "late-valid") == 0); + } + freeCheckpointRunner(fresh); + } + free(malformed); + free(encoded); +} + int main(void) { testCheckpointVariableIdentity(); testCheckpointVariableValidation(); + testCheckpointVariableExhaustion(); + testCheckpointRunnerRestore(); // A late lower physical slot must not change the saved legacy selection // when a new runner restores with both devices already connected. RunnerGamepadState originalPads = {0};