From 4537538bb2aac6b13a7c196839872ae6e0058ed2 Mon Sep 17 00:00:00 2001 From: agra Date: Wed, 3 Jun 2026 15:43:27 +0300 Subject: [PATCH] fix(ffi): replace silent `.void` arg-type fallback with loud `.unresolved` (issue 0074) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four FFI call-arg lowering sites resolved an argument's IR type via `getRefIRType(arg_ref) orelse .void` — a silent fallback to the load-bearing real type `.void`. A failed lookup there is a codegen invariant violation, but `.void` is treated by downstream `toLLVMType` → `abiCoerceParamType` → `coerceArg` as a legitimate void-typed foreign argument, corrupting the call ABI with no diagnostic. Add one shared resolver `LLVMEmitter.argIRTypeOrFail` that returns the dedicated `.unresolved` sentinel on a failed lookup — never `.void`/`.s64` — so the failure cannot masquerade as a real type and trips `toLLVMType`'s existing hard `@panic` tripwire at the call site. Route all four sites through it: - src/ir/emit_llvm.zig JNI constructor (NewObject) arg loop - src/backend/llvm/ops.zig objc_msgSend arg loop - src/backend/llvm/ops.zig JNI non-virtual call arg loop - src/backend/llvm/ops.zig JNI CallMethod arg loop Happy path is byte-identical (every real arg already has a resolved type); FFI examples stay green with zero snapshot churn. Regression test (fail-before/pass-after) in src/ir/emit_llvm.test.zig asserts an unresolvable FFI arg ref now yields `.unresolved`, not the old silent `.void`. --- issues/0074-ffi-arg-type-void-fallback.md | 84 +++++++++++++++++++++++ src/backend/llvm/ops.zig | 6 +- src/ir/emit_llvm.test.zig | 47 +++++++++++++ src/ir/emit_llvm.zig | 12 +++- 4 files changed, 145 insertions(+), 4 deletions(-) create mode 100644 issues/0074-ffi-arg-type-void-fallback.md diff --git a/issues/0074-ffi-arg-type-void-fallback.md b/issues/0074-ffi-arg-type-void-fallback.md new file mode 100644 index 0000000..e10efd9 --- /dev/null +++ b/issues/0074-ffi-arg-type-void-fallback.md @@ -0,0 +1,84 @@ +# 0074 — silent `getRefIRType(arg_ref) orelse .void` fallback in FFI call-arg lowering + +> **✅ RESOLVED.** Root cause: four FFI call-arg lowering loops resolved an +> argument's IR type via `getRefIRType(arg_ref) orelse .void` — a silent fallback +> to the load-bearing real type `.void`, which downstream `toLLVMType` → +> `abiCoerceParamType` → `coerceArg` treat as a legitimate (void-typed) foreign +> argument, corrupting the call ABI with no diagnostic. Fix: one shared resolver +> `LLVMEmitter.argIRTypeOrFail` ([src/ir/emit_llvm.zig]) returns the dedicated +> `.unresolved` sentinel on a failed lookup — never `.void`/`.s64` — so the failure +> cannot masquerade as a real type and trips `toLLVMType`'s existing hard `@panic` +> tripwire at the call site. All four sites +> ([src/ir/emit_llvm.zig] JNI constructor; [src/backend/llvm/ops.zig] objc_msgSend, +> JNI non-virtual, JNI `CallMethod`) now route through the helper. Happy path +> is byte-identical (every real arg already has a resolved type) — FFI examples stay +> green with zero snapshot churn. Regression test (fail-before/pass-after): +> `src/ir/emit_llvm.test.zig` — "argIRTypeOrFail surfaces .unresolved for an +> unresolvable FFI arg ref (issue 0074)". + +## Symptom +**One-line:** Four FFI call-arg lowering sites silently default a failed +argument-type lookup to `.void` — the forbidden silent-type-fallback anti-pattern +(`.void` as a failed-type-lookup sentinel), which would produce a void-typed +foreign-call argument (wrong LLVM param type → silent ABI corruption) with no +diagnostic if the lookup ever fails. + +**Observed:** `self.getRefIRType(arg_ref) orelse .void` at: +- `src/ir/emit_llvm.zig:2463` +- `src/backend/llvm/ops.zig:517` (Obj-C `objc_msgSend` arg loop) +- `src/backend/llvm/ops.zig:731` (JNI non-virtual call arg loop) +- `src/backend/llvm/ops.zig:761` (JNI `CallMethod` arg loop) + +Each then does `toLLVMType(raw_ty)` → `abiCoerceParamType` → `coerceArg`, so a +`.void` fallback silently mis-types the foreign-call argument. + +**Expected:** `getRefIRType` returning null for a real call argument is a +"must-succeed lookup" failure (every arg is a valid param/instruction ref). Per +`CLAUDE.md` REJECTED PATTERNS — *"`.void` is an UNACCEPTABLE sentinel for a failed +type lookup"* — the lookup failure must surface as a diagnostic / hard tripwire, not +a silently-corrupted argument type. + +## Provenance / scope +Pre-existing pattern (the `emit_llvm.zig` site is original; the three `ops.zig` sites +were relocated **verbatim, behavior-preserving** by step A7.4c of the arch-refactor — +the flow reviewer/observer correctly approved the relocations as equivalence-preserving). +Surfaced by the **A9.3 final fallback-audit** of the arch-refactor stream. Not +introduced by the refactor; filed per the IMPASSIBLE RULE (*"If you find an existing +default-return in the compiler that swallows a lookup failure, treat it as a +discovered bug — file an issue, do not just delete the default in place"*). + +## Reproduction +This is a **latent / static** finding: there is no known sx program that drives +`getRefIRType` to `null` for a valid foreign-call argument (well-formed IR always +has a type for every arg ref), so it cannot currently be triggered at runtime — which +is exactly why it is dangerous (a future IR change that breaks the invariant would +corrupt FFI ABI silently). The code paths are exercised (and must stay green after +the fix) by the existing FFI examples, e.g.: + +``` +examples/13xx-ffi-objc-* # objc_msgSend arg lowering (ops.zig:517) +examples/14xx-ffi-jni-* # JNI CallMethod / non-virtual (ops.zig:731/761) +``` + +(No new minimal repro `.sx` is meaningful for a latent defensive fallback; the fix is +verified by (a) the FFI suite staying green and (b) a unit test that asserts the new +loud-failure path, see below.) + +## Investigation prompt (ready to paste into a fresh session) +> In `/Users/agra/projects/sx`, four FFI call-arg lowering sites use the forbidden +> silent type-fallback `self.getRefIRType(arg_ref) orelse .void` +> (`src/ir/emit_llvm.zig:2463`; `src/backend/llvm/ops.zig:517`, `:731`, `:761`). +> `getRefIRType` (`src/ir/emit_llvm.zig:2229`, returns `?TypeId`) yields `null` only +> when a ref is neither a function param nor a block instruction result — a +> must-not-happen case for a real call argument. Replace the silent `.void` default +> with a loud failure that cannot be mistaken for a real type, per `CLAUDE.md` +> REJECTED PATTERNS: emit a diagnostic via `self.diagnostics.addFmt(.err, span, +> "...", .{...})` and/or a hard tripwire (`@panic`/`bailDetail`-style) naming the op +> and the bad ref — do NOT substitute another real type. Prefer a single shared +> helper (e.g. `argIRTypeOrFail(arg_ref, span)`) used by all four sites so the policy +> lives in one place. Then: (1) `/Users/agra/.zvm/bin/zig build && /Users/agra/.zvm/bin/zig +> build test && bash tests/run_examples.sh` must stay 361/0 with the FFI examples +> green (the happy path is unchanged); (2) add a `*.test.zig` unit test that +> constructs an FFI call op with a bogus arg ref and asserts the loud failure fires +> (not a `.void` silent default). Expected new behavior: an unresolved FFI arg type +> produces a clear compiler error / panic, never a void-typed foreign argument. diff --git a/src/backend/llvm/ops.zig b/src/backend/llvm/ops.zig index e34bbb1..e2f529e 100644 --- a/src/backend/llvm/ops.zig +++ b/src/backend/llvm/ops.zig @@ -514,7 +514,7 @@ pub const Ops = struct { // coercion applied so structs / strings decay the // same way they do for any C foreign call. for (msg.args, 0..) |arg_ref, i| { - const raw_ty = self.e.getRefIRType(arg_ref) orelse .void; + const raw_ty = self.e.argIRTypeOrFail(arg_ref); const raw_llvm = self.e.toLLVMType(raw_ty); const coerced_ty = self.e.abiCoerceParamType(raw_ty, raw_llvm); param_types[i + 2 + sret_off] = coerced_ty; @@ -728,7 +728,7 @@ pub const Ops = struct { call_args_nv[2] = cls; call_args_nv[3] = mid_val; for (msg.args, 0..) |arg_ref, i| { - const raw_ty = self.e.getRefIRType(arg_ref) orelse .void; + const raw_ty = self.e.argIRTypeOrFail(arg_ref); const raw_llvm = self.e.toLLVMType(raw_ty); const coerced_ty = self.e.abiCoerceParamType(raw_ty, raw_llvm); call_param_types_nv[i + 4] = coerced_ty; @@ -758,7 +758,7 @@ pub const Ops = struct { call_args[1] = target; call_args[2] = mid; for (msg.args, 0..) |arg_ref, i| { - const raw_ty = self.e.getRefIRType(arg_ref) orelse .void; + const raw_ty = self.e.argIRTypeOrFail(arg_ref); const raw_llvm = self.e.toLLVMType(raw_ty); const coerced_ty = self.e.abiCoerceParamType(raw_ty, raw_llvm); call_param_types[i + 3] = coerced_ty; diff --git a/src/ir/emit_llvm.test.zig b/src/ir/emit_llvm.test.zig index 8d9426f..ff20cda 100644 --- a/src/ir/emit_llvm.test.zig +++ b/src/ir/emit_llvm.test.zig @@ -1097,3 +1097,50 @@ test "emit: ERR E3.0 — no DWARF without a debug context (unit-test default)" { try std.testing.expect(std.mem.indexOf(u8, ir_str, "DICompileUnit") == null); try std.testing.expect(std.mem.indexOf(u8, ir_str, "!dbg") == null); } + +// ── issue 0074: FFI arg-type lookup must fail loudly, never silently `.void` ── +// `argIRTypeOrFail` backs the four FFI call-arg lowering sites (objc_msgSend, +// JNI CallMethod / non-virtual / constructor). A ref it cannot resolve is +// a codegen invariant violation; it must surface the dedicated `.unresolved` +// tripwire sentinel (which `toLLVMType` hard-panics on) rather than the old +// silent `.void` default that would emit a void-typed foreign-call argument. +test "emit: argIRTypeOrFail surfaces .unresolved for an unresolvable FFI arg ref (issue 0074)" { + const alloc = std.testing.allocator; + var module = Module.init(alloc); + defer module.deinit(); + + var b = Builder.init(&module); + + // func ffifn(a: s64, b: f64) -> void { } + const fid = b.beginFunction(str(&module, "ffifn"), &[_]Function.Param{ + .{ .name = str(&module, "a"), .ty = .s64 }, + .{ .name = str(&module, "b"), .ty = .f64 }, + }, .void); + const entry = b.appendBlock(str(&module, "entry"), &.{}); + b.switchToBlock(entry); + b.retVoid(); + b.finalize(); + + var emitter = LLVMEmitter.init(alloc, &module, "test_ffi_argty", .{}); + defer emitter.deinit(); + emitter.current_func_idx = fid.index(); + + // Happy path: a real arg ref (param 0 / param 1) resolves byte-identically + // to its declared IR type — the FFI fast path is unchanged. + try std.testing.expectEqual(TypeId.s64, emitter.argIRTypeOrFail(Ref.fromIndex(0))); + try std.testing.expectEqual(TypeId.f64, emitter.argIRTypeOrFail(Ref.fromIndex(1))); + + // A ref past every param and instruction is unresolvable. + const bogus = Ref.fromIndex(100_000); + try std.testing.expectEqual(@as(?TypeId, null), emitter.getRefIRType(bogus)); + + // Fail-before: the old `getRefIRType(arg) orelse .void` would silently + // yield `.void` here — a real, load-bearing type that downstream ABI + // coercion treats as a legitimate (void-typed) foreign argument. + try std.testing.expectEqual(TypeId.void, emitter.getRefIRType(bogus) orelse TypeId.void); + + // Pass-after: the helper returns the dedicated `.unresolved` sentinel, + // never `.void`, so the failure cannot masquerade as a real type. + try std.testing.expectEqual(TypeId.unresolved, emitter.argIRTypeOrFail(bogus)); + try std.testing.expect(emitter.argIRTypeOrFail(bogus) != .void); +} diff --git a/src/ir/emit_llvm.zig b/src/ir/emit_llvm.zig index 78853d2..0506f10 100644 --- a/src/ir/emit_llvm.zig +++ b/src/ir/emit_llvm.zig @@ -2239,6 +2239,16 @@ pub const LLVMEmitter = struct { return null; } + /// Resolve the IR type of a foreign-call argument ref. Every FFI arg ref is + /// a real function param or block instruction result, so a `null` here is a + /// codegen invariant violation, not a recoverable case: return the dedicated + /// `.unresolved` sentinel — never `.void`/`.s64` — so the failure cannot be + /// mistaken for a real type and trips `toLLVMType`'s hard tripwire at the call + /// site instead of silently emitting a void-typed foreign argument. + pub fn argIRTypeOrFail(self: *LLVMEmitter, arg_ref: Ref) TypeId { + return self.getRefIRType(arg_ref) orelse .unresolved; + } + /// Coerce both binary operands to match the instruction's result type. /// E.g. if result is i64 but one operand is i32, sext it. @@ -2460,7 +2470,7 @@ pub const LLVMEmitter = struct { call_args[1] = cls; call_args[2] = mid; for (msg.args, 0..) |arg_ref, i| { - const raw_ty = self.getRefIRType(arg_ref) orelse .void; + const raw_ty = self.argIRTypeOrFail(arg_ref); const raw_llvm = self.toLLVMType(raw_ty); const coerced_ty = self.abiCoerceParamType(raw_ty, raw_llvm); call_param_types[i + 3] = coerced_ty;