lang: fix struct-field null/undef over-store (issue 0154)
Assigning null/--- to a struct field picked up a leaked enclosing target_type (the function's return type, set for the whole body), so constNull/constUndef built a whole-struct-typed value. The oversized store overran the field's slot and clobbered the saved frame pointer, so the function returned to 0x0. Surfaced building a by-value-returned struct whose array field precedes a pointer field (Scheduler.init()). Fix: add null_literal/undef_literal to the needs_target switch in lowerAssignment so the field's own type is used. Regression: examples/types/0193-types-sret-array-before-pointer.sx.
This commit is contained in:
28
examples/types/0193-types-sret-array-before-pointer.sx
Normal file
28
examples/types/0193-types-sret-array-before-pointer.sx
Normal file
@@ -0,0 +1,28 @@
|
||||
// Assigning `null` / `---` to a struct field must store a FIELD-typed value,
|
||||
// not pick up an enclosing `target_type` (e.g. the function's return type while
|
||||
// lowering its body) — which would build a whole-struct-typed null and emit an
|
||||
// oversized store that overruns the field's slot.
|
||||
//
|
||||
// Regression (issue 0154): `s.p = null` inside a struct-returning fn used to
|
||||
// store a 32-byte struct `zeroinitializer` through the pointer field's GEP,
|
||||
// clobbering the saved x29/x30 so the fn `ret`'d to 0x0. The array-before-
|
||||
// pointer order puts the pointer at a non-zero offset, pushing the over-store
|
||||
// off the end of the alloca. Fixed by adding null/undef literals to
|
||||
// `needs_target` in lowerAssignment.
|
||||
#import "modules/std.sx";
|
||||
S :: struct {
|
||||
arr: [2]u64; // fixed-array field FIRST
|
||||
p: *i64; // pointer field AFTER the array
|
||||
n: i64;
|
||||
}
|
||||
mk :: () -> S {
|
||||
s : S = ---;
|
||||
s.p = null; // must store a *i64 null, not a whole-struct null
|
||||
s.n = 0;
|
||||
return s;
|
||||
}
|
||||
main :: () -> i64 {
|
||||
s := mk();
|
||||
print("n {}\n", s.n); // n 0
|
||||
return 0;
|
||||
}
|
||||
@@ -0,0 +1 @@
|
||||
0
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
n 0
|
||||
56
issues/0154-sret-array-before-pointer-field.md
Normal file
56
issues/0154-sret-array-before-pointer-field.md
Normal file
@@ -0,0 +1,56 @@
|
||||
# issue 0154 — `null` / `---` assigned to a struct field over-stores when an enclosing `target_type` leaks
|
||||
|
||||
> **RESOLVED** (fix landed). Root cause: `null_literal` / `undef_literal` were
|
||||
> absent from the `needs_target` switch in `lowerAssignment`
|
||||
> (`src/ir/lower/stmt.zig`), so `obj.field = null` (or `= ---`) did NOT set
|
||||
> `target_type` to the field's type. While lowering a function body,
|
||||
> `target_type` is set to the function's RETURN type (`decl.zig:2691`) for the
|
||||
> whole body, so that leaked type reached `constNull`/`constUndef`
|
||||
> (`expr.zig:1788`) and built a WHOLE-STRUCT-typed null. Emit then stored a
|
||||
> struct-sized `zeroinitializer` through a GEP at the field's offset — an
|
||||
> oversized store that overran the field's slot and clobbered the saved
|
||||
> x29/x30, so the function `ret`'d to `0x0`. Field order mattered only because
|
||||
> a pointer field *after* an array field sits at a non-zero offset, pushing the
|
||||
> over-store off the end of the alloca (pointer-first kept it in-bounds → silent
|
||||
> but harmless). Manifested at `-O0` (`sx run` JIT + `sx build --opt 0`);
|
||||
> `-O2` optimized the redundant stores away, masking it.
|
||||
>
|
||||
> **Fix:** add `.null_literal, .undef_literal` to the `needs_target` switch
|
||||
> (`src/ir/lower/stmt.zig`), so the field's type is resolved via
|
||||
> `fieldLvalueResolve` and `target_type` is set to the field type — `null`
|
||||
> builds an `*i64`-typed null (correct 8-byte `store ptr null`), not a struct.
|
||||
>
|
||||
> **Regression test:** `examples/types/0193-types-sret-array-before-pointer.sx`.
|
||||
|
||||
## Symptom
|
||||
|
||||
Returning a struct by value segfaults (read at `0x0`) when a fixed-array field
|
||||
precedes a pointer field. Observed: `Segmentation fault at address 0x0`;
|
||||
expected: the struct's fields read back correctly.
|
||||
|
||||
## Reproduction
|
||||
|
||||
```sx
|
||||
#import "modules/std.sx";
|
||||
S :: struct {
|
||||
arr: [2]u64; // fixed-array field FIRST
|
||||
p: *i64; // pointer field AFTER the array
|
||||
n: i64;
|
||||
}
|
||||
mk :: () -> S {
|
||||
s : S = ---;
|
||||
s.p = null; // ← leaked return-type target_type → whole-struct null → over-store
|
||||
s.n = 0;
|
||||
return s;
|
||||
}
|
||||
main :: () -> i64 {
|
||||
s := mk();
|
||||
print("n {}\n", s.n); // expected: n 0 ; actual: Segmentation fault at 0x0
|
||||
return 0;
|
||||
}
|
||||
```
|
||||
|
||||
Trigger matrix (aarch64-macos, `-O0`): array-before-pointer + by-value return →
|
||||
segfault; pointer-before-array → OK; array with no pointer → OK; by-value param
|
||||
pass → OK; local copy → OK. ⇒ the `null`/`---` field-store value type, not the
|
||||
sret ABI.
|
||||
@@ -629,7 +629,14 @@ pub fn lowerAssignment(self: *Lowering, asgn: *const ast.Assignment) void {
|
||||
// unchanged into method-call arg slots (`resolveCallParamTypes` can't
|
||||
// override target_type per-arg).
|
||||
const needs_target = switch (asgn.value.data) {
|
||||
.enum_literal, .struct_literal, .tuple_literal, .if_expr, .match_expr, .block, .unary_op, .binary_op => true,
|
||||
// `null` / `---` (undef) carry NO type of their own — they take the
|
||||
// store slot's type from `target_type`. Without setting it to the
|
||||
// field type here, a leaked enclosing `target_type` (e.g. the
|
||||
// function's return type while lowering its body, decl.zig:2691)
|
||||
// reaches `constNull`/`constUndef` and builds a WHOLE-STRUCT-typed
|
||||
// null, emitting an oversized store that overruns the field's slot
|
||||
// and corrupts neighboring stack (issue 0154).
|
||||
.enum_literal, .struct_literal, .tuple_literal, .if_expr, .match_expr, .block, .unary_op, .binary_op, .null_literal, .undef_literal => true,
|
||||
.call => |vc| vc.callee.data == .enum_literal,
|
||||
else => false,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user