From 787e755a2f36660b5bc22f2a3a5d051a8c34445f Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 16 Aug 2023 12:07:34 +0200 Subject: [PATCH 1/2] macho: tie FDEs and unwind records to all symbol aliases This is in particular very important to the Zig language which allows exporting the same symbol under different names. For instance, it is possible to have a case such that: ``` ... 4258 T _foo 4258 T _bar ... ``` In this case we need to keep track of both symbol names when resolving FDEs and unwind records. --- src/link/MachO/Object.zig | 65 ++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 11 deletions(-) diff --git a/src/link/MachO/Object.zig b/src/link/MachO/Object.zig index 06458ea3cd..bbcfbb7047 100644 --- a/src/link/MachO/Object.zig +++ b/src/link/MachO/Object.zig @@ -718,7 +718,7 @@ fn parseEhFrameSection(self: *Object, zld: *Zld, object_id: u32) !void { } try self.eh_frame_relocs_lookup.ensureTotalCapacity(gpa, record_count); - try self.eh_frame_records_lookup.ensureTotalCapacity(gpa, record_count); + try self.eh_frame_records_lookup.ensureUnusedCapacity(gpa, record_count); it.reset(); @@ -768,11 +768,28 @@ fn parseEhFrameSection(self: *Object, zld: *Zld, object_id: u32) !void { else => unreachable, } }; - log.debug("FDE at offset {x} tracks {s}", .{ offset, zld.getSymbolName(target) }); if (target.getFile() != object_id) { + log.debug("FDE at offset {x} marked DEAD", .{offset}); self.eh_frame_relocs_lookup.getPtr(offset).?.dead = true; } else { - self.eh_frame_records_lookup.putAssumeCapacityNoClobber(target, offset); + // You would think that we are done but turns out that the compilers may use + // whichever symbol alias they want for a target symbol. This in particular + // very problematic when using Zig's @export feature to re-export symbols under + // additional names. For that reason, we need to ensure we record aliases here + // too so that we can tie them with their matching unwind records and vice versa. + const aliases = self.getSymbolAliases(target.sym_index); + var i: u32 = 0; + while (i < aliases.len) : (i += 1) { + const actual_target = SymbolWithLoc{ + .sym_index = i + aliases.start, + .file = target.file, + }; + log.debug("FDE at offset {x} tracks {s}", .{ + offset, + zld.getSymbolName(actual_target), + }); + try self.eh_frame_records_lookup.putNoClobber(gpa, actual_target, offset); + } } } } @@ -803,7 +820,7 @@ fn parseUnwindInfo(self: *Object, zld: *Zld, object_id: u32) !void { const unwind_records = self.getUnwindRecords(); - try self.unwind_records_lookup.ensureTotalCapacity(gpa, @as(u32, @intCast(unwind_records.len))); + try self.unwind_records_lookup.ensureUnusedCapacity(gpa, @as(u32, @intCast(unwind_records.len))); const needs_eh_frame = for (unwind_records) |record| { if (UnwindInfo.UnwindEncoding.isDwarf(record.compactUnwindEncoding, cpu_arch)) break true; @@ -839,11 +856,28 @@ fn parseUnwindInfo(self: *Object, zld: *Zld, object_id: u32) !void { .code = mem.asBytes(&record), .base_offset = @as(i32, @intCast(offset)), }); - log.debug("unwind record {d} tracks {s}", .{ record_id, zld.getSymbolName(target) }); if (target.getFile() != object_id) { + log.debug("unwind record {d} marked DEAD", .{record_id}); self.unwind_relocs_lookup[record_id].dead = true; } else { - self.unwind_records_lookup.putAssumeCapacityNoClobber(target, @as(u32, @intCast(record_id))); + // You would think that we are done but turns out that the compilers may use + // whichever symbol alias they want for a target symbol. This in particular + // very problematic when using Zig's @export feature to re-export symbols under + // additional names. For that reason, we need to ensure we record aliases here + // too so that we can tie them with their matching unwind records and vice versa. + const aliases = self.getSymbolAliases(target.sym_index); + var i: u32 = 0; + while (i < aliases.len) : (i += 1) { + const actual_target = SymbolWithLoc{ + .sym_index = i + aliases.start, + .file = target.file, + }; + log.debug("unwind record {d} tracks {s}", .{ + record_id, + zld.getSymbolName(actual_target), + }); + try self.unwind_records_lookup.putNoClobber(gpa, actual_target, @intCast(record_id)); + } } } } @@ -991,6 +1025,18 @@ pub fn getSymbolName(self: Object, index: u32) []const u8 { return strtab[start..][0 .. len - 1 :0]; } +fn getSymbolAliases(self: Object, index: u32) Entry { + const addr = self.source_address_lookup[index]; + var start = index; + while (start > 0 and + self.source_address_lookup[start - 1] == addr) : (start -= 1) + {} + const end: u32 = for (self.source_address_lookup[start..], start..) |saddr, i| { + if (saddr != addr) break @as(u32, @intCast(i)); + } else @as(u32, @intCast(self.source_address_lookup.len)); + return .{ .start = start, .len = end - start }; +} + pub fn getSymbolByAddress(self: Object, addr: u64, sect_hint: ?u8) u32 { // Find containing atom const Predicate = struct { @@ -1012,11 +1058,8 @@ pub fn getSymbolByAddress(self: Object, addr: u64, sect_hint: ?u8) u32 { if (target_sym_index > 0) { // Hone in on the most senior alias of the target symbol. // See SymbolAtIndex.lessThan for more context. - var start = target_sym_index - 1; - while (start > 0 and - self.source_address_lookup[lookup.start..][start - 1] == addr) : (start -= 1) - {} - return @as(u32, @intCast(lookup.start + start)); + const aliases = self.getSymbolAliases(@intCast(lookup.start + target_sym_index - 1)); + return aliases.start; } } return self.getSectionAliasSymbolIndex(sect_id); From 573bb77ab6f4f00753bb257314d8115cbaac17ae Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 16 Aug 2023 12:19:34 +0200 Subject: [PATCH 2/2] macho: add smoke test for re-exports in static libs --- test/link.zig | 4 +++ test/link/macho/reexports/a.zig | 7 ++++++ test/link/macho/reexports/build.zig | 38 +++++++++++++++++++++++++++++ test/link/macho/reexports/main.c | 5 ++++ 4 files changed, 54 insertions(+) create mode 100644 test/link/macho/reexports/a.zig create mode 100644 test/link/macho/reexports/build.zig create mode 100644 test/link/macho/reexports/main.c diff --git a/test/link.zig b/test/link.zig index 56b1cf415d..53caefd64f 100644 --- a/test/link.zig +++ b/test/link.zig @@ -156,6 +156,10 @@ pub const cases = [_]Case{ .build_root = "test/link/macho/pagezero", .import = @import("link/macho/pagezero/build.zig"), }, + .{ + .build_root = "test/link/macho/reexports", + .import = @import("link/macho/reexports/build.zig"), + }, .{ .build_root = "test/link/macho/search_strategy", .import = @import("link/macho/search_strategy/build.zig"), diff --git a/test/link/macho/reexports/a.zig b/test/link/macho/reexports/a.zig new file mode 100644 index 0000000000..cfa7e8b3ac --- /dev/null +++ b/test/link/macho/reexports/a.zig @@ -0,0 +1,7 @@ +const x: i32 = 42; +export fn foo() i32 { + return x; +} +comptime { + @export(foo, .{ .name = "bar", .linkage = .Strong }); +} diff --git a/test/link/macho/reexports/build.zig b/test/link/macho/reexports/build.zig new file mode 100644 index 0000000000..d9a9481fa0 --- /dev/null +++ b/test/link/macho/reexports/build.zig @@ -0,0 +1,38 @@ +const std = @import("std"); + +pub const requires_symlinks = true; + +pub fn build(b: *std.Build) void { + const test_step = b.step("test", "Test it"); + b.default_step = test_step; + + add(b, test_step, .Debug); + add(b, test_step, .ReleaseFast); + add(b, test_step, .ReleaseSmall); + add(b, test_step, .ReleaseSafe); +} + +fn add(b: *std.Build, test_step: *std.Build.Step, optimize: std.builtin.OptimizeMode) void { + const target: std.zig.CrossTarget = .{ .os_tag = .macos }; + + const lib = b.addStaticLibrary(.{ + .name = "a", + .root_source_file = .{ .path = "a.zig" }, + .optimize = optimize, + .target = target, + }); + + const exe = b.addExecutable(.{ + .name = "test", + .optimize = optimize, + .target = target, + }); + exe.addCSourceFile(.{ .file = .{ .path = "main.c" }, .flags = &.{} }); + exe.linkLibrary(lib); + exe.linkLibC(); + + const run = b.addRunArtifact(exe); + run.skip_foreign_checks = true; + run.expectExitCode(0); + test_step.dependOn(&run.step); +} diff --git a/test/link/macho/reexports/main.c b/test/link/macho/reexports/main.c new file mode 100644 index 0000000000..2beb701f1f --- /dev/null +++ b/test/link/macho/reexports/main.c @@ -0,0 +1,5 @@ +extern int foo(); +extern int bar(); +int main() { + return bar() - foo(); +}