authorgravatar for kubkon@jakubkonka.comJakub Konka <kubkon@jakubkonka.com> 2021-07-22 16:00:31+02:00
committergravatar for kubkon@jakubkonka.comJakub Konka <kubkon@jakubkonka.com> 2021-07-22 16:02:31+02:00
log4fd0cb7618ffb5428981672f6a21c411599f51b2
tree5181986990d5273db055b879fc7c138ce794dbcc
parent773863150a96fcb9ddb3eccb585d10342d10cb78

macho: sort nlists within object before filtering by type

Previously, we'd filter the nlists assuming they were correctly ordered by type: local < extern defined < undefined within the object's symbol table but this doesn't seem to be guaranteed, therefore, we sort by type and address in one go, and filter defined from undefined afterwards.

1 files changed, 31 insertions(+), 51 deletions(-)

src/link/MachO/Object.zig+31-51
......@@ -295,7 +295,20 @@ const NlistWithIndex = struct {
295295 index: u32,
296296
297297 fn lessThan(_: void, lhs: NlistWithIndex, rhs: NlistWithIndex) bool {
298 return lhs.nlist.n_value < rhs.nlist.n_value;
298 // We sort by type: defined < undefined, and
299 // afterwards by address in each group. Normally, dysymtab should
300 // be enough to guarantee the sort, but turns out not every compiler
301 // is kind enough to specify the symbols in the correct order.
302 if (MachO.symbolIsSect(lhs.nlist)) {
303 if (MachO.symbolIsSect(rhs.nlist)) {
304 // Same group, sort by address.
305 return lhs.nlist.n_value < rhs.nlist.n_value;
306 } else {
307 return true;
308 }
309 } else {
310 return false;
311 }
299312 }
300313
301314 fn filterInSection(symbols: []NlistWithIndex, sect: macho.section_64) []NlistWithIndex {
......@@ -488,22 +501,27 @@ pub fn parseTextBlocks(self: *Object, macho_file: *MachO) !void {
488501
489502 log.debug("analysing {s}", .{self.name.?});
490503
491 const dysymtab = self.load_commands.items[self.dysymtab_cmd_index.?].Dysymtab;
492 // We only care about defined symbols, so filter every other out.
493 const nlists = self.symtab.items[dysymtab.ilocalsym..dysymtab.iundefsym];
494
495 var sorted_nlists = std.ArrayList(NlistWithIndex).init(self.allocator);
496 defer sorted_nlists.deinit();
497 try sorted_nlists.ensureTotalCapacity(nlists.len);
504 // You would expect that the symbol table is at least pre-sorted based on symbol's type:
505 // local < extern defined < undefined. Unfortunately, this is not guaranteed! For instance,
506 // the GO compiler does not necessarily respect that therefore we sort immediately by type
507 // and address within.
508 var sorted_all_nlists = std.ArrayList(NlistWithIndex).init(self.allocator);
509 defer sorted_all_nlists.deinit();
510 try sorted_all_nlists.ensureTotalCapacity(self.symtab.items.len);
498511
499 for (nlists) |nlist, index| {
500 sorted_nlists.appendAssumeCapacity(.{
512 for (self.symtab.items) |nlist, index| {
513 sorted_all_nlists.appendAssumeCapacity(.{
501514 .nlist = nlist,
502 .index = @intCast(u32, index + dysymtab.ilocalsym),
515 .index = @intCast(u32, index),
503516 });
504517 }
505518
506 sort.sort(NlistWithIndex, sorted_nlists.items, {}, NlistWithIndex.lessThan);
519 sort.sort(NlistWithIndex, sorted_all_nlists.items, {}, NlistWithIndex.lessThan);
520
521 const dysymtab = self.load_commands.items[self.dysymtab_cmd_index.?].Dysymtab;
522
523 // We only care about defined symbols, so filter every other out.
524 const sorted_nlists = sorted_all_nlists.items[dysymtab.ilocalsym..dysymtab.iundefsym];
507525
508526 for (seg.sections.items) |sect, id| {
509527 const sect_id = @intCast(u8, id);
......@@ -530,7 +548,7 @@ pub fn parseTextBlocks(self: *Object, macho_file: *MachO) !void {
530548 const relocs = mem.bytesAsSlice(macho.relocation_info, raw_relocs);
531549
532550 // Symbols within this section only.
533 const filtered_nlists = NlistWithIndex.filterInSection(sorted_nlists.items, sect);
551 const filtered_nlists = NlistWithIndex.filterInSection(sorted_nlists, sect);
534552
535553 // Is there any padding between symbols within the section?
536554 // const is_splittable = self.header.?.flags & macho.MH_SUBSECTIONS_VIA_SYMBOLS != 0;
......@@ -810,44 +828,6 @@ pub fn parseTextBlocks(self: *Object, macho_file: *MachO) !void {
810828 }
811829}
812830
813pub fn symbolFromReloc(self: *Object, macho_file: *MachO, rel: macho.relocation_info) !*Symbol {
814 const symbol = blk: {
815 if (rel.r_extern == 1) {
816 break :blk self.symbols.items[rel.r_symbolnum];
817 } else {
818 const sect_id = @intCast(u8, rel.r_symbolnum - 1);
819 const symbol = self.sections_as_symbols.get(sect_id) orelse symbol: {
820 // We need a valid pointer to Symbol even if there is no symbol, so we create a
821 // dummy symbol upfront which will later be populated when created a TextBlock from
822 // the target section here.
823 const seg = self.load_commands.items[self.segment_cmd_index.?].Segment;
824 const sect = seg.sections.items[sect_id];
825 const name = try std.fmt.allocPrint(self.allocator, "l_{s}_{s}_{s}", .{
826 self.name.?,
827 segmentName(sect),
828 sectionName(sect),
829 });
830 defer self.allocator.free(name);
831 const symbol = try macho_file.allocator.create(Symbol);
832 symbol.* = .{
833 .strx = try macho_file.makeString(name),
834 .payload = .{
835 .regular = .{
836 .linkage = .translation_unit,
837 .address = sect.addr,
838 .file = self,
839 },
840 },
841 };
842 try self.sections_as_symbols.putNoClobber(self.allocator, sect_id, symbol);
843 break :symbol symbol;
844 };
845 break :blk symbol;
846 }
847 };
848 return symbol;
849}
850
851831fn parseSymtab(self: *Object) !void {
852832 const index = self.symtab_cmd_index orelse return;
853833 const symtab_cmd = self.load_commands.items[index].Symtab;