diff --git a/examples/0136-types-global-array-element-store.sx b/examples/0136-types-global-array-element-store.sx new file mode 100644 index 0000000..1f41eb7 --- /dev/null +++ b/examples/0136-types-global-array-element-store.sx @@ -0,0 +1,57 @@ +// A store to a module-global array element writes the global's live storage, +// so a subsequent read sees the stored value — not the array initializer. +// Covers constant index, variable index, and a cross-function store, on a +// scalar global array, a struct-element global array (element-stride), and a +// nested-array global (recursive lvalue). +// Regression (issue 0079): global-array element stores were silently dropped +// (read returned the initializer) because the indexed lvalue base loaded the +// global by value into a temp instead of addressing the global's storage. + +#import "modules/std.sx"; + +g : [3]s64 = .[10, 20, 30]; + +Pair :: struct { a: s64; b: s64; } +gp : [2]Pair = .[ .{ a = 1, b = 2 }, .{ a = 3, b = 4 } ]; + +grid : [2][3]s64 = .[ .[0, 0, 0], .[0, 0, 0] ]; + +write_global :: (i: s64, v: s64) { g[i] = v; } + +main :: () { + // Scalar global array — const index. + g[1] = 222; + print("g[1]={}\n", g[1]); // 222 + + // Scalar global array — variable index. + k := 2; + g[k] = 333; + print("g[k]={}\n", g[k]); // 333 + + // Scalar global array — store from another function. + write_global(0, 111); + print("g[0]={}\n", g[0]); // 111 + + // Struct-element global array (16-byte stride) — const and var index. + gp[0] = .{ a = 10, b = 20 }; + j := 1; + gp[j] = .{ a = 30, b = 40 }; + print("gp[0]={},{}\n", gp[0].a, gp[0].b); // 10,20 + print("gp[j]={},{}\n", gp[j].a, gp[j].b); // 30,40 + + // Nested-array global — element is [3]s64, recursive indexed lvalue. + grid[1][2] = 7; + r := 0; + grid[r][0] = 5; + print("grid[1][2]={}\n", grid[1][2]); // 7 + print("grid[0][0]={}\n", grid[r][0]); // 5 + + if g[1] == 222 and g[2] == 333 and g[0] == 111 + and gp[0].a == 10 and gp[0].b == 20 + and gp[1].a == 30 and gp[1].b == 40 + and grid[1][2] == 7 and grid[0][0] == 5 { + print("PASS\n"); + } else { + print("FAIL: global array element store dropped\n"); + } +} diff --git a/examples/expected/0136-types-global-array-element-store.exit b/examples/expected/0136-types-global-array-element-store.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/examples/expected/0136-types-global-array-element-store.exit @@ -0,0 +1 @@ +0 diff --git a/examples/expected/0136-types-global-array-element-store.stderr b/examples/expected/0136-types-global-array-element-store.stderr new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/examples/expected/0136-types-global-array-element-store.stderr @@ -0,0 +1 @@ + diff --git a/examples/expected/0136-types-global-array-element-store.stdout b/examples/expected/0136-types-global-array-element-store.stdout new file mode 100644 index 0000000..9388ff7 --- /dev/null +++ b/examples/expected/0136-types-global-array-element-store.stdout @@ -0,0 +1,8 @@ +g[1]=222 +g[k]=333 +g[0]=111 +gp[0]=10,20 +gp[j]=30,40 +grid[1][2]=7 +grid[0][0]=5 +PASS diff --git a/issues/0079-global-array-element-store-dropped.md b/issues/0079-global-array-element-store-dropped.md new file mode 100644 index 0000000..e6d6ee7 --- /dev/null +++ b/issues/0079-global-array-element-store-dropped.md @@ -0,0 +1,102 @@ +# 0079 — stores to module-global array elements are silently dropped + +> **RESOLVED.** +> **Root cause:** `Lowering.lowerExprAsPtr` (`src/ir/lower.zig`) — the lvalue/ +> address path — handled only *local* identifiers (alloca pointers). A +> module-global identifier fell through to the value fallback `lowerExpr`, +> which emits `global_get` (loads the whole array *by value*). The LLVM +> backend's `emitIndexGep` then sees an array *value*, allocas a throwaway +> temp, copies the value in, and GEPs into the temp — so `g[i] = v` wrote a +> discarded copy and a later `g[i]` read the global's untouched initializer. +> Local arrays worked because they hit the alloca-pointer path; global +> *scalar* stores worked via `global_set`. +> **Fix:** teach `lowerExprAsPtr`'s identifier arm about globals — emit +> `global_addr` (a pointer into the global's live storage) for a normal +> global, or `global_get` for a pointer-typed global (mirroring the local +> pointer case). The same array-base resolution in the `address_of(index_expr)` +> path now routes through `lowerExprAsPtr` so `&g[i]` is also an lvalue into +> the global. `index_gep` then GEPs directly into `@g` for const AND variable +> index, across functions, in both `sx run` and `sx build`. +> **Regression:** `examples/0136-types-global-array-element-store.sx` +> (const-index, var-index, cross-function store on a scalar global array; a +> struct-element global array for element-stride; a nested-array global for the +> recursive indexed lvalue). FAILS on the pre-fix compiler (reads return the +> initializer / zero), PASSES after. + +## Symptom + +A store to a module-global (file-scope) ARRAY element is silently lost: after +`g[i] = v`, reading `g[i]` returns the array's INITIALIZER value, not `v`. No +diagnostic. Reproduces with a constant index, a variable index, and a store from +another function, in BOTH `sx run` (JIT) and `sx build` (AOT). Local-array stores +and global-SCALAR stores work correctly — so the indexed load/store on a *global* +array appears to read/write the initializer constant rather than the global's +live storage. This is a SILENT data-corruption bug (the dangerous class per the +project rules), not a crash. + +Manager-reproduced output (JIT): +``` +global[1] const-idx=20 (want 222) +global[k] var-idx=30 (want 333) +global[0] via fn=10 (want 111) +``` + +## Reproduction + +```sx +#import "modules/std.sx"; + +g : [3]s64 = .[10, 20, 30]; +write_global :: (i: s64, v: s64) { g[i] = v; } + +main :: () { + loc : [3]s64 = .[10, 20, 30]; + loc[1] = 222; + print("local[1]={}\n", loc[1]); // 222 (correct) + g[1] = 222; + print("global[1] const-idx={}\n", g[1]); // 20 (WRONG, want 222) + k := 2; + g[k] = 333; + print("global[k] var-idx={}\n", g[k]); // 30 (WRONG, want 333) + write_global(0, 111); + print("global[0] via fn={}\n", g[0]); // 10 (WRONG, want 111) +} +``` + +`./zig-out/bin/sx run ` → prints the WRONG values above. Expected: +`local[1]=222 / global[1]=222 / global[k]=333 / global[0]=111`. + +## Investigation prompt + +A store to a module-global array element (`g[i] = v`) is silently lost: a +subsequent `g[i]` yields the initializer value, in BOTH the JIT (`sx run`) and +compiled (`sx build`) paths. Global SCALAR stores and LOCAL array stores both +work, so the defect is specific to INDEXED lvalue access on a GLOBAL array. +Suspect the IR-gen / codegen for an indexed expression on a global symbol: +the load of `global_array[idx]` is likely decaying / constant-folding to the +array's initializer constant (or the address computation for the store targets a +private copy of the initializer rather than the global's live storage). Look in +the index-expression lowering and global-symbol address resolution (and how +global array initializers are materialized) — `src/ir/lower.zig` / +`src/ir/emit_llvm.zig` and the global-symbol/constant-initializer paths. + +The fix must make `load(global_array[idx])` read the global's live storage and +`store(global_array[idx], v)` write it — for constant AND variable indices, and +when the store happens in a different function than the read. Do NOT special-case; +fix the address resolution so a global array element is an lvalue into the +global's storage like any other. + +## Verification (once fixed) +- The repro above prints `222 / 333 / 111` for the global cases (exit 0). +- Add a pinned regression `examples/NNNN-*.sx` covering const-index, var-index, + and cross-function store to a global array (+ a global array of a struct/larger + element if practical). +- `zig build && zig build test && bash tests/run_examples.sh` all green. + +## Provenance + +Discovered by the `distribution` flow (sx-foundation step F3.1, std.cli argv +accessor) while exploring argv backing strategies. The shipped F3.1 code does NOT +depend on this (it uses caller-provided buffers + zero-copy views over the C argv +block), so F3.1 is correct independently — this is a separate latent +silent-miscompile surfaced per the STOP-on-compiler-bug rule. diff --git a/src/ir/lower.zig b/src/ir/lower.zig index 6bd5d98..e45b7e2 100644 --- a/src/ir/lower.zig +++ b/src/ir/lower.zig @@ -2324,20 +2324,29 @@ pub const Lowering = struct { fn lowerExprAsPtr(self: *Lowering, node: *const Node) Ref { switch (node.data) { .identifier => |id| { - if (self.scope) |scope| { - if (scope.lookup(id.name)) |binding| { - if (binding.is_alloca) { - // If the variable IS a pointer (e.g., p: *Vec2), load it - // to get the actual pointer value for GEP/store operations - if (!binding.ty.isBuiltin()) { - const info = self.module.types.get(binding.ty); - if (info == .pointer) { - return self.builder.load(binding.ref, binding.ty); - } + const local = if (self.scope) |scope| scope.lookup(id.name) else null; + if (local) |binding| { + if (binding.is_alloca) { + // If the variable IS a pointer (e.g., p: *Vec2), load it + // to get the actual pointer value for GEP/store operations + if (!binding.ty.isBuiltin()) { + const info = self.module.types.get(binding.ty); + if (info == .pointer) { + return self.builder.load(binding.ref, binding.ty); } - return binding.ref; } + return binding.ref; } + } else if (self.program_index.global_names.get(id.name)) |gi| { + // Module-global lvalue: address into the global's live storage + // so a downstream GEP/store targets the global itself, not a + // loaded copy. A pointer-typed global is loaded first to get + // the pointer value to GEP through (mirrors the local pointer + // case above); any other global yields its storage address. + if (!gi.ty.isBuiltin() and self.module.types.get(gi.ty) == .pointer) { + return self.builder.emit(.{ .global_get = gi.id }, gi.ty); + } + return self.builder.emit(.{ .global_addr = gi.id }, self.module.types.ptrTo(gi.ty)); } }, .field_access => |fa| { @@ -2697,9 +2706,11 @@ pub const Lowering = struct { const obj_ty = self.inferExprType(ie.object); const elem_ty = self.getElementType(obj_ty); const ptr_ty = self.module.types.ptrTo(elem_ty); - // For array targets, use the alloca directly so the pointer is persistent + // For array targets, use the storage pointer (alloca for a + // local, global_addr for a module global) so the resulting + // pointer is into live storage, not a loaded copy. const is_array = !obj_ty.isBuiltin() and self.module.types.get(obj_ty) == .array; - const base = if (is_array) (self.getExprAlloca(ie.object) orelse self.lowerExpr(ie.object)) else self.lowerExpr(ie.object); + const base = if (is_array) (self.getExprAlloca(ie.object) orelse self.lowerExprAsPtr(ie.object)) else self.lowerExpr(ie.object); break :blk self.builder.emit(.{ .index_gep = .{ .lhs = base, .rhs = idx } }, ptr_ty); } // address_of(field_access) → use lowerExprAsPtr for GEP chain