From ab30a0b9a0c05c62450639de8653d6695a7ac006 Mon Sep 17 00:00:00 2001 From: kcbanner Date: Sat, 5 Sep 2026 07:06:21 +0200 Subject: [PATCH] Coff: Incremental progress (#36706) Making progress on incremental support in the COFF linker: - Support reloc deletion by replacing them with .ABSOLUTE relocs (actually using the relocs from the free list will come later) - Fixup setting sizes during incremental updates - Fix incorrect modifications to the target_relocs linked list, add clarifying docs - Enable `x86_64-windows-selfhosted` for test-incremental Closes https://codeberg.org/ziglang/zig/issues/31773 Reviewed-on: https://codeberg.org/ziglang/zig/pulls/36706 --- lib/compiler/objdump.zig | 1 + src/link/Coff.zig | 83 +++++++++++++++++++++++++--------------- test/tests.zig | 15 ++++---- 3 files changed, 61 insertions(+), 38 deletions(-) diff --git a/lib/compiler/objdump.zig b/lib/compiler/objdump.zig index 3988dfb198c85dd5e9d5e6ffa122f9a52a1c2022..c3507d9e66f3f616a50e454ae5350b4a5a50d2c3 100644 --- a/lib/compiler/objdump.zig +++ b/lib/compiler/objdump.zig @@ -1780,4 +1780,5 @@ const usage = \\ --strings Display string tables \\ --symbols Display symbol tables \\ --tls Display TLS information + \\ ; diff --git a/src/link/Coff.zig b/src/link/Coff.zig index 1a553d8dc8a2f3ee69a6720ecb2439c974e317c1..11a2018056db7642d5a95f4b3c6dfc462c2aa78d 100644 --- a/src/link/Coff.zig +++ b/src/link/Coff.zig @@ -904,9 +904,11 @@ pub const Symbol = struct { weak_external_strat: WeakExternalStrat, _: u5 = 0, }, - /// Relocations contained within this symbol + /// The first index of the contiguous range of location relocs for this symbol. + /// The list is terminated by a reloc with a different .target than this symbol. + /// These are relocations that have a .loc that points to this symbol. loc_relocs: Reloc.Index, - /// Relocations targeting this symbol + /// The tail of a linked list of relocations with a .target that points to this symbol. target_relocs: Reloc.Index, section_number: SectionNumber, gmi: Node.GlobalMapIndex, @@ -998,8 +1000,20 @@ pub const Symbol = struct { }; } - pub fn size(sym: *const Symbol) u32 { - return if (sym.flags.extra_tag == .size) sym.extra.size else 0; + pub fn size(sym: *const Symbol, coff: *Coff) u32 { + var size_sym = sym; + while (size_sym.flags.extra_tag == .next_alias_si) + size_sym = size_sym.extra.next_alias_si.get(coff); + if (size_sym.flags.extra_tag != .size) return 0; + return size_sym.extra.size; + } + + pub fn setSize(sym: *Symbol, coff: *Coff, new_size: u32) void { + var size_sym = sym; + while (size_sym.flags.extra_tag == .next_alias_si) + size_sym = size_sym.extra.next_alias_si.get(coff); + assert(size_sym.flags.extra_tag == .size); + size_sym.extra.size = new_size; } pub const SectionNumber = enum(i16) { @@ -1097,7 +1111,7 @@ pub const Symbol = struct { assert(reloc.target == si); if (reloc.sri.entry(coff, reloc.loc.get(coff).section_number)) |entry| coff.targetStore(&entry.symbol_table_index, index); - ri = reloc.next; + ri = reloc.prev; } } @@ -1126,7 +1140,7 @@ pub const Symbol = struct { const reloc = ri.get(coff); assert(reloc.target == si); try reloc.apply(coff); - ri = reloc.next; + ri = reloc.prev; } } @@ -1494,24 +1508,25 @@ pub const Reloc = extern struct { } pub fn delete(reloc: *Reloc, coff: *Coff) void { + log.debug("deleteReloc({d})", .{reloc - coff.relocs.items.ptr}); if (reloc.sri != .none) { - // TODO: Need to remove this from the COFF relocation table (maybe removeswap?) - // TODO: If this was the last reloc causing something to be in the symbol table, we should remove - // the symbol table entry (and unset sti). That will require flushSymbolTableIndex on the - // swapped symbol if we exchange indices - @panic("TODO implement symbol table reloc deletions"); + const loc_sym = reloc.loc.get(coff); + const entry = reloc.sri.entry(coff, loc_sym.section_number).?; + + // On every supported architecture, a reloc type of 0 is .ABSOLUTE, and is a no-op + @memset(std.mem.asBytes(entry), 0); } switch (reloc.prev) { - .none => { - const target = reloc.target.get(coff); - assert(target.target_relocs.get(coff) == reloc); - target.target_relocs = reloc.next; - }, + .none => {}, else => |prev| prev.get(coff).next = reloc.next, } switch (reloc.next) { - .none => {}, + .none => { + const target = reloc.target.get(coff); + assert(target.target_relocs.get(coff) == reloc); + target.target_relocs = reloc.prev; + }, else => |next| next.get(coff).prev = reloc.prev, } @@ -3264,7 +3279,7 @@ fn flushSymbolTableEntry(coff: *Coff, index: u32) !void { }; coff.targetStore(&entry.value, switch (sym.section_number) { - .UNDEFINED => if (entry.storage_class == .WEAK_EXTERNAL) 0 else sym.size(), + .UNDEFINED => if (entry.storage_class == .WEAK_EXTERNAL) 0 else sym.size(coff), .ABSOLUTE, .DEBUG, => unreachable, @@ -3731,6 +3746,9 @@ fn addRelocAssumeCapacity( } else .none; const sri: Section.RelocationIndex = blk: { + // TODO: Once the API for using free relocs exist, if this is about to consume a + // free reloc, then we can use the existing (cleared) .sri on the reloc + const section = loc_sn.section(coff); const header = loc_sn.header(coff); const old_num_relocations = coff.targetLoad(&header.number_of_relocations); @@ -3770,8 +3788,8 @@ fn addRelocAssumeCapacity( coff.relocs.addOneAssumeCapacity().* = .{ .type = @"type", - .prev = .none, - .next = target.target_relocs, + .prev = target.target_relocs, + .next = .none, .loc = loc_si, .target = target_si, .sri = sri, @@ -3784,7 +3802,7 @@ fn addRelocAssumeCapacity( }; switch (target.target_relocs) { .none => {}, - else => |target_ri| target_ri.get(coff).prev = ri, + else => |target_ri| target_ri.get(coff).next = ri, } target.target_relocs = ri; } @@ -4894,7 +4912,7 @@ fn loadObject( symbol.si = global_gop.value_ptr.si; if (!global_gop.found_existing or symbol.si.get(coff).ni == .none) { const sym = symbol.si.get(coff); - sym.setExtra(.{ .size = @max(sym.size(), size) }); + sym.setExtra(.{ .size = @max(sym.size(coff), size) }); } }, else => unreachable, @@ -5483,7 +5501,8 @@ fn updateNavInner(coff: *Coff, pt: Zcu.PerThread, nav_index: InternPool.Nav.Inde error.WriteFailed => return nw.err.?, else => |e| return e, }; - si.get(coff).extra.size = @intCast(nw.interface.end); + + si.get(coff).setSize(coff, @intCast(nw.interface.end)); try si.applyLocationRelocs(coff); } @@ -5621,7 +5640,8 @@ fn updateFuncInner( error.WriteFailed => return nw.err.?, else => |e| return e, }; - si.get(coff).extra.size = @intCast(nw.interface.end); + + si.get(coff).setSize(coff, @intCast(nw.interface.end)); try si.applyLocationRelocs(coff); // The NAV's node is done---now generate any UAVs or lazy code/data which the NAV needs. @@ -6214,7 +6234,8 @@ fn genUav( error.WriteFailed => return nw.err.?, else => |e| return e, }; - si.get(coff).extra.size = @intCast(nw.interface.end); + + si.get(coff).setSize(coff, @intCast(nw.interface.end)); try si.applyLocationRelocs(coff); } @@ -6238,18 +6259,19 @@ fn aliasGlobal(coff: *Coff, gmi: Node.GlobalMapIndex, alias_si: Symbol.Index) !v const reloc = ri.get(coff); assert(reloc.target == si); reloc.target = alias_si; - if (reloc.next == .none) { - reloc.next = alias_sym.target_relocs; + if (reloc.prev == .none) { + reloc.prev = alias_sym.target_relocs; if (alias_sym.target_relocs != .none) - alias_sym.target_relocs.get(coff).prev = ri; + alias_sym.target_relocs.get(coff).next = ri; break; } - ri = reloc.next; + ri = reloc.prev; } const prev_target_relocs = alias_sym.target_relocs; if (sym.target_relocs != .none) alias_sym.target_relocs = sym.target_relocs; + sym.target_relocs = .none; sym.gmi = alias_sym.gmi; coff.globals.values()[gmi.unwrap().?].si = alias_si; @@ -6844,7 +6866,8 @@ fn genLazyInner(coff: *Coff, pt: Zcu.PerThread, lmr: Node.LazyMapRef) !void { error.WriteFailed => return nw.err.?, else => |e| return e, }; - si.get(coff).extra.size = @intCast(nw.interface.end); + + si.get(coff).setSize(coff, @intCast(nw.interface.end)); try si.applyLocationRelocs(coff); } diff --git a/test/tests.zig b/test/tests.zig index 7ecc7d93e22f5ba209ce8961c21c32a4f8112893..415aa23ff67a80afcc81c0034d865b53236d7ade 100644 --- a/test/tests.zig +++ b/test/tests.zig @@ -2286,14 +2286,13 @@ const incremental_targets = &[_]IncrementalTarget{ }, .backend = .selfhosted, }, - // https://codeberg.org/ziglang/zig/issues/31773 - // .{ - // .target = .{ - // .cpu_arch = .x86_64, - // .os_tag = .windows, - // }, - // .backend = .selfhosted, - // }, + .{ + .target = .{ + .cpu_arch = .x86_64, + .os_tag = .windows, + }, + .backend = .selfhosted, + }, .{ .target = .{ .cpu_arch = .wasm32, -- 2.54.0