From d94303be2bcee33e7efba22a186fd06eaa809707 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 2 Jan 2022 20:57:04 -0700 Subject: stage2: introduce renameTmpIntoCache into the linker API Doc comments reproduced here: This function is called by the frontend before flush(). It communicates that `options.bin_file.emit` directory needs to be renamed from `[zig-cache]/tmp/[random]` to `[zig-cache]/o/[digest]`. The frontend would like to simply perform a file system rename, however, some linker backends care about the file paths of the objects they are linking. So this function call tells linker backends to rename the paths of object files to observe the new directory path. Linker backends which do not have this requirement can fall back to the simple implementation at the bottom of this function. This function is only called when CacheMode is `whole`. This solves stack trace regressions on Windows and macOS because the linker backends do not observe object file paths until flush(). --- src/Compilation.zig | 84 +++++++++++++++++++++++++++++++--------------------- src/codegen/llvm.zig | 10 ++++--- src/link.zig | 59 +++++++++++++++++++++++++++++------- src/link/Coff.zig | 21 ++++++++----- src/link/Elf.zig | 26 +++++++++++----- src/link/MachO.zig | 23 +++++++++----- src/link/Wasm.zig | 22 +++++++++----- 7 files changed, 166 insertions(+), 79 deletions(-) (limited to 'src') diff --git a/src/Compilation.zig b/src/Compilation.zig index b01b4bbae4..cbb44c1e18 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -2093,39 +2093,15 @@ pub fn update(comp: *Compilation) !void { } if (comp.totalErrorCount() != 0) { - // Skip flushing. + // Skip flushing and keep source files loaded for error reporting. comp.link_error_flags = .{}; return; } - // This is needed before reading the error flags. - try comp.bin_file.flush(comp); - comp.link_error_flags = comp.bin_file.errorFlags(); - - if (!use_stage1) { - if (comp.bin_file.options.module) |module| { - try link.File.C.flushEmitH(module); - } - } - // Flush takes care of -femit-bin, but we still have -femit-llvm-ir, -femit-llvm-bc, and // -femit-asm to handle, in the case of C objects. comp.emitOthers(); - // If there are any errors, we anticipate the source files being loaded - // to report error messages. Otherwise we unload all source files to save memory. - // The ZIR needs to stay loaded in memory because (1) Decl objects contain references - // to it, and (2) generic instantiations, comptime calls, inline calls will need - // to reference the ZIR. - if (!comp.keep_source_files_loaded) { - if (comp.bin_file.options.module) |module| { - for (module.import_table.values()) |file| { - file.unloadTree(comp.gpa); - file.unloadSource(comp.gpa); - } - } - } - if (comp.whole_cache_manifest != null) { const digest = man.final(); @@ -2139,23 +2115,63 @@ pub fn update(comp: *Compilation) !void { const o_sub_path = try std.fs.path.join(comp.gpa, &[_][]const u8{ "o", &digest }); defer comp.gpa.free(o_sub_path); - try std.fs.rename( - comp.local_cache_directory.handle, - tmp_dir_sub_path, - comp.local_cache_directory.handle, - o_sub_path, - ); + try comp.bin_file.renameTmpIntoCache(comp.local_cache_directory, tmp_dir_sub_path, o_sub_path); + comp.wholeCacheModeSetBinFilePath(&digest); + + // This is intentionally sandwiched between renameTmpIntoCache() and writeManifest(). + if (comp.bin_file.options.module) |module| { + // We need to set the zig_cache_artifact_directory for -femit-asm, -femit-llvm-ir, + // etc to know where to output to. + var artifact_dir = try comp.local_cache_directory.handle.openDir(o_sub_path, .{}); + defer artifact_dir.close(); + + var dir_path = try comp.local_cache_directory.join(comp.gpa, &.{o_sub_path}); + defer comp.gpa.free(dir_path); + + module.zig_cache_artifact_directory = .{ + .handle = artifact_dir, + .path = dir_path, + }; + + try comp.flush(); + } else { + try comp.flush(); + } // Failure here only means an unnecessary cache miss. man.writeManifest() catch |err| { log.warn("failed to write cache manifest: {s}", .{@errorName(err)}); }; - comp.wholeCacheModeSetBinFilePath(&digest); - assert(comp.bin_file.lock == null); comp.bin_file.lock = man.toOwnedLock(); - return; + } else { + try comp.flush(); + } + + // Unload all source files to save memory. + // The ZIR needs to stay loaded in memory because (1) Decl objects contain references + // to it, and (2) generic instantiations, comptime calls, inline calls will need + // to reference the ZIR. + if (!comp.keep_source_files_loaded) { + if (comp.bin_file.options.module) |module| { + for (module.import_table.values()) |file| { + file.unloadTree(comp.gpa); + file.unloadSource(comp.gpa); + } + } + } +} + +fn flush(comp: *Compilation) !void { + try comp.bin_file.flush(comp); // This is needed before reading the error flags. + comp.link_error_flags = comp.bin_file.errorFlags(); + + const use_stage1 = build_options.is_stage1 and comp.bin_file.options.use_stage1; + if (!use_stage1) { + if (comp.bin_file.options.module) |module| { + try link.File.C.flushEmitH(module); + } } } diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index a248fb1718..e29116476d 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -324,10 +324,12 @@ pub const Object = struct { const mod = comp.bin_file.options.module.?; const cache_dir = mod.zig_cache_artifact_directory; - const emit_bin_path: ?[*:0]const u8 = if (comp.bin_file.options.emit) |emit| - try emit.directory.joinZ(arena, &[_][]const u8{self.sub_path}) - else - null; + const emit_bin_path: ?[*:0]const u8 = if (comp.bin_file.options.emit) |emit| blk: { + const full_out_path = try emit.directory.join(arena, &[_][]const u8{emit.sub_path}); + break :blk try std.fs.path.joinZ(arena, &.{ + std.fs.path.dirname(full_out_path).?, self.sub_path, + }); + } else null; const emit_asm_path = try locPath(arena, comp.emit_asm, cache_dir); const emit_llvm_ir_path = try locPath(arena, comp.emit_llvm_ir, cache_dir); diff --git a/src/link.zig b/src/link.zig index a720bdada2..5c33ce3b70 100644 --- a/src/link.zig +++ b/src/link.zig @@ -636,6 +636,36 @@ pub const File = struct { } } + /// This function is called by the frontend before flush(). It communicates that + /// `options.bin_file.emit` directory needs to be renamed from + /// `[zig-cache]/tmp/[random]` to `[zig-cache]/o/[digest]`. + /// The frontend would like to simply perform a file system rename, however, + /// some linker backends care about the file paths of the objects they are linking. + /// So this function call tells linker backends to rename the paths of object files + /// to observe the new directory path. + /// Linker backends which do not have this requirement can fall back to the simple + /// implementation at the bottom of this function. + /// This function is only called when CacheMode is `whole`. + pub fn renameTmpIntoCache( + base: *File, + cache_directory: Compilation.Directory, + tmp_dir_sub_path: []const u8, + o_sub_path: []const u8, + ) !void { + // So far, none of the linker backends need to respond to this event, however, + // it makes sense that they might want to. So we leave this mechanism here + // for now. Once the linker backends get more mature, if it turns out this + // is not needed we can refactor this into having the frontend do the rename + // directly, and remove this function from link.zig. + _ = base; + try std.fs.rename( + cache_directory.handle, + tmp_dir_sub_path, + cache_directory.handle, + o_sub_path, + ); + } + pub fn linkAsArchive(base: *File, comp: *Compilation) !void { const tracy = trace(@src()); defer tracy.end(); @@ -645,9 +675,11 @@ pub const File = struct { const arena = arena_allocator.allocator(); const directory = base.options.emit.?.directory; // Just an alias to make it shorter to type. + const full_out_path = try directory.join(arena, &[_][]const u8{base.options.emit.?.sub_path}); + const full_out_path_z = try arena.dupeZ(u8, full_out_path); - // If there is no Zig code to compile, then we should skip flushing the output file because it - // will not be part of the linker line anyway. + // If there is no Zig code to compile, then we should skip flushing the output file + // because it will not be part of the linker line anyway. const module_obj_path: ?[]const u8 = if (base.options.module) |module| blk: { const use_stage1 = build_options.is_stage1 and base.options.use_stage1; if (use_stage1) { @@ -656,20 +688,28 @@ pub const File = struct { .target = base.options.target, .output_mode = .Obj, }); - const o_directory = module.zig_cache_artifact_directory; - const full_obj_path = try o_directory.join(arena, &[_][]const u8{obj_basename}); - break :blk full_obj_path; + switch (base.options.cache_mode) { + .incremental => break :blk try module.zig_cache_artifact_directory.join( + arena, + &[_][]const u8{obj_basename}, + ), + .whole => break :blk try fs.path.join(arena, &.{ + fs.path.dirname(full_out_path_z).?, obj_basename, + }), + } } if (base.options.object_format == .macho) { try base.cast(MachO).?.flushObject(comp); } else { try base.flushModule(comp); } - const obj_basename = base.intermediary_basename.?; - const full_obj_path = try directory.join(arena, &[_][]const u8{obj_basename}); - break :blk full_obj_path; + break :blk try fs.path.join(arena, &.{ + fs.path.dirname(full_out_path_z).?, base.intermediary_basename.?, + }); } else null; + log.debug("module_obj_path={s}", .{if (module_obj_path) |s| s else "(null)"}); + const compiler_rt_path: ?[]const u8 = if (base.options.include_compiler_rt) comp.compiler_rt_obj.?.full_object_path else @@ -742,9 +782,6 @@ pub const File = struct { object_files.appendAssumeCapacity(try arena.dupeZ(u8, p)); } - const full_out_path = try directory.join(arena, &[_][]const u8{base.options.emit.?.sub_path}); - const full_out_path_z = try arena.dupeZ(u8, full_out_path); - if (base.options.verbose_link) { std.debug.print("ar rcs {s}", .{full_out_path_z}); for (object_files.items) |arg| { diff --git a/src/link/Coff.zig b/src/link/Coff.zig index e6788120a0..ec06c28b44 100644 --- a/src/link/Coff.zig +++ b/src/link/Coff.zig @@ -880,6 +880,7 @@ fn linkWithLLD(self: *Coff, comp: *Compilation) !void { const arena = arena_allocator.allocator(); const directory = self.base.options.emit.?.directory; // Just an alias to make it shorter to type. + const full_out_path = try directory.join(arena, &[_][]const u8{self.base.options.emit.?.sub_path}); // If there is no Zig code to compile, then we should skip flushing the output file because it // will not be part of the linker line anyway. @@ -891,15 +892,22 @@ fn linkWithLLD(self: *Coff, comp: *Compilation) !void { .target = self.base.options.target, .output_mode = .Obj, }); - const o_directory = module.zig_cache_artifact_directory; - const full_obj_path = try o_directory.join(arena, &[_][]const u8{obj_basename}); - break :blk full_obj_path; + switch (self.base.options.cache_mode) { + .incremental => break :blk try module.zig_cache_artifact_directory.join( + arena, + &[_][]const u8{obj_basename}, + ), + .whole => break :blk try fs.path.join(arena, &.{ + fs.path.dirname(full_out_path).?, obj_basename, + }), + } } try self.flushModule(comp); - const obj_basename = self.base.intermediary_basename.?; - const full_obj_path = try directory.join(arena, &[_][]const u8{obj_basename}); - break :blk full_obj_path; + + break :blk try fs.path.join(arena, &.{ + fs.path.dirname(full_out_path).?, self.base.intermediary_basename.?, + }); } else null; const is_lib = self.base.options.output_mode == .Lib; @@ -978,7 +986,6 @@ fn linkWithLLD(self: *Coff, comp: *Compilation) !void { }; } - const full_out_path = try directory.join(arena, &[_][]const u8{self.base.options.emit.?.sub_path}); if (self.base.options.output_mode == .Obj) { // LLD's COFF driver does not support the equivalent of `-r` so we do a simple file copy // here. TODO: think carefully about how we can avoid this redundant operation when doing diff --git a/src/link/Elf.zig b/src/link/Elf.zig index 5f3771a3cb..105b012fbf 100644 --- a/src/link/Elf.zig +++ b/src/link/Elf.zig @@ -297,6 +297,7 @@ pub fn createEmpty(gpa: Allocator, options: link.Options) !*Elf { else => return error.UnsupportedELFArchitecture, }; const self = try gpa.create(Elf); + errdefer gpa.destroy(self); self.* = .{ .base = .{ .tag = .elf, @@ -306,6 +307,9 @@ pub fn createEmpty(gpa: Allocator, options: link.Options) !*Elf { }, .ptr_width = ptr_width, }; + // TODO get rid of the sub_path parameter to LlvmObject.create + // and create the llvm_object here. Also openPath needs to + // not override this field or there will be a memory leak. return self; } @@ -1298,6 +1302,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void { const arena = arena_allocator.allocator(); const directory = self.base.options.emit.?.directory; // Just an alias to make it shorter to type. + const full_out_path = try directory.join(arena, &[_][]const u8{self.base.options.emit.?.sub_path}); // If there is no Zig code to compile, then we should skip flushing the output file because it // will not be part of the linker line anyway. @@ -1309,15 +1314,22 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void { .target = self.base.options.target, .output_mode = .Obj, }); - const o_directory = module.zig_cache_artifact_directory; - const full_obj_path = try o_directory.join(arena, &[_][]const u8{obj_basename}); - break :blk full_obj_path; + switch (self.base.options.cache_mode) { + .incremental => break :blk try module.zig_cache_artifact_directory.join( + arena, + &[_][]const u8{obj_basename}, + ), + .whole => break :blk try fs.path.join(arena, &.{ + fs.path.dirname(full_out_path).?, obj_basename, + }), + } } try self.flushModule(comp); - const obj_basename = self.base.intermediary_basename.?; - const full_obj_path = try directory.join(arena, &[_][]const u8{obj_basename}); - break :blk full_obj_path; + + break :blk try fs.path.join(arena, &.{ + fs.path.dirname(full_out_path).?, self.base.intermediary_basename.?, + }); } else null; const is_obj = self.base.options.output_mode == .Obj; @@ -1434,8 +1446,6 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void { }; } - const full_out_path = try directory.join(arena, &[_][]const u8{self.base.options.emit.?.sub_path}); - // Due to a deficiency in LLD, we need to special-case BPF to a simple file copy when generating // relocatables. Normally, we would expect `lld -r` to work. However, because LLD wants to resolve // BPF relocations which it shouldn't, it fails before even generating the relocatable. diff --git a/src/link/MachO.zig b/src/link/MachO.zig index f970ffb5f2..ed720080af 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -423,6 +423,7 @@ pub fn flushModule(self: *MachO, comp: *Compilation) !void { const arena = arena_allocator.allocator(); const directory = self.base.options.emit.?.directory; // Just an alias to make it shorter to type. + const full_out_path = try directory.join(arena, &[_][]const u8{self.base.options.emit.?.sub_path}); // If there is no Zig code to compile, then we should skip flushing the output file because it // will not be part of the linker line anyway. @@ -433,15 +434,24 @@ pub fn flushModule(self: *MachO, comp: *Compilation) !void { .target = self.base.options.target, .output_mode = .Obj, }); - const o_directory = module.zig_cache_artifact_directory; - const full_obj_path = try o_directory.join(arena, &[_][]const u8{obj_basename}); - break :blk full_obj_path; + switch (self.base.options.cache_mode) { + .incremental => break :blk try module.zig_cache_artifact_directory.join( + arena, + &[_][]const u8{obj_basename}, + ), + .whole => break :blk try fs.path.join(arena, &.{ + fs.path.dirname(full_out_path).?, obj_basename, + }), + } } const obj_basename = self.base.intermediary_basename orelse break :blk null; + try self.flushObject(comp); - const full_obj_path = try directory.join(arena, &[_][]const u8{obj_basename}); - break :blk full_obj_path; + + break :blk try fs.path.join(arena, &.{ + fs.path.dirname(full_out_path).?, obj_basename, + }); } else null; const is_lib = self.base.options.output_mode == .Lib; @@ -534,7 +544,6 @@ pub fn flushModule(self: *MachO, comp: *Compilation) !void { else => |e| return e, }; } - const full_out_path = try directory.join(arena, &[_][]const u8{self.base.options.emit.?.sub_path}); if (self.base.options.output_mode == .Obj) { // LLD's MachO driver does not support the equivalent of `-r` so we do a simple file copy @@ -1269,7 +1278,7 @@ fn parseInputFiles(self: *MachO, files: []const []const u8, syslibroot: ?[]const for (files) |file_name| { const full_path = full_path: { var buffer: [fs.MAX_PATH_BYTES]u8 = undefined; - const path = try std.fs.realpath(file_name, &buffer); + const path = try fs.realpath(file_name, &buffer); break :full_path try self.base.allocator.dupe(u8, path); }; defer self.base.allocator.free(full_path); diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index 5f8f48d1b1..220ab2f53c 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -1050,6 +1050,7 @@ fn linkWithLLD(self: *Wasm, comp: *Compilation) !void { const arena = arena_allocator.allocator(); const directory = self.base.options.emit.?.directory; // Just an alias to make it shorter to type. + const full_out_path = try directory.join(arena, &[_][]const u8{self.base.options.emit.?.sub_path}); // If there is no Zig code to compile, then we should skip flushing the output file because it // will not be part of the linker line anyway. @@ -1061,15 +1062,22 @@ fn linkWithLLD(self: *Wasm, comp: *Compilation) !void { .target = self.base.options.target, .output_mode = .Obj, }); - const o_directory = module.zig_cache_artifact_directory; - const full_obj_path = try o_directory.join(arena, &[_][]const u8{obj_basename}); - break :blk full_obj_path; + switch (self.base.options.cache_mode) { + .incremental => break :blk try module.zig_cache_artifact_directory.join( + arena, + &[_][]const u8{obj_basename}, + ), + .whole => break :blk try fs.path.join(arena, &.{ + fs.path.dirname(full_out_path).?, obj_basename, + }), + } } try self.flushModule(comp); - const obj_basename = self.base.intermediary_basename.?; - const full_obj_path = try directory.join(arena, &[_][]const u8{obj_basename}); - break :blk full_obj_path; + + break :blk try fs.path.join(arena, &.{ + fs.path.dirname(full_out_path).?, self.base.intermediary_basename.?, + }); } else null; const is_obj = self.base.options.output_mode == .Obj; @@ -1143,8 +1151,6 @@ fn linkWithLLD(self: *Wasm, comp: *Compilation) !void { }; } - const full_out_path = try directory.join(arena, &[_][]const u8{self.base.options.emit.?.sub_path}); - if (self.base.options.output_mode == .Obj) { // LLD's WASM driver does not support the equivalent of `-r` so we do a simple file copy // here. TODO: think carefully about how we can avoid this redundant operation when doing -- cgit v1.2.3