Several more bugs squashed.

This commit is contained in:
Scott Duensing 2026-07-06 19:19:16 -05:00
parent 8c5c5ac1d2
commit 8d0bac168e
9 changed files with 146 additions and 57 deletions

View file

@ -90,7 +90,8 @@ __start:
; Loader already did it. ; Loader already did it.
; See feedback_gsos_fopen_partial_diagnosis (root-caused this session). ; See feedback_gsos_fopen_partial_diagnosis (root-caused this session).
.Lbss_done: .Lbss_done:
rep #0x20 ; restore M=16 ; (M is already 16-bit; the heap-init block below re-asserts rep #0x30
; before it needs a 16-bit index, so no width fixup is needed here.)
; Back the C malloc heap with a locked Memory-Manager handle so the ; Back the C malloc heap with a locked Memory-Manager handle so the
; heap span is MM-owned -- otherwise a later NewHandle (ours or the ; heap span is MM-owned -- otherwise a later NewHandle (ours or the
@ -133,12 +134,14 @@ __start:
bra .Linit_loop bra .Linit_loop
.Linit_done: .Linit_done:
; NOTE: do NOT add TLStartUp/MMStartUp here. The GS/OS Loader has ; NOTE: do NOT add per-process TOOL startup here (TLStartUp, or a full
; already initialised the Tool Locator before transferring control to ; MM/QD/EM/WM StartUp chain). The GS/OS Loader has already initialised
; __start (helloBeep proves this — it does a SysBeep JSL $E10000 with ; the Tool Locator before transferring control to __start (helloBeep
; zero init code). Per-process tool init (MM/QD/EM/WM) is the ; proves this — it does a SysBeep JSL $E10000 with zero init code), and
; program's responsibility; the desktop demos use startdesk(640) ; per-process tool init is the program's responsibility (the desktop
; from runtime/include/iigs/desktop.h. ; demos use startdesk(640) from runtime/include/iigs/desktop.h). The
; bare MMStartUp above is NOT that: it only fetches our existing MM user
; ID for __heapInitMM and starts no tool set.
; Seed rand() from the IIgs RTC via ReadTimeHex ($0D03). Without ; Seed rand() from the IIgs RTC via ReadTimeHex ($0D03). Without
; this, srand defaults to seed=1 and every run produces an identical ; this, srand defaults to seed=1 and every run produces an identical

View file

@ -156,7 +156,14 @@ void enddesk(unsigned short userId) {
EMShutDown(); EMShutDown();
QDShutDown(); QDShutDown();
DisposeHandle(gDpHandle); DisposeHandle(gDpHandle);
MMShutDown(userId); // Do NOT call MMShutDown(userId) here. MMShutDown disposes EVERY handle
// owned by the app's user ID -- including the locked handle that backs
// the C malloc heap (crt0Gsos/__heapInitMM allocate it under this same
// ID). Tearing that down while the program keeps running, or holds
// malloc'd data past enddesk(), dangles every malloc pointer. An app
// need not call MMShutDown at all: GS/OS disposes all of the app's
// memory when it QUITs.
(void)userId;
} }

View file

@ -530,11 +530,23 @@ typedef struct FreeBlk {
static FreeBlk *freeList = (FreeBlk *)0; static FreeBlk *freeList = (FreeBlk *)0;
static char *bumpPtr = (char *)0; static char *bumpPtr = (char *)0;
static char *heapEnd = (char *)0; static char *heapEnd = (char *)0;
// Point the allocator at a fresh [start, end) window: bump pointer at the
// bottom, ceiling at the top, empty free list. Shared by mallocInitOnce
// (the link-time window) and __heapInitMM (the Memory-Manager-backed
// window) so the three heap-state fields are only ever set in one place.
static void heapSetWindow(char *start, char *end) {
bumpPtr = start;
heapEnd = end;
freeList = (FreeBlk *)0;
}
static void mallocInitOnce(void) { static void mallocInitOnce(void) {
if (bumpPtr) return; if (bumpPtr) return;
bumpPtr = __heap_start ? __heap_start : HEAP_DEFAULT_START; heapSetWindow(__heap_start ? __heap_start : HEAP_DEFAULT_START,
heapEnd = __heap_end ? __heap_end : HEAP_DEFAULT_END; __heap_end ? __heap_end : HEAP_DEFAULT_END);
freeList = (FreeBlk *)0;
} }
// GS/OS: back the malloc heap with a locked Memory-Manager handle so the // GS/OS: back the malloc heap with a locked Memory-Manager handle so the
@ -546,16 +558,24 @@ static void mallocInitOnce(void) {
// no-op __heapInitMM in libgcc.s resolves crt0Gsos's `jsl` for GS/OS // no-op __heapInitMM in libgcc.s resolves crt0Gsos's `jsl` for GS/OS
// programs that don't link libc.o. // programs that don't link libc.o.
// //
// On any failure -- NewHandle returns 0, a null master pointer, or a block // This reservation is deliberately eager (done once at startup, before any
// that straddles a bank boundary -- it returns without setting bumpPtr, so // malloc) so the Memory Manager cannot hand the heap span to another
// allocation first; it therefore costs the locked window's worth of RAM for
// the whole process lifetime even if the program never mallocs. On any
// failure -- NewHandle returns 0, a null master pointer, or a block that
// straddles a bank boundary -- it returns without setting bumpPtr, so
// mallocInitOnce falls back to the link-time [__heap_start,__heap_end) // mallocInitOnce falls back to the link-time [__heap_start,__heap_end)
// window. A tight-memory machine is therefore never worse off than before. // window; on that fallback path a tight-memory machine is no worse off than
extern void *NewHandle(unsigned long size, unsigned short userID, // before.
unsigned short attr, void *loc); extern void *NewHandle(unsigned long size, unsigned short userID, unsigned short attr, void *loc);
extern void DisposeHandle(void *h); extern void DisposeHandle(void *h);
void __heapInitMM(unsigned short userID) { void __heapInitMM(unsigned short userID) {
// Idempotency guard: keep the first window that was installed. The sole
// caller (crt0Gsos) invokes this once before any malloc, but this keeps
// a second call -- or a future reorder -- from stomping a heap that is
// already serving allocations.
if (bumpPtr) { if (bumpPtr) {
return; // already initialised return;
} }
// Heap size: keep the link-time window when it is in a sane range, // Heap size: keep the link-time window when it is in a sane range,
// else a 32 KB default. The window is bank-0-bounded, so it never // else a 32 KB default. The window is bank-0-bounded, so it never
@ -579,18 +599,26 @@ void __heapInitMM(unsigned short userID) {
DisposeHandle(h); DisposeHandle(h);
return; return;
} }
// The allocator does flat pointer arithmetic and assumes a single bank; // The allocator does flat, intra-bank (16-bit) pointer arithmetic and
// reject (rather than corrupt on) a straddling block. This also makes // assumes a single bank; reject (rather than corrupt on) a straddling
// correctness independent of the attrNoCross bit guess above. // block. This also makes correctness independent of the attrNoCross
// bit guess above.
unsigned long lo = (unsigned long)(void *)base; unsigned long lo = (unsigned long)(void *)base;
unsigned long hi = lo + want - 1UL; unsigned long hi = lo + want - 1UL;
if ((lo & 0xFF0000UL) != (hi & 0xFF0000UL)) { if ((lo & 0xFF0000UL) != (hi & 0xFF0000UL)) {
DisposeHandle(h); DisposeHandle(h);
return; return;
} }
bumpPtr = base; // heapEnd is one-PAST-end. A block flush against the bank top (last
heapEnd = base + want; // byte 0xXXFFFF) passes the straddle check above (its last byte is
freeList = (FreeBlk *)0; // in-bank), but base+want is then 0xXX0000 of the NEXT bank -- and since
// pointer arithmetic does not carry into the bank byte, heapEnd would
// wrap to the bank base, below bumpPtr, wedging every malloc. Give up
// the top byte so the exclusive end stays inside base's bank.
if (((lo + want) & 0xFFFFUL) == 0UL) {
want -= 1UL;
}
heapSetWindow(base, base + want);
} }
void *malloc(size_t n0) { void *malloc(size_t n0) {
@ -636,13 +664,18 @@ void *malloc(size_t n0) {
// straight from the GS/OS Memory Manager, not malloc. (There is no // straight from the GS/OS Memory Manager, not malloc. (There is no
// "halBigAlloc" routing -- an older comment here described one that was // "halBigAlloc" routing -- an older comment here described one that was
// never implemented; oversized requests simply hit the 0x7FF0 cap and // never implemented; oversized requests simply hit the 0x7FF0 cap and
// return NULL.) The `p + HDR_SZ + n > heapEnd` compare is correct and // return NULL.)
// is covered by smoke check #176. The historical "over-heap" failures //
// were the W65816BranchExpand/SepRepCleanup live-in bug (since fixed), // Ceiling test compares bytes-needed against bytes-remaining rather than
// not an i32 miscompile; `heapEnd - p` is equally correct now, MAME- // the obvious `p + HDR_SZ + n > heapEnd`: that sum can wrap past a bank
// verified -- the addition form is kept only because it is the tested one. // boundary under the target's intra-bank (16-bit) pointer arithmetic
// when the heap sits high in a bank (an MM-backed heap can land at
// $xx8000..$xxFFFF), spuriously passing the test and running off the
// heap. heapEnd - p stays in-bank -- __heapInitMM guarantees heapEnd
// and every p share a bank and heapEnd >= p -- so it is a small
// non-negative remaining-byte count. Covered by smoke check #176.
char *p = bumpPtr; char *p = bumpPtr;
if (p + HDR_SZ + n > heapEnd) return (void *)0; if ((size_t)(HDR_SZ + n) > (size_t)(heapEnd - p)) return (void *)0;
*(u16 *)p = (u16)n; *(u16 *)p = (u16)n;
bumpPtr = p + HDR_SZ + n; bumpPtr = p + HDR_SZ + n;
return p + HDR_SZ; return p + HDR_SZ;

View file

@ -115,10 +115,10 @@ $LUA_CHECKS
end) end)
EOF EOF
OUT=$(timeout 30 mame apple2gs \ OUT=$(SDL_VIDEODRIVER=dummy SDL_AUDIODRIVER=dummy timeout 30 mame apple2gs \
-rompath "$PROJECT_ROOT/tools/mame/roms" \ -rompath "$PROJECT_ROOT/tools/mame/roms" \
-plugins -autoboot_script "$LUA_PATH" \ -plugins -autoboot_script "$LUA_PATH" \
-window -sound none -nothrottle -seconds_to_run "$SECS" 2>&1 | grep "^MAME-") -video none -sound none -nothrottle -seconds_to_run "$SECS" 2>&1 | grep "^MAME-")
echo "$OUT" echo "$OUT"
mapfile -t GOT_LIST < <(printf '%s\n' "$OUT" | grep -oE 'val=0x[0-9a-f]+' | sed 's/val=0x//') mapfile -t GOT_LIST < <(printf '%s\n' "$OUT" | grep -oE 'val=0x[0-9a-f]+' | sed 's/val=0x//')

View file

@ -111,11 +111,11 @@ RAMSIZE_ARG=()
if [ -n "${MAME_RAMSIZE:-}" ]; then if [ -n "${MAME_RAMSIZE:-}" ]; then
RAMSIZE_ARG=(-ramsize "$MAME_RAMSIZE") RAMSIZE_ARG=(-ramsize "$MAME_RAMSIZE")
fi fi
OUT=$(timeout "${MAME_TIMEOUT:-30}" mame apple2gs \ OUT=$(SDL_VIDEODRIVER=dummy SDL_AUDIODRIVER=dummy timeout "${MAME_TIMEOUT:-30}" mame apple2gs \
"${RAMSIZE_ARG[@]}" \ "${RAMSIZE_ARG[@]}" \
-rompath "$PROJECT_ROOT/tools/mame/roms" \ -rompath "$PROJECT_ROOT/tools/mame/roms" \
-plugins -autoboot_script "$LUA_PATH" \ -plugins -autoboot_script "$LUA_PATH" \
-window -sound none -nothrottle -seconds_to_run "$SECS" 2>&1 | /usr/bin/grep -E "^(MAME-|SEG-)") -video none -sound none -nothrottle -seconds_to_run "$SECS" 2>&1 | /usr/bin/grep -E "^(MAME-|SEG-)")
echo "$OUT" echo "$OUT"
mapfile -t GOT_LIST < <(printf '%s\n' "$OUT" | /usr/bin/grep -oE 'val=0x[0-9a-f]+' | sed 's/val=0x//') mapfile -t GOT_LIST < <(printf '%s\n' "$OUT" | /usr/bin/grep -oE 'val=0x[0-9a-f]+' | sed 's/val=0x//')

View file

@ -158,8 +158,8 @@ emu.register_frame_done(function()
end) end)
LUA LUA
OUT=$(timeout 150 mame apple2gs -rompath "$PROJECT_ROOT/tools/mame/roms" \ OUT=$(SDL_VIDEODRIVER=dummy SDL_AUDIODRIVER=dummy timeout 150 mame apple2gs -rompath "$PROJECT_ROOT/tools/mame/roms" \
-window -nothrottle -sound none \ -video none -nothrottle -sound none \
-seconds_to_run 130 -flop3 "$WORK/disk.po" -flop4 "$WORK/data.po" \ -seconds_to_run 130 -flop3 "$WORK/disk.po" -flop4 "$WORK/data.po" \
-autoboot_script "$WORK/finder.lua" </dev/null 2>&1) -autoboot_script "$WORK/finder.lua" </dev/null 2>&1)

View file

@ -5622,8 +5622,11 @@ EOF
__attribute__((section(".text.seg2"), noinline)) __attribute__((section(".text.seg2"), noinline))
int pinnedFn(int x) { return x * 7 + 3; } int pinnedFn(int x) { return x * 7 + 3; }
volatile int gIn = 11; volatile int gIn = 11;
static volatile int gBss[16]; // zero-init -> .bss; exercises Stage A embed
int main(void) { int main(void) {
*(volatile unsigned short *)0x025000 = (unsigned short)pinnedFn(gIn); gBss[gIn & 15] = 0; // touch BSS so it is retained (stays 0)
*(volatile unsigned short *)0x025000 =
(unsigned short)(pinnedFn(gIn) + gBss[0]);
while (1) {} while (1) {}
} }
EOF EOF
@ -5635,20 +5638,33 @@ EOF
--output "$omfPhC" >/dev/null 2>&1 --output "$omfPhC" >/dev/null 2>&1
# Verify exactly 3 segments and correct KINDs. # Verify exactly 3 segments and correct KINDs.
nPhCSeg=$(python3 -c " nPhCSeg=$(python3 -c "
import struct import struct, re
data = open('$omfPhC','rb').read() data = open('$omfPhC','rb').read()
pos = 0; ok_kinds = [] mf = open('$mfPhC').read()
# Stage A must now wire a non-zero bss_size for the text segment into the
# manifest, and omfEmit must embed it so the root user seg's OMF LENGTH
# covers the BSS span.
bssVals = [int(x) for x in re.findall(r'\"bss_size\"\s*:\s*(\d+)', mf)]
bss = max(bssVals) if bssVals else 0
pos = 0; ok_kinds = []; rootLen = None
while pos < len(data): while pos < len(data):
bytecnt = struct.unpack_from('<I', data, pos)[0] bytecnt = struct.unpack_from('<I', data, pos)[0]
length = struct.unpack_from('<I', data, pos+8)[0]
kind = struct.unpack_from('<H', data, pos+20)[0] kind = struct.unpack_from('<H', data, pos+20)[0]
ok_kinds.append(kind) ok_kinds.append(kind)
if kind == 0x1100:
rootLen = length
pos += bytecnt pos += bytecnt
# Root user seg (seg 2 of the OMF) carries the DYNAMIC bit (0x1100 = # Root user seg (seg 2 of the OMF) carries the DYNAMIC bit (0x1100 =
# CODE|PRIV|DYNAMIC); non-root user segs stay 0x1000. Real GS/OS # CODE|PRIV|DYNAMIC); non-root user segs stay 0x1000. Real GS/OS
# rejects a multi-seg file whose root lacks DYNAMIC (see omfEmit # rejects a multi-seg file whose root lacks DYNAMIC (see omfEmit
# segKind logic) -- an earlier stale expectation here was [.,0x1000,.]. # segKind logic) -- an earlier stale expectation here was [.,0x1000,.].
# Stage A: bss_size wired in (bss>0) AND embedded zeros reach the OMF
# (root seg LENGTH covers the BSS span).
expected = [0x8001, 0x1100, 0x1000] expected = [0x8001, 0x1100, 0x1000]
print('OK' if ok_kinds == expected else 'BAD ' + str(ok_kinds)) ok = ((ok_kinds == expected) and (bss > 0) and
(rootLen is not None and rootLen >= bss))
print('OK' if ok else 'BAD kinds=%s bss=%s rootLen=%s' % (ok_kinds, bss, rootLen))
") ")
if [ "$nPhCSeg" != "OK" ]; then if [ "$nPhCSeg" != "OK" ]; then
die "Phase C.1: ExpressLoad-multi-seg KIND sequence wrong (got $nPhCSeg)" die "Phase C.1: ExpressLoad-multi-seg KIND sequence wrong (got $nPhCSeg)"

View file

@ -2148,7 +2148,9 @@ struct Linker {
if (mfPath.empty()) return; if (mfPath.empty()) return;
std::ofstream mf(mfPath); std::ofstream mf(mfPath);
if (!mf) die("cannot open '" + mfPath + "' for writing"); if (!mf) die("cannot open '" + mfPath + "' for writing");
char buf[512]; // Sized for the per-segment block below plus a long image path; the
// block's fixed text is ~250 bytes, leaving generous path headroom.
char buf[1024];
std::snprintf(buf, sizeof(buf), std::snprintf(buf, sizeof(buf),
"{\n" "{\n"
" \"version\": 1,\n" " \"version\": 1,\n"
@ -2188,6 +2190,19 @@ struct Linker {
// zeroed under GS/OS (crt0Gsos relies on the Loader LCONST-fill). // zeroed under GS/OS (crt0Gsos relies on the Loader LCONST-fill).
uint32_t segBssStart = (k == 0) ? lastLayout.bssBase : 0u; uint32_t segBssStart = (k == 0) ? lastLayout.bssBase : 0u;
uint32_t segBssSize = (k == 0) ? lastLayout.bssSize : 0u; uint32_t segBssSize = (k == 0) ? lastLayout.bssSize : 0u;
// omfEmit embeds this segment's BSS as trailing LCONST zeros, so
// BSS must live in THIS segment's bank. Multi-bank BSS
// (--bss-base in a higher bank) cannot be embedded here, and
// crt0Gsos relies on the Loader's LCONST fill (it runs no
// bank-by-bank STZ clear like the bare-metal/GNO path), so the
// out-of-bank BSS would be neither reserved nor zeroed. Reject
// the combination loudly rather than emit a broken file.
if (segBssSize > 0u &&
(segBssStart & 0xFF0000u) != (seg.base & 0xFF0000u)) {
die("multi-segment ExpressLoad build has BSS outside the "
"text segment's bank; multi-bank BSS + --manifest is not "
"supported (BSS cannot be embedded or zeroed for GS/OS)");
}
std::snprintf(buf, sizeof(buf), std::snprintf(buf, sizeof(buf),
" {\n" " {\n"
" \"num\": %u,\n" " \"num\": %u,\n"

View file

@ -646,13 +646,13 @@ static std::vector<uint8_t> emitOmfExpressLoad(
// N = total segments in the file (including ExpressLoad). For a // N = total segments in the file (including ExpressLoad). For a
// 1-user-seg layout count=0 (N=2) or count=1 (N=3 with DP/Stack); // 1-user-seg layout count=0 (N=2) or count=1 (N=3 with DP/Stack);
// for K user segs count = K - 1 (+ 1 if DP/Stack). // for K user segs count = K - 1 (+ 1 if DP/Stack).
// NOTE 2026-06-26: tried count=nSegs and count=0 -- neither makes a // History: multi-seg ExpressLoad OMFs once would not load under real
// multi-seg ExpressLoad OMF load under real GS/OS (the Loader rejects // GS/OS because the --manifest path emitted UserSegs with no
// the whole file regardless of count). The real blocker is upstream: // cRELOC/cINTERSEG records and no embedded BSS. That pipeline is now
// the multi-seg segments carry NO cRELOC/cINTERSEG records and NO // complete -- parseManifest wires per-seg reloc/inter sites (the
// embedded BSS (the --manifest path never wires link816's reloc/inter // .segN.reloc read) and bss_start/bss_size (embedded as LCONST zeros via
// sites or bssSize into the UserSegs), unlike the single-seg --relocs // computeBssGap) -- and such OMFs load under GS/OS 6.0.4. (count below
// path. Completing that pipeline is the open multi-seg work. // is unrelated: it is the segment total, tuned empirically.)
put32(elData, 0); // reserved put32(elData, 0); // reserved
put16(elData, (uint16_t)(nSegs - 1)); // count = N-2 = nSegs-1 put16(elData, (uint16_t)(nSegs - 1)); // count = N-2 = nSegs-1
@ -860,6 +860,24 @@ static uint32_t extractNumberField(const std::string &block,
nullptr, 0)); nullptr, 0));
} }
// Bytes of zero padding to insert between a segment's image and its embedded
// BSS-as-zeros so the BSS lands at its link-layout address. Zero when BSS
// abuts or overlaps the image end, or when bssStart precedes base (a
// malformed layout -- callers guard the multi-bank case upstream). Shared
// by the single-seg and multi-seg emit paths so the boundary rule lives in
// one place.
static uint32_t computeBssGap(uint32_t bssStart, uint32_t base, size_t imageSize) {
if (bssStart < base) {
return 0;
}
uint32_t bssOff = bssStart - base;
if (bssOff > (uint32_t)imageSize) {
return bssOff - (uint32_t)imageSize;
}
return 0;
}
static std::vector<ManifestSeg> parseManifest(const std::string &path) { static std::vector<ManifestSeg> parseManifest(const std::string &path) {
std::ifstream f(path); std::ifstream f(path);
if (!f) die("cannot open '" + path + "' for reading"); if (!f) die("cannot open '" + path + "' for reading");
@ -1061,11 +1079,7 @@ int main(int argc, char **argv) {
// from image-end up to the link-layout BSS address. // from image-end up to the link-layout BSS address.
if (s.bssSize > 0) { if (s.bssSize > 0) {
u.bssSize = s.bssSize; u.bssSize = s.bssSize;
if (s.bssStart >= s.base) { u.bssGap = computeBssGap(s.bssStart, s.base, u.image.size());
uint32_t bssOff = s.bssStart - s.base;
if (bssOff > u.image.size())
u.bssGap = bssOff - (uint32_t)u.image.size();
}
} }
if (!relocFile.empty()) { if (!relocFile.empty()) {
char perSegPath[512]; char perSegPath[512];
@ -1159,6 +1173,11 @@ int main(int argc, char **argv) {
// to a specific bank. CODE is the default (type 0). // to a specific bank. CODE is the default (type 0).
const uint16_t kind = OMF_KIND_CODE_STATIC_ABSBANK; const uint16_t kind = OMF_KIND_CODE_STATIC_ABSBANK;
uint32_t entryOff = (k == 0) ? s.entryOff : 0; uint32_t entryOff = (k == 0) ? s.entryOff : 0;
// BSS is intentionally NOT embedded on this STATIC|ABSBANK
// multi-seg path: real GS/OS rejects a multi-seg file whose root
// lacks the DYNAMIC bit, so this format is a debug/inspection
// artifact that is never loaded. The ExpressLoad path is the one
// that embeds BSS (via s.bssSize / computeBssGap).
auto seg = emitOneSeg(img, entryOff, s.base, auto seg = emitOneSeg(img, entryOff, s.base,
static_cast<uint16_t>(s.num), static_cast<uint16_t>(s.num),
kind, s.name); kind, s.name);
@ -1208,11 +1227,7 @@ int main(int argc, char **argv) {
auto bssStartIt = syms.find("__bss_start"); auto bssStartIt = syms.find("__bss_start");
if (bssSizeIt != syms.end()) bssSize = bssSizeIt->second; if (bssSizeIt != syms.end()) bssSize = bssSizeIt->second;
if (bssStartIt != syms.end()) bssStart = bssStartIt->second; if (bssStartIt != syms.end()) bssStart = bssStartIt->second;
if (bssStart >= base) { bssGap = computeBssGap(bssStart, base, image.size());
uint32_t bssOffInImage = bssStart - base;
if (bssOffInImage > image.size())
bssGap = bssOffInImage - (uint32_t)image.size();
}
auto blob = expressload auto blob = expressload
? emitOmfExpressLoad(image, entryOff, name, stackSize, bssSize, bssGap) ? emitOmfExpressLoad(image, entryOff, name, stackSize, bssSize, bssGap)