447 lines
24 KiB
Markdown
447 lines
24 KiB
Markdown
# Asks for the llvm816 toolchain session
|
|
|
|
JoeyLib-side findings that need toolchain-side fixes. Context:
|
|
PERF-AUDIT.md (findings #79, #77) and PERF-AUDIT-PLAN.md Phase 1.
|
|
JoeyLib does not modify toolchains/iigs/llvm-mos per the ownership
|
|
split -- these are requests, with evidence.
|
|
|
|
## 1. MVN-based memset/memcpy in runtime/src/libc.c (PERF-AUDIT #79)
|
|
|
|
`memset`, `memcpy` (and `memmove`'s copy loops) in
|
|
`toolchains/iigs/llvm-mos/runtime/src/libc.c` are one-byte-at-a-time
|
|
loops through 24-bit pointers:
|
|
|
|
void *memset(void *dst, int c, size_t n) {
|
|
char *d = (char *)dst;
|
|
while (n--) *d++ = (char)c;
|
|
return dst;
|
|
}
|
|
|
|
That is ~30 cycles/byte on the 65816. The ORCA/C baseline these
|
|
replaced lowers both to MVN block moves (~7 cycles/byte after setup).
|
|
|
|
Measured impact on the reference platform (UBER, honest post-Phase-0
|
|
clock): jlScbSetRange -- a 200-byte memset plus a flag write --
|
|
measures 163 ops/sec on the clang build vs 1005 ops/sec on the
|
|
ORCA-era capture (~4x regression at the whole-op level). Every
|
|
memset/memcpy-based JoeyLib path pays it: surface copies, sprite
|
|
save/restore fallbacks, SCB range fills, stage-alloc clears, the log
|
|
buffer, and all example/game code.
|
|
|
|
Ask: MVN-based memset/memcpy in libc.c (or intrinsic lowering in the
|
|
backend). Requirements from the JoeyLib side:
|
|
- Handle bank-crossing source/destination (MVN wraps within a bank;
|
|
the routine must split runs at bank boundaries).
|
|
- memset needs a 1-byte seed write + overlapping MVN (classic
|
|
self-copy fill), or an unrolled STA loop for short fills.
|
|
- Keep the size_t=unsigned long (32-bit) signature -- JoeyLib call
|
|
sites pass 32-bit counts today and the current behavior is correct,
|
|
just slow.
|
|
- A short-count fast path (n < ~16) matters too: the sprite
|
|
interpreter's per-row copies are 4-17 bytes.
|
|
|
|
## 2. FYI: sprite-codegen corruption under investigation (PERF-AUDIT #77)
|
|
|
|
JoeyLib's IIgs runtime sprite code generation corrupts memory when
|
|
enabled (bisected; disabled by default in spriteCompile.c until
|
|
root-caused). The current suspect list is JoeyLib-side (emit-time
|
|
buffer handling), but if the hunt lands on codegen/ABI behavior of
|
|
clang-emitted calls into arena-resident routines, it becomes a
|
|
toolchain question. Nothing to do yet -- flagged so the context is not
|
|
a surprise if it arrives.
|
|
|
|
## 3. fflush -> FST commit semantics (PERF-AUDIT #80)
|
|
|
|
JoeyLib's logger (src/core/debug.c) fflush()es after every line so a
|
|
crash/hang leaves the log on disk. On the clang runtime the lines do
|
|
not reach the disk image until fclose: a run that executed 33 logged
|
|
operations left 3 lines on disk after a forced emulator exit. Question:
|
|
does the libc fflush propagate to a GS/OS FST flush for the file, or
|
|
only drain the userland buffer? If the latter, an FST-flush call in
|
|
fflush (or an exposed fsync-like hook) would restore crash-durable
|
|
logging. Not urgent -- JoeyLib works around it with a memory mailbox
|
|
under emulation -- but real-hardware crash forensics wants it.
|
|
|
|
## 4. malloc/calloc returns 1 (not NULL) at heap exhaustion (PERF-AUDIT #81) -- CRITICAL
|
|
|
|
Empirically confirmed under MAME with in-app probes: with the default
|
|
link-layout heap (665 bytes at $00:BC67-$00:BF00 in the JoeyLib UBER
|
|
build), `calloc(1, ~720)` returns **1**. The bump-path overflow branch
|
|
in libc.c malloc -- whose comment already records a "historical
|
|
`p + HDR_SZ + n > heapEnd` over-heap miscompile (only manifested for
|
|
oversized n)" and assumes the path is unexercised -- is still
|
|
miscompiled in exactly that oversized case. Downstream, JoeyLib ran for
|
|
weeks on a phantom struct at (void*)1 whose first field happened to
|
|
alias the real framebuffer pointer; see PERF-AUDIT.md #81 for the full
|
|
forensics. JoeyLib no longer allocates anything large on the C heap,
|
|
but ANY program that exhausts this heap gets silent corruption instead
|
|
of NULL. Minimal repro: link any GS/OS app with default heap, call
|
|
calloc(1, 720), observe the return value.
|
|
|
|
## 5. Segment BSS/heap spans are not reserved from the Memory Manager (PERF-AUDIT #81/#77/#85) -- CRITICAL, EMPIRICALLY PROVEN
|
|
|
|
Three independent confirmations, in escalating severity:
|
|
1. The bank-0 C heap (unreserved) -- #81's phantom-stage chain.
|
|
2. The pinned SHR framebuffer $01:2000-$9CFF (unreserved) -- an MM
|
|
handle landed at $01:8B04 inside the display (#85).
|
|
3. THE GENERAL CASE: a 32KB NewHandle was placed at $08:4200 -- inside
|
|
the app's own ENTRY-SEGMENT BANK, overlapping seg1's BSS span. The
|
|
loader only claims each segment's image bytes; everything above
|
|
(BSS, heap) is MM-free. ANY GS/OS allocation -- the app's own or
|
|
the system's -- can land on live program data.
|
|
|
|
JoeyLib has stopgap claims for cases 1-2 (jlpInit reserves the
|
|
framebuffer and $00:B900-$BEFF). Case 3 was attempted app-side on
|
|
2026-07-05 (spacetaxi acceptance test) and the attempt produced two
|
|
findings you should have:
|
|
|
|
(a) The app CAN learn its BSS spans: the linker-emitted
|
|
__bss_seg{0..3}_{lo16,bank,size} symbols (the ones crt0.s zeroes
|
|
BSS through) are readable from C via a `.long` DATA32 table in an
|
|
asm TU -- direct C IMM16 references fail to link ("cross-segment
|
|
reloc ... uses 2-byte form; only IMM24 / DATA32 supports cINTERSEG
|
|
patching"). The DATA32 entries resolve as RELOCATED FULL ADDRESSES:
|
|
the lo16 entry reads back as the 24-bit runtime span base
|
|
($08:4F00 observed for the spacetaxi build), the size entry
|
|
carries the length in its low 16 bits, empty segments read as
|
|
bank-base-only. Recipe verified working end-to-end under MAME.
|
|
|
|
(b) BUT actually claiming the span kills the app: NewHandle(
|
|
attrAddr|attrFixed|attrLocked, base=$08:4F00, size=$6619) from
|
|
jlpInit SUCCEEDS -- and the app then dies before reaching main's
|
|
first log line, with the PC profile showing GS/OS desktop idle
|
|
(silent return-to-Finder). Reproduced twice; reverting only the
|
|
claim restores the previous behavior. Mechanism unknown from the
|
|
app side -- presumably the Loader/MM bookkeeping objects to a
|
|
user handle overlapping a loaded segment's address range, or the
|
|
claim starves something during the remaining init. The reverted
|
|
implementation was never committed; the full code (asm table +
|
|
claim loop) is reproduced verbatim in the copy of this item handed
|
|
to the toolchain session at /home/scott/claude/llvm816/JOEYLIB-ASKS.md.
|
|
|
|
Ask: crt0/omfEmit/loader cooperation so every segment's FULL
|
|
in-memory span (image + BSS + heap) is Memory-Manager-reserved at
|
|
load time -- with (b) suggesting the reservation must be made BY the
|
|
loader glue (or before/while segments are registered), not by app
|
|
code after the fact. This single fix unblocks IIgs sprite codegen
|
|
(PERF-AUDIT #77), the Space Taxi IIgs port (a 32KB font-surface
|
|
jlpBigAlloc lands on live state layout-dependently -- three distinct
|
|
failure modes observed as the binary layout shifted), and removes the
|
|
whole corruption class.
|
|
|
|
link816 defines [__heap_start, __heap_end) as the leftover entry-bank
|
|
space above BSS, but crt0Gsos never claims that range from the GS/OS
|
|
Memory Manager, so NewHandle can legally place blocks on top of live
|
|
malloc data (a second, independent corruption channel -- and the
|
|
crt0Gsos comments already record a Phase-1.1 fopen-hang caused by this
|
|
class). Ask: reserve the heap range at startup (attrAddr|attrBank|
|
|
attrFixed|attrLocked), or reimplement malloc on top of an MM handle.
|
|
Related hardening: pass attrNoSpec on NewHandle calls in the runtime,
|
|
and consider reserving the pinned SHR stage range $01:2000-$9FFF that
|
|
JoeyLib's jlpStageAllocPixels hands out as a bare constant.
|
|
|
|
## 6. Post-main exit path crashes to the monitor (COP $00 at $00:0002) -- EMPIRICALLY PROVEN, app-independent (2026-07-20)
|
|
|
|
Every JoeyLib IIgs example that RETURNS from main() dies in the
|
|
monitor instead of quitting to Finder. Reproduced identically for
|
|
KEYS and STAXI under MAME (deterministic, ~3 min): press the app's
|
|
exit key, and execution ends at `COP #$00` executing at $00:0002 with
|
|
S=$17FD. Nobody saw this before because every harness kills the
|
|
emulator after reading logs; an interactive ESC was the first real
|
|
exit ever exercised.
|
|
|
|
MAME debugger execution history at the crash (bp at $000002):
|
|
|
|
00:9EE0: sta $00c068 ; GS/OS call tail: state reg restore
|
|
00:9EE8: xce
|
|
00:9EE9: plp
|
|
00:9EEA: rtl ; returns to the app-side thunk:
|
|
08:0072: php ; <- pushes 1 byte...
|
|
08:0073: rtl ; <- ...then pulls 3: garbage return
|
|
00:0002: cop #$00 ; monitor
|
|
|
|
The thunk at segment offset $0072 (bank $08 at run time) executes
|
|
`php / rtl` after a GS/OS call returns to it -- the php mis-balances
|
|
the stack and the rtl returns to $00:0002. This is in the crt0Gsos /
|
|
runtime exit code (the offset is constant across different apps, so
|
|
it is runtime code, not app code). Suspects: a Quit stub whose body
|
|
was meant to sit between the php and the rtl, or a mis-assembled
|
|
epilogue.
|
|
|
|
Reproducer scripts (MAME, headless, breakpoint + history + RAM dump):
|
|
this session's scratchpad escCrash.sh / escCrashKeys.sh -- ask me and
|
|
I'll re-stage them anywhere durable.
|
|
|
|
WORKAROUND LANDED IN JOEYLIB (so this is no longer user-visible):
|
|
src/iigs/hal.c jlpShutdown now issues GS/OS QuitGS ($2029, pCount=0)
|
|
itself via a runtime-built jsl stub and never returns to the runtime
|
|
exit path. jlShutdown therefore does not return on IIgs. The root
|
|
cause still deserves a runtime fix: any app that returns from main
|
|
WITHOUT calling jlShutdown (or any non-JoeyLib program built on this
|
|
runtime) still crashes.
|
|
|
|
## 7. Volatile uint8_t loads through variable pointers widen to 16-bit bus reads (2026-08-18) -- EMPIRICALLY PROVEN
|
|
|
|
A `volatile uint8_t` LOAD through a variable (long) pointer is emitted as a
|
|
16-bit `lda [dp],y` with m=0 and the value masked afterwards (`and #$ff`) --
|
|
so the BUS performs a phantom read of address+1. For memory that is harmless;
|
|
for I/O it reads a neighbouring register, and hardware with adjacent registers
|
|
has read side effects. Concrete damage on the IIgs SCC (Z8530, four adjacent
|
|
registers $C038-$C03B): a data-register read of channel B ($C03A) phantom-pops
|
|
channel A's RX FIFO ($C03B -- the AppleTalk port), and in a printer-port
|
|
configuration a status poll of $C039 phantom-pops the MODEM port's data at
|
|
$C03A. (A per-poll RR1 harvest also correlates with MAME 0.264 SCC RX-deafness, but
|
|
that is NOT this bug: it reproduced through a guaranteed 8-bit read too, so
|
|
the phantom read is refuted as its mechanism and the harvest ships disabled
|
|
under emulation. The width violation itself stands proven by disassembly.)
|
|
|
|
The width handling is inconsistent, which is what makes it look like a bug
|
|
rather than a policy:
|
|
|
|
- STORES through the same pointers are correctly sep #$20-wrapped 8-bit.
|
|
- LOADS from CONSTANT addresses are correctly sep-wrapped 8-bit
|
|
(`sep #$20 / lda $c038 / rep #$20` -- verified via a probe TU).
|
|
- Only variable-pointer loads take the 16-bit `lda [dp],y` + mask form.
|
|
|
|
Ask: honour the declared access width for volatile loads in the long-indirect
|
|
form -- sep-wrap them exactly like the constant-address and store cases.
|
|
|
|
WORKAROUND LANDED IN JOEYLIB: src/iigs/serial.c routes every serial register
|
|
read through a hand-asm `jlpIoRead8` (global function, so no call site can be
|
|
inlined back into the widened form): `sep #$20 / lda [$e0],y / rep #$30 /
|
|
and #$ff`. ABI verified from a compiled probe: pointer lo16 in A, bank in X,
|
|
return in A, $e0-$e3 imaginary-register scratch. Any OTHER volatile I/O read
|
|
through a variable pointer (input/video HALs, future code) is still exposed
|
|
until the codegen fix lands.
|
|
|
|
## 8. Peeled-first-iteration address negation is one's complement, not two's (2026-08-20) -- FIXED IN TOOLCHAIN same day (see end of item)
|
|
|
|
A loop that indexes a local array at a base computed by SUBTRACTING a
|
|
uint8_t parameter (`k = 8u - nb;` then `t[(uint8_t)(k + i)]` inside a
|
|
counted loop) is compiled with the FIRST iteration peeled, and that
|
|
peeled iteration's address is computed as
|
|
|
|
base + sign_extend(~nb) ; one byte LOW: ~nb = -nb - 1
|
|
|
|
where two's complement (`-nb`) was needed. Every later iteration
|
|
increments correctly, so exactly the first element is read one byte
|
|
low and the remaining seven are right -- a silent wrong-VALUES class
|
|
with no crash, in the same derived-induction-variable family as the
|
|
modulo-advance item.
|
|
|
|
The emitted idiom (RetroNet src/crypto/rnSipHash.c sipRot, w65816
|
|
-O2 -ffreestanding -ffunction-sections, disassembly of the peeled
|
|
first read's address):
|
|
|
|
lda 0xb6,s ; nb
|
|
eor #0x1aff ; low byte: nb ^ 0xff = ~nb (high byte junk)
|
|
and #0xff
|
|
eor #0x80 ; sign-extend byte
|
|
clc
|
|
adc #0xff80 ; (x ^ 0x80) - 0x80
|
|
...
|
|
tsc
|
|
clc
|
|
adc #0xa3 ; t[] frame base (t + 8 region)
|
|
clc
|
|
adc 0x1f,s ; += sext(~nb) --> t + 8 - nb - 1 = t[k-1]
|
|
|
|
Blast radius measured before the workaround: EVERY SipHash-2-4 tag the
|
|
IIgs client computed was wrong (the rotate primitive feeds every round),
|
|
so every sealed uplink failed server-side authentication. Nothing
|
|
client-side ever caught it because the protocol's selective encryption
|
|
leaves downlink frames plaintext -- silent until a peer verifies.
|
|
Root-caused via on-target known-answer tests + a per-primitive state
|
|
probe (SIPBLOCK) diffed against the host build of the same source; the
|
|
"first iteration reads index k-1" model reproduces both on-target wrong
|
|
tags bit-for-bit end to end.
|
|
|
|
WORKAROUND LANDED IN RETRONET: sipRot now takes k (the precomputed read
|
|
base) instead of nb, removing the 8-nb negation from the function; the
|
|
emitted first-iteration address becomes a plain base + k and all KATs
|
|
pass on target. Any other loop whose peeled first iteration derives
|
|
its address from a subtracted narrow parameter is still exposed.
|
|
|
|
RESOLUTION (2026-08-20): root cause was NOT the address lowering -- the
|
|
IR and ISel were correct (a true NEGA8 pseudo). The AsmPrinter expanded
|
|
NEGA8 as an UNWRAPPED `EOR #$FF ; INA` on a false "already 8-bit M"
|
|
assumption: in the real 16-bit-M context the CPU decoded the 2-byte
|
|
EOR #imm8 as 3-byte EOR #imm16 and swallowed the INA opcode ($1A) as the
|
|
immediate's high byte -- the disassembly tell is `eor #$1AFF`. FIXED in
|
|
W65816AsmPrinter.cpp: NEGA8 now emits emitSepM/emitRepM around the pair,
|
|
exactly like LDA8absX/STA8absX (safe: SepRepCleanup's isMNeutral never
|
|
extends an M=8 window across the pseudo). Audited: NEGA8 was the only
|
|
unwrapped Imm8 expansion. Blast radius before the fix: 6 sites in the
|
|
RetroNet IIgs client -- the SipHash rotate (every sealed uplink failed to
|
|
authenticate) and 5 rnWidget functions (index corruption; wild-jump BRK
|
|
during the download offer frame; likely the phantom-FORM garbage too).
|
|
|
|
Original ask (kept for the repro): compile the pre-workaround sipRot
|
|
shape below at -O2 for w65816 and inspect the first read's address chain.
|
|
(Verbatim pre-workaround function; gSipV is a 40-byte global.)
|
|
|
|
```c
|
|
static void sipRot(uint8_t a, uint8_t nb, uint8_t mode) {
|
|
uint8_t t[16];
|
|
uint8_t i;
|
|
uint8_t k;
|
|
uint8_t x;
|
|
|
|
for (i = 0u; i < 8u; i++) {
|
|
x = gSipV[(uint8_t)(a + i)];
|
|
t[i] = x;
|
|
t[(uint8_t)(i + 8u)] = x;
|
|
}
|
|
k = (uint8_t)(8u - nb);
|
|
for (i = 0u; i < 8u; i++) {
|
|
x = t[(uint8_t)(k + i)];
|
|
if (mode == 3u) {
|
|
x = (uint8_t)((x >> 3) | (uint8_t)(t[(uint8_t)(k + i + 1u)] << 5));
|
|
} else if (mode == 1u) {
|
|
x = (uint8_t)((x << 1) | (uint8_t)(t[(uint8_t)(k + i - 1u)] >> 7));
|
|
}
|
|
gSipV[(uint8_t)(a + i)] = x;
|
|
}
|
|
}
|
|
```
|
|
|
|
## 9. i8 store to a 1-byte stack slot is a 16-bit STA d,s that overflows the return address (2026-08-20) -- FIXED IN TOOLCHAIN same day
|
|
|
|
M=16 is the default mode, so `sta d,s` writes TWO bytes. When the target
|
|
frame object is only 1 byte (an i8 spill / i8 arg-save) at the TOP slot of a
|
|
frame, the second byte lands on the caller's saved RETURN ADDRESS low byte and
|
|
the callee's RTL jumps mid-instruction into a different function. Live proof:
|
|
RetroNet's rnClientOnFileRecord saving its uint8_t `op` clobbered its own
|
|
return 0x80->0x01, so its RTL landed inside a different rasterizer switch case,
|
|
skipped the caller's arg cleanup, and left the DISPLAY record-walk's stack
|
|
pointer 4 low -> $F6 (big-frame pointer) wrong -> the walk byte-crawled forever
|
|
on the offer frame (the IIgs download "offer-silence": FILE_OFFER never
|
|
processed, client never drained the serial).
|
|
|
|
Minimal repro: `void fwd(uint8_t op, const uint8_t *r){ sink(op, r); }` at -O2
|
|
for w65816 emits `sta 0x2, s` (16-bit) to save op adjacent to the return.
|
|
|
|
FIX (W65816RegisterInfo.cpp eliminateFrameIndex, STAfi case): when
|
|
`MFI.getObjectSize(FI) == 1`, wrap the STA_StackRel in SEP #$20 / REP #$20 so a
|
|
1-byte slot gets a genuine 8-bit store. Only the STORE is a hazard; a 16-bit
|
|
LOAD of the extra byte is a harmless read (the high byte of an i8 value is
|
|
ignored), so LDAfi is left 16-bit. Verified: fwd now emits
|
|
`sep #$20 ; sta 0x1,s ; rep #$20`. Same defect family as ASKS #8 (NEGA8) and
|
|
the $F8 far-frame bank byte -- "an 8-bit operation emitted without the mode
|
|
wrap in default-16-bit-M code."
|
|
|
|
## 10. Pointer-deref to a GLOBAL hardcodes bank 0 (STZ $E2) under the GS/OS Loader (2026-08-20) -- FIXED IN TOOLCHAIN 2026-08-21 (provenance-based; see end)
|
|
|
|
The LDAptr / STAptr / STBptr custom inserter (W65816ISelLowering.cpp) lowers a
|
|
[dp],Y indirect-long deref with the BANK byte forced to 0 via `STZ $E2` (unless
|
|
the LoaderBankDeref path, which uses $BE, is taken). Bank-0 is correct for a
|
|
pointer to a STACK local (the 65816 stack is always bank 0) but WRONG for a
|
|
pointer to a GLOBAL: under the GS/OS Loader the program's BSS/globals are placed
|
|
in the program bank ($BE, e.g. $0A), not bank 0. So `&someGlobal` dereferenced
|
|
through this path reads bank-0 garbage, while the SAME global accessed directly
|
|
via `sta/lda abs` (DBR-relative, DBR=$BE) is correct -- a silent, layout-
|
|
dependent miscompile. The inserter's own comment already flags this tension
|
|
(gmtime's __gmtimeBuf hit it).
|
|
|
|
Live proof: RetroNet's inlined rnXferRequest materialised `&gX.id` (a static
|
|
global) and read it through LDAptr; the FILE_REQUEST then carried xferId=0xAD
|
|
(bank-0 garbage) instead of 1, and the server rejected the download. 0xAD is
|
|
not any valid gX field -- the tell of a wrong-bank read. Disasm:
|
|
`lda #<gX.id off> ; sta $e0 ; stz $e2 ; lda [$e0],y`.
|
|
|
|
WORKAROUND in app: `noinline` on the function so the global is read in the
|
|
caller through DBR-relative abs16 and passed by the ABI (same shape as the
|
|
existing rnXferState workaround).
|
|
|
|
Ask: the deref bank cannot be a blanket 0 or blanket $BE when stack pointers
|
|
(bank 0) and global pointers ($BE) are both derefed through one pseudo. The
|
|
principled fix is a 24-bit pointer representation (carry the bank in the pointer
|
|
value) or provenance tracking so a global-derived pointer derefs with $BE and a
|
|
stack-derived pointer with 0.
|
|
|
|
Fixes EVALUATED and rejected from the app side (2026-08-20):
|
|
1. -w65816-loader-bank-deref (existing flag): whole-program, flips EVERY
|
|
LDAptr to $BE. Fixes global derefs but breaks stack-pointer derefs (the
|
|
reason LDAptrBank0 exists for va_arg). Also the runtime *.o are shared
|
|
across Loader AND non-Loader (GNO/smoke) builds, so it can't be enabled
|
|
for just the S16/Loader target without a per-mode runtime split.
|
|
2. Fold (load <materialized global addr>) -> abs16+DBR at selection -- the
|
|
clean targeted fix, but NOT minimally reproducible: every small repro
|
|
(inlined forwarder, barrier-forced re-read, struct field) correctly
|
|
selects abs16. The mis-selection only appears inside a very large inlined
|
|
function (RetroNet rnFileRecord, ~0x916 bytes, three transfer routines
|
|
inlined), so it is a scheduling/CSE artifact of address materialization
|
|
under register pressure, not a stable pattern -- hard to fix safely
|
|
without provenance.
|
|
RESOLUTION (2026-08-21, W65816ISelLowering.cpp LDAptr/STAptr/STBptr custom
|
|
inserter): instead of a blanket bank policy, use PROVENANCE. When the pointer
|
|
being derefed traces (through COPYs) to an LDAi16imm of a global / external
|
|
symbol, the deref is exactly a DBR-relative absolute access to that global --
|
|
the same thing a direct `global` reference compiles to -- so emit LDAabs /
|
|
STAabs / STA8abs with the global operand and skip the [dp],Y deref entirely.
|
|
That lands in DBR's bank (correct for a global) with no bank guess, and only
|
|
GLOBAL-address derefs are touched -- stack pointers (bank 0) and heap/other
|
|
pointers keep the existing deref path, so no stack-out-param regression. NB the
|
|
bank canNOT be sourced from $BE: $BE is the code segment's PBR and in a
|
|
multi-segment program the data (BSS) segment is a different bank; only DBR (via
|
|
abs) is right. Single-segment builds are unaffected (DBR==0==old STZ bank).
|
|
Verified: RetroNet's rnXferRequest/rnXferState no longer need `noinline` (both
|
|
removed) and the inlined rnFileRecord emits zero bank-0 pointer-derefs; the IIgs
|
|
download DISK-VERIFIES from the pure-clean build. Also fixes the runtime gmtime
|
|
__gmtimeBuf case this item's parent note describes. What is still NOT covered:
|
|
a global address that flows through pointer ARITHMETIC the trace can't follow
|
|
(e.g. `p = &g; p += i; *p`) -- there the provenance is lost and the deref falls
|
|
back to bank 0; those remain candidates for the 24-bit-pointer rework.
|
|
|
|
## 11. ADDframe (stack-slot LEA) scheduled between a CMP and its branch clobbers the flags (2026-08-24) -- FIXED IN TOOLCHAIN same day
|
|
|
|
`ADDframe` - the LEA-equivalent that puts the address of a stack slot in A -
|
|
expands at PEI (W65816RegisterInfo.cpp eliminateFrameIndex) to `TSC; CLC;
|
|
ADC #disp`. That sequence rewrites N/Z (TSC, ADC), C (CLC, ADC) and V (ADC).
|
|
The pseudo deliberately carries no `Defs = [P]` (the same reasoning as the
|
|
LDA pseudos, see the NOTE above LDAi16imm in W65816InstrInfo.td) and is
|
|
`isReMaterializable`, so a post-RA pass may legally place it between a CMP
|
|
and the Bxx that consumes the CMP's flags. Machine Copy Propagation did
|
|
exactly that in RetroNet's src/platform/rnClient.c (main, the widget action
|
|
dispatch):
|
|
|
|
lda 0xf,s ; act.kind
|
|
cmp #0x3 ; == RN_WA_SUBMIT ?
|
|
tsc ; \
|
|
clc ; > ADDframe: &act, hoisted here for the SUBMIT arm
|
|
adc #0xc1 ; /
|
|
bne .LBB3_61 ; tests (SP+0xC1) != 0 - ALWAYS taken
|
|
|
|
so `act.kind == RN_WA_SUBMIT` could never be true: the LOGIN button
|
|
activated, rnWidgetActivate wrote kind=3, and rnClientOnSubmit was never
|
|
reached. On the IIgs the login form was never submitted; the harness's
|
|
Tab/Return retry loop then produced exactly nine no-widget Return uplinks,
|
|
which is what pinned it. A scan of every TU in the IIgs client found this
|
|
ONE instance (`cmp` immediately followed by `tsc` before the branch), which
|
|
is why only the submit was affected.
|
|
|
|
The existing mitigation - the PHP/PLP wrap in W65816StackSlotCleanup.cpp
|
|
Pass -2.5 - did not catch it for two reasons: (1) ADDframe was not in the
|
|
pass's corrupting set (isLdaLike), so the backward walk from the branch hit
|
|
it, took isFlagDefining's default ("anything else defines flags") and called
|
|
it the TEST, then declined to wrap; (2) the pass only collects BEQ/BNE/BMI/
|
|
BPL, on the (true) grounds that LDA-like ops leave C alone - but ADDframe
|
|
also clobbers C and V, so a `cmp; ADDframe; bcc` would have been miscompiled
|
|
with no protection at all.
|
|
|
|
RESOLUTION (2026-08-24, W65816StackSlotCleanup.cpp Pass -2.5):
|
|
* ADDframe is now in isLdaLike (N/Z-corrupting), so the walk continues
|
|
past it to the real CMP and wraps it: `cmp #3; php; tsc; clc; adc #disp+1;
|
|
plp; bne`. It was already in isStackRel and the bump list, so its
|
|
ImmOffset gets the +1 that compensates PHP's S decrement; the FP-relative
|
|
exemption in the bump is skipped for ADDframe because it has no FP-
|
|
relative expansion (always TSC+ADC, i.e. always SP-relative).
|
|
* BCC/BCS/BVC/BVS are collected as well, with a separate
|
|
isCarryCorrupting predicate (ADDframe only): under a C/V branch an
|
|
N/Z-only corrupter is walked past without a wrap, so the size of every
|
|
existing carry-branch sequence is unchanged.
|
|
* Pinned by llvm/test/CodeGen/W65816/addframe-flag-wrap.mir (nz, carry,
|
|
and the lda-under-carry no-wrap case).
|
|
Verified: the rnClient.c TU rescans clean (zero cmp/tsc/branch sequences in
|
|
all 20 IIgs client TUs) and the IIgs login submits live - see the RetroNet
|
|
e2e matrix (`joey-iigs session`).
|