From 87448654256d334e1d5877736a9747de67b7eedf Mon Sep 17 00:00:00 2001 From: mlugg Date: Sun, 14 Sep 2025 11:10:46 +0100 Subject: [PATCH] frontend: fix reference tracking through coerced function bodies This bug was manifesting for user as a nasty link error because they were calling their application's main entry point as a coerced function, which essentially broke reference tracking for the entire ZCU, causing exported symbols to silently not get exported. I've been a little unsure about how coerced functions should interact with the unit graph before, but the solution is actually really obvious now: they shouldn't! `Sema` is now responsible for unwrapping possibly-coerced functions *before* queuing analysis or marking unit references. This makes the reference graph optimal (there are no redundant edges representing coerced versions of the same function) and simplifies logic elsewhere at the expense of just a few lines in Sema. --- src/Sema.zig | 56 ++++++++++++------- src/Zcu.zig | 3 + src/Zcu/PerThread.zig | 10 ++-- test/cases/export_from_body_of_coerced_fn.zig | 19 +++++++ 4 files changed, 64 insertions(+), 24 deletions(-) create mode 100644 test/cases/export_from_body_of_coerced_fn.zig diff --git a/src/Sema.zig b/src/Sema.zig index 51b93fb30830576350000e249507a2aad2804e33..385e1ae74a84dbaefd7a9a1a0e245e72e2379bb0 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -4376,8 +4376,9 @@ fn zirResolveInferredAlloc(sema: *Sema, block: *Block, inst: Zir.Inst.Index) Com if (zcu.intern_pool.isFuncBody(val)) { const ty: Type = .fromInterned(zcu.intern_pool.typeOf(val)); if (try ty.fnHasRuntimeBitsSema(pt)) { - try sema.addReferenceEntry(block, src, AnalUnit.wrap(.{ .func = val })); - try zcu.ensureFuncBodyAnalysisQueued(val); + const orig_fn_index = zcu.intern_pool.unwrapCoercedFunc(val); + try sema.addReferenceEntry(block, src, .wrap(.{ .func = orig_fn_index })); + try zcu.ensureFuncBodyAnalysisQueued(orig_fn_index); } } @@ -5589,16 +5590,21 @@ fn zirPanic(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void } try sema.ensureMemoizedStateResolved(src, .panic); - try zcu.ensureFuncBodyAnalysisQueued(zcu.builtin_decl_values.get(.@"panic.call")); - - const panic_fn = Air.internedToRef(zcu.builtin_decl_values.get(.@"panic.call")); - + const panic_fn_index = zcu.builtin_decl_values.get(.@"panic.call"); const opt_usize_ty = try pt.optionalType(.usize_type); const null_ret_addr = Air.internedToRef((try pt.intern(.{ .opt = .{ .ty = opt_usize_ty.toIntern(), .val = .none, } }))); - try sema.callBuiltin(block, src, panic_fn, .auto, &.{ coerced_msg, null_ret_addr }, .@"@panic"); + // `callBuiltin` also calls `addReferenceEntry` to the function body for us. + try sema.callBuiltin( + block, + src, + .fromIntern(panic_fn_index), + .auto, + &.{ coerced_msg, null_ret_addr }, + .@"@panic", + ); } fn zirTrap(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void { @@ -7567,8 +7573,9 @@ fn analyzeCall( ref_func: { const runtime_func_val = try sema.resolveValue(runtime_func) orelse break :ref_func; if (!ip.isFuncBody(runtime_func_val.toIntern())) break :ref_func; - try sema.addReferenceEntry(block, call_src, .wrap(.{ .func = runtime_func_val.toIntern() })); - try zcu.ensureFuncBodyAnalysisQueued(runtime_func_val.toIntern()); + const orig_fn_index = ip.unwrapCoercedFunc(runtime_func_val.toIntern()); + try sema.addReferenceEntry(block, call_src, .wrap(.{ .func = orig_fn_index })); + try zcu.ensureFuncBodyAnalysisQueued(orig_fn_index); } const call_tag: Air.Inst.Tag = switch (modifier) { @@ -26383,23 +26390,27 @@ fn explainWhyTypeIsNotPacked( /// instructions. This function ensures the panic function will be available to /// be called during that time. fn preparePanicId(sema: *Sema, src: LazySrcLoc, panic_id: Zcu.SimplePanicId) !void { + const zcu = sema.pt.zcu; + // If the backend doesn't support `.panic_fn`, it doesn't want us to lower the panic handlers. // The backend will transform panics into traps instead. - if (sema.pt.zcu.backendSupportsFeature(.panic_fn)) { - _ = try sema.getPanicIdFunc(src, panic_id); - } + if (!zcu.backendSupportsFeature(.panic_fn)) return; + + const fn_index = try sema.getPanicIdFunc(src, panic_id); + const orig_fn_index = zcu.intern_pool.unwrapCoercedFunc(fn_index); + try sema.addReferenceEntry(null, src, .wrap(.{ .func = orig_fn_index })); + try zcu.ensureFuncBodyAnalysisQueued(orig_fn_index); } fn getPanicIdFunc(sema: *Sema, src: LazySrcLoc, panic_id: Zcu.SimplePanicId) !InternPool.Index { const zcu = sema.pt.zcu; try sema.ensureMemoizedStateResolved(src, .panic); - const panic_func = zcu.builtin_decl_values.get(panic_id.toBuiltin()); - try zcu.ensureFuncBodyAnalysisQueued(panic_func); + const panic_fn_index = zcu.builtin_decl_values.get(panic_id.toBuiltin()); switch (sema.owner.unwrap()) { .@"comptime", .nav_ty, .nav_val, .type, .memoized_state => {}, .func => |owner_func| zcu.intern_pool.funcSetHasErrorTrace(owner_func, true), } - return panic_func; + return panic_fn_index; } fn addSafetyCheck( @@ -31143,6 +31154,11 @@ fn addReferenceEntry( referenced_unit: AnalUnit, ) !void { const zcu = sema.pt.zcu; + const ip = &zcu.intern_pool; + switch (referenced_unit.unwrap()) { + .func => |f| assert(ip.unwrapCoercedFunc(f) == f), // for `.{ .func = f }`, `f` must be uncoerced + else => {}, + } if (!zcu.comp.incremental and zcu.comp.reference_trace == 0) return; const gop = try sema.references.getOrPut(sema.gpa, referenced_unit); if (gop.found_existing) return; @@ -31329,8 +31345,9 @@ fn maybeQueueFuncBodyAnalysis(sema: *Sema, block: *Block, src: LazySrcLoc, nav_i const nav_val = zcu.navValue(nav_index); if (!ip.isFuncBody(nav_val.toIntern())) return; - try sema.addReferenceEntry(block, src, AnalUnit.wrap(.{ .func = nav_val.toIntern() })); - try zcu.ensureFuncBodyAnalysisQueued(nav_val.toIntern()); + const orig_fn_index = ip.unwrapCoercedFunc(nav_val.toIntern()); + try sema.addReferenceEntry(block, src, .wrap(.{ .func = orig_fn_index })); + try zcu.ensureFuncBodyAnalysisQueued(orig_fn_index); } fn analyzeRef( @@ -34963,8 +34980,9 @@ fn resolveInferredErrorSet( } // In this case we are dealing with the actual InferredErrorSet object that // corresponds to the function, not one created to track an inline/comptime call. - try sema.addReferenceEntry(block, src, AnalUnit.wrap(.{ .func = func_index })); - try pt.ensureFuncBodyUpToDate(func_index); + const orig_func_index = ip.unwrapCoercedFunc(func_index); + try sema.addReferenceEntry(block, src, .wrap(.{ .func = orig_func_index })); + try pt.ensureFuncBodyUpToDate(orig_func_index); } // This will now have been resolved by the logic at the end of `Zcu.analyzeFnBody` diff --git a/src/Zcu.zig b/src/Zcu.zig index d86f422b981d48eccb4d2c9ba36e21c611da1972..706bdb51655409a6e2c23e36d580d0c81c01f350 100644 --- a/src/Zcu.zig +++ b/src/Zcu.zig @@ -3451,8 +3451,11 @@ pub fn mapOldZirToNew( /// will be analyzed when it returns: for that, see `ensureFuncBodyAnalyzed`. pub fn ensureFuncBodyAnalysisQueued(zcu: *Zcu, func_index: InternPool.Index) !void { const ip = &zcu.intern_pool; + const func = zcu.funcInfo(func_index); + assert(func.ty == func.uncoerced_ty); // analyze the body of the original function, not a coerced one + if (zcu.func_body_analysis_queued.contains(func_index)) return; if (func.analysisUnordered(ip).is_analyzed) { diff --git a/src/Zcu/PerThread.zig b/src/Zcu/PerThread.zig index 3c2bb27dab716576098a6f7aa24b48af11205b67..684b3a8658fef4b0b79e2d56c8483af83a1dee3f 100644 --- a/src/Zcu/PerThread.zig +++ b/src/Zcu/PerThread.zig @@ -1571,7 +1571,7 @@ fn analyzeNavType(pt: Zcu.PerThread, nav_id: InternPool.Nav.Index) Zcu.CompileEr return .{ .type_changed = true }; } -pub fn ensureFuncBodyUpToDate(pt: Zcu.PerThread, maybe_coerced_func_index: InternPool.Index) Zcu.SemaError!void { +pub fn ensureFuncBodyUpToDate(pt: Zcu.PerThread, func_index: InternPool.Index) Zcu.SemaError!void { dev.check(.sema); const tracy = trace(@src()); @@ -1581,15 +1581,15 @@ pub fn ensureFuncBodyUpToDate(pt: Zcu.PerThread, maybe_coerced_func_index: Inter const gpa = zcu.gpa; const ip = &zcu.intern_pool; - _ = zcu.func_body_analysis_queued.swapRemove(maybe_coerced_func_index); + _ = zcu.func_body_analysis_queued.swapRemove(func_index); - // We only care about the uncoerced function. - const func_index = ip.unwrapCoercedFunc(maybe_coerced_func_index); const anal_unit: AnalUnit = .wrap(.{ .func = func_index }); log.debug("ensureFuncBodyUpToDate {f}", .{zcu.fmtAnalUnit(anal_unit)}); - const func = zcu.funcInfo(maybe_coerced_func_index); + const func = zcu.funcInfo(func_index); + + assert(func.ty == func.uncoerced_ty); // analyze the body of the original function, not a coerced one const was_outdated = zcu.outdated.swapRemove(anal_unit) or zcu.potentially_outdated.swapRemove(anal_unit); diff --git a/test/cases/export_from_body_of_coerced_fn.zig b/test/cases/export_from_body_of_coerced_fn.zig new file mode 100644 index 0000000000000000000000000000000000000000..541cd0c86f97601a1e0c8a1920077017502bd9ed --- /dev/null +++ b/test/cases/export_from_body_of_coerced_fn.zig @@ -0,0 +1,19 @@ +fn original() usize { + _ = struct { + export const val: u32 = 123; + }; + return 0; +} + +pub fn main() void { + const coerced: fn () u64 = original; + _ = coerced(); + + const S = struct { + extern const val: u32; + }; + if (S.val != 123) @panic("wrong value"); +} + +// run +// target=x86_64-linux -- 2.54.0