From 8144a88a212842e1076d70807c439680cecf73cc Mon Sep 17 00:00:00 2001 From: agra Date: Sat, 20 Jun 2026 09:26:53 +0300 Subject: [PATCH] atomics A.0c: harden guards (scalar-kind, ordering validity, align bail) Adversarial review of A.0 found two silent-wrong defects reachable via the public atomic_load/atomic_store intrinsics (raw LLVM verifier errors, not clean sx diagnostics) + a latent alignment fallback. All fixed: - scalar-kind allowlist (call.zig): the size-only T guard admitted same-sized aggregates ([8]u8, 8-byte structs) -> invalid 'load atomic [8 x i8]'. Now an allowlist switch (integer/float/bool/pointer/enum/vector) rejects loudly. - per-op ordering validity (call.zig): load cannot release/acq_rel, store cannot acquire/acq_rel -> loud diagnostic instead of invalid LLVM. - val_ty align fallback (ops.zig): the 'else .i64' (align 8) default would over-align a sub-8 store -> now bails loudly on a missing val_ty. Locked by examples 1130 (non-scalar) + 1131 (bad ordering). Suite green (713/0). --- current/CHECKPOINT-ATOMICS.md | 25 ++++++++++++++++++- examples/1130-diagnostics-atomic-nonscalar.sx | 9 +++++++ .../1131-diagnostics-atomic-bad-ordering.sx | 9 +++++++ .../1130-diagnostics-atomic-nonscalar.exit | 1 + .../1130-diagnostics-atomic-nonscalar.stderr | 5 ++++ .../1130-diagnostics-atomic-nonscalar.stdout | 1 + .../1131-diagnostics-atomic-bad-ordering.exit | 1 + ...131-diagnostics-atomic-bad-ordering.stderr | 5 ++++ ...131-diagnostics-atomic-bad-ordering.stdout | 1 + src/backend/llvm/ops.zig | 13 ++++++++-- src/ir/lower/call.zig | 23 +++++++++++++++-- 11 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 examples/1130-diagnostics-atomic-nonscalar.sx create mode 100644 examples/1131-diagnostics-atomic-bad-ordering.sx create mode 100644 examples/expected/1130-diagnostics-atomic-nonscalar.exit create mode 100644 examples/expected/1130-diagnostics-atomic-nonscalar.stderr create mode 100644 examples/expected/1130-diagnostics-atomic-nonscalar.stdout create mode 100644 examples/expected/1131-diagnostics-atomic-bad-ordering.exit create mode 100644 examples/expected/1131-diagnostics-atomic-bad-ordering.stderr create mode 100644 examples/expected/1131-diagnostics-atomic-bad-ordering.stdout diff --git a/current/CHECKPOINT-ATOMICS.md b/current/CHECKPOINT-ATOMICS.md index 4bc185ef..c7955a17 100644 --- a/current/CHECKPOINT-ATOMICS.md +++ b/current/CHECKPOINT-ATOMICS.md @@ -4,7 +4,27 @@ Companion to [PLAN-ATOMICS.md](PLAN-ATOMICS.md). Update after every step (one st time, per the cadence rule). New corpus category: `17xx`. ## Last completed step -**A.0b (green) — DONE.** Real atomic load/store emission: `LLVMBuildLoad2`/`LLVMBuildStore` +**A.0c (guard hardening) — DONE.** Adversarial review of A.0 found two CRITICAL silent-wrong +defects (raw LLVM verifier errors leaking via the public intrinsics) + a latent align +fallback; all fixed: +- **Scalar-kind guard** (call.zig): the size-only check admitted same-sized aggregates + (`[8]u8`, 8-byte struct) → invalid `load atomic [8 x i8]`. Now an allowlist switch + (integer/float/bool/pointer/enum/vector) rejects non-scalars loudly. +- **Per-op ordering validity** (call.zig): a load can't `release`/`acq_rel`, a store can't + `acquire`/`acq_rel` (LLVM rejects). Now a loud diagnostic, not invalid IR. +- **`val_ty` align fallback** (ops.zig): the `else .i64` (align-8) default would over-align a + sub-8 store → now bails loudly if `val_ty` is missing (a compiler bug, not a guess). +- Locked by `examples/1130-diagnostics-atomic-nonscalar.sx` + + `1131-diagnostics-atomic-bad-ordering.sx`. Suite green (713, 0 failed). + +**Capability landed (worker, commit 3c4305f):** comptime enum value params — `$o: Ordering` +now binds the variant tag and resolves in the body; during LOWERING the tag is readable via +`self.comptimeIntNamed("")` → `?i64`. This unblocks A.0.5 (full ordering surface). +Worker limitation: `.tagged_union` constraints were skipped — user wants this lifted (any +value should be comptime-able); separate generalization worker spawned. + +### Earlier — A.0b (green) +Real atomic load/store emission: `LLVMBuildLoad2`/`LLVMBuildStore` + `LLVMSetOrdering` + mandatory `LLVMSetAlignment`, ordering via an explicit sx-tag→`LLVMAtomicOrdering` switch (`llvmOrdering`). `examples/1700` green (7/42/43); IR shows `load atomic i64, ptr … seq_cst, align 8` + `store atomic …`. Added unit test @@ -75,3 +95,6 @@ Full atomic load/store plumbing with LLVM emission deliberately bailing loudly; - **A.0b** — real atomic load/store emission (LLVMBuildLoad2/Store + SetOrdering + SetAlignment; explicit sx→LLVM ordering switch). 1700 green (7/42/43, `load atomic … seq_cst, align 8`). Unit test added. Suite green (710 + units). +- **A.0c** — guard hardening from the adversarial review: scalar-kind allowlist + per-op + ordering validity (call.zig), val_ty align bail (ops.zig), + diagnostic examples + 1130/1131. Suite green (713/0). (comptime enum value params landed via worker 3c4305f.) diff --git a/examples/1130-diagnostics-atomic-nonscalar.sx b/examples/1130-diagnostics-atomic-nonscalar.sx new file mode 100644 index 00000000..985dc70f --- /dev/null +++ b/examples/1130-diagnostics-atomic-nonscalar.sx @@ -0,0 +1,9 @@ +// An atomic op on a non-scalar type is rejected with a clean diagnostic +// (not a raw LLVM verifier error). Stream A (atomics) guard. +#import "modules/std.sx"; +#import "modules/std/atomic.sx"; + +main :: () { + arr : [8]u8 = .[0, 0, 0, 0, 0, 0, 0, 0]; + x := atomic_load([8]u8, @arr, .seq_cst); +} diff --git a/examples/1131-diagnostics-atomic-bad-ordering.sx b/examples/1131-diagnostics-atomic-bad-ordering.sx new file mode 100644 index 00000000..138e3280 --- /dev/null +++ b/examples/1131-diagnostics-atomic-bad-ordering.sx @@ -0,0 +1,9 @@ +// An atomic load with an ordering LLVM forbids (.release / .acq_rel) is +// rejected with a clean diagnostic. Stream A (atomics) guard. +#import "modules/std.sx"; +#import "modules/std/atomic.sx"; + +main :: () { + n : i64 = 0; + x := atomic_load(i64, @n, .release); +} diff --git a/examples/expected/1130-diagnostics-atomic-nonscalar.exit b/examples/expected/1130-diagnostics-atomic-nonscalar.exit new file mode 100644 index 00000000..d00491fd --- /dev/null +++ b/examples/expected/1130-diagnostics-atomic-nonscalar.exit @@ -0,0 +1 @@ +1 diff --git a/examples/expected/1130-diagnostics-atomic-nonscalar.stderr b/examples/expected/1130-diagnostics-atomic-nonscalar.stderr new file mode 100644 index 00000000..1cfdf6a5 --- /dev/null +++ b/examples/expected/1130-diagnostics-atomic-nonscalar.stderr @@ -0,0 +1,5 @@ +error: atomic ops require a scalar type (integer/float/bool/pointer/enum/vector) of size 1/2/4/8/16 bytes — '[8]u8' is not eligible + --> examples/1130-diagnostics-atomic-nonscalar.sx:8:22 + | + 8 | x := atomic_load([8]u8, @arr, .seq_cst); + | ^^^^^ diff --git a/examples/expected/1130-diagnostics-atomic-nonscalar.stdout b/examples/expected/1130-diagnostics-atomic-nonscalar.stdout new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/examples/expected/1130-diagnostics-atomic-nonscalar.stdout @@ -0,0 +1 @@ + diff --git a/examples/expected/1131-diagnostics-atomic-bad-ordering.exit b/examples/expected/1131-diagnostics-atomic-bad-ordering.exit new file mode 100644 index 00000000..d00491fd --- /dev/null +++ b/examples/expected/1131-diagnostics-atomic-bad-ordering.exit @@ -0,0 +1 @@ +1 diff --git a/examples/expected/1131-diagnostics-atomic-bad-ordering.stderr b/examples/expected/1131-diagnostics-atomic-bad-ordering.stderr new file mode 100644 index 00000000..7b07b31b --- /dev/null +++ b/examples/expected/1131-diagnostics-atomic-bad-ordering.stderr @@ -0,0 +1,5 @@ +error: atomic load ordering cannot be .release or .acq_rel (use .relaxed / .acquire / .seq_cst) + --> examples/1131-diagnostics-atomic-bad-ordering.sx:8:31 + | + 8 | x := atomic_load(i64, @n, .release); + | ^^^^^^^^ diff --git a/examples/expected/1131-diagnostics-atomic-bad-ordering.stdout b/examples/expected/1131-diagnostics-atomic-bad-ordering.stdout new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/examples/expected/1131-diagnostics-atomic-bad-ordering.stdout @@ -0,0 +1 @@ + diff --git a/src/backend/llvm/ops.zig b/src/backend/llvm/ops.zig index 32c5e606..bea3cc0f 100644 --- a/src/backend/llvm/ops.zig +++ b/src/backend/llvm/ops.zig @@ -410,10 +410,19 @@ pub const Ops = struct { }; if (target_ty) |tt| val = self.e.coerceArg(val, tt); } + // Alignment MUST come from the actual stored type — never a fixed + // fallback (an `.i64`/align-8 default silently over-aligns a sub-8 + // store, which the verifier rejects). Lowering always sets val_ty; a + // missing one is a compiler bug, so bail loudly rather than guess. + if (a.val_ty == .void) { + std.debug.print("error: atomic store missing val_ty (cannot derive alignment)\n", .{}); + self.e.comptime_failed = true; + self.e.advanceRefCounter(); + return; + } const st = c.LLVMBuildStore(self.e.builder, val, ptr); c.LLVMSetOrdering(st, llvmOrdering(a.ordering)); - const align_ty = if (a.val_ty != .void) a.val_ty else .i64; - c.LLVMSetAlignment(st, @intCast(self.e.ir_mod.types.typeSizeBytes(align_ty))); + c.LLVMSetAlignment(st, @intCast(self.e.ir_mod.types.typeSizeBytes(a.val_ty))); } self.e.advanceRefCounter(); } diff --git a/src/ir/lower/call.zig b/src/ir/lower/call.zig index 0d495aea..40bae749 100644 --- a/src/ir/lower/call.zig +++ b/src/ir/lower/call.zig @@ -1705,9 +1705,18 @@ pub fn tryLowerAtomicIntrinsic(self: *Lowering, name: []const u8, c: *const ast. } const elem_ty = self.resolveTypeArg(c.args[0]); + // Atomic-eligible T = a SCALAR that LLVM can load/store atomically: integer, + // float, bool, pointer, enum (integer-backed), or vector. Aggregates + // (struct/array/slice/string/tuple/…) are rejected LOUDLY here — without the + // kind check a same-sized aggregate (`[8]u8`, an 8-byte struct) slips through + // and the user gets a raw LLVM verifier error instead of a clean diagnostic. + const scalar_ok = switch (self.module.types.get(elem_ty)) { + .signed, .unsigned, .f32, .f64, .bool, .pointer, .many_pointer, .cstring, .@"enum", .vector => true, + else => false, + }; const size = self.typeSizeBytes(elem_ty); - if (size != 1 and size != 2 and size != 4 and size != 8 and size != 16) { - if (self.diagnostics) |d| d.addFmt(.err, c.args[0].span, "atomic ops require a scalar type of size 1/2/4/8/16 bytes — '{s}' is {d} bytes", .{ self.formatTypeName(elem_ty), size }); + if (!scalar_ok or (size != 1 and size != 2 and size != 4 and size != 8 and size != 16)) { + if (self.diagnostics) |d| d.addFmt(.err, c.args[0].span, "atomic ops require a scalar type (integer/float/bool/pointer/enum/vector) of size 1/2/4/8/16 bytes — '{s}' is not eligible", .{self.formatTypeName(elem_ty)}); return Ref.none; } @@ -1716,6 +1725,16 @@ pub fn tryLowerAtomicIntrinsic(self: *Lowering, name: []const u8, c: *const ast. if (self.diagnostics) |d| d.addFmt(.err, ord_node.span, "atomic ordering must be a constant ordering literal (.relaxed / .acquire / .release / .acq_rel / .seq_cst)", .{}); return Ref.none; }; + // Per-op ordering validity (LLVM rejects these). A load can't release; a + // store can't acquire; neither can acq_rel. Loud diagnostic, not invalid IR. + if (is_load and (ordering == .release or ordering == .acq_rel)) { + if (self.diagnostics) |d| d.addFmt(.err, ord_node.span, "atomic load ordering cannot be .release or .acq_rel (use .relaxed / .acquire / .seq_cst)", .{}); + return Ref.none; + } + if (is_store and (ordering == .acquire or ordering == .acq_rel)) { + if (self.diagnostics) |d| d.addFmt(.err, ord_node.span, "atomic store ordering cannot be .acquire or .acq_rel (use .relaxed / .release / .seq_cst)", .{}); + return Ref.none; + } const ptr = self.lowerExpr(c.args[1]); if (is_load) {