More bug fixes.

runtime/build.sh

@@ -84,10 +84,11 @@ asm "$SRC/iigsToolbox.s"
 # generated iigsToolbox.s so a `python3 scripts/genToolbox.py` regen
 # doesn't clobber them.  See runtime/src/iigsToolboxExtra.s.
 asm "$SRC/iigsToolboxExtra.s"
-# softDouble.c builds at -O2.  dpack is noinline to dodge a backend
+# softDouble.c builds at -O2.  dpack is a plain inline static now (the
-# stack-slot aliasing bug; dclass stays inline because pointer-arg
+# old stack-slot aliasing bug was fixed in-backend, see
-# stores from a noinline boundary use DBR-relative addressing (broken
+# feedback_dpack_inline_fixed).  dclass is KEPT inline because pointer-arg
-# under DBR != 0).  Both choices documented in the source.
+# stores from a noinline boundary lower to DBR-relative `sta (d,s),y`
+# (broken under DBR != 0).  That choice is documented in the source.
 cc  "$SRC/softDouble.c"
 
 # Phase 6.2 UBSan-min runtime.  MUST be compiled with

runtime/src/libc.c

@@ -542,10 +542,12 @@ void *malloc(size_t n0) {
     // Heap ceiling is ~32KB so anything > 0x7FF0 is unsatisfiable.
     if (n0 > (size_t)0x7FF0) return (void *)0;
     // Round up to 2-byte alignment, with a minimum of FREE_NODE_SZ-HDR_SZ.
-    // Keep this in 16-bit arithmetic — the 0x7FF0 cap above guarantees the
+    // Keep this in 16-bit arithmetic: the 0x7FF0 cap above guarantees the
-    // value fits.  Going through `unsigned long` here triggers an i32 umax
+    // value fits, and 16-bit ops are cheaper than i32 on the 65816.  (An
-    // pattern that our backend currently miscompiles; staying 16-bit dodges
+    // earlier comment here blamed an "i32 umax miscompile" for staying
-    // that path entirely.
+    // 16-bit; that was a misattribution.  The i32 path is correct, verified
+    // in MAME; the real over-heap bug was a control-flow live-in staleness
+    // bug in W65816BranchExpand, since fixed.)
     u16 n = (u16)n0;
     if (n == 0) n = 1;
     n = (u16)((n + 1) & ~(u16)1);
@@ -572,13 +574,17 @@ void *malloc(size_t n0) {
         link = &cur->next;
         cur  = cur->next;
     }
-    // Bump-allocate from the high end. Big allocations (e.g. the 16 KB sprite
+    // Bump-allocate from the high end.  This heap only serves small
-    // codegen scratch) are routed through halBigAlloc -> the Memory Manager,
+    // allocations: the 0x7FF0 cap at the top of malloc() bounds n, and
-    // NOT this heap, so n is always small here and the historical
+    // callers needing big buffers (e.g. a 16 KB sprite scratch) get them
-    // `p + HDR_SZ + n > heapEnd` over-heap miscompile (only manifested for
+    // straight from the GS/OS Memory Manager, not malloc.  (There is no
-    // oversized n) is never exercised. A `heapEnd - p` reformulation went
+    // "halBigAlloc" routing -- an older comment here described one that was
-    // through the same i32 path and spuriously failed small mallocs, so keep
+    // never implemented; oversized requests simply hit the 0x7FF0 cap and
-    // the original simple compare.
+    // 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.
     char *p = bumpPtr;
     if (p + HDR_SZ + n > heapEnd) return (void *)0;
     *(u16 *)p = (u16)n;

scripts/smokeTest.sh

@@ -2894,6 +2894,94 @@ EOF
         fi
         rm -f "$cMcFile" "$oMcFile" "$binMcFile"
 
+        # Regression: W65816SepRepCleanup's PHI-copy hoist must NOT move a
+        # NULL-return materialization (lda #0) across the bump bounds-compare
+        # when the over-heap edge leaves via a BranchExpand-created Bridge
+        # (BRL) block.  Pre-fix that hoist was unsound because the Bridge
+        # block's live-ins were left empty, so the guard wrongly concluded A
+        # was dead and the over-heap path returned the compare result instead
+        # of NULL — letting a drain run off the heap.  Fixed by recomputing
+        # live-ins for newly-created blocks in W65816BranchExpand.  This
+        # self-contained malloc-shaped reproducer (free-list walk + split +
+        # i32 cap raise the register pressure / force the Bridge block) drains
+        # a fixed nine-block heap; pre-fix it ran to the 0x40 cap (over-run),
+        # post-fix it stops at 9.
+        log "check: MAME malloc-shape drain stops at heap end (#176 SepRep/BranchExpand live-in)"
+        cBrFile="$(mktemp --suffix=.c)"
+        oBrFile="$(mktemp --suffix=.o)"
+        binBrFile="$(mktemp --suffix=.bin)"
+        cat > "$cBrFile" <<'EOF'
+typedef unsigned long  u32;
+typedef unsigned short u16;
+typedef struct B { u16 size; struct B *next; } B;
+#define HDR        ((u16)sizeof(u16))
+#define MIN_SPLIT  ((u16)(sizeof(u16) + sizeof(B *) + 2))
+#define MAX_REQ    ((u32)0x7FF0)
+#define DRAIN_CAP  ((u16)0x40)
+static B    *freeList;
+static char *bumpPtr;
+static char *heapEnd;
+volatile u32 vStart = 0x2E80;
+volatile u32 vEnd   = 0xBF00;
+volatile u32 vReq   = 0x1000;
+__attribute__((noinline)) static void *mal(u32 n0) {
+    if (n0 > MAX_REQ) {
+        return (void *)0;
+    }
+    u16 n = (u16)n0;
+    B **link = &freeList;
+    B  *cur  = freeList;
+    while (cur) {
+        if (cur->size >= n) {
+            if (cur->size >= n + MIN_SPLIT) {
+                u16 rem = (u16)(cur->size - n - HDR);
+                B *tail = (B *)((char *)cur + HDR + n);
+                tail->size = rem;
+                tail->next = cur->next;
+                cur->size  = n;
+                *link = tail;
+            } else {
+                *link = cur->next;
+            }
+            return (char *)cur + HDR;
+        }
+        link = &cur->next;
+        cur  = cur->next;
+    }
+    char *p = bumpPtr;
+    if (p + HDR + n > heapEnd) {
+        return (void *)0;
+    }
+    *(u16 *)p = n;
+    bumpPtr = p + HDR + n;
+    return p + HDR;
+}
+int main(void) {
+    bumpPtr  = (char *)vStart;
+    heapEnd  = (char *)vEnd;
+    freeList = (B *)0;
+    u16 count = 0;
+    for (u16 i = 0; i < DRAIN_CAP; i++) {
+        if (!mal(vReq)) {
+            break;
+        }
+        count++;
+    }
+    *(volatile u16 *)0x025000 = count;
+    while (1) {
+    }
+}
+EOF
+        "$CLANG" --target=w65816 -O2 -ffunction-sections -c "$cBrFile" -o "$oBrFile"
+        "$PROJECT_ROOT/tools/link816" -o "$binBrFile" --text-base 0x1000 \
+            "$oCrt0F" "$oLibgccFile" "$oBrFile" >/dev/null 2>&1
+        if ! bash "$PROJECT_ROOT/scripts/runInMame.sh" "$binBrFile" --check \
+                  0x025000=0009 >/dev/null 2>&1; then
+            die "MAME: malloc-shape drain over-ran heap (BranchExpand live-in regression)"
+        fi
+        log "OK: malloc-shape drain stopped at the 9-block heap end (no over-run)"
+        rm -f "$cBrFile" "$oBrFile" "$binBrFile"
+
         log "check: MAME runs strtok 'a,b,,c' continuation (#84 fixed)"
         cTkFile="$(mktemp --suffix=.c)"
         oTkFile="$(mktemp --suffix=.o)"

src/link816/omfEmit.cpp

@@ -1030,11 +1030,11 @@ int main(int argc, char **argv) {
         // --manifest + --expressload: wrap N user segments in a single
         // ~ExpressLoad descriptor (Phase C.1).  Each user seg gets
         // KIND=0x1000 (CODE|PRIV), so the Loader picks banks dynamically
-        // — programs MUST NOT depend on link-time bank placement of
+        // — programs MUST NOT depend on link-time bank placement.  Both
-        // cross-segment IMM24 targets (those need cINTERSEG, deferred
+        // intra-segment cRELOC and cross-segment IMM24 targets (via
-        // to Phase C.2).  Intra-segment cRELOC is supported via the
+        // cINTERSEG, 0xF6) are handled: Phase C.2 (per-seg sidecars with
-        // global gReloc24Sites (single-sidecar path; per-seg sidecars
+        // intra + inter site lists) is implemented in the loop below.
-        // also deferred to Phase C.2).
+        // Intra-only single-sidecar builds still use global gReloc24Sites.
         if (expressload) {
             std::vector<UserSeg> users;
             users.reserve(segs.size());

src/llvm/lib/Target/W65816/W65816BranchExpand.cpp

@@ -42,6 +42,7 @@
 #include "W65816.h"
 #include "W65816InstrInfo.h"
 #include "W65816Subtarget.h"
+#include "llvm/CodeGen/LivePhysRegs.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineInstr.h"
@@ -152,7 +153,8 @@ static unsigned estimateDistance(MachineFunction &MF,
 // sliced after each non-final conditional, so every MBB ends up with
 // at most one conditional terminator.  Returns true if any MBB was
 // split.
-static bool splitMultiBranchMBBs(MachineFunction &MF) {
+static bool splitMultiBranchMBBs(MachineFunction &MF,
+                                 SmallVectorImpl<MachineBasicBlock *> &NewBlocks) {
   bool Changed = false;
   // Snapshot MBBs first (we mutate the list during iteration).
   SmallVector<MachineBasicBlock *, 16> MBBs;
@@ -183,6 +185,7 @@ static bool splitMultiBranchMBBs(MachineFunction &MF) {
       // Create new MBB; transfer everything after splitAfter to it.
       auto *NewMBB = MF.CreateMachineBasicBlock(MBB->getBasicBlock());
       MF.insert(std::next(MBB->getIterator()), NewMBB);
+      NewBlocks.push_back(NewMBB);
       // Move instructions [splitAfter+1 .. end) to NewMBB.
       auto moveStart = std::next(splitAfter);
       NewMBB->splice(NewMBB->end(), MBB, moveStart, MBB->end());
@@ -331,6 +334,13 @@ bool W65816BranchExpand::runOnMachineFunction(MachineFunction &MF) {
   const auto &STI = MF.getSubtarget<W65816Subtarget>();
   const auto *TII = STI.getInstrInfo();
   bool AnyChanged = false;
+  // Blocks created below (split tails + `BRL Target` bridges).  These are
+  // wired into the CFG with successors but no live-in lists; we recompute
+  // live-ins for exactly these blocks at the end so downstream passes that
+  // query successor live-ins (W65816SepRepCleanup) stay sound.  Existing
+  // blocks' live-ins are unaffected by inserting predecessors, so there's
+  // no need to recompute the whole function.
+  SmallVector<MachineBasicBlock *, 8> NewBlocks;
 
   // Step -1: invert Bxx + BRA when cond target is the next MBB.  Saves
   // 3 cyc per backedge in tight loops (sumOfSquares 50 iters: -150 cyc).
@@ -342,7 +352,7 @@ bool W65816BranchExpand::runOnMachineFunction(MachineFunction &MF) {
   AnyChanged |= dropDeadConditionalsToBRATarget(MF);
 
   // Step 1: split multi-conditional-terminator MBBs.
-  AnyChanged |= splitMultiBranchMBBs(MF);
+  AnyChanged |= splitMultiBranchMBBs(MF, NewBlocks);
 
   // Step 2: iterate to fixed-point.  Each expansion adds 3 bytes
   // (bridge BRA), which may push another previously-OK branch over
@@ -414,6 +424,7 @@ bool W65816BranchExpand::runOnMachineFunction(MachineFunction &MF) {
       MachineBasicBlock *Bridge =
           MF.CreateMachineBasicBlock(MBB->getBasicBlock());
       MF.insert(std::next(MBB->getIterator()), Bridge);
+      NewBlocks.push_back(Bridge);
 
       // Replace successor edges: MBB used to have {Target, Skip}; now
       // it has {Bridge, Skip}.  Bridge has {Target}.
@@ -485,5 +496,21 @@ bool W65816BranchExpand::runOnMachineFunction(MachineFunction &MF) {
     Last->eraseFromParent();
     AnyChanged = true;
   }
+
+  // Bridge/split blocks created above are wired into the CFG with successors
+  // but no live-in lists, so they are left with EMPTY live-ins even though
+  // values flow through them to their targets.  Downstream peepholes that
+  // query successor live-in sets (W65816SepRepCleanup's PHI-copy hoist and
+  // lagged-ptr sink guards both call Succ->isLiveIn(A)) would then wrongly
+  // conclude A is dead at a block exit and hoist a clobbering def across a
+  // still-live value.  This is the root cause of the malloc over-heap
+  // miscompile: the over-heap NULL return (A=0) lived out only through a
+  // Bridge block, so the hoist replaced it with the bounds-compare result.
+  // Recompute live-ins for exactly the new blocks (their successors are
+  // existing blocks whose live-ins are already correct; fullyRecomputeLiveIns
+  // converges across chained new blocks).  Existing blocks are untouched, so
+  // this stays O(new blocks) instead of O(function) per invocation.
+  if (!NewBlocks.empty())
+    fullyRecomputeLiveIns(NewBlocks);
   return AnyChanged;
 }