Skip to content

Commit 725c825

Browse files
mluggjacobly0
authored andcommitted
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 commit 725c825

File tree

3 files changed

+61
-25
lines changed

3 files changed

+61
-25
lines changed

src/Compilation.zig

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2397,7 +2397,7 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) !void {
23972397

23982398
// Work around windows `AccessDenied` if any files within this
23992399
// directory are open by closing and reopening the file handles.
2400-
const need_writable_dance = w: {
2400+
const need_writable_dance: enum { no, lf_only, lf_and_debug } = w: {
24012401
if (builtin.os.tag == .windows) {
24022402
if (comp.bin_file) |lf| {
24032403
// 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 {
24102410
if (lf.file) |f| {
24112411
f.close();
24122412
lf.file = null;
2413-
break :w true;
2413+
2414+
if (lf.closeDebugInfo()) break :w .lf_and_debug;
2415+
break :w .lf_only;
24142416
}
24152417
}
24162418
}
2417-
break :w false;
2419+
break :w .no;
24182420
};
24192421

24202422
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 {
24412443
};
24422444

24432445
// Has to be after the `wholeCacheModeSetBinFilePath` above.
2444-
if (need_writable_dance) {
2445-
try lf.makeWritable();
2446+
switch (need_writable_dance) {
2447+
.no => {},
2448+
.lf_only => try lf.makeWritable(),
2449+
.lf_and_debug => {
2450+
try lf.makeWritable();
2451+
try lf.reopenDebugInfo();
2452+
},
24462453
}
24472454
}
24482455

src/link.zig

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,20 @@ pub const File = struct {
600600
}
601601
}
602602

603+
/// Some linkers create a separate file for debug info, which we might need to temporarily close
604+
/// when moving the compilation result directory due to the host OS not allowing moving a
605+
/// file/directory while a handle remains open.
606+
/// Returns `true` if a debug info file was closed. In that case, `reopenDebugInfo` may be called.
607+
pub fn closeDebugInfo(base: *File) bool {
608+
const macho = base.cast(.macho) orelse return false;
609+
return macho.closeDebugInfo();
610+
}
611+
612+
pub fn reopenDebugInfo(base: *File) !void {
613+
const macho = base.cast(.macho).?;
614+
return macho.reopenDebugInfo();
615+
}
616+
603617
pub fn makeExecutable(base: *File) !void {
604618
dev.check(.make_executable);
605619
const comp = base.comp;

src/link/MachO.zig

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3277,6 +3277,40 @@ const InitMetadataOptions = struct {
32773277
program_code_size_hint: u64,
32783278
};
32793279

3280+
pub fn closeDebugInfo(self: *MachO) bool {
3281+
const d_sym = &(self.d_sym orelse return false);
3282+
d_sym.deinit();
3283+
self.d_sym = null;
3284+
return true;
3285+
}
3286+
3287+
pub fn reopenDebugInfo(self: *MachO) !void {
3288+
assert(self.d_sym == null);
3289+
3290+
assert(!self.base.comp.config.use_llvm);
3291+
assert(self.base.comp.config.debug_format == .dwarf);
3292+
3293+
const gpa = self.base.comp.gpa;
3294+
const sep = fs.path.sep_str;
3295+
const d_sym_path = try std.fmt.allocPrint(
3296+
gpa,
3297+
"{s}.dSYM" ++ sep ++ "Contents" ++ sep ++ "Resources" ++ sep ++ "DWARF",
3298+
.{self.base.emit.sub_path},
3299+
);
3300+
defer gpa.free(d_sym_path);
3301+
3302+
var d_sym_bundle = try self.base.emit.root_dir.handle.makeOpenPath(d_sym_path, .{});
3303+
defer d_sym_bundle.close();
3304+
3305+
const d_sym_file = try d_sym_bundle.createFile(self.base.emit.sub_path, .{
3306+
.truncate = false,
3307+
.read = true,
3308+
});
3309+
3310+
self.d_sym = .{ .allocator = gpa, .file = d_sym_file };
3311+
try self.d_sym.?.initMetadata(self);
3312+
}
3313+
32803314
// TODO: move to ZigObject
32813315
fn initMetadata(self: *MachO, options: InitMetadataOptions) !void {
32823316
if (!self.base.isRelocatable()) {
@@ -3333,26 +3367,7 @@ fn initMetadata(self: *MachO, options: InitMetadataOptions) !void {
33333367
if (options.zo.dwarf) |*dwarf| {
33343368
// Create dSYM bundle.
33353369
log.debug("creating {s}.dSYM bundle", .{options.emit.sub_path});
3336-
3337-
const gpa = self.base.comp.gpa;
3338-
const sep = fs.path.sep_str;
3339-
const d_sym_path = try std.fmt.allocPrint(
3340-
gpa,
3341-
"{s}.dSYM" ++ sep ++ "Contents" ++ sep ++ "Resources" ++ sep ++ "DWARF",
3342-
.{options.emit.sub_path},
3343-
);
3344-
defer gpa.free(d_sym_path);
3345-
3346-
var d_sym_bundle = try options.emit.root_dir.handle.makeOpenPath(d_sym_path, .{});
3347-
defer d_sym_bundle.close();
3348-
3349-
const d_sym_file = try d_sym_bundle.createFile(options.emit.sub_path, .{
3350-
.truncate = false,
3351-
.read = true,
3352-
});
3353-
3354-
self.d_sym = .{ .allocator = gpa, .file = d_sym_file };
3355-
try self.d_sym.?.initMetadata(self);
3370+
try self.reopenDebugInfo();
33563371
try dwarf.initMetadata();
33573372
}
33583373
}

0 commit comments

Comments
 (0)