fabi/Syplheed-Reborn
Archived
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

re: fix the wrong ISL conditions -- the cause was a coroutine boundary, not the filter

Browse Source 
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 "<unknown: reached after a coroutine boundary>".

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.
This commit is contained in:
Sylpheed RE agent
2026-08-27 05:36:02 +00:00
parent eac5b3e22e
commit 1ac9aa1d7d
4 changed files with 90 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
docs/re/BACKLOG.md
View File

@@ -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

31
docs/re/data/isl-stage02-conditions.txt
View File

@@ -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 <unknown: reached after a coroutine boundary> == 28 -> 0xFB1C
0x0105D8 if <unknown: reached after a coroutine boundary> == 29 -> 0xFBD0
0x0105EC if <unknown: reached after a coroutine boundary> == 30 -> 0xFC84
0x010600 if <unknown: reached after a coroutine boundary> == 31 -> 0xFD38
0x010614 if <unknown: reached after a coroutine boundary> == 32 -> 0xFDEC
0x010628 if <unknown: reached after a coroutine boundary> == 33 -> 0xFEA0
0x01063C if <unknown: reached after a coroutine boundary> == 34 -> 0xFF54
0x010650 if <unknown: reached after a coroutine boundary> == 35 -> 0x10008
0x010664 if <unknown: reached after a coroutine boundary> == 36 -> 0x100BC
0x010678 if <unknown: reached after a coroutine boundary> == 37 -> 0x10170
0x01068C if <unknown: reached after a coroutine boundary> == 38 -> 0x10224
0x0106A0 if <unknown: reached after a coroutine boundary> == 39 -> 0x102D8
0x0106B4 if <unknown: reached after a coroutine boundary> == 40 -> 0x1038C
0x0106C8 if <unknown: reached after a coroutine boundary> == 41 -> 0x10440
0x0106DC if <unknown: reached after a coroutine boundary> == 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

56
docs/re/structures/isl-conditions.md
View File

@@ -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
`<unknown: reached after a coroutine boundary>` 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