Diag/issue27 tso fences - #56
Open
ITSeniy wants to merge 30 commits into
Open
Conversation
rc2 telemetry from issue SansNope#27 devices shows the corruption is deeper than unset pointers: object vtable slots contain other owners' data (the float 0.7f, XML text " </"), the game's freelist allocator pops a head of -1, and KeBugCheck fires - i.e. the game's lightly-synchronized structures tear. They demonstrably work on in-order Xenon and on x86-TSO (desktop upstream), so the working hypothesis is ARM64's weak memory ordering. Give every guest load acquire semantics and every guest store release semantics via thread fences (not ldar/stlr - guest accesses may be unaligned). Measured cost on TB710: none observable (~59.5 FPS). If this eliminates the crashes on the affected devices, a production variant follows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The TSO fence experiment did not stop the issue SansNope#27 corruption, but the rc2 telemetry narrowed the pattern: foreign STACK-shaped data (ASCII spaces, XML parser text, lerp floats) overwrites heap objects, the crash moved into o1heapFree on a wild pointer, and the game's own CRT abort handler reaches KeBugCheck. All guest threads were given a fixed 256 KiB stack carved from the shared user heap with no guard page - and the stack size the game requests through ExCreateThread was ignored, so any thread needing more silently overran into neighbouring heap blocks. - Honor the requested stack size (rounded up to 64 KiB, min 256 KiB). - Fill the bottom 256 bytes of every guest stack with a canary pattern; the Android watchdog checks all live stacks once per second and the thread teardown re-checks, logging "GUEST STACK OVERFLOW" with the thread id - one log line per overrun. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The stack canaries never fired (diag6), and the garbage found in corrupted objects turned out to be asset data (Havok "Mopp" tags, XML) rather than stack frames, while a new crash surfaced in NtResumeThread with a null kernel handle - every crash so far is a downstream symptom of the same primary corruption. Stop deducing and catch the writer directly: - When the animation-node guard sees a broken data pointer, it arms an mprotect(PROT_READ) trap on that node's page. - The SIGSEGV handler recognizes write faults on the watched page, logs "PAGEWATCH HIT ... pc=libmain+0x..." (async-signal-safe, through the stderr mirror), restores RW and resumes execution; the next guard hit re-arms the page. Up to 64 hits are logged, 512 arms max. The EXACT-tagged hits are writes to the broken field itself; their pc symbolizes offline to the corrupting code path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
diag7's page watch showed sub_82BB5870 (main thread) allocating a count*48 buffer through sub_82EA8A30 and copying data right over the live animation node 16 bytes above it - the Havok hkThreadMemory-style bucketed freelist handed out overlapping blocks. Track every alloc/free from sub_82EA8A30 / sub_82EA8AB0 in a 64K-event ring buffer plus a live-block bitmap (1 bit per 8-byte granule of the first 512 MB): a block handed out while still live is reported immediately with both owners, and the evaluator guard dumps the lifetime history of the broken node and its parent to distinguish use-after-free from double allocation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
main carries the earlier issue SansNope#27 diagnostic builds (PRs SansNope#48/SansNope#52, diag2-4). This branch supersedes most of that: memory.cpp keeps the version with the readable null page plus the indirect-call guard, the log banner stays diag8-alloctrack, and the launcher icons stay at their final versions. misc_patches.cpp merges cleanly into a union of all diagnostics: the evaluator guard with page watch and allocator lifetime history, diag3's deep node dumps, diag4's healthy-run reference dumps and the sub_82F76698 registration dump. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Brings the 0.4.0 release into the diagnostic branch: version code 13 / version name 0.4.0 and the release CHANGELOG. The log banner stays diag8-alloctrack so tester logs identify the build. The diag2-4 deep node dumps drop out of misc_patches.cpp (the release removed them from main and they already served their purpose - the layouts are identified); the evaluator guard, page watch and allocator lifetime tracker remain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
diag8 logs from AYN Thor, Odin2 and Poco F5 all show the identical use-after-free chain: a 512-byte node created by sub_822EBF60 is destroyed by its own deallocating destructor (sub_82ED4BB8) while a live parent blend node (sub_82F76248/sub_82F76698 lifecycle) still references it, and within a dozen allocator events sub_82BB5870 reuses the memory for transform buffers whose floats then get read through the dangling child pointer. Hold freed blocks of <= 512 bytes in a per-thread FIFO of 1024 entries before really freeing them, and poison the payload with 0xFF so a dangling reader deterministically sees -1, which the evaluator guard already skips. The heap instance is per-thread TLS state, so deferring the tail call from the same thread preserves the allocator's threading contract. If the ring crash disappears with this build, the chain is proven end to end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
diag9 proved the use-after-free end to end: with freed blocks quarantined and poisoned, the dangling child reads became deterministic 0xFFFFFFFF and the ring pickup stopped crashing - but the crash moved to sub_82F768E0, and its code explains the whole corruption cascade. The function clones a parent node (copying the child pointers at +16/+20) and registers the clone by writing through each child's slot-table pointer at child+76. Through a destroyed child this re-propagates the dangling pointer into every clone generation and scatters writes through garbage table pointers - the heap corruption amplifier behind the wild downstream crashes. Repair the source node before cloning: a dead child pointer (destructed or poisoned payload) is replaced with the surviving sibling, degrading the type-4 blend to a self-blend instead of corrupting memory; if both children are dead the clone is skipped (callers iterate a batch and ignore the return value). Repairs are logged rate-limited. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SansNope#58) The mod scanner walked directories alphabetically with a 1000-file budget, and a mod's content folder ("mod") sorts before its manifest ("mod.ini"). A mod with thousands of content files - Low End Mod ships 2494 - burned the whole budget inside its own content tree, so neither its mod.ini nor any mod that sorted after it was ever discovered. Parse the ini files of each directory level before descending into subfolders, and stop descending once a directory is identified as a mod root: mods never nest inside one another, so a mod now costs a handful of scanned files no matter how much content it ships. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bring back the classic Sonic artwork as the launcher icon, fixing what was wrong with both previous sets: the pre-1.5 icons were legacy-only (rendered as a small square in a white circle on modern launchers), and the 1.5 set declared a transparent adaptive background, which the spec forbids and some launchers render as a black plate with a padded picture. The artwork now fills the full 432px adaptive canvas on both layers (parallax-safe), the legacy densities are the crisp originals, and the themed-icon monochrome is regenerated from the same art. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The launcher gains "Install game files" and "Install a mod" actions. Both accept a ZIP archive or a folder through the system picker and copy in the background with progress and cancel. The content root is located automatically: for game files, the folder holding game/default.xex (however deeply nested the dump is); for mods, every folder holding a mod.ini, so multi-mod archives install in one go. Raw dumps without the desktop installer's patched/ output are now bootable: the native side creates patched/default.xex from game+update with XexPatcher on first launch, and the launcher no longer counts the missing file against install readiness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The touch overlay now follows the game context. While a navigation menu is up (title screen, main menu, world map - stamped from CGameModeStageTitle::Update - or the pause HUD / options menu) the left analog stick becomes an 8-way D-pad and the camera finger is released so it cannot swallow taps. During Inspire cutscenes every control collapses into a single wide SKIP button at the Start position that sends START. Menu presence uses a decaying per-frame timestamp, so no destructor hooks are needed; the cutscene flag is edge-triggered from the existing CScene ctor/dtor hooks. The WMV movie manager singleton was deliberately not used as a cutscene signal: it outlives playback and locked the whole title screen into SKIP mode when tried. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The in-game options menu no longer shows settings that are meaningless on a phone: Window Size, Monitor, Fullscreen and VSync (the app always runs fullscreen on the one display and the compositor vsyncs regardless), plus Allow Background Input (Android apps pause in background). Everything else - aspect ratio, resolution scale, FPS and the quality options - stays. The launcher, mod manager and installer UI are now translated into the game's supported languages - Japanese, German, French, Spanish, Italian - plus Portuguese for the sizeable Brazilian tester community. Russian gains the previously missing DocumentsProvider strings. Locale selection follows the system (or per-app) language setting. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Texture Quality (Video category: Full / Half / Quarter) serves every game texture from further down its own mip chain at load time - the DDS data already contains the smaller levels, so Half skips one level and Quarter two, cutting GPU memory and sampling bandwidth roughly 4x/16x for large textures. This covers what low-end texture-pack mods do with hundreds of megabytes of repacked archives, but applies to the base game, DLC, mods and cutscenes alike with zero extra files. Textures without mip chains (UI, fonts) and anything that would drop below 32px are left untouched, so HUD art keeps its pixels. Works with the BC fallback path on non-BC GPUs since transcoding is per-slice. Shadow Resolution additionally gains a 256 step below 512 for the weakest devices. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The reflection scene pass is gated by a virtual method (sub_82BAC240) that early-outs when the global reflection-enable flag at 0x832FA0D8 reads zero, which is the normal path on every stage without reflective surfaces. The new Video option presents that zero to just this function, skipping the whole reflection render pass on stages that use it (towns, water surfaces) while leaving the flag intact for the rest of the engine - host-side pipeline compilation reads the same address to pick MSAA variants. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
diag10 closed the clone path, but the Odin 2 Max log showed the same registration pattern crashing in a third place: sub_82F76698, the pool swap-remove, copies the pool's last node over the removed one and then re-registers it through u32[child+76] of both children - reading the quarantine poison as a pointer (fault at base+0xFFFFFFFF+76 = 0x200000000). Extract the child repair into a shared helper and apply it to both the clone and the flusher's top-of-pool node before the originals run. When both children are dead the repair now substitutes a synthetic sink node (data pointer reads as destroyed so the evaluator skips it; slot table points at a private scratch buffer so registrations land harmlessly), since the flusher cannot be skipped without breaking pool bookkeeping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The diag11 Odin 2 Max log validated the repairs (four fired, zero corruption) but the game itself then hit its own pose consistency assert (KeBugCheck from sub_82BB5870) within milliseconds, on the same thread. Substituting the surviving sibling for a dead child re-arms the type-4 blend with a mismatched clip: the node evaluates again and produces pose data whose bone counts do not match, which the game's own validation rejects. Repair with the sink node in every case instead: registration writes stay safe, and the sink's destroyed-reading data pointer keeps the evaluator guard skipping the node - the same silent skip diag10 already proved visually harmless. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dget The diag12 Odin 2 Max run got much further (past the ring segment) and kept the node repairs clean, but two symptoms showed stale references into blocks above the 512-byte quarantine cutoff: a live parent node's child pointer overwritten with float data, and a poison pointer read out of a CRT lock list (sub_831E1890, fault at base+0xFFFFFFFF). The count*48 transform buffers reach 13792 bytes and were being reused immediately. Quarantine everything this allocator frees (up to 64KB per block) under an 8MB per-thread byte budget with oldest-first eviction, so both stale reads and stale writes land in held, poisoned memory instead of someone else's fresh allocation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
diag13 crashed inside the node pool's grow path reading the freed pool descriptor - the fifth distinct site executing read-modify-write sequences on freed objects. Guarding consumers one by one is confirmed whack-a-mole; the poison itself turns every remaining site into a crash. Recreate what made the Xbox 360 tolerate this race instead: - Quarantined blocks keep their contents (poison removed). Stale code now sees consistent stale data and dies off naturally within frames, just like on console, while the deferred reuse still prevents the actual corruption. - Destructors do leave 0/-1 in pointer fields, and 64-bit recompiled address arithmetic sends those dereferences past the 4GB arena instead of wrapping. Map a 16MB anonymous RW absorber region right after the arena so 0xFFFFFFFF+offset accesses read zeros / write into scratch instead of faulting. The pages cost nothing until touched. The evaluator guard, sink repairs and ALLOCTRACK stay for diagnostics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
diag14 (no poison) brought back wild writes: with block contents intact, teardown-racing code follows chains of plausible stale pointers for multiple hops and eventually writes through memory reused at another allocator level - an o1heap fragment header died exactly that way. diag13 (poison, no absorber) failed the opposite way: every stale chain was cut at its first hop, but the cut itself (dereferencing 0xFFFFFFFF plus a field offset) faulted past the arena end - all five observed crash sites were exactly that fault. Ship the pair: poison cuts every stale chain immediately, and the 16MB absorber region turns the cut into a harmless zero-read or scratch-write. u32-wrapped variants of the same accesses land in the low arena, which is already mapped RW on Android. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The diag15 run reached the loop-de-loop with every memory protection holding: sinks, skips and absorbed indirect calls all fired cleanly and no corruption followed. The remaining death is the game's own CRT assert (KeBugCheck via sub_82BB5870's pose validator) noticing the incomplete animation output that the guards necessarily produce when they skip destroyed nodes. The guest assert handler restores its stack and returns normally when the bugcheck comes back, so a returning KeBugCheck/KeBugCheckEx costs one imperfect pose frame instead of the process. Log each occurrence rate-limited; desktop keeps the debug trap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The diag16 pair of logs proved the tolerate-and-skip strategy has hit its ceiling: memory protection held everywhere (three guest asserts survived, zero corruption), but skipping destroyed nodes broke game logic instead - intangible walls (the skipped nodes are collision agents), a job-counter freeze with every guest thread parked on futexes, and a cleanup path that freed garbage after surviving its assert. Fix the lifetime bug itself: defer sub_82ED4BB8, the shared deallocating destructor of the node family, by 100ms. The object stays fully alive and registered while the queued collision pairs that still reference it drain, then the original destructor runs unchanged; its free then lands in the allocator quarantine, adding a second grace layer. Only deallocating calls (r4 & 1) are deferred - non-deallocating destructors target stack or embedded objects whose memory the caller reclaims immediately. All previous layers (quarantine, poison, absorber, guards, soft asserts) stay as the safety net beneath. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
diag17's ALLOCTRACK shows the node frees still coming from inside the deferred destructor - the hook worked, the destructor ran 100ms late, and the evaluator guards still caught dead children afterwards. The stale references therefore outlive 100ms. One second is a cheap decisive experiment: if the guards still fire, the references are effectively unbounded, which means the collision-pair removal event itself is being lost and the drain in sub_82EF2348 is the only remaining fix site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reverse engineering the pair pipeline pinned the loss mechanism the deferral experiments pointed at. During a physics step, pair adds and removes are queued as raw collidable pointers (sub_82EF18B0 at dispatcher+108, sub_82EF19F0 at dispatcher+176) with nothing pinning the entities. The drain (sub_82EF2348) then: - creates agents for queued adds via sub_82EF5680 even when the collidable is already freed - the agent is born dangling, registered into the live partner's slot table, evaluated inline (the original ring crash) and unremovable forever, since its identity lives in freed memory; these undead agents are the references that outlive any deferral window; - looks up agents for queued removes via sub_82F760A8, which walks the collidable's slot table at +76/+80 and silently misses when that memory reads as poison. Both drain call sites tolerate a null result, so the fix recognises destroyed pair members through a global quarantine bitmap and returns null from both functions: no agent is created for a dead pair, and lookups on dead members miss immediately instead of walking poison. This is the same tolerance the Xbox 360's intact-until-reuse memory provided, expressed explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pre-handoff review of every assumption behind the pair filter: - the drain call site passes the same interior entry pointers as the immediate path (r3/r4 = collidable + s8 offset; poison makes the offset -1, keeping the pointer inside the quarantined block for the bitmap); - all five sub_82F760A8 callers null-check the result; sub_82EF5680's drain caller null-checks, the immediate caller ignores the result, and the tail-call wrapper propagates the same contract; - the 16MB absorber region is confirmed mapped RW on-device (100000000-201000000 in /proc/maps). The diag17/18 destructor deferral is removed: it disproved its own timing-window premise and running destructors on whichever thread later drains the queue is a concurrency pattern the game never had. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Six diag19 tester logs across two devices produced a clean negative: the pair-entry pointers handed to sub_82EF5680 are never quarantined (zero filter hits), yet the evaluator still capped out on poisoned children and diag8's event ordering shows agents born after a node's death. The two facts reconcile in the drain disassembly: the factory takes the actual animation/physics node from entry+184 (the same field the enqueue dedup compares), so a live collidable can carry a dead node. Check the +184 node pointers of both entries in the filter. The local repro perturbation (drain-entry sleep) also produced nothing in 16 runs, consistent with the drain not being the race site; the markers were removed from the tablet for the smoke. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
diag20's zero filter hits eliminated the last indirection hypothesis, so
this round went back to enumeration: agent nodes are born in exactly one
function, sub_82F764C8(?, childA, childB, ...), which stores both children
at +16/+20 and registers the newborn into each child's registration table
(u32[child+76], count at +80, 8-byte {agent, partner} entries - the layout
sub_82F760A8 walks). The family destructor never walks that table: parents
are simply not told their child died. That missing notification is the
whole defect.
Two hooks close it end to end:
1. births: sub_82F764C8 refuses to create an agent whose child is already
destroyed (quarantine bitmap check on the direct child arguments);
2. deaths: the destructor hook walks the dying node's registration table
and points every live parent's matching child slot at the sink node -
delivering the notification the game never sends.
Both paths log rate-limited markers ("refused agent birth", "notified N
live parents") so tester logs directly show which half fires in the wild.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
diag21's own marker diagnosed its bug: notified=0 in every log while the evaluator still skipped poisoned children. Parents never reference the object base r3 - every broken child pointer across every log is base+0x10 (090E4D10 in block 090E4D00, 0A6A3510 in 0A6A3500...). The node is embedded at +0x10, so its registration table pointer lives at object+0x5C, which is precisely the field the first crashes of this issue faulted on (guest reads of 0x5C through a null node pointer). Walk the table from the embedded node and match parent child-slots against the node address. Everything else unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The o1heap crashes present in the diag20 AND diag21 logs (o1heapAllocate unbin on garbage fragment pointers, via RtlAllocateHeap) are a second leg the node-notification work never touched: the same teardown races leave stale guest pointers into RtlAllocateHeap memory, and a stale write into a freed-and-reused user-heap block corrupts o1heap fragment headers. Defer the real o1heapFree on the user heap behind a FIFO (4096 entries / 16MB, oldest completes first). Contents stay intact - no poison, since o1heap only writes its metadata once the fragment is actually freed - so stale readers and writers see the Xbox 360's memory-intact-until-reuse behaviour on this heap as well. Shipping this together with the diag22 notification fix instead of waiting for testers to rediscover the second leg one build later. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
diag23 confirmed the notification fires in the wild (notified=16, one crash-free run, furthest progress yet) and precisely exposed the cost of the sink approach: skipped nodes drive the game's pose-consistency assert into a storm - 16+ survived KeBugChecks in one run - whose handler frames overflowed a guest stack (the diag6 canaries fired for the first time ever) into the user heap, ending in the memcpy crash. Replace sink substitution in the destructor hook with the game's own agent removal: sub_82EF58A0(agent) is self-contained (derives its dispatch context from the agent itself) and is exactly what the remove-pair drain calls. Dying nodes now remove every registered agent properly - deregistered from both children's tables and the animation job list - so the pose graph stays consistent: no skips, no asserts, no zombie agents. Progress guards force the loop forward on unremovable entries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.