fix(frame): stop a re-anchor during layout from freeing the point being read

Crash #132 at 0x00768323 (FUN_REGION_GET_RECT reading [5+0x3C]): Show lays a frame out before its children leave their layout deferral, so its OnSizeChanged ran; the ScrollBox handler read a region's size, and laying that region out reached an anchor point on a still-deferred child. The point getter (0x007A2F90 / 0x007A3070) ends the relative's deferral with FUN_REGION_LAYOUT_REALIZE(0), which ran the child's OnSizeChanged; that script re-anchored the region, freeing the point, and the getter then read the point again (0x007A2FC5) from freed heap. The getters now end the deferral themselves with the point marked busy. The point's deleting destructor (0x00767D80, the only free path) flags a busy point; a flagged getter returns the layout's undefined value without touching the point, and the point walk (0x007671A0 / 0x007671F0) is run again with the current anchors, up to ten times. Otherwise the engine's own getter reads the value, with the deferred bit hidden so a script that hid the relative again cannot start a second resolve. No hooks: the three getters/destructor are patched in the relative-point vtable (0x0081C44C, their only reference), and the six edge resolvers' calls to the walks are redirected. Adds Common::RedirectSlot for the vtable entries.
2026-11-10 20:07:37 +00:00 · 2026-11-03 20:47:46 -06:00
parent 6f3a8ffc1d
commit 0a3f8324ea
4 changed files with 221 additions and 0 deletions
@@ -64,6 +64,17 @@ bool RedirectJcc(uintptr_t site, uintptr_t expected, const void *target) {
    return PatchBytes(reinterpret_cast<uint8_t *>(site) + 2, &rel, sizeof rel);
 }

+bool RedirectSlot(uintptr_t slot, uintptr_t expected, const void *target) {
+    uintptr_t current = 0;
+    std::memcpy(&current, reinterpret_cast<const void *>(slot), sizeof current);
+    const uintptr_t to = reinterpret_cast<uintptr_t>(target);
+    if (current == to)
+        return true;
+    if (current != expected)
+        return false;
+    return PatchBytes(reinterpret_cast<void *>(slot), &to, sizeof to);
+}
+
 bool ReplaceWithCall(uintptr_t site, const uint8_t *original, size_t len, const void *target) {
    uint8_t code[64];
    if (len < 5 || len > sizeof code)
@@ -28,6 +28,10 @@ bool RedirectCall(uintptr_t site, uintptr_t expected, const void *target);
 // at `site` to `target` when it still reaches `expected`. Same contract.
 bool RedirectJcc(uintptr_t site, uintptr_t expected, const void *target);

+// RedirectCall for a function-pointer slot (a vtable entry): points the slot
+// at `slot` to `target` when it still holds `expected`. Same contract.
+bool RedirectSlot(uintptr_t slot, uintptr_t expected, const void *target);
+
 // Writes `call target` plus NOP padding over the `len` bytes (5 to 64) of
 // code at `site` when they still hold `original`; true when the site
 // now calls `target`. A site holding anything else (another DLL's patch, a
@@ -671,6 +671,39 @@ enum Offsets {
    OFF_ANCHOR_NODE_Y = 0x08,
    OFF_ANCHOR_NODE_RELATIVE_POINT = 0x10,
    VTBL_ANCHOR_NODE_GET_RELATIVE = 0x0C,
+    // The relative node's other three slots (Ghidra: each function's only
+    // reference is its slot): +0x00 the deleting destructor 0x00767D80, `void
+    // *__thiscall(node, int flags)`, `ret 4` — every free of a point (SetPoint,
+    // ClearAllPoints, FUN_REGION_CLEAR_POINT_BY_RELATIVE, the layout destructor)
+    // is `push 1; call [vtbl]`; +0x04 / +0x08 the x / y getters 0x007A2F90 /
+    // 0x007A3070, `float __thiscall(node, float scale)`, `ret 4` (benilla-re
+    // ui/anchors "Relative point coordinates"). A getter ends a deferred
+    // relative's deferral with FUN_REGION_LAYOUT_REALIZE(0) (0x007A2FC0), then
+    // reads the node again (0x007A2FC5).
+    VTBL_ANCHOR_NODE_DELETE = 0x00,
+    VTBL_ANCHOR_NODE_X = 0x04,
+    VTBL_ANCHOR_NODE_Y = 0x08,
+    FUN_ANCHOR_NODE_DELETE = 0x00767D80,
+    FUN_ANCHOR_NODE_X = 0x007A2F90,
+    FUN_ANCHOR_NODE_Y = 0x007A3070,
+    // A layout's point walks, 4.3.4's GetFramePointX / Y: `float
+    // __thiscall(layout, const int *points, int count)`, `ret 8` — calls
+    // VTBL_ANCHOR_NODE_X (Y) of each set slot in `points` with the layout's
+    // scale (+0x58) and returns the first result that is not
+    // VAR_LAYOUT_UNDEFINED. The six edge resolvers are the only callers
+    // (Ghidra xrefs): left / centre x / right call the x walk, top / centre y /
+    // bottom the y walk.
+    FUN_LAYOUT_POINTS_X = 0x007671A0,
+    FUN_LAYOUT_POINTS_Y = 0x007671F0,
+    CALL_LAYOUT_LEFT_POINTS = 0x007673F1,
+    CALL_LAYOUT_CENTER_X_POINTS = 0x00767281,
+    CALL_LAYOUT_RIGHT_POINTS = 0x00767561,
+    CALL_LAYOUT_TOP_POINTS = 0x007674F1,
+    CALL_LAYOUT_CENTER_Y_POINTS = 0x00767381,
+    CALL_LAYOUT_BOTTOM_POINTS = 0x007675D1,
+    // float: the "no value" an edge, a point or a walk returns (benilla-re
+    // s_layoutUndefined); the walk compares with `fcomp [0x00CF550C]`.
+    VAR_LAYOUT_UNDEFINED = 0x00CF550C,

    // The point names the XML loader and SetPoint share (benilla-re
    // ui/anchors "Point slots and point names"), 4.3.4's StringToFramePoint /
@@ -0,0 +1,173 @@
+// This file is part of ClassicAPI.
+//
+// ClassicAPI is free software: you can redistribute it and/or modify it under the terms
+// of the GNU General Public License as published by the Free Software Foundation, either
+// version 3 of the License, or (at your option) any later version.
+//
+// ClassicAPI is distributed in the hope that it will be useful, but WITHOUT ANY
+// WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+// PURPOSE. See the GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along with
+// ClassicAPI. If not, see <https://www.gnu.org/licenses/>.
+
+// A script that re-anchors a region while that region is being laid out no
+// longer frees the anchor point the layout is reading.
+//
+// An edge resolver walks the region's points (FUN_LAYOUT_POINTS_X / Y) and asks
+// each for its coordinate (VTBL_ANCHOR_NODE_X / Y). When the point's relative
+// frame is deferred, the getter first ends the deferral with
+// FUN_REGION_LAYOUT_REALIZE(0), which lays the relative out at once: its
+// OnSizeChanged runs right there. The getter then reads the point again. If
+// that script moved the region with SetPoint or ClearAllPoints, the point is
+// freed, and the read gets heap memory: an access violation in
+// FUN_REGION_GET_RECT on the "relative frame" the freed point holds (crash
+// log 2026-11-03: `[ecx+0x3c]`, ecx = 5). The path is easy to reach: Show runs
+// a frame's OnSizeChanged before its children leave their deferral, and the
+// ported ScrollBox reads sizes from that handler.
+//
+// 3.3.5 fixed it in CFramePoint::GetRelativeRect (0x0049C830): the point is
+// marked busy around its relative's resolve, a point changed while busy is
+// flagged, and the getter stops on the flag without reading the point again.
+// GetFramePointX / Y then return a failure, and GetHorizontalFramePoint /
+// GetVerticalFramePoint (0x00488ED0 / 0x00488F10) walk the points again, up to
+// ten times, with the anchors as they are now. 1.12 has no flag word in its
+// point, and SetPoint never changes a point in place (it frees the old one and
+// allocates a new one), so "changed" here means "freed": the point's deleting
+// destructor (VTBL_ANCHOR_NODE_DELETE, every free goes through it) flags a busy
+// point.
+//
+// The getters end the deferral themselves, as 3.3.5 does, and stop when the
+// point is flagged. Otherwise they call the engine's own getter for the value.
+// If the resolve deferred the relative again (a Hide in its script), the
+// getter would end the deferral a second time. 3.3.5 ends it once and then
+// reads the rect, so the bit is hidden from the engine's getter during the
+// read. The walks are the engine's own, retried as 3.3.5 retries them.
+//
+// Nothing is hooked: the point vtable (VAR_ANCHOR_VTABLE_WITH_RELATIVE) is the
+// only way to reach the getters and the destructor, and the six edge
+// resolvers are the only callers of the walks.
+
+#include "Common.h"
+#include "Game.h"
+#include "Offsets.h"
+
+#include <cstdint>
+
+namespace Frame::AnchorResolve {
+
+namespace {
+
+using Coordinate_t = float(__thiscall *)(void *point, float scale);
+using Walk_t = float(__thiscall *)(void *layout, const int *points, int count);
+using Delete_t = void *(__thiscall *)(void *point, int flags);
+using Realize_t = void(__thiscall *)(void *layout, int defer);
+
+// 3.3.5 GetHorizontalFramePoint / GetVerticalFramePoint: `for (i = 0; i < 10; i++)`.
+constexpr int kWalkAttempts = 10;
+
+// A point whose relative is being laid out. Its destructor sets `freed`.
+struct BusyPoint {
+    void *point;
+    bool freed;
+    BusyPoint *outer;
+};
+
+// A point walk in progress. A getter sets `changed` when its point was freed.
+struct PointWalk {
+    bool changed;
+    PointWalk *outer;
+};
+
+BusyPoint *g_busy = nullptr;
+PointWalk *g_walk = nullptr;
+
+float Undefined() { return Game::Read<float>(Offsets::VAR_LAYOUT_UNDEFINED); }
+
+float Coordinate(uintptr_t engine, void *point, float scale) {
+    if (g_walk != nullptr && g_walk->changed)
+        return Undefined(); // 3.3.5 stops the walk at the changed point
+
+    auto *relative = Game::Read<void *>(point, Offsets::OFF_ANCHOR_NODE_RELATIVE_TO);
+    if ((Game::Read<uint32_t>(relative, Offsets::OFF_LAYOUT_FLAGS) & Offsets::LAYOUT_FLAG_DEFERRED) != 0) {
+        BusyPoint busy{point, false, g_busy};
+        g_busy = &busy;
+        reinterpret_cast<Realize_t>(Offsets::FUN_REGION_LAYOUT_REALIZE)(relative, 0);
+        g_busy = busy.outer;
+        if (busy.freed) {
+            if (g_walk != nullptr)
+                g_walk->changed = true;
+            return Undefined();
+        }
+    }
+
+    uint32_t &flags = Game::Ref<uint32_t>(relative, Offsets::OFF_LAYOUT_FLAGS);
+    const uint32_t deferred = flags & Offsets::LAYOUT_FLAG_DEFERRED;
+    flags &= ~deferred;
+    const float value = reinterpret_cast<Coordinate_t>(engine)(point, scale);
+    flags |= deferred;
+    return value;
+}
+
+float __fastcall PointX(void *point, void * /*edx*/, float scale) {
+    return Coordinate(Offsets::FUN_ANCHOR_NODE_X, point, scale);
+}
+
+float __fastcall PointY(void *point, void * /*edx*/, float scale) {
+    return Coordinate(Offsets::FUN_ANCHOR_NODE_Y, point, scale);
+}
+
+void *__fastcall DeletePoint(void *point, void * /*edx*/, int flags) {
+    for (BusyPoint *busy = g_busy; busy != nullptr; busy = busy->outer) {
+        if (busy->point == point)
+            busy->freed = true;
+    }
+    return reinterpret_cast<Delete_t>(Offsets::FUN_ANCHOR_NODE_DELETE)(point, flags);
+}
+
+float Walk(uintptr_t engine, void *layout, const int *points, int count) {
+    for (int attempt = 0; attempt < kWalkAttempts; ++attempt) {
+        PointWalk walk{false, g_walk};
+        g_walk = &walk;
+        const float value = reinterpret_cast<Walk_t>(engine)(layout, points, count);
+        g_walk = walk.outer;
+        if (!walk.changed)
+            return value;
+    }
+    return Undefined();
+}
+
+float __fastcall WalkX(void *layout, void * /*edx*/, const int *points, int count) {
+    return Walk(Offsets::FUN_LAYOUT_POINTS_X, layout, points, count);
+}
+
+float __fastcall WalkY(void *layout, void * /*edx*/, const int *points, int count) {
+    return Walk(Offsets::FUN_LAYOUT_POINTS_Y, layout, points, count);
+}
+
+void PatchCode() {
+    constexpr uintptr_t vtable = Offsets::VAR_ANCHOR_VTABLE_WITH_RELATIVE;
+    Common::RedirectSlot(vtable + Offsets::VTBL_ANCHOR_NODE_DELETE, Offsets::FUN_ANCHOR_NODE_DELETE,
+                         reinterpret_cast<const void *>(&DeletePoint));
+    Common::RedirectSlot(vtable + Offsets::VTBL_ANCHOR_NODE_X, Offsets::FUN_ANCHOR_NODE_X,
+                         reinterpret_cast<const void *>(&PointX));
+    Common::RedirectSlot(vtable + Offsets::VTBL_ANCHOR_NODE_Y, Offsets::FUN_ANCHOR_NODE_Y,
+                         reinterpret_cast<const void *>(&PointY));
+
+    const auto walkX = reinterpret_cast<const void *>(&WalkX);
+    Common::RedirectCall(Offsets::CALL_LAYOUT_LEFT_POINTS, Offsets::FUN_LAYOUT_POINTS_X, walkX);
+    Common::RedirectCall(Offsets::CALL_LAYOUT_CENTER_X_POINTS, Offsets::FUN_LAYOUT_POINTS_X, walkX);
+    Common::RedirectCall(Offsets::CALL_LAYOUT_RIGHT_POINTS, Offsets::FUN_LAYOUT_POINTS_X, walkX);
+    const auto walkY = reinterpret_cast<const void *>(&WalkY);
+    Common::RedirectCall(Offsets::CALL_LAYOUT_TOP_POINTS, Offsets::FUN_LAYOUT_POINTS_Y, walkY);
+    Common::RedirectCall(Offsets::CALL_LAYOUT_CENTER_Y_POINTS, Offsets::FUN_LAYOUT_POINTS_Y, walkY);
+    Common::RedirectCall(Offsets::CALL_LAYOUT_BOTTOM_POINTS, Offsets::FUN_LAYOUT_POINTS_Y, walkY);
+}
+
+// Each pass runs before its GlueXML or FrameXML loads; the patch is idempotent.
+const Game::GlueModuleAutoRegister _gluePatch{&PatchCode};
+const Game::ModuleAutoRegister _autoreg{&PatchCode};
+
+} // namespace
+
+} // namespace Frame::AnchorResolve