diff --git a/runtime/src/crt0Gsos.s b/runtime/src/crt0Gsos.s index 0424fec..3cdb4a8 100644 --- a/runtime/src/crt0Gsos.s +++ b/runtime/src/crt0Gsos.s @@ -90,7 +90,8 @@ __start: ; Loader already did it. ; See feedback_gsos_fopen_partial_diagnosis (root-caused this session). .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 ; heap span is MM-owned -- otherwise a later NewHandle (ours or the @@ -133,12 +134,14 @@ __start: bra .Linit_loop .Linit_done: - ; NOTE: do NOT add TLStartUp/MMStartUp here. The GS/OS Loader has - ; already initialised the Tool Locator before transferring control to - ; __start (helloBeep proves this — it does a SysBeep JSL $E10000 with - ; zero init code). Per-process tool init (MM/QD/EM/WM) is the - ; program's responsibility; the desktop demos use startdesk(640) - ; from runtime/include/iigs/desktop.h. + ; NOTE: do NOT add per-process TOOL startup here (TLStartUp, or a full + ; MM/QD/EM/WM StartUp chain). The GS/OS Loader has already initialised + ; the Tool Locator before transferring control to __start (helloBeep + ; proves this — it does a SysBeep JSL $E10000 with zero init code), and + ; per-process tool init is the program's responsibility (the desktop + ; 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 ; this, srand defaults to seed=1 and every run produces an identical diff --git a/runtime/src/desktop.c b/runtime/src/desktop.c index a7c7d00..c4ea184 100644 --- a/runtime/src/desktop.c +++ b/runtime/src/desktop.c @@ -156,7 +156,14 @@ void enddesk(unsigned short userId) { EMShutDown(); QDShutDown(); 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; } diff --git a/runtime/src/libc.c b/runtime/src/libc.c index a795712..e43c672 100644 --- a/runtime/src/libc.c +++ b/runtime/src/libc.c @@ -530,11 +530,23 @@ typedef struct FreeBlk { static FreeBlk *freeList = (FreeBlk *)0; static char *bumpPtr = (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) { if (bumpPtr) return; - bumpPtr = __heap_start ? __heap_start : HEAP_DEFAULT_START; - heapEnd = __heap_end ? __heap_end : HEAP_DEFAULT_END; - freeList = (FreeBlk *)0; + heapSetWindow(__heap_start ? __heap_start : HEAP_DEFAULT_START, + __heap_end ? __heap_end : HEAP_DEFAULT_END); } // 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 // programs that don't link libc.o. // -// On any failure -- NewHandle returns 0, a null master pointer, or a block -// that straddles a bank boundary -- it returns without setting bumpPtr, so +// This reservation is deliberately eager (done once at startup, before any +// 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) -// window. A tight-memory machine is therefore never worse off than before. -extern void *NewHandle(unsigned long size, unsigned short userID, - unsigned short attr, void *loc); +// window; on that fallback path a tight-memory machine is no worse off than +// before. +extern void *NewHandle(unsigned long size, unsigned short userID, unsigned short attr, void *loc); extern void DisposeHandle(void *h); 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) { - return; // already initialised + return; } // 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 @@ -579,18 +599,26 @@ void __heapInitMM(unsigned short userID) { DisposeHandle(h); return; } - // The allocator does flat pointer arithmetic and assumes a single bank; - // reject (rather than corrupt on) a straddling block. This also makes - // correctness independent of the attrNoCross bit guess above. + // The allocator does flat, intra-bank (16-bit) pointer arithmetic and + // assumes a single bank; reject (rather than corrupt on) a straddling + // block. This also makes correctness independent of the attrNoCross + // bit guess above. unsigned long lo = (unsigned long)(void *)base; unsigned long hi = lo + want - 1UL; if ((lo & 0xFF0000UL) != (hi & 0xFF0000UL)) { DisposeHandle(h); return; } - bumpPtr = base; - heapEnd = base + want; - freeList = (FreeBlk *)0; + // heapEnd is one-PAST-end. A block flush against the bank top (last + // byte 0xXXFFFF) passes the straddle check above (its last byte is + // 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) { @@ -636,13 +664,18 @@ void *malloc(size_t n0) { // straight from the GS/OS Memory Manager, not malloc. (There is no // "halBigAlloc" routing -- an older comment here described one that was // never implemented; oversized requests simply hit the 0x7FF0 cap and - // return NULL.) The `p + HDR_SZ + n > heapEnd` compare is correct and - // is covered by smoke check #176. The historical "over-heap" failures - // were the W65816BranchExpand/SepRepCleanup live-in bug (since fixed), - // not an i32 miscompile; `heapEnd - p` is equally correct now, MAME- - // verified -- the addition form is kept only because it is the tested one. + // return NULL.) + // + // Ceiling test compares bytes-needed against bytes-remaining rather than + // the obvious `p + HDR_SZ + n > heapEnd`: that sum can wrap past a bank + // 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; - 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; bumpPtr = p + HDR_SZ + n; return p + HDR_SZ; diff --git a/scripts/runInMameWithGsosStub.sh b/scripts/runInMameWithGsosStub.sh index 3d81d2f..f38bbf4 100755 --- a/scripts/runInMameWithGsosStub.sh +++ b/scripts/runInMameWithGsosStub.sh @@ -115,10 +115,10 @@ $LUA_CHECKS end) EOF -OUT=$(timeout 30 mame apple2gs \ +OUT=$(SDL_VIDEODRIVER=dummy SDL_AUDIODRIVER=dummy timeout 30 mame apple2gs \ -rompath "$PROJECT_ROOT/tools/mame/roms" \ -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" mapfile -t GOT_LIST < <(printf '%s\n' "$OUT" | grep -oE 'val=0x[0-9a-f]+' | sed 's/val=0x//') diff --git a/scripts/runMultiSeg.sh b/scripts/runMultiSeg.sh index 8843c26..38f5813 100755 --- a/scripts/runMultiSeg.sh +++ b/scripts/runMultiSeg.sh @@ -111,11 +111,11 @@ RAMSIZE_ARG=() if [ -n "${MAME_RAMSIZE:-}" ]; then RAMSIZE_ARG=(-ramsize "$MAME_RAMSIZE") fi -OUT=$(timeout "${MAME_TIMEOUT:-30}" mame apple2gs \ +OUT=$(SDL_VIDEODRIVER=dummy SDL_AUDIODRIVER=dummy timeout "${MAME_TIMEOUT:-30}" mame apple2gs \ "${RAMSIZE_ARG[@]}" \ -rompath "$PROJECT_ROOT/tools/mame/roms" \ -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" mapfile -t GOT_LIST < <(printf '%s\n' "$OUT" | /usr/bin/grep -oE 'val=0x[0-9a-f]+' | sed 's/val=0x//') diff --git a/scripts/runViaFinder.sh b/scripts/runViaFinder.sh index 17ca1ac..490968a 100755 --- a/scripts/runViaFinder.sh +++ b/scripts/runViaFinder.sh @@ -158,8 +158,8 @@ emu.register_frame_done(function() end) LUA -OUT=$(timeout 150 mame apple2gs -rompath "$PROJECT_ROOT/tools/mame/roms" \ - -window -nothrottle -sound none \ +OUT=$(SDL_VIDEODRIVER=dummy SDL_AUDIODRIVER=dummy timeout 150 mame apple2gs -rompath "$PROJECT_ROOT/tools/mame/roms" \ + -video none -nothrottle -sound none \ -seconds_to_run 130 -flop3 "$WORK/disk.po" -flop4 "$WORK/data.po" \ -autoboot_script "$WORK/finder.lua" &1) diff --git a/scripts/smokeTest.sh b/scripts/smokeTest.sh index a577590..e64cfb9 100755 --- a/scripts/smokeTest.sh +++ b/scripts/smokeTest.sh @@ -5622,8 +5622,11 @@ EOF __attribute__((section(".text.seg2"), noinline)) int pinnedFn(int x) { return x * 7 + 3; } volatile int gIn = 11; +static volatile int gBss[16]; // zero-init -> .bss; exercises Stage A embed 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) {} } EOF @@ -5635,20 +5638,33 @@ EOF --output "$omfPhC" >/dev/null 2>&1 # Verify exactly 3 segments and correct KINDs. nPhCSeg=$(python3 -c " -import struct +import struct, re 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): bytecnt = struct.unpack_from('0) AND embedded zeros reach the OMF +# (root seg LENGTH covers the BSS span). 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 die "Phase C.1: ExpressLoad-multi-seg KIND sequence wrong (got $nPhCSeg)" diff --git a/src/link816/link816.cpp b/src/link816/link816.cpp index feb1897..8fd91c6 100644 --- a/src/link816/link816.cpp +++ b/src/link816/link816.cpp @@ -2148,7 +2148,9 @@ struct Linker { if (mfPath.empty()) return; std::ofstream mf(mfPath); 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), "{\n" " \"version\": 1,\n" @@ -2188,6 +2190,19 @@ struct Linker { // zeroed under GS/OS (crt0Gsos relies on the Loader LCONST-fill). uint32_t segBssStart = (k == 0) ? lastLayout.bssBase : 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), " {\n" " \"num\": %u,\n" diff --git a/src/link816/omfEmit.cpp b/src/link816/omfEmit.cpp index 66f5404..3562a57 100644 --- a/src/link816/omfEmit.cpp +++ b/src/link816/omfEmit.cpp @@ -646,13 +646,13 @@ static std::vector emitOmfExpressLoad( // 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); // for K user segs count = K - 1 (+ 1 if DP/Stack). - // NOTE 2026-06-26: tried count=nSegs and count=0 -- neither makes a - // multi-seg ExpressLoad OMF load under real GS/OS (the Loader rejects - // the whole file regardless of count). The real blocker is upstream: - // the multi-seg segments carry NO cRELOC/cINTERSEG records and NO - // embedded BSS (the --manifest path never wires link816's reloc/inter - // sites or bssSize into the UserSegs), unlike the single-seg --relocs - // path. Completing that pipeline is the open multi-seg work. + // History: multi-seg ExpressLoad OMFs once would not load under real + // GS/OS because the --manifest path emitted UserSegs with no + // cRELOC/cINTERSEG records and no embedded BSS. That pipeline is now + // complete -- parseManifest wires per-seg reloc/inter sites (the + // .segN.reloc read) and bss_start/bss_size (embedded as LCONST zeros via + // computeBssGap) -- and such OMFs load under GS/OS 6.0.4. (count below + // is unrelated: it is the segment total, tuned empirically.) put32(elData, 0); // reserved 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)); } +// 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 parseManifest(const std::string &path) { std::ifstream f(path); 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. if (s.bssSize > 0) { u.bssSize = s.bssSize; - if (s.bssStart >= s.base) { - uint32_t bssOff = s.bssStart - s.base; - if (bssOff > u.image.size()) - u.bssGap = bssOff - (uint32_t)u.image.size(); - } + u.bssGap = computeBssGap(s.bssStart, s.base, u.image.size()); } if (!relocFile.empty()) { char perSegPath[512]; @@ -1159,6 +1173,11 @@ int main(int argc, char **argv) { // to a specific bank. CODE is the default (type 0). const uint16_t kind = OMF_KIND_CODE_STATIC_ABSBANK; 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, static_cast(s.num), kind, s.name); @@ -1208,11 +1227,7 @@ int main(int argc, char **argv) { auto bssStartIt = syms.find("__bss_start"); if (bssSizeIt != syms.end()) bssSize = bssSizeIt->second; if (bssStartIt != syms.end()) bssStart = bssStartIt->second; - if (bssStart >= base) { - uint32_t bssOffInImage = bssStart - base; - if (bssOffInImage > image.size()) - bssGap = bssOffInImage - (uint32_t)image.size(); - } + bssGap = computeBssGap(bssStart, base, image.size()); auto blob = expressload ? emitOmfExpressLoad(image, entryOff, name, stackSize, bssSize, bssGap)