Commit 725c825829

mlugg <mlugg@mlugg.co.uk>
2025-02-26 00:58:14
link: make sure MachO closes the damn files
Windows is a ridiculous operating system designed by toddlers, and so requires us to close all file handles in the `tmp/xxxxxxx` cache dir before renaming it into `o/xxxxxxx`. We have a hack in place to handle this for the main output file, but the MachO linker also outputs a file with debug symbols, and we weren't closing it! This led to a fuckton of CI failures when we enabled `.whole` cache mode by default for self-hosted backends. thanks jacob for figuring this out while i sat there
1 parent c2983a3
Changed files (3)
src/link/MachO.zig
@@ -3277,6 +3277,40 @@ const InitMetadataOptions = struct {
     program_code_size_hint: u64,
 };
 
+pub fn closeDebugInfo(self: *MachO) bool {
+    const d_sym = &(self.d_sym orelse return false);
+    d_sym.deinit();
+    self.d_sym = null;
+    return true;
+}
+
+pub fn reopenDebugInfo(self: *MachO) !void {
+    assert(self.d_sym == null);
+
+    assert(!self.base.comp.config.use_llvm);
+    assert(self.base.comp.config.debug_format == .dwarf);
+
+    const gpa = self.base.comp.gpa;
+    const sep = fs.path.sep_str;
+    const d_sym_path = try std.fmt.allocPrint(
+        gpa,
+        "{s}.dSYM" ++ sep ++ "Contents" ++ sep ++ "Resources" ++ sep ++ "DWARF",
+        .{self.base.emit.sub_path},
+    );
+    defer gpa.free(d_sym_path);
+
+    var d_sym_bundle = try self.base.emit.root_dir.handle.makeOpenPath(d_sym_path, .{});
+    defer d_sym_bundle.close();
+
+    const d_sym_file = try d_sym_bundle.createFile(self.base.emit.sub_path, .{
+        .truncate = false,
+        .read = true,
+    });
+
+    self.d_sym = .{ .allocator = gpa, .file = d_sym_file };
+    try self.d_sym.?.initMetadata(self);
+}
+
 // TODO: move to ZigObject
 fn initMetadata(self: *MachO, options: InitMetadataOptions) !void {
     if (!self.base.isRelocatable()) {
@@ -3333,26 +3367,7 @@ fn initMetadata(self: *MachO, options: InitMetadataOptions) !void {
         if (options.zo.dwarf) |*dwarf| {
             // Create dSYM bundle.
             log.debug("creating {s}.dSYM bundle", .{options.emit.sub_path});
-
-            const gpa = self.base.comp.gpa;
-            const sep = fs.path.sep_str;
-            const d_sym_path = try std.fmt.allocPrint(
-                gpa,
-                "{s}.dSYM" ++ sep ++ "Contents" ++ sep ++ "Resources" ++ sep ++ "DWARF",
-                .{options.emit.sub_path},
-            );
-            defer gpa.free(d_sym_path);
-
-            var d_sym_bundle = try options.emit.root_dir.handle.makeOpenPath(d_sym_path, .{});
-            defer d_sym_bundle.close();
-
-            const d_sym_file = try d_sym_bundle.createFile(options.emit.sub_path, .{
-                .truncate = false,
-                .read = true,
-            });
-
-            self.d_sym = .{ .allocator = gpa, .file = d_sym_file };
-            try self.d_sym.?.initMetadata(self);
+            try self.reopenDebugInfo();
             try dwarf.initMetadata();
         }
     }
src/Compilation.zig
@@ -2397,7 +2397,7 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void {
 
             // Work around windows `AccessDenied` if any files within this
             // directory are open by closing and reopening the file handles.
-            const need_writable_dance = w: {
+            const need_writable_dance: enum { no, lf_only, lf_and_debug } = w: {
                 if (builtin.os.tag == .windows) {
                     if (comp.bin_file) |lf| {
                         // We cannot just call `makeExecutable` as it makes a false
@@ -2410,11 +2410,13 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void {
                         if (lf.file) |f| {
                             f.close();
                             lf.file = null;
-                            break :w true;
+
+                            if (lf.closeDebugInfo()) break :w .lf_and_debug;
+                            break :w .lf_only;
                         }
                     }
                 }
-                break :w false;
+                break :w .no;
             };
 
             renameTmpIntoCache(comp.local_cache_directory, tmp_dir_sub_path, o_sub_path) catch |err| {
@@ -2441,8 +2443,13 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void {
                 };
 
                 // Has to be after the `wholeCacheModeSetBinFilePath` above.
-                if (need_writable_dance) {
-                    try lf.makeWritable();
+                switch (need_writable_dance) {
+                    .no => {},
+                    .lf_only => try lf.makeWritable(),
+                    .lf_and_debug => {
+                        try lf.makeWritable();
+                        try lf.reopenDebugInfo();
+                    },
                 }
             }
 
src/link.zig
@@ -600,6 +600,20 @@ pub const File = struct {
         }
     }
 
+    /// Some linkers create a separate file for debug info, which we might need to temporarily close
+    /// when moving the compilation result directory due to the host OS not allowing moving a
+    /// file/directory while a handle remains open.
+    /// Returns `true` if a debug info file was closed. In that case, `reopenDebugInfo` may be called.
+    pub fn closeDebugInfo(base: *File) bool {
+        const macho = base.cast(.macho) orelse return false;
+        return macho.closeDebugInfo();
+    }
+
+    pub fn reopenDebugInfo(base: *File) !void {
+        const macho = base.cast(.macho).?;
+        return macho.reopenDebugInfo();
+    }
+
     pub fn makeExecutable(base: *File) !void {
         dev.check(.make_executable);
         const comp = base.comp;