frontend sends dead navs to the linker

[andrewrk]

I found this while working on the wasm linker but it affects all backends and I believe ultimately stems from the frontend.

pub fn main() void {}

zig build-exe test.zig -target wasm32-wasi -fno-llvm -fno-lld -OReleaseFast

Looking at the data segment:

  - 1000000: 0400 0000 0000 0000 aaaa aaaa aaaa aaaa  ................
  - 1000010: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  - 1000020: 3200 0000 0000 0000 7002 0001 0700 0000  2.......p.......
  - 1000030: 7002 0001 0700 0000 c00a 0000 0000 0000  p...............
  - 1000040: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  - 1000050: 0000 0000 0000 0000 0000 0000 2000 0000  ............ ...
  - 1000060: 0000 0000 0100 0000 0000 0000 0000 0000  ................
  - 1000070: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  - 1000080: 0200 0000 0200 0000 0000 0000 0000 0000  ................
  - 1000090: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  - 10000a0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  - 10000b0: 0000 0000 0000 0000 2900 0000 2800 0001  ........)...(...
  - 10000c0: c00a 0000 0000 0000 0000 0000 0000 0000  ................
  - 10000d0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  - 10000e0: 0000 0000 2900 0000 2800 0001 c00a 0000  ....)...(.......
  - 10000f0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  - 1000100: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  - 1000110: 2000 0000 0000 0000 0100 0000 0000 0000   ...............
  - 1000120: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  - 1000130: 0000 0000 0200 0000 0200 0000 0000 0000  ................
  - 1000140: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  - 1000150: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  - 1000160: 0000 0000 0000 0000 0000 0000 100a aaaa  ................

Look at all that crap, what's that doing there?

Turning on --debug-log link, we see that updateNav is called for lots of things:

debug(link): updateNav std.start InternPool.Nav.Index(94)
debug(link): updateNav std.debug InternPool.Nav.Index(56)
debug(link): updateNav start.root InternPool.Nav.Index(99)
debug(link): updateNav start.builtin InternPool.Nav.Index(101)
debug(link): updateNav builtin.std InternPool.Nav.Index(225)
debug(link): updateNav std.builtin InternPool.Nav.Index(50)
debug(link): updateNav builtin.CompilerBackend InternPool.Nav.Index(287)
debug(link): updateNav builtin.zig_backend InternPool.Nav.Index(228)
debug(link): updateNav start.simplified_logic InternPool.Nav.Index(108)
debug(link): updateNav builtin.OutputMode InternPool.Nav.Index(271)
debug(link): updateNav builtin.output_mode InternPool.Nav.Index(229)
debug(link): updateNav builtin.link_libc InternPool.Nav.Index(240)
debug(link): updateNav std.Target InternPool.Nav.Index(41)
debug(link): updateNav Target.Os InternPool.Nav.Index(303)
debug(link): updateNav Target.Os.Tag InternPool.Nav.Index(373)
debug(link): updateNav Target.Os.VersionRange InternPool.Nav.Index(377)
debug(link): updateNav Target.std InternPool.Nav.Index(369)
debug(link): updateNav std.SemanticVersion InternPool.Nav.Index(34)
debug(link): updateNav SemanticVersion.Range InternPool.Nav.Index(396)
debug(link): updateNav Target.Os.HurdVersionRange InternPool.Nav.Index(375)
debug(link): updateNav Target.Os.LinuxVersionRange InternPool.Nav.Index(376)
debug(link): updateNav Target.Os.WindowsVersion InternPool.Nav.Index(374)
debug(link): updateNav Target.Os.WindowsVersion.Range InternPool.Nav.Index(415)
debug(link): updateNav SemanticVersion.Version InternPool.Nav.Index(395)
debug(link): updateNav builtin.os InternPool.Nav.Index(236)
debug(link): updateNav start.native_os InternPool.Nav.Index(106)
debug(link): updateNav builtin.WasiExecModel InternPool.Nav.Index(274)
debug(link): updateNav builtin.wasi_exec_model InternPool.Nav.Index(251)
debug(link): updateNav builtin.Signedness InternPool.Nav.Index(270)
debug(link): updateNav builtin.AddressSpace InternPool.Nav.Index(264)
debug(link): updateNav builtin.CallingConvention InternPool.Nav.Index(263)
debug(link): updateNav builtin.CallingConvention.CommonOptions InternPool.Nav.Index(443)
debug(link): updateNav builtin.CallingConvention.X86RegparmOptions InternPool.Nav.Index(444)
debug(link): updateNav builtin.CallingConvention.ArmInterruptOptions InternPool.Nav.Index(445)
debug(link): updateNav builtin.CallingConvention.MipsInterruptOptions InternPool.Nav.Index(446)
debug(link): updateNav builtin.CallingConvention.RiscvInterruptOptions InternPool.Nav.Index(447)
debug(link): updateNav builtin.CallingConvention.ArmInterruptOptions.InterruptType InternPool.Nav.Index(451)
debug(link): updateNav builtin.CallingConvention.MipsInterruptOptions.InterruptMode InternPool.Nav.Index(452)
debug(link): updateNav builtin.CallingConvention.RiscvInterruptOptions.PrivilegeMode InternPool.Nav.Index(453)
debug(link): updateNav builtin.StackTrace InternPool.Nav.Index(254)
debug(link): updateNav builtin.SourceLocation InternPool.Nav.Index(265)
debug(link): updateNav builtin.CallModifier InternPool.Nav.Index(275)
debug(link): updateNav builtin.AtomicOrder InternPool.Nav.Index(257)
debug(link): updateNav builtin.AtomicRmwOp InternPool.Nav.Index(259)
debug(link): updateNav builtin.ReduceOp InternPool.Nav.Index(258)
debug(link): updateNav builtin.FloatMode InternPool.Nav.Index(268)
debug(link): updateNav builtin.PrefetchOptions InternPool.Nav.Index(283)
debug(link): updateNav builtin.PrefetchOptions.Rw InternPool.Nav.Index(455)
debug(link): updateNav builtin.PrefetchOptions.Cache InternPool.Nav.Index(456)
debug(link): updateNav builtin.ExportOptions InternPool.Nav.Index(284)
debug(link): updateNav builtin.GlobalLinkage InternPool.Nav.Index(255)
debug(link): updateNav builtin.SymbolVisibility InternPool.Nav.Index(256)
debug(link): updateNav builtin.ExternOptions InternPool.Nav.Index(285)
debug(link): updateNav builtin.BranchHint InternPool.Nav.Index(286)
debug(link): updateNav builtin.Type InternPool.Nav.Index(267)
debug(link): updateNav builtin.Type.Int InternPool.Nav.Index(457)
debug(link): updateNav builtin.Type.Float InternPool.Nav.Index(458)
debug(link): updateNav builtin.Type.Pointer InternPool.Nav.Index(459)
debug(link): updateNav builtin.Type.Array InternPool.Nav.Index(460)
debug(link): updateNav builtin.Type.Struct InternPool.Nav.Index(463)
debug(link): updateNav builtin.Type.Optional InternPool.Nav.Index(464)
debug(link): updateNav builtin.Type.ErrorUnion InternPool.Nav.Index(465)
debug(link): updateNav builtin.Type.Error InternPool.Nav.Index(466)
debug(link): updateNav builtin.Type.ErrorSet InternPool.Nav.Index(467)
debug(link): updateNav builtin.Type.Enum InternPool.Nav.Index(469)
debug(link): updateNav builtin.Type.Union InternPool.Nav.Index(471)
debug(link): updateNav builtin.Type.Fn InternPool.Nav.Index(472)
debug(link): updateNav builtin.Type.Opaque InternPool.Nav.Index(473)
debug(link): updateNav builtin.Type.Frame InternPool.Nav.Index(474)
debug(link): updateNav builtin.Type.AnyFrame InternPool.Nav.Index(475)
debug(link): updateNav builtin.Type.Vector InternPool.Nav.Index(476)
debug(link): updateNav builtin.Type.ContainerLayout InternPool.Nav.Index(461)
debug(link): updateNav builtin.Type.StructField InternPool.Nav.Index(462)
debug(link): updateNav builtin.Type.Declaration InternPool.Nav.Index(477)
debug(link): updateNav builtin.Type.UnionField InternPool.Nav.Index(470)
debug(link): updateNav builtin.Type.Fn.Param InternPool.Nav.Index(479)
debug(link): updateNav builtin.Type.Pointer.Size InternPool.Nav.Index(478)
debug(link): updateNav builtin.Type.EnumField InternPool.Nav.Index(468)
debug(link): updateNav builtin.builtin InternPool.Nav.Index(252)
debug(link): updateNav Target.Cpu InternPool.Nav.Index(334)

Anything here that is not a zero-bit type ends up in the output binary even though it is dead (not referenced).

This would be fair game for -ODebug since arguably any referenced nav by the code should be inspectable by the debugger.

However, when -OReleaseFast is specified, this justification disappears; the binary should not include dead data. Especially if strip is enabled.

One could argue that the linker should do this work by doing garbage collection. That's typically something linkers do. However, the frontend is already doing dependency chasing and handling the logic of which symbols are referenced, and only calling updateNav for those things which have been analyzed. It seems reasonable for linker implementations to rely on updateNav being called as the authority on whether a nav is supposed to be included in the binary.

[mlugg]

This is a little tricky.

The obvious thing to do here would be to give all const declarations the treatment we currently give functions. For functions, they are only analyzed at runtime and lowered into the binary if a) they are called at runtime, or b) a reference is ever taken to them (through &myFunc or similar). We could do this for const declarations too, and only rule (b) is necessary there: if you ever write &x, then the declaration must be lowered.

Note that the reason we already do this for functions is because it not only bloats the binary more, but it also has semantic implications for functions; see #6256 and #15891.

The downside to implementing this rule for global declarations is that taking references to things which aren't functions happens... kind of a lot. This is mainly because of method calls. Even if a method doesn't actually take a pointer, we have to emit ZIR which takes a reference to the callee in case we need one. I've thought about this, and I don't think we can really emit ZIR which does this conditionally without an unreasonable amount of work in AstGen and complexity in the langspec, so I don't think that's an avenue worth exploring.

I don't think there's any more sophisticated analysis the frontend could reasonably do here: it would need to do quite a bit of extra tracking to figure out if a symbol is actually referenced at runtime, and I don't fancy slowing down (and increasing RSS of) the frontend to try and slightly optimize the output binary size.

So, to omit all of these symbols, we'd need linker GC anyway. The question then is whether it's worth also having this mechanism in the frontend. My instinct is probably not, just because it would be extra complexity, but I'm open to other opinions here.

[andrewrk]

OK, thanks for that analysis. Let's run with your intuition then - this is a job for linker GC.

[mlugg]

I recall @Snektron having issues with this when working on the SPIR-V backend. Robin, I just want to check: why was it a specific problem there? Was there some fundamental issue with lowering something you were getting (due to SPIR-V limitations), or was it just frustrating that you couldn't test the backend until you could lower those decls? The former would, to me, be a motivating use case for doing something in the frontend here, but otherwise I think this is settled.