diff --git a/Makefile b/Makefile index 5059f1a5..7f366a0a 100644 --- a/Makefile +++ b/Makefile @@ -235,6 +235,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_fnlabel_mangle \ $(BIN)/test_cgreturn_variant_zero \ $(BIN)/test_param_shadow_mod \ + $(BIN)/test_localoff_scope \ $(BIN)/test_use_promote_alias \ $(BIN)/test_field_signed $(BIN)/test_frame_argcount \ $(BIN)/test_selfhost $(BIN)/test_w6a_ww $(BIN)/test_w6l_ww \ @@ -436,6 +437,12 @@ $(BIN)/test_param_shadow_mod: test/wcc/708_param_shadow_mod.c \ $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_localoff_scope: test/wcc/709_localoff_scope.c \ + $(BIN)/ww $(BIN)/w6c $(BIN)/w6a $(BIN)/w6l \ + $(BIN)/ww_ww $(BIN)/w6c_ww $(BIN)/w6a_ww $(BIN)/w6l_ww \ + $(LIB)/libwwrt.a | $(BIN) + $(CC) $(CFLAGS) -o $@ $< + $(BIN)/test_use_promote_alias: test/wcc/699_use_promote_alias.c \ $(BIN)/ww $(BIN)/w6c $(BIN)/w6a $(BIN)/w6l \ $(LIB)/libwwrt.a | $(BIN) diff --git a/cmd/w6c/cgen.c b/cmd/w6c/cgen.c index e4cfc326..16c6ac8a 100644 --- a/cmd/w6c/cgen.c +++ b/cmd/w6c/cgen.c @@ -880,11 +880,18 @@ struct Local { Local *next; }; +/* localoff — push a fresh stack slot for this binding and return its + * BP offset. Never dedups by name (post-#27): two `let a: T` in disjoint + * scopes within one fn must each get their own slot, sized to their own + * declared T. Pre-fix the dedup loop returned the first-allocated slot + * regardless of the new declaration's size, so an outer `let a: [128]u8` + * after an inner `let a: i64` would collapse onto the 8B slot and + * `a[127]` would land at +119(BP), past the saved RIP, into the + * caller's frame. localfind walks from the head, so the most recent + * binding still wins lookups inside its scope. */ static int localoff(Cg *c, Local **head, const char *name, int size, int *frame) { - for (Local *l = *head; l; l = l->next) - if (strcmp(l->name, name) == 0) return l->off; int al = 8; *frame = (*frame + size + al - 1) & ~(al - 1); int off = -*frame; @@ -896,12 +903,11 @@ localoff(Cg *c, Local **head, const char *name, int size, int *frame) return off; } -/* local_alloc — always push a fresh slot, never dedup by name. Used for - * match-arm bindings, where `case let e: T` must shadow any outer `e` - * with a slot sized to T — localoff's dedup would reuse the outer's - * (possibly smaller) slot and let multi-word writes overflow into the - * saved BP / return address. localfind walks from the head, so the - * fresh entry still wins inside the arm body. */ +/* local_alloc — synonym for localoff. Pre-#27 localoff deduped by name + * and local_alloc was the always-fresh escape hatch (match-arm bindings, + * synthetic scratch slots). Post-#27 localoff is also always-fresh, so + * the two are functionally identical; both names are kept so the call + * sites read intentfully (let-decl vs scratch). */ static int local_alloc(Cg *c, Local **head, const char *name, int size, int *frame) { @@ -5723,10 +5729,27 @@ cgstmt(Cg *c, Node *n, Local **locals, int *frame) { if (n == NULL) return; switch (n->kind) { - case N_BLOCK: + case N_BLOCK: { + /* Save/restore the locals head across the block (post-#27). + * Inner-scope `let` bindings prepend to *locals via localoff; + * without this restore, the prepended stubs leak into sibling + * and ancestor scopes, and localfind (head-first) returns the + * inner binding's offset for an identifier that semantically + * belongs to the outer scope. The frame is left grown — slot + * lifetimes don't overlap with later siblings observably (the + * popped stubs' offsets are no longer reachable by name), but + * we don't reclaim the frame bytes; that's the conservative + * choice C compilers make for simple lowering. + * + * cgfn iterates fn->body->list directly to bypass this + * save/restore at the function's outermost block — defers + * (and the implicit-return epilogue) need locals intact. */ + Local *saved = *locals; for (Node *s = n->list; s; s = s->next) cgstmt(c, s, locals, frame); + *locals = saved; break; + } case N_EXPRSTMT: cgexpr(c, n->lhs, *locals); break; @@ -6700,7 +6723,19 @@ cgfn(Cg *c, FILE *out, Node *fn) if (tp) tp = tp->next; } - cgstmt(c, fn->body, &locals, &frame); + /* Iterate the fn body's statements directly rather than dispatching + * the outermost N_BLOCK through cgstmt — N_BLOCK now save/restores + * the locals head to scope inner shadows, but the function body is + * not "an inner block": defers (queued during the body) and the + * implicit-return epilogue both call cgexpr after this loop and + * resolve identifiers via localfind, so the body's locals must + * still be in *locals when we get there. */ + if (fn->body && fn->body->kind == N_BLOCK) { + for (Node *s = fn->body->list; s; s = s->next) + cgstmt(c, s, &locals, &frame); + } else { + cgstmt(c, fn->body, &locals, &frame); + } /* implicit return for void functions */ if (c->tail->as != A_RET) { diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index b5f69496..bfc367b3 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -14845,11 +14845,24 @@ fn cgyield(c: *cgen, n: *node) void = { }; fn cgblock(c: *cgen, n: *node) void = { + // Save/restore the locals head across the block (post-#27). + // Inner-scope `let` bindings prepend to c.locals via localadd; + // without this restore, the prepended stubs leak into sibling + // and ancestor scopes, and localfind (head-first) returns the + // inner binding's offset for an identifier that semantically + // belongs to the outer scope. The frame is left grown — we + // don't reclaim popped slots, matching cstage's lowering. + // + // cgfn iterates fn_.body.list directly to bypass this save/ + // restore at the function's outermost block — defers (and the + // implicit-return epilogue) need locals intact. + let saved: *local = c.locals; let s: *node = n.list; for (s != nil) { cgstmt(c, s); s = s.next; }; + c.locals = saved; return; }; @@ -16065,24 +16078,23 @@ fn scanlocals(c: *cgen, n: *node) i32 = { // scanlocals must agree with localadd or the prologue // SUBQ undersizes the frame and lets overflow into the // caller's stack — corrupting whatever's at -frameSize..-1 - // of the caller. Same-name re-declarations share the first - // slot (see scanseenmark / localadd). - if (!scanseenmark(c, n.str)) { - let sz: i32 = letslotsize(c, n); - if (sz < 8) { sz = 8; }; - if ((sz & 7) != 0) { sz = (sz + 7) & ~7; }; - total += sz; - }; - // Carry the let's tnode into the stub so scanlocals can - // dispatch on type later in the walk — e.g. detecting - // `arr[i] = ...` where arr is a tagged-element array, - // which needs an @tagscr scratch slot reservation. - let stub: *local = localfindnode(c, n.str); - if (stub != nil) { - if (stub.tnode == nil) { - if (n.lhs != nil) { stub.tnode = n.lhs; }; - }; - }; + // of the caller. Post-#27 every let allocates fresh (no + // name dedup), so we always count + always append a stub. + // The stub carries n.lhs as tnode so later scanlocals + // nodes can dispatch on type — e.g. detecting `arr[i] = ...` + // where arr is a tagged-element array (needs @tagscr). + // localfindnode walks head-first, so the freshest stub + // (innermost binding) wins lookup. + let sz: i32 = letslotsize(c, n); + if (sz < 8) { sz = 8; }; + if ((sz & 7) != 0) { sz = (sz + 7) & ~7; }; + total += sz; + let stub: *local = amalloc(c.a, 48u64): *local; + stub.name = n.str; + stub.off = 0; + stub.tnode = n.lhs; + stub.lnext = c.locals; + c.locals = stub; }; // Multi-let from a tuple-returning call: each binding's size // comes from its annotated type (l.lhs) when present, else from @@ -16116,18 +16128,25 @@ fn scanlocals(c: *cgen, n: *node) i32 = { let pt: *node = p0t; let bidx: i32 = 0; for (l != nil) { - if (!scanseenmark(c, l.str)) { - let t: *node = l.lhs; - if (t == nil) { - if (bidx == 0) { t = p0t; }; - if (bidx == 1) { t = p1t; }; - }; - let sz: i32 = 8; - if (t != nil) { sz = slotsize(c, t); }; - if (sz < 8) { sz = 8; }; - if ((sz & 7) != 0) { sz = (sz + 7) & ~7; }; - total += sz; + let t: *node = l.lhs; + if (t == nil) { + if (bidx == 0) { t = p0t; }; + if (bidx == 1) { t = p1t; }; }; + let sz: i32 = 8; + if (t != nil) { sz = slotsize(c, t); }; + if (sz < 8) { sz = 8; }; + if ((sz & 7) != 0) { sz = (sz + 7) & ~7; }; + total += sz; + // Always-fresh stub (post-#27); tnode carries the + // binding's type so later array-index dispatch can + // resolve the let through localfindnode. + let stub: *local = amalloc(c.a, 48u64): *local; + stub.name = l.str; + stub.off = 0; + stub.tnode = t; + stub.lnext = c.locals; + c.locals = stub; l = l.next; bidx += 1; }; @@ -16144,24 +16163,35 @@ fn scanlocals(c: *cgen, n: *node) i32 = { // those, so the simple count tracks C cgen for current fixtures. if (n.kind == nkind.N_FORRANGE) { total += 16; // .rgi + .rgl scratch + // Each forrange binding gets a fresh 8B slot (post-#27). + // Stub is also appended so the body's references resolve + // to this binding via head-first localfindnode lookup. if (n.list != nil) { let m: *node = n.list; for (m != nil) { let bnm: str = m.str; - if (bnm.len == 0) { - total += 8; // discard binding still gets a slot - } else { if (!scanseenmark(c, bnm)) { - total += 8; - };}; + total += 8; + if (bnm.len > 0) { + let stub: *local = amalloc(c.a, 48u64): *local; + stub.name = bnm; + stub.off = 0; + stub.tnode = m.lhs; + stub.lnext = c.locals; + c.locals = stub; + }; m = m.next; }; } else { let bnm: str = n.str; - if (bnm.len == 0) { - total += 8; - } else { if (!scanseenmark(c, bnm)) { - total += 8; - };}; + total += 8; + if (bnm.len > 0) { + let stub: *local = amalloc(c.a, 48u64): *local; + stub.name = bnm; + stub.off = 0; + stub.tnode = nil; + stub.lnext = c.locals; + c.locals = stub; + }; }; }; // `match (non-ident)` needs a 24B `@match_spill` scratch slot for @@ -16839,7 +16869,24 @@ fn cgfn(c: *cgen, fn_: *node) void = { cgfnparams(c, fn_.list); c.lastwasreturn = 0; - if (fn_.body != nil) { cgstmt(c, fn_.body); }; + // Iterate the fn body's statements directly rather than dispatching + // the outermost N_BLOCK through cgstmt — cgblock now save/restores + // c.locals to scope inner shadows (post-#27), but the function body + // is not "an inner block": defers (queued during the body) and the + // implicit-return epilogue both call cgexpr after this loop and + // resolve identifiers via localfind, so the body's locals must + // still be in c.locals when we get there. + if (fn_.body != nil) { + if (fn_.body.kind == nkind.N_BLOCK) { + let s: *node = fn_.body.list; + for (s != nil) { + cgstmt(c, s); + s = s.next; + }; + } else { + cgstmt(c, fn_.body); + }; + }; if (c.lastwasreturn == 0) { // Run any registered defers in LIFO order before the @@ -17374,29 +17421,37 @@ fn localaddstack(c: *cgen, name: str, tnode: *node, off: i32) void = { }; fn localadd(c: *cgen, name: str, sz: i32, tnode: *node) i32 = { - // Name-based slot reuse for N_LETs and params: if `name` is - // already declared in this function, return its existing - // offset. Mirrors C cgen (cmd/w6c/cgen.c:localoff). Two - // disjoint scopes that declare the same name share one slot — - // so `escape` in wwdump (three `let cp: pos;` across separate - // branches) reserves one slot, not three. scanlocals does - // the matching dedup at prologue time so the SUBQ stays in - // sync. + // User-let path (post-#27): always allocate a fresh slot per + // binding. Pre-fix this deduped by name to share one slot + // across same-name lets in disjoint scopes — inherited from + // C cgen's localoff. Both stages had the same silent-stack- + // corruption bug: an inner 8B `let a: i64` allocated first + // would force a later outer `let a: [128]u8` onto the 8B slot, + // and `a[127]` would write at +119(BP), past the saved RIP. + // Localfind walks head-first, so the most-recent binding still + // wins lookups inside its scope. Tnode is carried on the + // freshly-pushed entry, so type dispatch in cgenutil never + // sees a stale predecessor. // - // On a dedup hit we also overwrite the stored tnode to match - // the new declaration's type. C reads `n->lhs->type` (filled - // by the checker) at every nkind.N_DOT/nkind.N_CAST site; we read - // `lc.tnode`, so it must follow source order. Without this, - // a later `let m: *node` inside a branch keeps an earlier - // `let m: i32`'s tnode and `m.next` falls into the SB fallback. - let cur: *local = c.locals; - for (cur != nil) { - let cn: str = cur.name; - if (streq(cn, name)) { - cur.tnode = tnode; - return cur.off; + // Synthetic scratch slots (`@tagscr`, `@retscr`, `@tagbase`) + // keep the per-fn dedup. Each scratch is sized identically + // across its call sites and intended to be shared — the + // scanlocals pre-pass also dedups via scanseenmark, so frame + // reservation and emit-time allocation stay in sync. The + // `@`-prefix carve-out preserves that contract; user names + // can never start with `@` (lexer-rejected). + if (name.len > 0) { + if (name[0] == 64u8) { // '@' + let cur: *local = c.locals; + for (cur != nil) { + let cn: str = cur.name; + if (streq(cn, name)) { + cur.tnode = tnode; + return cur.off; + }; + cur = cur.lnext; + }; }; - cur = cur.lnext; }; return localalloc(c, name, sz, tnode); }; diff --git a/selfhost/cmd/wcc/cgen.ww b/selfhost/cmd/wcc/cgen.ww index d027f2eb..0709fc7b 100644 --- a/selfhost/cmd/wcc/cgen.ww +++ b/selfhost/cmd/wcc/cgen.ww @@ -484,29 +484,37 @@ fn localaddstack(c: *cgen, name: str, tnode: *node, off: i32) void = { }; fn localadd(c: *cgen, name: str, sz: i32, tnode: *node) i32 = { - // Name-based slot reuse for N_LETs and params: if `name` is - // already declared in this function, return its existing - // offset. Mirrors C cgen (cmd/w6c/cgen.c:localoff). Two - // disjoint scopes that declare the same name share one slot — - // so `escape` in wwdump (three `let cp: pos;` across separate - // branches) reserves one slot, not three. scanlocals does - // the matching dedup at prologue time so the SUBQ stays in - // sync. + // User-let path (post-#27): always allocate a fresh slot per + // binding. Pre-fix this deduped by name to share one slot + // across same-name lets in disjoint scopes — inherited from + // C cgen's localoff. Both stages had the same silent-stack- + // corruption bug: an inner 8B `let a: i64` allocated first + // would force a later outer `let a: [128]u8` onto the 8B slot, + // and `a[127]` would write at +119(BP), past the saved RIP. + // Localfind walks head-first, so the most-recent binding still + // wins lookups inside its scope. Tnode is carried on the + // freshly-pushed entry, so type dispatch in cgenutil never + // sees a stale predecessor. // - // On a dedup hit we also overwrite the stored tnode to match - // the new declaration's type. C reads `n->lhs->type` (filled - // by the checker) at every nkind.N_DOT/nkind.N_CAST site; we read - // `lc.tnode`, so it must follow source order. Without this, - // a later `let m: *node` inside a branch keeps an earlier - // `let m: i32`'s tnode and `m.next` falls into the SB fallback. - let cur: *local = c.locals; - for (cur != nil) { - let cn: str = cur.name; - if (streq(cn, name)) { - cur.tnode = tnode; - return cur.off; + // Synthetic scratch slots (`@tagscr`, `@retscr`, `@tagbase`) + // keep the per-fn dedup. Each scratch is sized identically + // across its call sites and intended to be shared — the + // scanlocals pre-pass also dedups via scanseenmark, so frame + // reservation and emit-time allocation stay in sync. The + // `@`-prefix carve-out preserves that contract; user names + // can never start with `@` (lexer-rejected). + if (name.len > 0) { + if (name[0] == 64u8) { // '@' + let cur: *local = c.locals; + for (cur != nil) { + let cn: str = cur.name; + if (streq(cn, name)) { + cur.tnode = tnode; + return cur.off; + }; + cur = cur.lnext; + }; }; - cur = cur.lnext; }; return localalloc(c, name, sz, tnode); }; diff --git a/selfhost/cmd/wcc/cgendecl.ww b/selfhost/cmd/wcc/cgendecl.ww index 8e01b8d1..e51c4323 100644 --- a/selfhost/cmd/wcc/cgendecl.ww +++ b/selfhost/cmd/wcc/cgendecl.ww @@ -30,24 +30,23 @@ fn scanlocals(c: *cgen, n: *node) i32 = { // scanlocals must agree with localadd or the prologue // SUBQ undersizes the frame and lets overflow into the // caller's stack — corrupting whatever's at -frameSize..-1 - // of the caller. Same-name re-declarations share the first - // slot (see scanseenmark / localadd). - if (!scanseenmark(c, n.str)) { - let sz: i32 = letslotsize(c, n); - if (sz < 8) { sz = 8; }; - if ((sz & 7) != 0) { sz = (sz + 7) & ~7; }; - total += sz; - }; - // Carry the let's tnode into the stub so scanlocals can - // dispatch on type later in the walk — e.g. detecting - // `arr[i] = ...` where arr is a tagged-element array, - // which needs an @tagscr scratch slot reservation. - let stub: *local = localfindnode(c, n.str); - if (stub != nil) { - if (stub.tnode == nil) { - if (n.lhs != nil) { stub.tnode = n.lhs; }; - }; - }; + // of the caller. Post-#27 every let allocates fresh (no + // name dedup), so we always count + always append a stub. + // The stub carries n.lhs as tnode so later scanlocals + // nodes can dispatch on type — e.g. detecting `arr[i] = ...` + // where arr is a tagged-element array (needs @tagscr). + // localfindnode walks head-first, so the freshest stub + // (innermost binding) wins lookup. + let sz: i32 = letslotsize(c, n); + if (sz < 8) { sz = 8; }; + if ((sz & 7) != 0) { sz = (sz + 7) & ~7; }; + total += sz; + let stub: *local = amalloc(c.a, 48u64): *local; + stub.name = n.str; + stub.off = 0; + stub.tnode = n.lhs; + stub.lnext = c.locals; + c.locals = stub; }; // Multi-let from a tuple-returning call: each binding's size // comes from its annotated type (l.lhs) when present, else from @@ -81,18 +80,25 @@ fn scanlocals(c: *cgen, n: *node) i32 = { let pt: *node = p0t; let bidx: i32 = 0; for (l != nil) { - if (!scanseenmark(c, l.str)) { - let t: *node = l.lhs; - if (t == nil) { - if (bidx == 0) { t = p0t; }; - if (bidx == 1) { t = p1t; }; - }; - let sz: i32 = 8; - if (t != nil) { sz = slotsize(c, t); }; - if (sz < 8) { sz = 8; }; - if ((sz & 7) != 0) { sz = (sz + 7) & ~7; }; - total += sz; + let t: *node = l.lhs; + if (t == nil) { + if (bidx == 0) { t = p0t; }; + if (bidx == 1) { t = p1t; }; }; + let sz: i32 = 8; + if (t != nil) { sz = slotsize(c, t); }; + if (sz < 8) { sz = 8; }; + if ((sz & 7) != 0) { sz = (sz + 7) & ~7; }; + total += sz; + // Always-fresh stub (post-#27); tnode carries the + // binding's type so later array-index dispatch can + // resolve the let through localfindnode. + let stub: *local = amalloc(c.a, 48u64): *local; + stub.name = l.str; + stub.off = 0; + stub.tnode = t; + stub.lnext = c.locals; + c.locals = stub; l = l.next; bidx += 1; }; @@ -109,24 +115,35 @@ fn scanlocals(c: *cgen, n: *node) i32 = { // those, so the simple count tracks C cgen for current fixtures. if (n.kind == nkind.N_FORRANGE) { total += 16; // .rgi + .rgl scratch + // Each forrange binding gets a fresh 8B slot (post-#27). + // Stub is also appended so the body's references resolve + // to this binding via head-first localfindnode lookup. if (n.list != nil) { let m: *node = n.list; for (m != nil) { let bnm: str = m.str; - if (bnm.len == 0) { - total += 8; // discard binding still gets a slot - } else { if (!scanseenmark(c, bnm)) { - total += 8; - };}; + total += 8; + if (bnm.len > 0) { + let stub: *local = amalloc(c.a, 48u64): *local; + stub.name = bnm; + stub.off = 0; + stub.tnode = m.lhs; + stub.lnext = c.locals; + c.locals = stub; + }; m = m.next; }; } else { let bnm: str = n.str; - if (bnm.len == 0) { - total += 8; - } else { if (!scanseenmark(c, bnm)) { - total += 8; - };}; + total += 8; + if (bnm.len > 0) { + let stub: *local = amalloc(c.a, 48u64): *local; + stub.name = bnm; + stub.off = 0; + stub.tnode = nil; + stub.lnext = c.locals; + c.locals = stub; + }; }; }; // `match (non-ident)` needs a 24B `@match_spill` scratch slot for @@ -804,7 +821,24 @@ fn cgfn(c: *cgen, fn_: *node) void = { cgfnparams(c, fn_.list); c.lastwasreturn = 0; - if (fn_.body != nil) { cgstmt(c, fn_.body); }; + // Iterate the fn body's statements directly rather than dispatching + // the outermost N_BLOCK through cgstmt — cgblock now save/restores + // c.locals to scope inner shadows (post-#27), but the function body + // is not "an inner block": defers (queued during the body) and the + // implicit-return epilogue both call cgexpr after this loop and + // resolve identifiers via localfind, so the body's locals must + // still be in c.locals when we get there. + if (fn_.body != nil) { + if (fn_.body.kind == nkind.N_BLOCK) { + let s: *node = fn_.body.list; + for (s != nil) { + cgstmt(c, s); + s = s.next; + }; + } else { + cgstmt(c, fn_.body); + }; + }; if (c.lastwasreturn == 0) { // Run any registered defers in LIFO order before the diff --git a/selfhost/cmd/wcc/cgenstmt.ww b/selfhost/cmd/wcc/cgenstmt.ww index 51a9e889..eaa0a968 100644 --- a/selfhost/cmd/wcc/cgenstmt.ww +++ b/selfhost/cmd/wcc/cgenstmt.ww @@ -74,11 +74,24 @@ fn cgyield(c: *cgen, n: *node) void = { }; fn cgblock(c: *cgen, n: *node) void = { + // Save/restore the locals head across the block (post-#27). + // Inner-scope `let` bindings prepend to c.locals via localadd; + // without this restore, the prepended stubs leak into sibling + // and ancestor scopes, and localfind (head-first) returns the + // inner binding's offset for an identifier that semantically + // belongs to the outer scope. The frame is left grown — we + // don't reclaim popped slots, matching cstage's lowering. + // + // cgfn iterates fn_.body.list directly to bypass this save/ + // restore at the function's outermost block — defers (and the + // implicit-return epilogue) need locals intact. + let saved: *local = c.locals; let s: *node = n.list; for (s != nil) { cgstmt(c, s); s = s.next; }; + c.locals = saved; return; }; diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index e29082f8..e4e73824 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -14845,11 +14845,24 @@ fn cgyield(c: *cgen, n: *node) void = { }; fn cgblock(c: *cgen, n: *node) void = { + // Save/restore the locals head across the block (post-#27). + // Inner-scope `let` bindings prepend to c.locals via localadd; + // without this restore, the prepended stubs leak into sibling + // and ancestor scopes, and localfind (head-first) returns the + // inner binding's offset for an identifier that semantically + // belongs to the outer scope. The frame is left grown — we + // don't reclaim popped slots, matching cstage's lowering. + // + // cgfn iterates fn_.body.list directly to bypass this save/ + // restore at the function's outermost block — defers (and the + // implicit-return epilogue) need locals intact. + let saved: *local = c.locals; let s: *node = n.list; for (s != nil) { cgstmt(c, s); s = s.next; }; + c.locals = saved; return; }; @@ -16065,24 +16078,23 @@ fn scanlocals(c: *cgen, n: *node) i32 = { // scanlocals must agree with localadd or the prologue // SUBQ undersizes the frame and lets overflow into the // caller's stack — corrupting whatever's at -frameSize..-1 - // of the caller. Same-name re-declarations share the first - // slot (see scanseenmark / localadd). - if (!scanseenmark(c, n.str)) { - let sz: i32 = letslotsize(c, n); - if (sz < 8) { sz = 8; }; - if ((sz & 7) != 0) { sz = (sz + 7) & ~7; }; - total += sz; - }; - // Carry the let's tnode into the stub so scanlocals can - // dispatch on type later in the walk — e.g. detecting - // `arr[i] = ...` where arr is a tagged-element array, - // which needs an @tagscr scratch slot reservation. - let stub: *local = localfindnode(c, n.str); - if (stub != nil) { - if (stub.tnode == nil) { - if (n.lhs != nil) { stub.tnode = n.lhs; }; - }; - }; + // of the caller. Post-#27 every let allocates fresh (no + // name dedup), so we always count + always append a stub. + // The stub carries n.lhs as tnode so later scanlocals + // nodes can dispatch on type — e.g. detecting `arr[i] = ...` + // where arr is a tagged-element array (needs @tagscr). + // localfindnode walks head-first, so the freshest stub + // (innermost binding) wins lookup. + let sz: i32 = letslotsize(c, n); + if (sz < 8) { sz = 8; }; + if ((sz & 7) != 0) { sz = (sz + 7) & ~7; }; + total += sz; + let stub: *local = amalloc(c.a, 48u64): *local; + stub.name = n.str; + stub.off = 0; + stub.tnode = n.lhs; + stub.lnext = c.locals; + c.locals = stub; }; // Multi-let from a tuple-returning call: each binding's size // comes from its annotated type (l.lhs) when present, else from @@ -16116,18 +16128,25 @@ fn scanlocals(c: *cgen, n: *node) i32 = { let pt: *node = p0t; let bidx: i32 = 0; for (l != nil) { - if (!scanseenmark(c, l.str)) { - let t: *node = l.lhs; - if (t == nil) { - if (bidx == 0) { t = p0t; }; - if (bidx == 1) { t = p1t; }; - }; - let sz: i32 = 8; - if (t != nil) { sz = slotsize(c, t); }; - if (sz < 8) { sz = 8; }; - if ((sz & 7) != 0) { sz = (sz + 7) & ~7; }; - total += sz; + let t: *node = l.lhs; + if (t == nil) { + if (bidx == 0) { t = p0t; }; + if (bidx == 1) { t = p1t; }; }; + let sz: i32 = 8; + if (t != nil) { sz = slotsize(c, t); }; + if (sz < 8) { sz = 8; }; + if ((sz & 7) != 0) { sz = (sz + 7) & ~7; }; + total += sz; + // Always-fresh stub (post-#27); tnode carries the + // binding's type so later array-index dispatch can + // resolve the let through localfindnode. + let stub: *local = amalloc(c.a, 48u64): *local; + stub.name = l.str; + stub.off = 0; + stub.tnode = t; + stub.lnext = c.locals; + c.locals = stub; l = l.next; bidx += 1; }; @@ -16144,24 +16163,35 @@ fn scanlocals(c: *cgen, n: *node) i32 = { // those, so the simple count tracks C cgen for current fixtures. if (n.kind == nkind.N_FORRANGE) { total += 16; // .rgi + .rgl scratch + // Each forrange binding gets a fresh 8B slot (post-#27). + // Stub is also appended so the body's references resolve + // to this binding via head-first localfindnode lookup. if (n.list != nil) { let m: *node = n.list; for (m != nil) { let bnm: str = m.str; - if (bnm.len == 0) { - total += 8; // discard binding still gets a slot - } else { if (!scanseenmark(c, bnm)) { - total += 8; - };}; + total += 8; + if (bnm.len > 0) { + let stub: *local = amalloc(c.a, 48u64): *local; + stub.name = bnm; + stub.off = 0; + stub.tnode = m.lhs; + stub.lnext = c.locals; + c.locals = stub; + }; m = m.next; }; } else { let bnm: str = n.str; - if (bnm.len == 0) { - total += 8; - } else { if (!scanseenmark(c, bnm)) { - total += 8; - };}; + total += 8; + if (bnm.len > 0) { + let stub: *local = amalloc(c.a, 48u64): *local; + stub.name = bnm; + stub.off = 0; + stub.tnode = nil; + stub.lnext = c.locals; + c.locals = stub; + }; }; }; // `match (non-ident)` needs a 24B `@match_spill` scratch slot for @@ -16839,7 +16869,24 @@ fn cgfn(c: *cgen, fn_: *node) void = { cgfnparams(c, fn_.list); c.lastwasreturn = 0; - if (fn_.body != nil) { cgstmt(c, fn_.body); }; + // Iterate the fn body's statements directly rather than dispatching + // the outermost N_BLOCK through cgstmt — cgblock now save/restores + // c.locals to scope inner shadows (post-#27), but the function body + // is not "an inner block": defers (queued during the body) and the + // implicit-return epilogue both call cgexpr after this loop and + // resolve identifiers via localfind, so the body's locals must + // still be in c.locals when we get there. + if (fn_.body != nil) { + if (fn_.body.kind == nkind.N_BLOCK) { + let s: *node = fn_.body.list; + for (s != nil) { + cgstmt(c, s); + s = s.next; + }; + } else { + cgstmt(c, fn_.body); + }; + }; if (c.lastwasreturn == 0) { // Run any registered defers in LIFO order before the @@ -17374,29 +17421,37 @@ fn localaddstack(c: *cgen, name: str, tnode: *node, off: i32) void = { }; fn localadd(c: *cgen, name: str, sz: i32, tnode: *node) i32 = { - // Name-based slot reuse for N_LETs and params: if `name` is - // already declared in this function, return its existing - // offset. Mirrors C cgen (cmd/w6c/cgen.c:localoff). Two - // disjoint scopes that declare the same name share one slot — - // so `escape` in wwdump (three `let cp: pos;` across separate - // branches) reserves one slot, not three. scanlocals does - // the matching dedup at prologue time so the SUBQ stays in - // sync. + // User-let path (post-#27): always allocate a fresh slot per + // binding. Pre-fix this deduped by name to share one slot + // across same-name lets in disjoint scopes — inherited from + // C cgen's localoff. Both stages had the same silent-stack- + // corruption bug: an inner 8B `let a: i64` allocated first + // would force a later outer `let a: [128]u8` onto the 8B slot, + // and `a[127]` would write at +119(BP), past the saved RIP. + // Localfind walks head-first, so the most-recent binding still + // wins lookups inside its scope. Tnode is carried on the + // freshly-pushed entry, so type dispatch in cgenutil never + // sees a stale predecessor. // - // On a dedup hit we also overwrite the stored tnode to match - // the new declaration's type. C reads `n->lhs->type` (filled - // by the checker) at every nkind.N_DOT/nkind.N_CAST site; we read - // `lc.tnode`, so it must follow source order. Without this, - // a later `let m: *node` inside a branch keeps an earlier - // `let m: i32`'s tnode and `m.next` falls into the SB fallback. - let cur: *local = c.locals; - for (cur != nil) { - let cn: str = cur.name; - if (streq(cn, name)) { - cur.tnode = tnode; - return cur.off; + // Synthetic scratch slots (`@tagscr`, `@retscr`, `@tagbase`) + // keep the per-fn dedup. Each scratch is sized identically + // across its call sites and intended to be shared — the + // scanlocals pre-pass also dedups via scanseenmark, so frame + // reservation and emit-time allocation stay in sync. The + // `@`-prefix carve-out preserves that contract; user names + // can never start with `@` (lexer-rejected). + if (name.len > 0) { + if (name[0] == 64u8) { // '@' + let cur: *local = c.locals; + for (cur != nil) { + let cn: str = cur.name; + if (streq(cn, name)) { + cur.tnode = tnode; + return cur.off; + }; + cur = cur.lnext; + }; }; - cur = cur.lnext; }; return localalloc(c, name, sz, tnode); }; diff --git a/test/wcc/709_localoff_scope.c b/test/wcc/709_localoff_scope.c new file mode 100644 index 00000000..fe3c4445 --- /dev/null +++ b/test/wcc/709_localoff_scope.c @@ -0,0 +1,364 @@ +/* + * 709_localoff_scope — cgen localoff/localadd no longer dedups stack + * slots by name across disjoint scopes (task #27). + * + * Pre-fix: cstage `localoff` and wwstage `localadd` both keyed slot + * lookup on name alone. Two `let a: T` in disjoint scopes within one + * fn collapsed to a single slot, sized by whoever was allocated first. + * The smaller of the two then ran with an offset that, when used at + * its declared size, walked past the SUBQ'd frame and into the saved + * frame zero / saved RIP / caller stack. Silent stack corruption. + * + * The bug fired in either direction: + * - inner small allocated FIRST, outer big allocated SECOND → + * outer's writes near the end of its declared size land at + * positive BP offsets (past the saved RIP) → SIGSEGV. + * - outer big allocated FIRST, inner small allocated SECOND → + * inner's full-size store stomps the outer's first bytes. + * + * worker-19's commit (c9bbfcb) sidestepped one instance in selfhost/ + * cmd/w6a/main.ww by renaming an outer `let asm: asm_;` to `s` so the + * inner `let a: *u8 = ...;` (in the for-loop) wouldn't share a slot. + * That rename can be reverted once this fix lands (sibling cleanup). + * + * Fix: drop the dedup. Every `let` allocates a fresh slot (cstage + * localoff / wwstage localadd). localfind walks head-first, so the + * most-recent (innermost) binding still wins lookups inside its + * scope. Wwstage scanlocals stops deduping user-let names in lockstep + * so the prologue SUBQ matches the emit-time offsets. Synthetic + * scratch slots (`@tagscr`, `@retscr`, `@tagbase`) keep the per-fn + * dedup via an `@`-prefix carve-out — they're sized identically at + * every call site and intended to be shared. + * + * row | shape | gate + * --------------------------+------------------------------------+--------- + * inner_first_outer_bigger | 8B inner, then 128B outer; write | exit=99 + * | a[127]=99 in outer scope. Pre-fix | + * | SEGV; post-fix slot is 128B. | + * outer_first_inner_writes | 64B outer first, nested 8B inner | exit=7 + * | writes -1; outer's a[0] still the | + * | original 7. | + * nested_3_deep | same name `x` at three nesting | exit=6 + * | depths, returns the sum of values | (1+2+3) + * | read at each scope. | + * same_name_diff_type | `let a: i32 = 5;` then disjoint | exit=2 + * | block `let a: str = "hi";`. Returns| + * | a.len from the str scope. | + * same_block_redecl_pin | `let a: i32 = 1; let a: i32 = 9;` | exit=9 + * | in same block (checker silently | + * | accepts today). Pins last-write- | + * | wins via head-first localfind. | + * defer_shadow | outer `a`, deferred call captures | exit=42 + * | &outer-a, inner-block shadow `a`, | + * | return outer a. Pins both: cgfn | + * | body-bypass (defer's cgexpr after | + * | body iteration must still resolve | + * | outer `a`) and inner-block | + * | save/restore (inner shadow can't | + * | leak past `}`). | + * forrange_body_shadow | for (let x .. s) iterates [1,2,3], | exit=47 + * | body reads x then declares `let | (1+2+3) + * | x: i32 = 99;` then reads x again. | +99-3*8 + * | First read = iter, second = inner. | =6+47-24 + * | Pins that the iter slot's per-iter | + * | load (cgforrange cached offset) is | + * | unaffected by the body's shadow, | + * | AND that the body's pre-shadow | + * | reads still find the iter. | + * if_body_shadow | outer `a=7`, then `if (..) | exit=7 + * | { let a=99; }`, return a. Pins the | + * | if-arm's N_BLOCK save/restore pops | + * | cleanly so the return reads outer. | + * + * Per-row exit-code agreement across cstage + wwstage drivers is the + * contract this test pins. Asm byte-identity is NOT diffed here: no + * wwstage divergence specific to this fix; 995_self_rebuild covers + * cross-stage drift broadly. + */ +#include +#include +#include +#include +#include +#include + +static int +runwait(const char *cmd) +{ + int rc = system(cmd); + if (rc == -1) return -1; + if (WIFEXITED(rc)) return WEXITSTATUS(rc); + return -1; +} + +struct row { const char *label; const char *src; int want; }; + +static const struct row rows[] = { + /* 1. Inner allocated first (in for-loop body), outer allocated + * second and bigger. Pre-fix: outer dedups onto inner's 8B slot + * at -8(BP); a[127] = 99 emits MOVB AX, 119(BP) — past saved + * RIP — SEGV. Post-fix: outer gets its own 128B slot, write + * lands inside the slot, exit = 99. */ + { "inner_first_outer_bigger", + "fn main() i32 = {\n" + " let i: i32 = 0;\n" + " for (i < 3) { let a: i64 = 5i64; i += 1; };\n" + " let a: [128]u8 = [0u8...];\n" + " a[127] = 99u8;\n" + " return a[127]: i32;\n" + "};\n", + 99 }, + /* 2. Outer allocated first (64B), inner inside a nested block + * writes an 8B i64. Pre-fix: inner's full 8B store overwrites + * outer's bytes 0..7; outer's a[0] read after the inner block + * returns the low byte of -1 (= 0xff = 255). Post-fix: inner + * gets its own slot, outer's a[0] keeps the value 7 written + * before the inner block. */ + { "outer_first_inner_writes", + "fn main() i32 = {\n" + " let a: [64]u8 = [0u8...];\n" + " a[0] = 7u8;\n" + " {\n" + " let a: i64 = 0i64 - 1i64;\n" + " if (a == 0i64) { return 99i32; };\n" + " };\n" + " return a[0]: i32;\n" + "};\n", + 7 }, + /* 3. Same name `x` redeclared at three nesting depths, each with + * a distinct value. Inside each scope, x reads its own binding + * (head-first localfind). The inner reads happen WHILE the + * outer slots are still live, so the test catches both + * cross-scope slot collision (corrupt outer) and miss-up-the- + * chain lookup (returns wrong inner value). Sum is 1 + 2 + 3 = 6. */ + { "nested_3_deep", + "fn main() i32 = {\n" + " let x: i32 = 1;\n" + " let s1: i32 = x;\n" + " let s2: i32 = 0;\n" + " let s3: i32 = 0;\n" + " {\n" + " let x: i32 = 2;\n" + " s2 = x;\n" + " {\n" + " let x: i32 = 3;\n" + " s3 = x;\n" + " };\n" + " };\n" + " return s1 + s2 + s3;\n" + "};\n", + 6 }, + /* 4. Same name, different types in disjoint scopes. Outer + * `let a: i32 = 5;` (8B slot, scalar), inner `let a: str = "hi"` + * (16B slot, ptr+len). Pre-fix: inner reuses outer's 8B slot + * and the str ptr/len writes land at -8/0(BP) — corrupting + * the saved BP. Post-fix: inner gets its own 16B slot. + * Returns inner a.len = 2. */ + { "same_name_diff_type", + "fn main() i32 = {\n" + " let a: i32 = 5;\n" + " let r: i32 = 0;\n" + " {\n" + " let a: str = \"hi\";\n" + " r = a.len: i32;\n" + " };\n" + " return r;\n" + "};\n", + 2 }, + /* 5. Same-block re-declaration. Today's checker silently accepts + * `let a: i32 = 0; let a: i32 = 9;` (scope_define returns NULL + * on dup but the caller in cmd/wcc/check.c:1443 doesn't error; + * wwstage check.ww behaves the same). Pre-fix: both lets shared + * one slot, last write wins by storage. Post-fix: each let gets + * its own slot but localfind walks head-first → still last- + * write-wins observably. This row pins that observable contract + * — if the checker tightens later to reject same-scope redecl + * (task #32), this row is the canary that flips from "exit 9" + * to "build fails", explicitly opting in to the new shape. */ + { "same_block_redecl_pin", + "fn main() i32 = {\n" + " let a: i32 = 1;\n" + " let a: i32 = 9;\n" + " return a;\n" + "};\n", + 9 }, + /* 6. Defer + inner-block shadow + outer-scope post-defer read. + * + * `defer touch(&a)` queues the call; at fn-exit the defer's + * cgexpr runs (resolving `&a`) BEFORE the return value is + * loaded. We expect &a to bind to the OUTER a (the one in + * scope at fn-exit), so touch sets outer a to 42 and the + * return reads 42. + * + * Three independent invariants gate this row: + * (i) cgfn body-bypass: cgstmt(N_BLOCK) on fn->body would + * restore c.locals to params-only before the deferred + * cgexpr runs. With the bypass cgfn iterates fn.body.list + * directly so c.locals stays populated for the defer's + * cgexpr — &a resolves to outer a's slot. + * (ii) cgblock save/restore: the inner block's `let a: i32 = 99` + * prepends a stub to c.locals. Without restore, the inner + * stub leaks past `}` and head-first localfind picks it + * up; touch then writes to the inner slot and the return + * reads outer a → 7, not 42. + * (iii) localadd always-fresh: even with save/restore, if + * outer & inner share one slot (pre-#27), touch writing + * 42 stomps the (deceased) inner slot which IS the outer + * — so this gate alone happens to land at 42 either way. + * Combined with (ii), exit=42 means BOTH (ii) and (iii) + * hold; either regressing flips the gate. + * + * touch(&a) returns 0 to keep its own scalar exit out of the + * way; only the side effect through *p matters. */ + { "defer_shadow", + "fn touch(p: *i32) i32 = { *p = 42i32; return 0i32; };\n" + "fn main() i32 = {\n" + " let a: i32 = 7;\n" + " defer touch(&a);\n" + " {\n" + " let a: i32 = 99;\n" + " if (a == 0i32) { return 1i32; };\n" + " };\n" + " return a;\n" + "};\n", + 42 }, + /* 7. Forrange body shadow. Distinct codegen path from N_FOR + * (cstage cgforrange / wwstage cgenstmt cgforrange + cgendecl + * scanlocals N_FORRANGE arm). The iter binding `x` is added to + * c.locals via localadd before the body is emitted; the body + * declares its own `let x: i32 = 7;` partway through. Two + * observations per iteration: + * - first `sum += x` (pre-shadow): finds iter x via head-first + * localfind on c.locals at that point (no inner stub yet). + * Sums to 10 + 20 + 30 = 60 across iterations. + * - second `sum += x` (post-shadow): finds inner x = 7. + * Sums to 7 * 3 = 21 across iterations. + * Total: 81. The iter slot's per-iter rewrite is driven by + * cgforrange via cached offset, NOT by name lookup, so the + * shadow can't hijack the iter-write — but a regression in + * scanlocals' N_FORRANGE arm (e.g. forgetting to append a stub) + * would still surface here as the pre-shadow read failing to + * resolve `x`. */ + { "forrange_body_shadow", + "fn main() i32 = {\n" + " let arr: [3]i32 = [10i32, 20i32, 30i32];\n" + " let sum: i32 = 0;\n" + " for (let x .. arr) {\n" + " sum += x;\n" + " let x: i32 = 7;\n" + " sum += x;\n" + " };\n" + " return sum;\n" + "};\n", + 81 }, + /* 8. If-body shadow. The if's body is N_BLOCK and reaches + * cgblock via cgstmt — distinct visual path from a bare + * `{ ... }` at fn-body level (rows 2/3/4). `touched` stashes + * the inner-a value so we confirm the if-arm actually executed + * before checking the outer-scope visibility. With cgblock + * save/restore: return reads outer a = 7. Without: head-first + * localfind picks up the inner stub still on the chain → 99. */ + { "if_body_shadow", + "fn main() i32 = {\n" + " let a: i32 = 7;\n" + " let touched: i32 = 0;\n" + " if (a > 0i32) {\n" + " let a: i32 = 99;\n" + " touched = a;\n" + " };\n" + " if (touched != 99i32) { return 1i32; };\n" + " return a;\n" + "};\n", + 7 }, +}; + +static int +run_driver(const char *driver, const struct row *r, int i) +{ + char src[64], tmpdir[64], cmd[1024]; + snprintf(src, sizeof src, "/tmp/wclo_%d_%d.ww", getpid(), i); + snprintf(tmpdir, sizeof tmpdir, "/tmp/wclo_%d_d_%d", getpid(), i); + + FILE *f = fopen(src, "wb"); + if (!f) return -1; + fputs(r->src, f); + fclose(f); + + mkdir(tmpdir, 0755); + snprintf(cmd, sizeof cmd, "cd %s && %s build %s", + tmpdir, driver, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: build via %s failed\n", + r->label, driver); + unlink(src); rmdir(tmpdir); + return -1; + } + + const char *base = strrchr(src, '/'); + base = base ? base + 1 : src; + char outbin[128]; + snprintf(outbin, sizeof outbin, "%s/%s", tmpdir, base); + char *dot = strrchr(outbin, '.'); + if (dot && strcmp(dot, ".ww") == 0) *dot = '\0'; + int got = runwait(outbin); + + unlink(src); unlink(outbin); rmdir(tmpdir); + return got; +} + +int +main(void) +{ + const char *bin = getenv("BIN"); + if (!bin) bin = "out/bin"; + char absbin[512]; + if (bin[0] != '/') { + char cwd[256]; + if (getcwd(cwd, sizeof cwd) == NULL) return 1; + snprintf(absbin, sizeof absbin, "%s/%s", cwd, bin); + bin = absbin; + } + + char cdrv[640]; + snprintf(cdrv, sizeof cdrv, "%s/ww", bin); + char wdrv[640]; + snprintf(wdrv, sizeof wdrv, "%s/ww_ww", bin); + + struct { const char *name; const char *path; int gated_on_existence; } + drivers[] = { + { "cstage", cdrv, 0 }, + { "wwstage", wdrv, 1 }, + { NULL, NULL, 0 }, + }; + + int n = (int)(sizeof rows / sizeof rows[0]); + int total = 0, fail = 0; + + for (int d = 0; drivers[d].name; d++) { + if (drivers[d].gated_on_existence + && access(drivers[d].path, X_OK) != 0) { + fprintf(stderr, "localoff_scope: skip %s (no %s)\n", + drivers[d].name, drivers[d].path); + continue; + } + for (int i = 0; i < n; i++) { + int got = run_driver(drivers[d].path, &rows[i], i); + total++; + if (got != rows[i].want) { + fprintf(stderr, + "localoff_scope[%s][%s]: exit=%d want=%d\n", + drivers[d].name, rows[i].label, + got, rows[i].want); + fail++; + } + } + } + + if (fail) { + fprintf(stderr, + "localoff_scope: %d/%d fixtures failed\n", fail, total); + return 1; + } + printf("localoff_scope: %d/%d ok\n", total, total); + return 0; +}