fix(ir): store to module-global array element targets live storage (issue 0079)
A store to a module-global array element (`g[i] = v`) was silently dropped: a subsequent `g[i]` read the array's initializer, not `v`. Constant index, variable index, and cross-function stores were all affected, in both `sx run` and `sx build`. Global scalars and local arrays were fine. Root cause: `Lowering.lowerExprAsPtr` (the lvalue/address path) handled only local identifiers. A module-global identifier fell through to the value fallback `lowerExpr`, which emits `global_get` — loading the whole array by value. The LLVM backend's `emitIndexGep` then allocas a throwaway temp, copies the value in, and GEPs into the temp, so the store wrote a discarded copy. Fix: teach `lowerExprAsPtr`'s identifier arm about globals — emit `global_addr` (a pointer into the global's live storage), or `global_get` for a pointer-typed global (mirroring the local pointer case). Route the `address_of(index_expr)` array base through `lowerExprAsPtr` too so `&g[i]` is likewise an lvalue into the global. `index_gep` now GEPs directly into the global for const and variable index, across functions. This also fixes global struct field stores, which shared the same root cause. Regression: examples/0136-types-global-array-element-store.sx (const-index, var-index, cross-function store on a scalar global array; struct-element array for stride; nested-array global for the recursive lvalue). Fails on the pre-fix compiler, passes after.
This commit is contained in:
57
examples/0136-types-global-array-element-store.sx
Normal file
57
examples/0136-types-global-array-element-store.sx
Normal file
@@ -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");
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1 @@
|
||||
0
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
@@ -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
|
||||
102
issues/0079-global-array-element-store-dropped.md
Normal file
102
issues/0079-global-array-element-store-dropped.md
Normal file
@@ -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 <file>` → 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.
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user