From d94303be2bcee33e7efba22a186fd06eaa809707 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 2 Jan 2022 20:57:04 -0700 Subject: [PATCH] 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(-) diff --git a/src/Compilation.zig b/src/Compilation.zig index b01b4bbae423e46e228c0ae06391aa4d6d8eee76..cbb44c1e183a3c10a34182fd5634248342878ce6 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 a248fb17188ecd3eea20b8f45220ab976d1ff542..e29116476d06cb393ea58490eef8e5ad7cf40f9a 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 a720bdada2b9b17d6822f48d536a4f6105bf3158..5c33ce3b70080c5b7a8008c50fc95f5d8150ab19 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 e6788120a0503193c90c367a5fd0f1ece458381c..ec06c28b448a4b93f2b2146b6470588eedd9a47e 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 5f3771a3cb5b9136d4c1665f46d158c94ff7de85..105b012fbfb43f6da9b367f0ac4a0e3f9785ccbe 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 f970ffb5f206bfa22c7911d847fa4081043ae235..ed720080af02afec281ed4fcb252d8ae0e93dfae 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 5f8f48d1b19a003e17e633c3938330d882e5a924..220ab2f53c683a28519221434eb8366780c12643 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 -- 2.54.0