authorgravatar for kubkon@jakubkonka.comJakub Konka <kubkon@jakubkonka.com> 2022-10-30 16:38:28+01:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2022-10-30 16:38:28+01:00
log2b25d3c33394edcf8760ed49495c162aac80f59b
tree29ab93c9bed76e33302cece2290d0e901b17c325
parent5339cac2ef69d81913444ae50c7b060e97e80709
parent9b0c555db4bf60432b22504bc03abdd84d494175
signaturebadge-question-mark Signed by PGP key 4AEE18F83AFDEB23

Merge pull request #13356 from ziglang/macho-no-dupe

macho: fix performance regression for x86_64 and -dead_strip

4 files changed, 104 insertions(+), 56 deletions(-)

src/link/MachO/Object.zig+18-10
...@@ -44,6 +44,10 @@ symtab: []macho.nlist_64 = undefined,...@@ -44,6 +44,10 @@ symtab: []macho.nlist_64 = undefined,
44/// Can be undefined as set together with in_symtab.44/// Can be undefined as set together with in_symtab.
45source_symtab_lookup: []u32 = undefined,45source_symtab_lookup: []u32 = undefined,
46/// Can be undefined as set together with in_symtab.46/// Can be undefined as set together with in_symtab.
47source_address_lookup: []i64 = undefined,
48/// Can be undefined as set together with in_symtab.
49source_section_index_lookup: []i64 = undefined,
50/// Can be undefined as set together with in_symtab.
47strtab_lookup: []u32 = undefined,51strtab_lookup: []u32 = undefined,
48/// Can be undefined as set together with in_symtab.52/// Can be undefined as set together with in_symtab.
49atom_by_index_table: []AtomIndex = undefined,53atom_by_index_table: []AtomIndex = undefined,
...@@ -58,6 +62,8 @@ pub fn deinit(self: *Object, gpa: Allocator) void {...@@ -58,6 +62,8 @@ pub fn deinit(self: *Object, gpa: Allocator) void {
58 gpa.free(self.contents);62 gpa.free(self.contents);
59 if (self.in_symtab) |_| {63 if (self.in_symtab) |_| {
60 gpa.free(self.source_symtab_lookup);64 gpa.free(self.source_symtab_lookup);
65 gpa.free(self.source_address_lookup);
66 gpa.free(self.source_section_index_lookup);
61 gpa.free(self.strtab_lookup);67 gpa.free(self.strtab_lookup);
62 gpa.free(self.symtab);68 gpa.free(self.symtab);
63 gpa.free(self.atom_by_index_table);69 gpa.free(self.atom_by_index_table);
...@@ -116,6 +122,10 @@ pub fn parse(self: *Object, allocator: Allocator, cpu_arch: std.Target.Cpu.Arch)...@@ -116,6 +122,10 @@ pub fn parse(self: *Object, allocator: Allocator, cpu_arch: std.Target.Cpu.Arch)
116 self.strtab_lookup = try allocator.alloc(u32, self.in_symtab.?.len);122 self.strtab_lookup = try allocator.alloc(u32, self.in_symtab.?.len);
117 self.globals_lookup = try allocator.alloc(i64, self.in_symtab.?.len);123 self.globals_lookup = try allocator.alloc(i64, self.in_symtab.?.len);
118 self.atom_by_index_table = try allocator.alloc(AtomIndex, self.in_symtab.?.len + nsects);124 self.atom_by_index_table = try allocator.alloc(AtomIndex, self.in_symtab.?.len + nsects);
125 // This is wasteful but we need to be able to lookup source symbol address after stripping and
126 // allocating of sections.
127 self.source_address_lookup = try allocator.alloc(i64, self.in_symtab.?.len);
128 self.source_section_index_lookup = try allocator.alloc(i64, nsects);
119129
120 for (self.symtab) |*sym| {130 for (self.symtab) |*sym| {
121 sym.* = .{131 sym.* = .{
...@@ -129,6 +139,7 @@ pub fn parse(self: *Object, allocator: Allocator, cpu_arch: std.Target.Cpu.Arch)...@@ -129,6 +139,7 @@ pub fn parse(self: *Object, allocator: Allocator, cpu_arch: std.Target.Cpu.Arch)
129139
130 mem.set(i64, self.globals_lookup, -1);140 mem.set(i64, self.globals_lookup, -1);
131 mem.set(AtomIndex, self.atom_by_index_table, 0);141 mem.set(AtomIndex, self.atom_by_index_table, 0);
142 mem.set(i64, self.source_section_index_lookup, -1);
132143
133 // You would expect that the symbol table is at least pre-sorted based on symbol's type:144 // You would expect that the symbol table is at least pre-sorted based on symbol's type:
134 // local < extern defined < undefined. Unfortunately, this is not guaranteed! For instance,145 // local < extern defined < undefined. Unfortunately, this is not guaranteed! For instance,
...@@ -150,8 +161,13 @@ pub fn parse(self: *Object, allocator: Allocator, cpu_arch: std.Target.Cpu.Arch)...@@ -150,8 +161,13 @@ pub fn parse(self: *Object, allocator: Allocator, cpu_arch: std.Target.Cpu.Arch)
150 for (sorted_all_syms.items) |sym_id, i| {161 for (sorted_all_syms.items) |sym_id, i| {
151 const sym = sym_id.getSymbol(self);162 const sym = sym_id.getSymbol(self);
152163
164 if (sym.sect() and self.source_section_index_lookup[sym.n_sect - 1] == -1) {
165 self.source_section_index_lookup[sym.n_sect - 1] = @intCast(i64, i);
166 }
167
153 self.symtab[i] = sym;168 self.symtab[i] = sym;
154 self.source_symtab_lookup[i] = sym_id.index;169 self.source_symtab_lookup[i] = sym_id.index;
170 self.source_address_lookup[i] = if (sym.undf()) -1 else @intCast(i64, sym.n_value);
155171
156 const sym_name_len = mem.sliceTo(@ptrCast([*:0]const u8, self.in_strtab.?.ptr + sym.n_strx), 0).len + 1;172 const sym_name_len = mem.sliceTo(@ptrCast([*:0]const u8, self.in_strtab.?.ptr + sym.n_strx), 0).len + 1;
157 self.strtab_lookup[i] = @intCast(u32, sym_name_len);173 self.strtab_lookup[i] = @intCast(u32, sym_name_len);
...@@ -244,13 +260,12 @@ fn filterSymbolsBySection(symbols: []macho.nlist_64, n_sect: u8) struct {...@@ -244,13 +260,12 @@ fn filterSymbolsBySection(symbols: []macho.nlist_64, n_sect: u8) struct {
244 return .{ .index = @intCast(u32, index), .len = @intCast(u32, len) };260 return .{ .index = @intCast(u32, index), .len = @intCast(u32, len) };
245}261}
246262
247fn filterSymbolsByAddress(symbols: []macho.nlist_64, n_sect: u8, start_addr: u64, end_addr: u64) struct {263fn filterSymbolsByAddress(symbols: []macho.nlist_64, start_addr: u64, end_addr: u64) struct {
248 index: u32,264 index: u32,
249 len: u32,265 len: u32,
250} {266} {
251 const Predicate = struct {267 const Predicate = struct {
252 addr: u64,268 addr: u64,
253 n_sect: u8,
254269
255 pub fn predicate(pred: @This(), symbol: macho.nlist_64) bool {270 pub fn predicate(pred: @This(), symbol: macho.nlist_64) bool {
256 return symbol.n_value >= pred.addr;271 return symbol.n_value >= pred.addr;
...@@ -259,11 +274,9 @@ fn filterSymbolsByAddress(symbols: []macho.nlist_64, n_sect: u8, start_addr: u64...@@ -259,11 +274,9 @@ fn filterSymbolsByAddress(symbols: []macho.nlist_64, n_sect: u8, start_addr: u64
259274
260 const index = @import("zld.zig").lsearch(macho.nlist_64, symbols, Predicate{275 const index = @import("zld.zig").lsearch(macho.nlist_64, symbols, Predicate{
261 .addr = start_addr,276 .addr = start_addr,
262 .n_sect = n_sect,
263 });277 });
264 const len = @import("zld.zig").lsearch(macho.nlist_64, symbols[index..], Predicate{278 const len = @import("zld.zig").lsearch(macho.nlist_64, symbols[index..], Predicate{
265 .addr = end_addr,279 .addr = end_addr,
266 .n_sect = n_sect,
267 });280 });
268281
269 return .{ .index = @intCast(u32, index), .len = @intCast(u32, len) };282 return .{ .index = @intCast(u32, index), .len = @intCast(u32, len) };
...@@ -412,12 +425,7 @@ pub fn splitIntoAtoms(self: *Object, zld: *Zld, object_id: u31) !void {...@@ -412,12 +425,7 @@ pub fn splitIntoAtoms(self: *Object, zld: *Zld, object_id: u31) !void {
412 while (next_sym_index < sect_start_index + sect_loc.len) {425 while (next_sym_index < sect_start_index + sect_loc.len) {
413 const next_sym = symtab[next_sym_index];426 const next_sym = symtab[next_sym_index];
414 const addr = next_sym.n_value;427 const addr = next_sym.n_value;
415 const atom_loc = filterSymbolsByAddress(428 const atom_loc = filterSymbolsByAddress(symtab[next_sym_index..], addr, addr + 1);
416 symtab[next_sym_index..],
417 sect_id + 1,
418 addr,
419 addr + 1,
420 );
421 assert(atom_loc.len > 0);429 assert(atom_loc.len > 0);
422 const atom_sym_index = atom_loc.index + next_sym_index;430 const atom_sym_index = atom_loc.index + next_sym_index;
423 const nsyms_trailing = atom_loc.len - 1;431 const nsyms_trailing = atom_loc.len - 1;
src/link/MachO/ZldAtom.zig+85-29
...@@ -181,6 +181,27 @@ const RelocContext = struct {...@@ -181,6 +181,27 @@ const RelocContext = struct {
181 base_offset: i32 = 0,181 base_offset: i32 = 0,
182};182};
183183
184pub fn getRelocContext(zld: *Zld, atom_index: AtomIndex) RelocContext {
185 const atom = zld.getAtom(atom_index);
186 assert(atom.getFile() != null); // synthetic atoms do not have relocs
187
188 const object = zld.objects.items[atom.getFile().?];
189 if (object.getSourceSymbol(atom.sym_index)) |source_sym| {
190 const source_sect = object.getSourceSection(source_sym.n_sect - 1);
191 return .{
192 .base_addr = source_sect.addr,
193 .base_offset = @intCast(i32, source_sym.n_value - source_sect.addr),
194 };
195 }
196 const nbase = @intCast(u32, object.in_symtab.?.len);
197 const sect_id = @intCast(u16, atom.sym_index - nbase);
198 const source_sect = object.getSourceSection(sect_id);
199 return .{
200 .base_addr = source_sect.addr,
201 .base_offset = 0,
202 };
203}
204
184pub fn parseRelocTarget(205pub fn parseRelocTarget(
185 zld: *Zld,206 zld: *Zld,
186 atom_index: AtomIndex,207 atom_index: AtomIndex,
...@@ -192,6 +213,52 @@ pub fn parseRelocTarget(...@@ -192,6 +213,52 @@ pub fn parseRelocTarget(
192213
193 if (rel.r_extern == 0) {214 if (rel.r_extern == 0) {
194 const sect_id = @intCast(u8, rel.r_symbolnum - 1);215 const sect_id = @intCast(u8, rel.r_symbolnum - 1);
216 const ctx = getRelocContext(zld, atom_index);
217 const atom_code = getAtomCode(zld, atom_index);
218 const rel_offset = @intCast(u32, rel.r_address - ctx.base_offset);
219
220 const address_in_section = if (rel.r_pcrel == 0) blk: {
221 break :blk if (rel.r_length == 3)
222 mem.readIntLittle(i64, atom_code[rel_offset..][0..8])
223 else
224 mem.readIntLittle(i32, atom_code[rel_offset..][0..4]);
225 } else blk: {
226 const correction: u3 = switch (@intToEnum(macho.reloc_type_x86_64, rel.r_type)) {
227 .X86_64_RELOC_SIGNED => 0,
228 .X86_64_RELOC_SIGNED_1 => 1,
229 .X86_64_RELOC_SIGNED_2 => 2,
230 .X86_64_RELOC_SIGNED_4 => 4,
231 else => unreachable,
232 };
233 const addend = mem.readIntLittle(i32, atom_code[rel_offset..][0..4]);
234 const target_address = @intCast(i64, ctx.base_addr) + rel.r_address + 4 + correction + addend;
235 break :blk target_address;
236 };
237
238 // Find containing atom
239 const Predicate = struct {
240 addr: i64,
241
242 pub fn predicate(pred: @This(), other: i64) bool {
243 return if (other == -1) true else other > pred.addr;
244 }
245 };
246
247 if (object.source_section_index_lookup[sect_id] > -1) {
248 const first_sym_index = @intCast(usize, object.source_section_index_lookup[sect_id]);
249 const target_sym_index = @import("zld.zig").lsearch(i64, object.source_address_lookup[first_sym_index..], Predicate{
250 .addr = address_in_section,
251 });
252
253 if (target_sym_index > 0) {
254 return SymbolWithLoc{
255 .sym_index = @intCast(u32, first_sym_index + target_sym_index - 1),
256 .file = atom.file,
257 };
258 }
259 }
260
261 // Start of section is not contained anywhere, return synthetic atom.
195 const sym_index = object.getSectionAliasSymbolIndex(sect_id);262 const sym_index = object.getSectionAliasSymbolIndex(sect_id);
196 return SymbolWithLoc{ .sym_index = sym_index, .file = atom.file };263 return SymbolWithLoc{ .sym_index = sym_index, .file = atom.file };
197 }264 }
...@@ -405,29 +472,13 @@ pub fn resolveRelocs(...@@ -405,29 +472,13 @@ pub fn resolveRelocs(
405 const atom = zld.getAtom(atom_index);472 const atom = zld.getAtom(atom_index);
406 assert(atom.getFile() != null); // synthetic atoms do not have relocs473 assert(atom.getFile() != null); // synthetic atoms do not have relocs
407474
408 const object = zld.objects.items[atom.getFile().?];
409 const ctx: RelocContext = blk: {
410 if (object.getSourceSymbol(atom.sym_index)) |source_sym| {
411 const source_sect = object.getSourceSection(source_sym.n_sect - 1);
412 break :blk .{
413 .base_addr = source_sect.addr,
414 .base_offset = @intCast(i32, source_sym.n_value - source_sect.addr),
415 };
416 }
417 const nbase = @intCast(u32, object.in_symtab.?.len);
418 const sect_id = @intCast(u16, atom.sym_index - nbase);
419 const source_sect = object.getSourceSection(sect_id);
420 break :blk .{
421 .base_addr = source_sect.addr,
422 .base_offset = 0,
423 };
424 };
425
426 log.debug("resolving relocations in ATOM(%{d}, '{s}')", .{475 log.debug("resolving relocations in ATOM(%{d}, '{s}')", .{
427 atom.sym_index,476 atom.sym_index,
428 zld.getSymbolName(atom.getSymbolWithLoc()),477 zld.getSymbolName(atom.getSymbolWithLoc()),
429 });478 });
430479
480 const ctx = getRelocContext(zld, atom_index);
481
431 return switch (arch) {482 return switch (arch) {
432 .aarch64 => resolveRelocsArm64(zld, atom_index, atom_code, atom_relocs, reverse_lookup, ctx),483 .aarch64 => resolveRelocsArm64(zld, atom_index, atom_code, atom_relocs, reverse_lookup, ctx),
433 .x86_64 => resolveRelocsX86(zld, atom_index, atom_code, atom_relocs, reverse_lookup, ctx),484 .x86_64 => resolveRelocsX86(zld, atom_index, atom_code, atom_relocs, reverse_lookup, ctx),
...@@ -744,8 +795,11 @@ fn resolveRelocsArm64(...@@ -744,8 +795,11 @@ fn resolveRelocsArm64(
744 mem.readIntLittle(i32, atom_code[rel_offset..][0..4]);795 mem.readIntLittle(i32, atom_code[rel_offset..][0..4]);
745796
746 if (rel.r_extern == 0) {797 if (rel.r_extern == 0) {
747 const target_sect_base_addr = object.getSourceSection(@intCast(u16, rel.r_symbolnum - 1)).addr;798 const base_addr = if (target.sym_index > object.source_address_lookup.len)
748 ptr_addend -= @intCast(i64, target_sect_base_addr);799 @intCast(i64, object.getSourceSection(@intCast(u16, rel.r_symbolnum - 1)).addr)
800 else
801 object.source_address_lookup[target.sym_index];
802 ptr_addend -= base_addr;
749 }803 }
750804
751 const result = blk: {805 const result = blk: {
...@@ -878,13 +932,12 @@ fn resolveRelocsX86(...@@ -878,13 +932,12 @@ fn resolveRelocsX86(
878 var addend = mem.readIntLittle(i32, atom_code[rel_offset..][0..4]) + correction;932 var addend = mem.readIntLittle(i32, atom_code[rel_offset..][0..4]) + correction;
879933
880 if (rel.r_extern == 0) {934 if (rel.r_extern == 0) {
881 // Note for the future self: when r_extern == 0, we should subtract correction from the935 const base_addr = if (target.sym_index > object.source_address_lookup.len)
882 // addend.936 @intCast(i64, object.getSourceSection(@intCast(u16, rel.r_symbolnum - 1)).addr)
883 const target_sect_base_addr = object.getSourceSection(@intCast(u16, rel.r_symbolnum - 1)).addr;937 else
884 // We need to add base_offset, i.e., offset of this atom wrt to the source938 object.source_address_lookup[target.sym_index];
885 // section. Otherwise, the addend will over-/under-shoot.939 addend += @intCast(i32, @intCast(i64, context.base_addr) + rel.r_address + 4 -
886 addend += @intCast(i32, @intCast(i64, context.base_addr + rel_offset + 4) -940 @intCast(i64, base_addr));
887 @intCast(i64, target_sect_base_addr) + context.base_offset);
888 }941 }
889942
890 const adjusted_target_addr = @intCast(u64, @intCast(i64, target_addr) + addend);943 const adjusted_target_addr = @intCast(u64, @intCast(i64, target_addr) + addend);
...@@ -902,8 +955,11 @@ fn resolveRelocsX86(...@@ -902,8 +955,11 @@ fn resolveRelocsX86(
902 mem.readIntLittle(i32, atom_code[rel_offset..][0..4]);955 mem.readIntLittle(i32, atom_code[rel_offset..][0..4]);
903956
904 if (rel.r_extern == 0) {957 if (rel.r_extern == 0) {
905 const target_sect_base_addr = object.getSourceSection(@intCast(u16, rel.r_symbolnum - 1)).addr;958 const base_addr = if (target.sym_index > object.source_address_lookup.len)
906 addend -= @intCast(i64, target_sect_base_addr);959 @intCast(i64, object.getSourceSection(@intCast(u16, rel.r_symbolnum - 1)).addr)
960 else
961 object.source_address_lookup[target.sym_index];
962 addend -= base_addr;
907 }963 }
908964
909 const result = blk: {965 const result = blk: {
src/link/MachO/dead_strip.zig-15
...@@ -162,21 +162,6 @@ fn markLive(...@@ -162,21 +162,6 @@ fn markLive(
162 };162 };
163 const target_sym = zld.getSymbol(target);163 const target_sym = zld.getSymbol(target);
164164
165 if (rel.r_extern == 0) {
166 // We are pessimistic and mark all atoms within the target section as live.
167 // TODO: this can be improved by marking only the relevant atoms.
168 const sect_id = target_sym.n_sect;
169 const object = zld.objects.items[target.getFile().?];
170 for (object.atoms.items) |other_atom_index| {
171 const other_atom = zld.getAtom(other_atom_index);
172 const other_sym = zld.getSymbol(other_atom.getSymbolWithLoc());
173 if (other_sym.n_sect == sect_id) {
174 markLive(zld, other_atom_index, alive, reverse_lookups);
175 }
176 }
177 continue;
178 }
179
180 if (target_sym.undf()) continue;165 if (target_sym.undf()) continue;
181 if (target.getFile() == null) {166 if (target.getFile() == null) {
182 const target_sym_name = zld.getSymbolName(target);167 const target_sym_name = zld.getSymbolName(target);
src/link/MachO/zld.zig+1-2
...@@ -966,10 +966,9 @@ pub const Zld = struct {...@@ -966,10 +966,9 @@ pub const Zld = struct {
966966
967 const global_index = resolver.table.get(sym_name) orelse {967 const global_index = resolver.table.get(sym_name) orelse {
968 const gpa = self.gpa;968 const gpa = self.gpa;
969 const name = try resolver.arena.dupe(u8, sym_name);
970 const global_index = @intCast(u32, self.globals.items.len);969 const global_index = @intCast(u32, self.globals.items.len);
971 try self.globals.append(gpa, sym_loc);970 try self.globals.append(gpa, sym_loc);
972 try resolver.table.putNoClobber(name, global_index);971 try resolver.table.putNoClobber(sym_name, global_index);
973 if (sym.undf() and !sym.tentative()) {972 if (sym.undf() and !sym.tentative()) {
974 try resolver.unresolved.putNoClobber(global_index, {});973 try resolver.unresolved.putNoClobber(global_index, {});
975 }974 }