forked from Ports/goldeneye-pc-port
D248: fix port-wide 30fps halving -- pointer-width scheduler ABI bug
osScAddClient() stashes a per-client "every retrace vs every other retrace" flag in the next array slot's .next field (c[1].next = next); __scHandleRetrace read it back via a hardcoded 8-byte offset that only lands on the right memory when OSScClient is 8 bytes (32-bit pointers). On this 64-bit port the struct is 16 bytes, so the read landed inside the gfx client's own msgQ pointer (always non-NULL) instead, silently collapsing the gfx client onto the audio client's 30Hz path even though the VI-retrace pacemaker itself was always correctly ticking at 60Hz. Fix indexes the struct (client[1].next) instead of hand-rolling the byte offset, so it scales with pointer width on either platform -- same class as D122/D126/D132/D209, just in the scheduler instead of a ROM-serialized struct. Verified via the Video.DisplayFPS overlay: 30 FPS pre-fix, 60 FPS post-fix, same build/scene (-level_09). -level_09/-level_20 both crash-free post-fix.
This commit is contained in:
1 parent
dac4ae7c0c
commit
52477b44af
4 files changed
+42
No files matched your search
@@ -178,6 +178,29 @@ reaches the site with the union arm you are naming actually active.
|
||||
real struct, under `#ifdef PORT`. (`[0]`/`rooms` often still works by luck
|
||||
on LE — low half at +0 — which masks the bug for one of the two fields.)
|
||||
|
||||
- **D248 — a hardcoded byte offset used to index "one struct past the end of
|
||||
the array" desyncs from a real field write once the struct grows.**
|
||||
`OSScClient` (`src/sched.h`) is `{next; msgQ;}`, 8B on N64 / 16B on PC.
|
||||
`osScAddClient()` deliberately writes a per-client flag into the *next
|
||||
array element's* first field (`c[1].next = next`) instead of adding a
|
||||
real member — cheap on N64 since the caller over-allocates the array by
|
||||
one slot. The reader, `__scHandleRetrace`, doesn't index the struct back;
|
||||
it recomputes the same address by hand as `*((s32*)client + 2)` (a
|
||||
hardcoded 8-byte offset). That offset only equals `sizeof(OSScClient)`
|
||||
(and thus `&client[1]`) when pointers are 32-bit. On PC it lands 8 bytes
|
||||
into `client[0]` itself — inside `msgQ`, a real non-NULL pointer — so the
|
||||
flag reads as permanently "set" for every client, silently collapsing a
|
||||
60Hz/30Hz per-client split onto whichever branch a nonzero flag takes.
|
||||
**General tell:** any place a struct is deliberately under-declared and a
|
||||
sibling array slot is used as ad-hoc extra storage (`c[1].field`,
|
||||
`arr[i+1].x`) is fine as a *write* (indexing scales with the real
|
||||
`sizeof`), but a matching *read* done via a raw `(T*)p + N`/byte-offset
|
||||
cast instead of `p[1].field` will silently desync once the struct's size
|
||||
changes across the port. Fix: always index the struct on both ends
|
||||
(`client[1].next`), never hand-roll the byte math — it's the direct
|
||||
decompilation of what the write side already does, `#ifdef PORT` with the
|
||||
N64 hand-rolled offset kept verbatim under `#else`. §F **D248**.
|
||||
|
||||
## B. 16-byte PC `Gfx` / `Vtx` vs 8-byte N64
|
||||
|
||||
Any buffer reservation, `memcpy` size, slot stride, or pool budget
|
||||
|
||||
Reference in new issue
Block a user