From 45143c6f04c84e8a519aa24e4d79e9385b5d5e2f Mon Sep 17 00:00:00 2001 From: mlugg Date: Fri, 10 Oct 2025 11:20:19 +0100 Subject: [PATCH] MachO: emit absolute path in N_OSO stabs This path being relative is unconventional and causes issues for us if the output artifact is ever used from a different cwd than the one it was built from. The behavior implemented by this commit of always emitting these paths as absolute was actually the behavior in 0.14.x, but it regressed in 0.15.1 due to internal reworks to path handling which led to relative paths being more common in the compiler internals. Resolves: #25433 --- src/link/MachO.zig | 16 ++++--- src/link/MachO/Archive.zig | 24 ++++++---- src/link/MachO/Object.zig | 81 +++++++++++++--------------------- src/link/MachO/relocatable.zig | 2 +- 4 files changed, 58 insertions(+), 65 deletions(-) diff --git a/src/link/MachO.zig b/src/link/MachO.zig index 3e7e3a4e8a08b9c9825f2d536bc10c2b5e436fd6..11127495ab4b2580aaacb477d4d40901f1361574 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -919,7 +919,16 @@ fn addObject(self: *MachO, path: Path, handle: File.HandleIndex, offset: u64) !v const tracy = trace(@src()); defer tracy.end(); - const gpa = self.base.comp.gpa; + const comp = self.base.comp; + const gpa = comp.gpa; + + const abs_path = try std.fs.path.resolvePosix(gpa, &.{ + comp.dirs.cwd, + path.root_dir.path orelse ".", + path.sub_path, + }); + errdefer gpa.free(abs_path); + const mtime: u64 = mtime: { const file = self.getFileHandle(handle); const stat = file.stat() catch break :mtime 0; @@ -928,10 +937,7 @@ fn addObject(self: *MachO, path: Path, handle: File.HandleIndex, offset: u64) !v const index = @as(File.Index, @intCast(try self.files.addOne(gpa))); self.files.set(index, .{ .object = .{ .offset = offset, - .path = .{ - .root_dir = path.root_dir, - .sub_path = try gpa.dupe(u8, path.sub_path), - }, + .path = abs_path, .file_handle = handle, .mtime = mtime, .index = index, diff --git a/src/link/MachO/Archive.zig b/src/link/MachO/Archive.zig index 007e630bfb3842cf20731896728292b51cda9934..058efdbafcc13351dda13b8a29e91ecb8c49ba43 100644 --- a/src/link/MachO/Archive.zig +++ b/src/link/MachO/Archive.zig @@ -5,8 +5,9 @@ pub fn deinit(self: *Archive, allocator: Allocator) void { } pub fn unpack(self: *Archive, macho_file: *MachO, path: Path, handle_index: File.HandleIndex, fat_arch: ?fat.Arch) !void { - const gpa = macho_file.base.comp.gpa; - const diags = &macho_file.base.comp.link_diags; + const comp = macho_file.base.comp; + const gpa = comp.gpa; + const diags = &comp.link_diags; var arena = std.heap.ArenaAllocator.init(gpa); defer arena.deinit(); @@ -55,23 +56,30 @@ pub fn unpack(self: *Archive, macho_file: *MachO, path: Path, handle_index: File mem.eql(u8, name, SYMDEF_SORTED) or mem.eql(u8, name, SYMDEF64_SORTED)) continue; + const abs_path = try std.fs.path.resolvePosix(gpa, &.{ + comp.dirs.cwd, + path.root_dir.path orelse ".", + path.sub_path, + }); + errdefer gpa.free(abs_path); + + const o_basename = try gpa.dupe(u8, name); + errdefer gpa.free(o_basename); + const object: Object = .{ .offset = pos, .in_archive = .{ - .path = .{ - .root_dir = path.root_dir, - .sub_path = try gpa.dupe(u8, path.sub_path), - }, + .path = abs_path, .size = hdr_size, }, - .path = Path.initCwd(try gpa.dupe(u8, name)), + .path = o_basename, .file_handle = handle_index, .index = undefined, .alive = false, .mtime = hdr.date() catch 0, }; - log.debug("extracting object '{f}' from archive '{f}'", .{ object.path, path }); + log.debug("extracting object '{s}' from archive '{f}'", .{ o_basename, path }); try self.objects.append(gpa, object); } diff --git a/src/link/MachO/Object.zig b/src/link/MachO/Object.zig index 90d96b85d9566f6f37a607c6493f2c1367b0c657..7cec09ba911440488e91f2c70de94fb19ed5833c 100644 --- a/src/link/MachO/Object.zig +++ b/src/link/MachO/Object.zig @@ -1,8 +1,9 @@ /// Non-zero for fat object files or archives offset: u64, -/// Archive files cannot contain subdirectories, so only the basename is needed -/// for output. However, the full path is kept for error reporting. -path: Path, +/// If `in_archive` is not `null`, this is the basename of the object in the archive. Otherwise, +/// this is a fully-resolved absolute path, because that is the path we need to embed in stabs to +/// ensure the output does not depend on its cwd. +path: []u8, file_handle: File.HandleIndex, mtime: u64, index: File.Index, @@ -41,8 +42,8 @@ output_symtab_ctx: MachO.SymtabCtx = .{}, output_ar_state: Archive.ArState = .{}, pub fn deinit(self: *Object, allocator: Allocator) void { - if (self.in_archive) |*ar| allocator.free(ar.path.sub_path); - allocator.free(self.path.sub_path); + if (self.in_archive) |*ar| allocator.free(ar.path); + allocator.free(self.path); for (self.sections.items(.relocs), self.sections.items(.subsections)) |*relocs, *sub| { relocs.deinit(allocator); sub.deinit(allocator); @@ -1703,7 +1704,7 @@ pub fn updateArSize(self: *Object, macho_file: *MachO) !void { pub fn writeAr(self: Object, ar_format: Archive.Format, macho_file: *MachO, writer: anytype) !void { // Header const size = try macho_file.cast(usize, self.output_ar_state.size); - const basename = std.fs.path.basename(self.path.sub_path); + const basename = std.fs.path.basename(self.path); try Archive.writeHeader(basename, size, ar_format, writer); // Data const file = macho_file.getFileHandle(self.file_handle); @@ -1756,12 +1757,7 @@ pub fn calcSymtabSize(self: *Object, macho_file: *MachO) void { self.calcStabsSize(macho_file); } -fn pathLen(path: Path) usize { - // +1 for the path separator - return (if (path.root_dir.path) |p| p.len + @intFromBool(path.sub_path.len != 0) else 0) + path.sub_path.len; -} - -pub fn calcStabsSize(self: *Object, macho_file: *MachO) void { +fn calcStabsSize(self: *Object, macho_file: *MachO) void { if (self.compile_unit) |cu| { const comp_dir = cu.getCompDir(self.*); const tu_name = cu.getTuName(self.*); @@ -1771,9 +1767,11 @@ pub fn calcStabsSize(self: *Object, macho_file: *MachO) void { self.output_symtab_ctx.strsize += @as(u32, @intCast(tu_name.len + 1)); // tu_name if (self.in_archive) |ar| { - self.output_symtab_ctx.strsize += @intCast(pathLen(ar.path) + 1 + self.path.basename().len + 1 + 1); + // "/path/to/archive.a(object.o)\x00" + self.output_symtab_ctx.strsize += @intCast(ar.path.len + self.path.len + 3); } else { - self.output_symtab_ctx.strsize += @intCast(pathLen(self.path) + 1); + // "/path/to/object.o\x00" + self.output_symtab_ctx.strsize += @intCast(self.path.len + 1); } for (self.symbols.items, 0..) |sym, i| { @@ -2018,7 +2016,7 @@ pub fn writeSymtab(self: Object, macho_file: *MachO, ctx: anytype) void { self.writeStabs(n_strx, macho_file, ctx); } -pub fn writeStabs(self: Object, stroff: u32, macho_file: *MachO, ctx: anytype) void { +fn writeStabs(self: Object, stroff: u32, macho_file: *MachO, ctx: anytype) void { const writeFuncStab = struct { inline fn writeFuncStab( n_strx: u32, @@ -2103,38 +2101,20 @@ pub fn writeStabs(self: Object, stroff: u32, macho_file: *MachO, ctx: anytype) v }; index += 1; if (self.in_archive) |ar| { - if (ar.path.root_dir.path) |p| { - @memcpy(ctx.strtab.items[n_strx..][0..p.len], p); - n_strx += @intCast(p.len); - if (ar.path.sub_path.len != 0) { - ctx.strtab.items[n_strx] = '/'; - n_strx += 1; - } - } - @memcpy(ctx.strtab.items[n_strx..][0..ar.path.sub_path.len], ar.path.sub_path); - n_strx += @intCast(ar.path.sub_path.len); - ctx.strtab.items[n_strx] = '('; - n_strx += 1; - const basename = self.path.basename(); - @memcpy(ctx.strtab.items[n_strx..][0..basename.len], basename); - n_strx += @intCast(basename.len); - ctx.strtab.items[n_strx] = ')'; - n_strx += 1; - ctx.strtab.items[n_strx] = 0; + // "/path/to/archive.a(object.o)\x00" + @memcpy(ctx.strtab.items[n_strx..][0..ar.path.len], ar.path); + n_strx += @intCast(ar.path.len); + ctx.strtab.items[n_strx..][0] = '('; n_strx += 1; + @memcpy(ctx.strtab.items[n_strx..][0..self.path.len], self.path); + n_strx += @intCast(self.path.len); + ctx.strtab.items[n_strx..][0..2].* = ")\x00".*; + n_strx += 2; } else { - if (self.path.root_dir.path) |p| { - @memcpy(ctx.strtab.items[n_strx..][0..p.len], p); - n_strx += @intCast(p.len); - if (self.path.sub_path.len != 0) { - ctx.strtab.items[n_strx] = '/'; - n_strx += 1; - } - } - @memcpy(ctx.strtab.items[n_strx..][0..self.path.sub_path.len], self.path.sub_path); - n_strx += @intCast(self.path.sub_path.len); - ctx.strtab.items[n_strx] = 0; - n_strx += 1; + // "/path/to/object.o\x00" + @memcpy(ctx.strtab.items[n_strx..][0..self.path.len], self.path); + ctx.strtab.items[n_strx..][self.path.len] = 0; + n_strx += @intCast(self.path.len + 1); } for (self.symbols.items, 0..) |sym, i| { @@ -2621,11 +2601,9 @@ pub fn fmtPath(self: Object) std.fmt.Alt(Object, formatPath) { fn formatPath(object: Object, w: *Writer) Writer.Error!void { if (object.in_archive) |ar| { - try w.print("{f}({s})", .{ - ar.path, object.path.basename(), - }); + try w.print("{s}({s})", .{ ar.path, object.path }); } else { - try w.print("{f}", .{object.path}); + try w.writeAll(object.path); } } @@ -2716,7 +2694,9 @@ const CompileUnit = struct { }; const InArchive = struct { - path: Path, + /// This is a fully-resolved absolute path, because that is the path we need to embed in stabs + /// to ensure the output does not depend on its cwd. + path: []u8, size: u32, }; @@ -3094,7 +3074,6 @@ const log = std.log.scoped(.link); const macho = std.macho; const math = std.math; const mem = std.mem; -const Path = std.Build.Cache.Path; const Allocator = std.mem.Allocator; const Writer = std.Io.Writer; diff --git a/src/link/MachO/relocatable.zig b/src/link/MachO/relocatable.zig index 920125136ba1153d209608c32d79ed756e4a3442..09807a28458cd72c6db954d4fc6a429159623571 100644 --- a/src/link/MachO/relocatable.zig +++ b/src/link/MachO/relocatable.zig @@ -191,7 +191,7 @@ pub fn flushStaticLib(macho_file: *MachO, comp: *Compilation, module_obj_path: ? pos = mem.alignForward(usize, pos, 2); state.file_off = pos; pos += @sizeOf(Archive.ar_hdr); - pos += mem.alignForward(usize, o.path.basename().len + 1, ptr_width); + pos += mem.alignForward(usize, std.fs.path.basename(o.path).len + 1, ptr_width); pos += try macho_file.cast(usize, state.size); }, else => unreachable, -- 2.54.0