From e5aef96293336fa25e6f094bf82d189176e8d1a7 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 13 Sep 2020 22:54:16 -0700 Subject: [PATCH] stage2: CRT files retain locks on the build artifacts --- src-self-hosted/Compilation.zig | 17 ++++++++++++++--- src-self-hosted/glibc.zig | 5 ++++- src-self-hosted/link.zig | 18 ++++++++++++++++++ src-self-hosted/link/Elf.zig | 27 ++++++--------------------- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src-self-hosted/Compilation.zig b/src-self-hosted/Compilation.zig index c4eeaf7354..a3a99aee6f 100644 --- a/src-self-hosted/Compilation.zig +++ b/src-self-hosted/Compilation.zig @@ -70,13 +70,24 @@ libc_static_lib: ?[]const u8 = null, /// For example `Scrt1.o` and `libc.so.6`. These are populated after building libc from source, /// The set of needed CRT (C runtime) files differs depending on the target and compilation settings. /// The key is the basename, and the value is the absolute path to the completed build artifact. -crt_files: std.StringHashMapUnmanaged([]const u8) = .{}, +crt_files: std.StringHashMapUnmanaged(CRTFile) = .{}, /// Keeping track of this possibly open resource so we can close it later. owned_link_dir: ?std.fs.Dir, pub const InnerError = Module.InnerError; +pub const CRTFile = struct { + lock: std.cache_hash.Lock, + full_object_path: []const u8, + + fn deinit(self: *CRTFile, gpa: *Allocator) void { + self.lock.release(); + gpa.free(self.full_object_path); + self.* = undefined; + } +}; + /// For passing to a C compiler. pub const CSourceFile = struct { src_path: []const u8, @@ -625,7 +636,7 @@ pub fn destroy(self: *Compilation) void { var it = self.crt_files.iterator(); while (it.next()) |entry| { gpa.free(entry.key); - gpa.free(entry.value); + entry.value.deinit(gpa); } self.crt_files.deinit(gpa); } @@ -1512,7 +1523,7 @@ fn detectLibCFromLibCInstallation(arena: *Allocator, target: Target, lci: *const pub fn get_libc_crt_file(comp: *Compilation, arena: *Allocator, basename: []const u8) ![]const u8 { if (comp.wantBuildGLibCFromSource()) { - return comp.crt_files.get(basename).?; + return comp.crt_files.get(basename).?.full_object_path; } const lci = comp.bin_file.options.libc_installation orelse return error.LibCInstallationNotAvailable; const crt_dir_path = lci.crt_dir orelse return error.LibCInstallationMissingCRTDir; diff --git a/src-self-hosted/glibc.zig b/src-self-hosted/glibc.zig index 0f7fabdcac..cd25baef8c 100644 --- a/src-self-hosted/glibc.zig +++ b/src-self-hosted/glibc.zig @@ -648,5 +648,8 @@ fn build_libc_object(comp: *Compilation, basename: []const u8, c_source_file: Co try comp.gpa.dupe(u8, basename); // TODO obtain a lock on the artifact and put that in crt_files as well. - comp.crt_files.putAssumeCapacityNoClobber(basename, artifact_path); + comp.crt_files.putAssumeCapacityNoClobber(basename, .{ + .full_object_path = artifact_path, + .lock = sub_compilation.bin_file.toOwnedLock(), + }); } diff --git a/src-self-hosted/link.zig b/src-self-hosted/link.zig index 8158e7ee55..f75274eb58 100644 --- a/src-self-hosted/link.zig +++ b/src-self-hosted/link.zig @@ -90,6 +90,10 @@ pub const File = struct { /// this location, and then this path can be placed on the LLD linker line. intermediary_basename: ?[]const u8 = null, + /// Prevents other processes from clobbering files in the output directory + /// of this linking operation. + lock: ?std.cache_hash.Lock = null, + pub const LinkBlock = union { elf: Elf.TextBlock, coff: Coff.TextBlock, @@ -228,7 +232,21 @@ pub const File = struct { } } + pub fn releaseLock(self: *File) void { + if (self.lock) |*lock| { + lock.release(); + self.lock = null; + } + } + + pub fn toOwnedLock(self: *File) std.cache_hash.Lock { + const lock = self.lock.?; + self.lock = null; + return lock; + } + pub fn destroy(base: *File) void { + base.releaseLock(); if (base.file) |f| f.close(); if (base.intermediary_basename) |sub_path| base.allocator.free(sub_path); switch (base.tag) { diff --git a/src-self-hosted/link/Elf.zig b/src-self-hosted/link/Elf.zig index d419cbc297..e3a28a5fd4 100644 --- a/src-self-hosted/link/Elf.zig +++ b/src-self-hosted/link/Elf.zig @@ -123,9 +123,6 @@ dbg_info_decl_free_list: std.AutoHashMapUnmanaged(*TextBlock, void) = .{}, dbg_info_decl_first: ?*TextBlock = null, dbg_info_decl_last: ?*TextBlock = null, -/// Prevents other processes from clobbering the output file this is linking. -lock: ?std.cache_hash.Lock = null, - /// `alloc_num / alloc_den` is the factor of padding when allocating. const alloc_num = 4; const alloc_den = 3; @@ -290,21 +287,7 @@ pub fn createEmpty(gpa: *Allocator, options: link.Options) !*Elf { return self; } -pub fn releaseLock(self: *Elf) void { - if (self.lock) |*lock| { - lock.release(); - self.lock = null; - } -} - -pub fn toOwnedLock(self: *Elf) std.cache_hash.Lock { - const lock = self.lock.?; - self.lock = null; - return lock; -} - pub fn deinit(self: *Elf) void { - self.releaseLock(); self.sections.deinit(self.base.allocator); self.program_headers.deinit(self.base.allocator); self.shstrtab.deinit(self.base.allocator); @@ -1253,7 +1236,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void { const id_symlink_basename = "id.txt"; // We are about to obtain this lock, so here we give other processes a chance first. - self.releaseLock(); + self.base.releaseLock(); var ch = comp.cache_parent.obtain(); defer ch.deinit(); @@ -1302,14 +1285,16 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void { const digest = ch.final(); var prev_digest_buf: [digest.len]u8 = undefined; - const prev_digest: []u8 = directory.handle.readLink(id_symlink_basename, &prev_digest_buf) catch blk: { + const prev_digest: []u8 = directory.handle.readLink(id_symlink_basename, &prev_digest_buf) catch |err| blk: { + log.debug("ELF LLD new_digest={} readlink error: {}", .{digest, @errorName(err)}); // Handle this as a cache miss. mem.set(u8, &prev_digest_buf, 0); break :blk &prev_digest_buf; }; + log.debug("ELF LLD prev_digest={} new_digest={}", .{prev_digest, digest}); if (mem.eql(u8, prev_digest, &digest)) { // Hot diggity dog! The output binary is already there. - self.lock = ch.toOwnedLock(); + self.base.lock = ch.toOwnedLock(); return; } @@ -1611,7 +1596,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void { }; // We hang on to this lock so that the output file path can be used without // other processes clobbering it. - self.lock = ch.toOwnedLock(); + self.base.lock = ch.toOwnedLock(); } fn append_diagnostic(context: usize, ptr: [*]const u8, len: usize) callconv(.C) void {