Skip to content

Commit 8744865

Browse files
committed
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.
1 parent 377a8b2 commit 8744865

File tree

4 files changed

+64
-24
lines changed

4 files changed

+64
-24
lines changed

src/Sema.zig

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4376,8 +4376,9 @@ fn zirResolveInferredAlloc(sema: *Sema, block: *Block, inst: Zir.Inst.Index) Com
43764376
if (zcu.intern_pool.isFuncBody(val)) {
43774377
const ty: Type = .fromInterned(zcu.intern_pool.typeOf(val));
43784378
if (try ty.fnHasRuntimeBitsSema(pt)) {
4379-
try sema.addReferenceEntry(block, src, AnalUnit.wrap(.{ .func = val }));
4380-
try zcu.ensureFuncBodyAnalysisQueued(val);
4379+
const orig_fn_index = zcu.intern_pool.unwrapCoercedFunc(val);
4380+
try sema.addReferenceEntry(block, src, .wrap(.{ .func = orig_fn_index }));
4381+
try zcu.ensureFuncBodyAnalysisQueued(orig_fn_index);
43814382
}
43824383
}
43834384

@@ -5589,16 +5590,21 @@ fn zirPanic(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void
55895590
}
55905591

55915592
try sema.ensureMemoizedStateResolved(src, .panic);
5592-
try zcu.ensureFuncBodyAnalysisQueued(zcu.builtin_decl_values.get(.@"panic.call"));
5593-
5594-
const panic_fn = Air.internedToRef(zcu.builtin_decl_values.get(.@"panic.call"));
5595-
5593+
const panic_fn_index = zcu.builtin_decl_values.get(.@"panic.call");
55965594
const opt_usize_ty = try pt.optionalType(.usize_type);
55975595
const null_ret_addr = Air.internedToRef((try pt.intern(.{ .opt = .{
55985596
.ty = opt_usize_ty.toIntern(),
55995597
.val = .none,
56005598
} })));
5601-
try sema.callBuiltin(block, src, panic_fn, .auto, &.{ coerced_msg, null_ret_addr }, .@"@panic");
5599+
// `callBuiltin` also calls `addReferenceEntry` to the function body for us.
5600+
try sema.callBuiltin(
5601+
block,
5602+
src,
5603+
.fromIntern(panic_fn_index),
5604+
.auto,
5605+
&.{ coerced_msg, null_ret_addr },
5606+
.@"@panic",
5607+
);
56025608
}
56035609

56045610
fn zirTrap(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void {
@@ -7567,8 +7573,9 @@ fn analyzeCall(
75677573
ref_func: {
75687574
const runtime_func_val = try sema.resolveValue(runtime_func) orelse break :ref_func;
75697575
if (!ip.isFuncBody(runtime_func_val.toIntern())) break :ref_func;
7570-
try sema.addReferenceEntry(block, call_src, .wrap(.{ .func = runtime_func_val.toIntern() }));
7571-
try zcu.ensureFuncBodyAnalysisQueued(runtime_func_val.toIntern());
7576+
const orig_fn_index = ip.unwrapCoercedFunc(runtime_func_val.toIntern());
7577+
try sema.addReferenceEntry(block, call_src, .wrap(.{ .func = orig_fn_index }));
7578+
try zcu.ensureFuncBodyAnalysisQueued(orig_fn_index);
75727579
}
75737580

75747581
const call_tag: Air.Inst.Tag = switch (modifier) {
@@ -26383,23 +26390,27 @@ fn explainWhyTypeIsNotPacked(
2638326390
/// instructions. This function ensures the panic function will be available to
2638426391
/// be called during that time.
2638526392
fn preparePanicId(sema: *Sema, src: LazySrcLoc, panic_id: Zcu.SimplePanicId) !void {
26393+
const zcu = sema.pt.zcu;
26394+
2638626395
// If the backend doesn't support `.panic_fn`, it doesn't want us to lower the panic handlers.
2638726396
// The backend will transform panics into traps instead.
26388-
if (sema.pt.zcu.backendSupportsFeature(.panic_fn)) {
26389-
_ = try sema.getPanicIdFunc(src, panic_id);
26390-
}
26397+
if (!zcu.backendSupportsFeature(.panic_fn)) return;
26398+
26399+
const fn_index = try sema.getPanicIdFunc(src, panic_id);
26400+
const orig_fn_index = zcu.intern_pool.unwrapCoercedFunc(fn_index);
26401+
try sema.addReferenceEntry(null, src, .wrap(.{ .func = orig_fn_index }));
26402+
try zcu.ensureFuncBodyAnalysisQueued(orig_fn_index);
2639126403
}
2639226404

2639326405
fn getPanicIdFunc(sema: *Sema, src: LazySrcLoc, panic_id: Zcu.SimplePanicId) !InternPool.Index {
2639426406
const zcu = sema.pt.zcu;
2639526407
try sema.ensureMemoizedStateResolved(src, .panic);
26396-
const panic_func = zcu.builtin_decl_values.get(panic_id.toBuiltin());
26397-
try zcu.ensureFuncBodyAnalysisQueued(panic_func);
26408+
const panic_fn_index = zcu.builtin_decl_values.get(panic_id.toBuiltin());
2639826409
switch (sema.owner.unwrap()) {
2639926410
.@"comptime", .nav_ty, .nav_val, .type, .memoized_state => {},
2640026411
.func => |owner_func| zcu.intern_pool.funcSetHasErrorTrace(owner_func, true),
2640126412
}
26402-
return panic_func;
26413+
return panic_fn_index;
2640326414
}
2640426415

2640526416
fn addSafetyCheck(
@@ -31143,6 +31154,11 @@ fn addReferenceEntry(
3114331154
referenced_unit: AnalUnit,
3114431155
) !void {
3114531156
const zcu = sema.pt.zcu;
31157+
const ip = &zcu.intern_pool;
31158+
switch (referenced_unit.unwrap()) {
31159+
.func => |f| assert(ip.unwrapCoercedFunc(f) == f), // for `.{ .func = f }`, `f` must be uncoerced
31160+
else => {},
31161+
}
3114631162
if (!zcu.comp.incremental and zcu.comp.reference_trace == 0) return;
3114731163
const gop = try sema.references.getOrPut(sema.gpa, referenced_unit);
3114831164
if (gop.found_existing) return;
@@ -31329,8 +31345,9 @@ fn maybeQueueFuncBodyAnalysis(sema: *Sema, block: *Block, src: LazySrcLoc, nav_i
3132931345
const nav_val = zcu.navValue(nav_index);
3133031346
if (!ip.isFuncBody(nav_val.toIntern())) return;
3133131347

31332-
try sema.addReferenceEntry(block, src, AnalUnit.wrap(.{ .func = nav_val.toIntern() }));
31333-
try zcu.ensureFuncBodyAnalysisQueued(nav_val.toIntern());
31348+
const orig_fn_index = ip.unwrapCoercedFunc(nav_val.toIntern());
31349+
try sema.addReferenceEntry(block, src, .wrap(.{ .func = orig_fn_index }));
31350+
try zcu.ensureFuncBodyAnalysisQueued(orig_fn_index);
3133431351
}
3133531352

3133631353
fn analyzeRef(
@@ -34963,8 +34980,9 @@ fn resolveInferredErrorSet(
3496334980
}
3496434981
// In this case we are dealing with the actual InferredErrorSet object that
3496534982
// corresponds to the function, not one created to track an inline/comptime call.
34966-
try sema.addReferenceEntry(block, src, AnalUnit.wrap(.{ .func = func_index }));
34967-
try pt.ensureFuncBodyUpToDate(func_index);
34983+
const orig_func_index = ip.unwrapCoercedFunc(func_index);
34984+
try sema.addReferenceEntry(block, src, .wrap(.{ .func = orig_func_index }));
34985+
try pt.ensureFuncBodyUpToDate(orig_func_index);
3496834986
}
3496934987

3497034988
// This will now have been resolved by the logic at the end of `Zcu.analyzeFnBody`

src/Zcu.zig

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3451,8 +3451,11 @@ pub fn mapOldZirToNew(
34513451
/// will be analyzed when it returns: for that, see `ensureFuncBodyAnalyzed`.
34523452
pub fn ensureFuncBodyAnalysisQueued(zcu: *Zcu, func_index: InternPool.Index) !void {
34533453
const ip = &zcu.intern_pool;
3454+
34543455
const func = zcu.funcInfo(func_index);
34553456

3457+
assert(func.ty == func.uncoerced_ty); // analyze the body of the original function, not a coerced one
3458+
34563459
if (zcu.func_body_analysis_queued.contains(func_index)) return;
34573460

34583461
if (func.analysisUnordered(ip).is_analyzed) {

src/Zcu/PerThread.zig

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1571,7 +1571,7 @@ fn analyzeNavType(pt: Zcu.PerThread, nav_id: InternPool.Nav.Index) Zcu.CompileEr
15711571
return .{ .type_changed = true };
15721572
}
15731573

1574-
pub fn ensureFuncBodyUpToDate(pt: Zcu.PerThread, maybe_coerced_func_index: InternPool.Index) Zcu.SemaError!void {
1574+
pub fn ensureFuncBodyUpToDate(pt: Zcu.PerThread, func_index: InternPool.Index) Zcu.SemaError!void {
15751575
dev.check(.sema);
15761576

15771577
const tracy = trace(@src());
@@ -1581,15 +1581,15 @@ pub fn ensureFuncBodyUpToDate(pt: Zcu.PerThread, maybe_coerced_func_index: Inter
15811581
const gpa = zcu.gpa;
15821582
const ip = &zcu.intern_pool;
15831583

1584-
_ = zcu.func_body_analysis_queued.swapRemove(maybe_coerced_func_index);
1584+
_ = zcu.func_body_analysis_queued.swapRemove(func_index);
15851585

1586-
// We only care about the uncoerced function.
1587-
const func_index = ip.unwrapCoercedFunc(maybe_coerced_func_index);
15881586
const anal_unit: AnalUnit = .wrap(.{ .func = func_index });
15891587

15901588
log.debug("ensureFuncBodyUpToDate {f}", .{zcu.fmtAnalUnit(anal_unit)});
15911589

1592-
const func = zcu.funcInfo(maybe_coerced_func_index);
1590+
const func = zcu.funcInfo(func_index);
1591+
1592+
assert(func.ty == func.uncoerced_ty); // analyze the body of the original function, not a coerced one
15931593

15941594
const was_outdated = zcu.outdated.swapRemove(anal_unit) or
15951595
zcu.potentially_outdated.swapRemove(anal_unit);
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
fn original() usize {
2+
_ = struct {
3+
export const val: u32 = 123;
4+
};
5+
return 0;
6+
}
7+
8+
pub fn main() void {
9+
const coerced: fn () u64 = original;
10+
_ = coerced();
11+
12+
const S = struct {
13+
extern const val: u32;
14+
};
15+
if (S.val != 123) @panic("wrong value");
16+
}
17+
18+
// run
19+
// target=x86_64-linux

0 commit comments

Comments
 (0)