Prevent clearing flag gradients from reading past resource switches

[genixpro]

A clearing flag has five basic-resource switches, but fruit IDs are 5–7. The building-gradient builder indexed that array with any nonempty resource ID, reading object padding for fruit. Different allocation histories, including loading a save, could therefore change clearing targets and unit paths.

Check resource.type < BASIC_COUNT before indexing the switches. Fruit remains an obstacle, consistent with the supported switches. No serialized layout changes; mixed-version replay/network equivalence is not established because the invalid read can affect paths.

Regression

scons -j4 release=1 server=0 clearing-gradient-test
python3 test/run-savegame-safety-tests.py --check-preferences build/src/ClearingFlagGradientTest

Use .exe on Windows. The real-engine fixture varies only padding after the switch array and checks every fruit, enabled/disabled basic-resource switches, empty tiles, and all swimming classes. It uses the existing shared runner with a disposable profile and working directory, and verifies preference preservation. Linux and Windows CI execute it.

Review follow-up and validation

Removed the standalone regression document and dedicated Python runner as requested. Concise usage remains in the existing test README. Follow-up 9e2f6b5f0 incorporates current master: its local-gradient implementation has been removed, so the test now covers the current 16-bit weighted building gradient and all swimming classes. The remaining production fix is one bounds check.

macOS / Apple Clang and Ubuntu 24.04 / GCC 13: the updated optimized clearing regression and existing savegame-safety harness pass. Targeted ASan/UBSan instrumentation of MapGradientBuilding.cpp and the fixture passes against optimized engine objects. The negative control substitutes the unfixed source from master 88934ecfb and fails on the out-of-bounds fruit index. Leak detection is disabled because of the separately observed script-binding leak; this is not a full-engine sanitizer or leak-clean claim.

Final-head CI passes on Ubuntu 22.04, Ubuntu 24.04 and Windows. Kyle's re-review is pending. This author-owned submission requires separate review and merge.

[genixpro]

Verification and policy follow-up on e4dd4d9649c5238b26fa78078bf2c41d0ff2cdbb:

• The two production bounds checks are unchanged and fit a focused memory-safety/determinism bug fix. Fruit IDs cannot index the five basic-resource switches. No architecture or serialized-format changes.
• Replaced the ad-hoc linker script with clearing-gradient-test in SCons, so tests use the configured compiler/platform libraries and sanitizer flags. Added explicit Linux and Windows CI execution; ordinary client CI did not previously run this regression.
• Fixed test ownership with a live headless GameGUI. The runner isolates HOME and its working directory and checks seeded preferences remain byte-for-byte and timestamp-identical. Assertions remain enabled and Windows uses a console entry point.
• On devlaptop.local (Ubuntu 24.04/GCC 13), scons -j4 release=1 server=0 clearing-gradient-test and python3 test/run_clearing_regression.py --build-dir build pass on the final sources.
• Recompiled both affected gradient sources and the harness with -fsanitize=address,undefined -fno-omit-frame-pointer -O1 -g, linking the remaining optimized client objects. With UBSAN_OPTIONS=halt_on_error=1 and ASAN_OPTIONS=detect_leaks=0, the fixed code passes. Replacing only the two conditions with their original forms makes UBSan stop at resource ID 5 indexing bool[5] in MapGradientLocal.cpp. This is targeted instrumentation, not a fully instrumented engine.
• The initial leak-enabled run reports four 32-byte MapScriptUSL native-binding allocations (128 bytes total). Retained that report; leak detection was then disabled for the bounds check. No leak-clean claim or unrelated runtime fix is included here.

New CI is pending. Correcting undefined behavior can change clearing paths relative to the unfixed engine; same save format does not imply mixed-version simulation compatibility. @Giszmo @kylelutze, please independently review the bounds fix and this verification before merging. This will not be self-merged.

[Giszmo]

Verified against master bfc8fd7: clearingResources is bool[BASIC_COUNT] followed by Sint32 minLevelToFlag, so fruit IDs 5-7 read the padding bytes; both gradient builders now use the same < BASIC_COUNT guard that updateLocalResources and Map::isResourceTakeable already had. CI green on all three jobs. Approving.

[kylelutze]

why keep the doc around?

[genixpro]

Removed the dedicated reproduction document in edbb6ae. The test command and coverage now live in the existing test/README.md; the before/after evidence stays on this PR.

[kylelutze]

why does every test need a python runner?

[genixpro]

Agreed. edbb6ae deletes the clearing-specific Python runner and reuses run-savegame-safety-tests.py with an optional --check-preferences flag. Clearing and the existing save-safety suite both pass on Ubuntu 24.04/GCC 13. The shared runner uses a disposable HOME and working directory. New Linux/Windows CI is pending; please re-review this follow-up. The production bounds checks are unchanged.

[genixpro]

Updated to current master in 9e2f6b5 and resolved the merge conflicts. Master removed local gradients and changed building gradients to 16-bit values, so the retained test now exercises the current API and every swimming class. The remaining production change is the same resource-index bounds check.

Optimized clearing and shared save-safety regressions pass on macOS and Ubuntu 24.04. Targeted ASan/UBSan passes on Linux with leak detection disabled; the unfixed master source triggers index 5 out of bounds for bool[5]. CI on this final head now passes on Ubuntu 22.04, Ubuntu 24.04 and Windows, including the clearing regression and savegame-safety checks.

@kylelutze, the document and dedicated-runner requests remain addressed; your re-review request is still open. This has not been merged.

[Giszmo]

Resolve conflicts and merge