From 1ac9aa1d7d6051a551481986ee8b711df73bccdc Mon Sep 17 00:00:00 2001 From: Sylpheed RE agent Date: Thu, 27 Aug 2026 05:36:02 +0000 Subject: [PATCH] re: fix the wrong ISL conditions -- the cause was a coroutine boundary, not the filter The listing showed end_coroutine as the left-hand side of 34 comparisons disc-wide. That is impossible -- it returns no value a script can test -- so it was the bug reporting itself. The recorded fix ("set special[0] only for built-ins that write [phase+164]") is REFUTED. end_coroutine's handler 0x82272624 is `addi r11,r0,1 ; addi r3,r0,3 ; stw r11,164(r31)` -- it DOES write [phase+164], so that filter would have kept it. Reading the handler before writing the filter is what caught this. The real cause: end_coroutine returns 3, which DESTROYS the thread. Execution does not continue past it, so the instructions following it in the flat stream belong to a different routine and every tracked value is stale. The linear walk that makes the decode possible is exactly what walks across that boundary. A/B over all 28 stages, 7563 sites, resetting the tracker at end_coroutine: sites whose operands change 34 (0.45%) LHS = end_coroutine, before -> after 34 -> 0 left as an explicit unknown 34 (0.45%) The two counts being equal is the result: the leak was confined to exactly the sites that displayed the impossible value, so the other 7529 conditions were never affected. Those 34 now print "". Not done, and said so: their RHS is still exact and the LHS is recoverable by seeding the tracker at coroutine entries, whose targets are staged slot 0 of start_coroutine. data/isl-stage02-conditions.txt regenerated; calls and phase-ends both byte-identical. --- docs/re/BACKLOG.md | 16 +++++-- docs/re/data/isl-stage02-conditions.txt | 31 +++++++------- docs/re/structures/isl-conditions.md | 56 ++++++++++++++++++++----- tools/re-capture/isl.py | 18 +++++++- 4 files changed, 90 insertions(+), 31 deletions(-) diff --git a/docs/re/BACKLOG.md b/docs/re/BACKLOG.md index 9245870..5c52f02 100644 --- a/docs/re/BACKLOG.md +++ b/docs/re/BACKLOG.md @@ -1425,10 +1425,18 @@ premise was wrong.** `data/isl-stage02-conditions.txt` finally has a generator (`isl_report.py conditions`); the other two artefacts regenerate identical. Top predicates: `hp_pct_test` 1955, `unit_state` 1257, `unit_relation` 796, - `dist_lt` 450. 🟡 **1.6 % are WRONG:** 15 sites attribute the LHS to - `end_coroutine`, which returns nothing — the tracker sets `special[0]` on every - call, so those show a stale value. Fix = only set it for built-ins that write - `[phase+164]`. ▶️ **Still open:** the 35 unnamed built-ins (`builtin16` 132 + `dist_lt` 450. ~~🟡 **1.6 % are WRONG:** 15 sites attribute the LHS to + `end_coroutine`… Fix = only set it for built-ins that write `[phase+164]`.~~ + ✅ **(2026-08-27) FIXED, and that proposed fix was REFUTED** — `end_coroutine`'s + handler `0x82272624` *does* `stw r11,164(r31)`, so the filter would have kept + it. Real cause: `end_coroutine` returns **3 = destroy the thread**, so the flat + stream continues into a DIFFERENT routine and the tracked state is stale. A/B + over 28 stages: exactly **34 of 7563** sites change, **34 → 0** with an + `end_coroutine` LHS, and the two counts being equal proves the leak was + confined to them — the other 7529 were never affected. They now print an + explicit unknown. 🟡 Recoverable but not recovered: the RHS of each is still + exact, and seeding the tracker at coroutine ENTRIES (`start_coroutine`'s target + is staged slot 0) would resolve the LHS. ▶️ **Still open:** the 35 unnamed built-ins (`builtin16` 132 sites, `builtin105` 117, `builtin103` 115 — each now a vtable-slot lookup); which condition guards each `END_PHASE` (needs the control flow between them); the vtable's length. The condition lives in the `op10`/`op13` poll loop upstream of diff --git a/docs/re/data/isl-stage02-conditions.txt b/docs/re/data/isl-stage02-conditions.txt index c5f6e14..f49f0f3 100644 --- a/docs/re/data/isl-stage02-conditions.txt +++ b/docs/re/data/isl-stage02-conditions.txt @@ -18,7 +18,6 @@ the right-hand side is evaluated (its result lands in `special[0]`), builtin105 38 builtin80 28 request_script_message 26 - end_coroutine 15 read_freg 10 unit_hp_pct 9 builtin7 9 @@ -274,21 +273,21 @@ the right-hand side is evaluated (its result lands in `special[0]`), 0x0103D0 if unit_state(ADT113) == 1 -> 0x10438 0x010484 if unit_state(ADN114) == 1 -> 0x104EC 0x010538 if unit_state(ADN115) == 1 -> 0x105A0 - 0x0105C4 if end_coroutine() == 28 -> 0xFB1C - 0x0105D8 if end_coroutine() == 29 -> 0xFBD0 - 0x0105EC if end_coroutine() == 30 -> 0xFC84 - 0x010600 if end_coroutine() == 31 -> 0xFD38 - 0x010614 if end_coroutine() == 32 -> 0xFDEC - 0x010628 if end_coroutine() == 33 -> 0xFEA0 - 0x01063C if end_coroutine() == 34 -> 0xFF54 - 0x010650 if end_coroutine() == 35 -> 0x10008 - 0x010664 if end_coroutine() == 36 -> 0x100BC - 0x010678 if end_coroutine() == 37 -> 0x10170 - 0x01068C if end_coroutine() == 38 -> 0x10224 - 0x0106A0 if end_coroutine() == 39 -> 0x102D8 - 0x0106B4 if end_coroutine() == 40 -> 0x1038C - 0x0106C8 if end_coroutine() == 41 -> 0x10440 - 0x0106DC if end_coroutine() == 42 -> 0x104F4 + 0x0105C4 if == 28 -> 0xFB1C + 0x0105D8 if == 29 -> 0xFBD0 + 0x0105EC if == 30 -> 0xFC84 + 0x010600 if == 31 -> 0xFD38 + 0x010614 if == 32 -> 0xFDEC + 0x010628 if == 33 -> 0xFEA0 + 0x01063C if == 34 -> 0xFF54 + 0x010650 if == 35 -> 0x10008 + 0x010664 if == 36 -> 0x100BC + 0x010678 if == 37 -> 0x10170 + 0x01068C if == 38 -> 0x10224 + 0x0106A0 if == 39 -> 0x102D8 + 0x0106B4 if == 40 -> 0x1038C + 0x0106C8 if == 41 -> 0x10440 + 0x0106DC if == 42 -> 0x104F4 0x010728 if builtin103() != 0 -> 0x10748 0x010788 if hp_pct_test(TCN004, 30) != 1 -> 0x108D4 0x010814 if dist_lt(TCN004, TCN001, 10000) == 1 -> 0x108D4 diff --git a/docs/re/structures/isl-conditions.md b/docs/re/structures/isl-conditions.md index d44291a..937f69a 100644 --- a/docs/re/structures/isl-conditions.md +++ b/docs/re/structures/isl-conditions.md @@ -69,18 +69,54 @@ And they read as conditions: `dist_lt(ADT308, TCN000, 15000)` — with the world unit established as 1 metre, that is a **15 km** proximity test. -## 🟡 A caveat the numbers themselves expose +## ✅ FIXED — and the cause was not what I expected -15 of Stage 02's 965 sites (1.6 %) attribute the left-hand side to -**`end_coroutine`**, which does not return a value. The tracker sets -`special[0]` on *every* call, so where the right-hand side of a comparison was -not actually a value-producing built-in, the LHS shown is a **stale** -`special[0]` from an earlier call. Those sites are wrong, not merely imprecise. +The first version of this listing showed **`end_coroutine`** as the left-hand +side of 34 comparisons disc-wide (15 in Stage 02). That is impossible — +`end_coroutine` returns no value a script can test — and an impossible output is +the bug reporting itself. -The fix is to set `special[0]` only for built-ins that write `[phase+164]` — -which [`isl-builtin-dispatch`](isl-builtin-dispatch.md) makes checkable, since -112 of 147 are vtable slots whose implementations can be tested for that store. -Not done; recorded so the 1.6 % is not read as accurate. +### 🔴 The obvious fix is REFUTED + +The plan recorded here was *"set `special[0]` only for built-ins that write +`[phase+164]`"*. That would not have worked: `end_coroutine`'s handler +`0x82272624` is + +``` +addi r11, r0, 1 +addi r3, r0, 3 +stw r11, 164(r31) ; it DOES write [phase+164] +``` + +so the filter would have kept it. Checking the handler before writing the filter +is what caught this. + +### ✅ The real cause: a coroutine boundary + +`end_coroutine` returns **3**, which *destroys the thread*. Execution does not +continue past it — so the instructions that follow it in the **flat** stream +belong to a **different routine**, and every value the tracker was carrying is +stale. The linear walk that makes this decode possible at all is precisely what +walks across that boundary. + +Resetting the tracker at `end_coroutine`: + +| A/B over all 28 stages, 7563 sites | | +|---|---| +| sites whose operands change | **34 — 0.45 %** | +| LHS `end_coroutine` before → after | **34 → 0** | +| left as an explicit unknown afterwards | **34 — 0.45 %** | + +**The two counts are equal, so the leak was confined to exactly the sites that +displayed the impossible value** — the other 7 529 conditions were never +affected. Those 34 now print +`` rather than a wrong answer. + +🟡 **They are recoverable but not recovered.** The right-hand side of each is +still exact; only the left is lost. Resolving them means seeding the tracker at +each coroutine **entry** rather than walking in from the previous routine, and +the entries are available — `start_coroutine`'s target is staged slot 0. Not +done. ## 🟡 Not settled diff --git a/tools/re-capture/isl.py b/tools/re-capture/isl.py index 6e4a228..1543ff1 100755 --- a/tools/re-capture/isl.py +++ b/tools/re-capture/isl.py @@ -368,6 +368,11 @@ def conditions(b, sym1=None, sym2=None): MEASURED, and this is why the model is trusted: across all 28 stages push and pop balance at 1877 each with ZERO underflows, and in Stage 02 all 319 `pop.i` sites are immediately followed by `cmp.i`. + + The tracker resets at `end_coroutine`, which destroys the thread. A/B over + all 28 stages: exactly 34 of 7563 sites change, and all 34 are the ones that + previously reported the impossible `end_coroutine` as a left-hand side. They + become an explicit unknown rather than a wrong answer. """ bases = phase_bases(b) sp, loc, stack = {}, {}, [] @@ -412,6 +417,14 @@ def conditions(b, sym1=None, sym2=None): args.append(v) sp[0] = '%s(%s)' % (nm, ', '.join(args)) loc = {} + if words[0] == 11: + # `end_coroutine` returns 3, which DESTROYS the thread -- execution + # does not continue past it, so the instructions that follow in the + # flat stream belong to a different routine and every tracked value + # is stale. Without this reset, 34 sites disc-wide reported + # `end_coroutine` itself as the left-hand side of a comparison, + # which is impossible: it returns no value a script can test. + sp, loc, stack, pend = {}, {}, [], None elif op == 21: stack.append(sp.get(1)) elif op == 22: @@ -423,7 +436,10 @@ def conditions(b, sym1=None, sym2=None): elif op in REL and pend: ph = sum(1 for x in bases if x <= off) tgt = bases[ph - 1] + words[0] if words else None - out.append({'off': pend[0], 'phase': ph, 'lhs': pend[1], + lhs = pend[1] + if lhs.startswith('special['): + lhs = '' + out.append({'off': pend[0], 'phase': ph, 'lhs': lhs, 'rel': REL[op], 'rhs': pend[2], 'branch': off, 'target': tgt}) pend = None