From 5334972f657da09c96a370b7aa07ae609909797c Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Mon, 1 Jul 2024 06:31:22 +0200 Subject: [PATCH] macho: redo deduping literals yet again This time we can completely get rid of the mutex in Symbol. --- src/MachO.zig | 2 +- src/MachO/Atom.zig | 6 ++-- src/MachO/InternalObject.zig | 56 ++++++++++++++++-------------------- src/MachO/Object.zig | 53 ++++++++++++++++++++++------------ src/MachO/Symbol.zig | 2 -- src/MachO/relocatable.zig | 2 +- src/MachO/thunks.zig | 4 +-- 7 files changed, 66 insertions(+), 59 deletions(-) diff --git a/src/MachO.zig b/src/MachO.zig index ee314849..602fda4f 100644 --- a/src/MachO.zig +++ b/src/MachO.zig @@ -1595,7 +1595,7 @@ fn calcSectionSizeWorker(self: *MachO, sect_id: u8) void { ) !void { for (atoms) |ref| { const atom = ref.getAtom(macho_file).?; - const p2align = atom.alignment.load(.seq_cst); + const p2align = atom.alignment; const atom_alignment = try math.powi(u32, 2, p2align); const offset = mem.alignForward(u64, header.size, atom_alignment); const padding = offset - header.size; diff --git a/src/MachO/Atom.zig b/src/MachO/Atom.zig index abf8ccb7..0b996035 100644 --- a/src/MachO/Atom.zig +++ b/src/MachO/Atom.zig @@ -11,7 +11,7 @@ file: File.Index = 0, size: u64 = 0, /// Alignment of this atom as a power of two. -alignment: std.atomic.Value(u32) = std.atomic.Value(u32).init(0), +alignment: u32 = 0, /// Index of the input section. n_sect: u32 = 0, @@ -861,8 +861,8 @@ fn format2( const macho_file = ctx.macho_file; const file = atom.getFile(macho_file); try writer.print("atom({d}) : {s} : @{x} : sect({d}) : align({x}) : size({x}) : thunk({d})", .{ - atom.atom_index, atom.getName(macho_file), atom.getAddress(macho_file), - atom.out_n_sect, atom.alignment.load(.seq_cst), atom.size, + atom.atom_index, atom.getName(macho_file), atom.getAddress(macho_file), + atom.out_n_sect, atom.alignment, atom.size, atom.getExtra(macho_file).thunk, }); if (!atom.alive.load(.seq_cst)) try writer.writeAll(" : [*]"); diff --git a/src/MachO/InternalObject.zig b/src/MachO/InternalObject.zig index f297de12..3de9fe37 100644 --- a/src/MachO/InternalObject.zig +++ b/src/MachO/InternalObject.zig @@ -250,7 +250,7 @@ fn addObjcMethnameSection(self: *InternalObject, methname: []const u8, macho_fil try self.atoms_indexes.append(gpa, atom_index); const atom = self.getAtom(atom_index).?; atom.size = methname.len + 1; - atom.alignment.store(0, .seq_cst); + atom.alignment = 0; const n_sect = try self.addSection(gpa, "__TEXT", "__objc_methname"); const sect = &self.sections.items(.header)[n_sect]; @@ -291,7 +291,7 @@ fn addObjcSelrefsSection(self: *InternalObject, methname_sym_index: Symbol.Index try self.atoms_indexes.append(gpa, atom_index); const atom = self.getAtom(atom_index).?; atom.size = @sizeOf(u64); - atom.alignment.store(3, .seq_cst); + atom.alignment = 3; const n_sect = try self.addSection(gpa, "__DATA", "__objc_selrefs"); const sect = &self.sections.items(.header)[n_sect]; @@ -422,6 +422,11 @@ pub fn resolveLiterals(self: *InternalObject, lp: *MachO.LiteralPool, macho_file buffer.clearRetainingCapacity(); if (!res.found_existing) { res.ref.* = .{ .index = atom.getExtra(macho_file).literal_symbol_index, .file = self.index }; + } else { + const lp_sym = lp.getSymbol(res.index, macho_file); + const lp_atom = lp_sym.getAtom(macho_file).?; + lp_atom.alignment = @max(lp_atom.alignment, atom.alignment); + _ = atom.alive.swap(false, .seq_cst); } atom.addExtra(.{ .literal_pool_index = res.index }, macho_file); } @@ -443,22 +448,16 @@ pub fn dedupLiterals(self: *InternalObject, lp: MachO.LiteralPool, macho_file: * for (relocs) |*rel| { if (rel.tag != .@"extern") continue; const target_sym_ref = rel.getTargetSymbolRef(atom.*, macho_file); - if (target_sym_ref.getFile(macho_file) != null) { - const target_sym = target_sym_ref.getSymbol(macho_file).?; - if (target_sym.getAtom(macho_file)) |target_atom| { - if (!Object.isPtrLiteral(target_atom.getInputSection(macho_file))) continue; - const lp_index = target_atom.getExtra(macho_file).literal_pool_index; - const lp_sym = lp.getSymbol(lp_index, macho_file); - const lp_atom_ref = lp_sym.atom_ref; - if (target_atom.atom_index != lp_atom_ref.index or target_atom.file != lp_atom_ref.file) { - const lp_atom = lp_sym.getAtom(macho_file).?; - _ = lp_atom.alignment.fetchMax(target_atom.alignment.load(.seq_cst), .seq_cst); - _ = target_atom.alive.swap(false, .seq_cst); - target_sym.mutex.lock(); - defer target_sym.mutex.unlock(); - target_sym.atom_ref = lp_atom_ref; - } - } + const file = target_sym_ref.getFile(macho_file) orelse continue; + if (file.getIndex() != self.index) continue; + const target_sym = target_sym_ref.getSymbol(macho_file).?; + const target_atom = target_sym.getAtom(macho_file) orelse continue; + if (!Object.isPtrLiteral(target_atom.getInputSection(macho_file))) continue; + const lp_index = target_atom.getExtra(macho_file).literal_pool_index; + const lp_sym = lp.getSymbol(lp_index, macho_file); + const lp_atom_ref = lp_sym.atom_ref; + if (target_atom.atom_index != lp_atom_ref.index or target_atom.file != lp_atom_ref.file) { + target_sym.atom_ref = lp_atom_ref; } } } @@ -467,23 +466,18 @@ pub fn dedupLiterals(self: *InternalObject, lp: MachO.LiteralPool, macho_file: * if (!sym.getSectionFlags().objc_stubs) continue; const extra = sym.getExtra(macho_file); const file = sym.getFile(macho_file).?; + if (file.getIndex() != self.index) continue; const tsym = switch (file) { .dylib => unreachable, inline else => |x| &x.symbols.items[extra.objc_selrefs], }; - if (tsym.getAtom(macho_file)) |atom| { - if (!Object.isPtrLiteral(atom.getInputSection(macho_file))) continue; - const lp_index = atom.getExtra(macho_file).literal_pool_index; - const lp_sym = lp.getSymbol(lp_index, macho_file); - const lp_atom_ref = lp_sym.atom_ref; - if (atom.atom_index != lp_atom_ref.index or atom.file != lp_atom_ref.file) { - const lp_atom = lp_sym.getAtom(macho_file).?; - _ = lp_atom.alignment.fetchMax(atom.alignment.load(.seq_cst), .seq_cst); - _ = atom.alive.swap(false, .seq_cst); - tsym.mutex.lock(); - defer tsym.mutex.unlock(); - tsym.atom_ref = lp_atom_ref; - } + const atom = tsym.getAtom(macho_file) orelse continue; + if (!Object.isPtrLiteral(atom.getInputSection(macho_file))) continue; + const lp_index = atom.getExtra(macho_file).literal_pool_index; + const lp_sym = lp.getSymbol(lp_index, macho_file); + const lp_atom_ref = lp_sym.atom_ref; + if (atom.atom_index != lp_atom_ref.index or atom.file != lp_atom_ref.file) { + tsym.atom_ref = lp_atom_ref; } } } diff --git a/src/MachO/Object.zig b/src/MachO/Object.zig index f64e543c..e8126fd5 100644 --- a/src/MachO/Object.zig +++ b/src/MachO/Object.zig @@ -631,6 +631,11 @@ pub fn resolveLiterals(self: *Object, lp: *MachO.LiteralPool, macho_file: *MachO const res = try lp.insert(gpa, header.type(), atom_data); if (!res.found_existing) { res.ref.* = .{ .index = atom.getExtra(macho_file).literal_symbol_index, .file = self.index }; + } else { + const lp_sym = lp.getSymbol(res.index, macho_file); + const lp_atom = lp_sym.getAtom(macho_file).?; + lp_atom.alignment = @max(lp_atom.alignment, atom.alignment); + _ = atom.alive.swap(false, .seq_cst); } atom.addExtra(.{ .literal_pool_index = res.index }, macho_file); } @@ -666,6 +671,11 @@ pub fn resolveLiterals(self: *Object, lp: *MachO.LiteralPool, macho_file: *MachO buffer.clearRetainingCapacity(); if (!res.found_existing) { res.ref.* = .{ .index = atom.getExtra(macho_file).literal_symbol_index, .file = self.index }; + } else { + const lp_sym = lp.getSymbol(res.index, macho_file); + const lp_atom = lp_sym.getAtom(macho_file).?; + lp_atom.alignment = @max(lp_atom.alignment, atom.alignment); + _ = atom.alive.swap(false, .seq_cst); } atom.addExtra(.{ .literal_pool_index = res.index }, macho_file); } @@ -689,27 +699,32 @@ pub fn dedupLiterals(self: *Object, lp: MachO.LiteralPool, macho_file: *MachO) v for (relocs) |*rel| { if (rel.tag != .@"extern") continue; const target_sym_ref = rel.getTargetSymbolRef(atom.*, macho_file); - if (target_sym_ref.getFile(macho_file) != null) { - const target_sym = target_sym_ref.getSymbol(macho_file).?; - if (target_sym.getAtom(macho_file)) |target_atom| { - const isec = target_atom.getInputSection(macho_file); - if (Object.isCstringLiteral(isec) or Object.isFixedSizeLiteral(isec) or Object.isPtrLiteral(isec)) { - const lp_index = target_atom.getExtra(macho_file).literal_pool_index; - const lp_sym = lp.getSymbol(lp_index, macho_file); - const lp_atom_ref = lp_sym.atom_ref; - if (target_atom.atom_index != lp_atom_ref.index or target_atom.file != lp_atom_ref.file) { - const lp_atom = lp_sym.getAtom(macho_file).?; - _ = lp_atom.alignment.fetchMax(target_atom.alignment.load(.seq_cst), .seq_cst); - _ = target_atom.alive.swap(false, .seq_cst); - target_sym.mutex.lock(); - defer target_sym.mutex.unlock(); - target_sym.atom_ref = lp_atom_ref; - } - } - } + const file = target_sym_ref.getFile(macho_file) orelse continue; + if (file.getIndex() != self.index) continue; + const target_sym = target_sym_ref.getSymbol(macho_file).?; + const target_atom = target_sym.getAtom(macho_file) orelse continue; + const isec = target_atom.getInputSection(macho_file); + if (!Object.isCstringLiteral(isec) and !Object.isFixedSizeLiteral(isec) and !Object.isPtrLiteral(isec)) continue; + const lp_index = target_atom.getExtra(macho_file).literal_pool_index; + const lp_sym = lp.getSymbol(lp_index, macho_file); + const lp_atom_ref = lp_sym.atom_ref; + if (target_atom.atom_index != lp_atom_ref.index or target_atom.file != lp_atom_ref.file) { + target_sym.atom_ref = lp_atom_ref; } } } + + for (self.symbols.items) |*sym| { + const atom = sym.getAtom(macho_file) orelse continue; + const isec = atom.getInputSection(macho_file); + if (!Object.isCstringLiteral(isec) and !Object.isFixedSizeLiteral(isec) and !Object.isPtrLiteral(isec)) continue; + const lp_index = atom.getExtra(macho_file).literal_pool_index; + const lp_sym = lp.getSymbol(lp_index, macho_file); + const lp_atom_ref = lp_sym.atom_ref; + if (atom.atom_index != lp_atom_ref.index or self.index != lp_atom_ref.file) { + sym.atom_ref = lp_atom_ref; + } + } } pub fn findAtom(self: Object, addr: u64) ?Atom.Index { @@ -2258,8 +2273,8 @@ fn addAtom(self: *Object, allocator: Allocator, args: AddAtomArgs) !Atom.Index { .size = args.size, .off = args.off, .extra = try self.addAtomExtra(allocator, .{}), + .alignment = args.alignment, }; - atom.alignment.store(args.alignment, .seq_cst); return atom_index; } diff --git a/src/MachO/Symbol.zig b/src/MachO/Symbol.zig index 1bf060be..e33d0868 100644 --- a/src/MachO/Symbol.zig +++ b/src/MachO/Symbol.zig @@ -29,8 +29,6 @@ visibility: Visibility = .local, extra: u32 = 0, -mutex: std.Thread.Mutex = .{}, - pub fn isLocal(symbol: Symbol) bool { return !(symbol.flags.import or symbol.flags.@"export"); } diff --git a/src/MachO/relocatable.zig b/src/MachO/relocatable.zig index a115e44c..d4cc5546 100644 --- a/src/MachO/relocatable.zig +++ b/src/MachO/relocatable.zig @@ -149,7 +149,7 @@ fn calcSectionSizeWorker(macho_file: *MachO, sect_id: u8) void { fn doWork(mfile: *MachO, header: *macho.section_64, atoms: []const MachO.Ref) !void { for (atoms) |ref| { const atom = ref.getAtom(mfile).?; - const p2align = atom.alignment.load(.seq_cst); + const p2align = atom.alignment; const atom_alignment = try math.powi(u32, 2, p2align); const offset = mem.alignForward(u64, header.size, atom_alignment); const padding = offset - header.size; diff --git a/src/MachO/thunks.zig b/src/MachO/thunks.zig index fbae2912..390f0fff 100644 --- a/src/MachO/thunks.zig +++ b/src/MachO/thunks.zig @@ -18,7 +18,7 @@ pub fn createThunks(sect_id: u8, macho_file: *MachO) !void { const start = i; const start_atom = atoms[start].getAtom(macho_file).?; assert(start_atom.alive.load(.seq_cst)); - start_atom.value = try advance(header, start_atom.size, start_atom.alignment.load(.seq_cst)); + start_atom.value = try advance(header, start_atom.size, start_atom.alignment); i += 1; while (i < atoms.len and @@ -26,7 +26,7 @@ pub fn createThunks(sect_id: u8, macho_file: *MachO) !void { { const atom = atoms[i].getAtom(macho_file).?; assert(atom.alive.load(.seq_cst)); - atom.value = try advance(header, atom.size, atom.alignment.load(.seq_cst)); + atom.value = try advance(header, atom.size, atom.alignment); } // Insert a thunk at the group end