Commit 70d7d7e919

Andrew Kelley <andrew@ziglang.org>
2020-09-26 03:01:35
stage2: disable lld caching when output dir is owned by user
Normally when using LLD to link, Zig uses a file named "lld.id" in the same directory as the output binary which contains the hash of the link operation, allowing Zig to skip linking when the hash would be unchanged. In the case that the output binary is being emitted into a directory which is externally modified - essentially anything other than zig-cache - then this flag would be set to disable this machinery to avoid false positives. * Better defaults when using -fno-LLVM * Fix compiler_rt and libc static libraries were getting a .zig extension instead of .a extension. * when using the stage1 backend, put the object file next to the stage1.id file in the cache directory. this prevents an object file from polluting the cwd when using zig from the CLI.
1 parent 670e7d4
src/link/Elf.zig
@@ -23,6 +23,7 @@ const File = link.File;
 const build_options = @import("build_options");
 const target_util = @import("../target.zig");
 const glibc = @import("../glibc.zig");
+const Cache = @import("../Cache.zig");
 
 const default_entry_addr = 0x8000000;
 
@@ -1225,7 +1226,8 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void {
         const use_stage1 = build_options.is_stage1 and self.base.options.use_llvm;
         if (use_stage1) {
             const obj_basename = try std.fmt.allocPrint(arena, "{}.o", .{self.base.options.root_name});
-            const full_obj_path = try directory.join(arena, &[_][]const u8{obj_basename});
+            const o_directory = self.base.options.module.?.zig_cache_artifact_directory;
+            const full_obj_path = try o_directory.join(arena, &[_][]const u8{obj_basename});
             break :blk full_obj_path;
         }
 
@@ -1235,6 +1237,12 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void {
         break :blk full_obj_path;
     } else null;
 
+    const is_lib = self.base.options.output_mode == .Lib;
+    const is_dyn_lib = self.base.options.link_mode == .Dynamic and is_lib;
+    const is_exe_or_dyn_lib = is_dyn_lib or self.base.options.output_mode == .Exe;
+    const have_dynamic_linker = self.base.options.link_libc and
+        self.base.options.link_mode == .Dynamic and is_exe_or_dyn_lib;
+
     // Here we want to determine whether we can save time by not invoking LLD when the
     // output is unchanged. None of the linker options or the object files that are being
     // linked are in the hash that namespaces the directory we are outputting to. Therefore,
@@ -1245,78 +1253,78 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void {
     // our digest. If so, we can skip linking. Otherwise, we proceed with invoking LLD.
     const id_symlink_basename = "lld.id";
 
-    // We are about to obtain this lock, so here we give other processes a chance first.
-    self.base.releaseLock();
-
-    var ch = comp.cache_parent.obtain();
-    defer ch.deinit();
+    var man: Cache.Manifest = undefined;
+    defer if (!self.base.options.disable_lld_caching) man.deinit();
+
+    var digest: [Cache.hex_digest_len]u8 = undefined;
+
+    if (!self.base.options.disable_lld_caching) {
+        man = comp.cache_parent.obtain();
+
+        // We are about to obtain this lock, so here we give other processes a chance first.
+        self.base.releaseLock();
+
+        try man.addOptionalFile(self.base.options.linker_script);
+        try man.addOptionalFile(self.base.options.version_script);
+        try man.addListOfFiles(self.base.options.objects);
+        for (comp.c_object_table.items()) |entry| {
+            _ = try man.addFile(entry.key.status.success.object_path, null);
+        }
+        try man.addOptionalFile(module_obj_path);
+        // We can skip hashing libc and libc++ components that we are in charge of building from Zig
+        // installation sources because they are always a product of the compiler version + target information.
+        man.hash.addOptional(self.base.options.stack_size_override);
+        man.hash.addOptional(self.base.options.gc_sections);
+        man.hash.add(self.base.options.eh_frame_hdr);
+        man.hash.add(self.base.options.rdynamic);
+        man.hash.addListOfBytes(self.base.options.extra_lld_args);
+        man.hash.addListOfBytes(self.base.options.lib_dirs);
+        man.hash.addListOfBytes(self.base.options.rpath_list);
+        man.hash.add(self.base.options.each_lib_rpath);
+        man.hash.add(self.base.options.is_compiler_rt_or_libc);
+        man.hash.add(self.base.options.z_nodelete);
+        man.hash.add(self.base.options.z_defs);
+        if (self.base.options.link_libc) {
+            man.hash.add(self.base.options.libc_installation != null);
+            if (self.base.options.libc_installation) |libc_installation| {
+                man.hash.addBytes(libc_installation.crt_dir.?);
+            }
+            if (have_dynamic_linker) {
+                man.hash.addOptionalBytes(self.base.options.dynamic_linker);
+            }
+        }
+        if (is_dyn_lib) {
+            man.hash.addOptionalBytes(self.base.options.override_soname);
+            man.hash.addOptional(self.base.options.version);
+        }
+        man.hash.addListOfBytes(self.base.options.system_libs);
+        man.hash.addOptional(self.base.options.allow_shlib_undefined);
+        man.hash.add(self.base.options.bind_global_refs_locally);
 
-    const is_lib = self.base.options.output_mode == .Lib;
-    const is_dyn_lib = self.base.options.link_mode == .Dynamic and is_lib;
-    const is_exe_or_dyn_lib = is_dyn_lib or self.base.options.output_mode == .Exe;
-    const have_dynamic_linker = self.base.options.link_libc and
-        self.base.options.link_mode == .Dynamic and is_exe_or_dyn_lib;
+        // We don't actually care whether it's a cache hit or miss; we just need the digest and the lock.
+        _ = try man.hit();
+        digest = man.final();
 
-    try ch.addOptionalFile(self.base.options.linker_script);
-    try ch.addOptionalFile(self.base.options.version_script);
-    try ch.addListOfFiles(self.base.options.objects);
-    for (comp.c_object_table.items()) |entry| {
-        _ = try ch.addFile(entry.key.status.success.object_path, null);
-    }
-    try ch.addOptionalFile(module_obj_path);
-    // We can skip hashing libc and libc++ components that we are in charge of building from Zig
-    // installation sources because they are always a product of the compiler version + target information.
-    ch.hash.addOptional(self.base.options.stack_size_override);
-    ch.hash.addOptional(self.base.options.gc_sections);
-    ch.hash.add(self.base.options.eh_frame_hdr);
-    ch.hash.add(self.base.options.rdynamic);
-    ch.hash.addListOfBytes(self.base.options.extra_lld_args);
-    ch.hash.addListOfBytes(self.base.options.lib_dirs);
-    ch.hash.addListOfBytes(self.base.options.rpath_list);
-    ch.hash.add(self.base.options.each_lib_rpath);
-    ch.hash.add(self.base.options.is_compiler_rt_or_libc);
-    ch.hash.add(self.base.options.z_nodelete);
-    ch.hash.add(self.base.options.z_defs);
-    if (self.base.options.link_libc) {
-        ch.hash.add(self.base.options.libc_installation != null);
-        if (self.base.options.libc_installation) |libc_installation| {
-            ch.hash.addBytes(libc_installation.crt_dir.?);
-        }
-        if (have_dynamic_linker) {
-            ch.hash.addOptionalBytes(self.base.options.dynamic_linker);
+        var prev_digest_buf: [digest.len]u8 = undefined;
+        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.
+            break :blk prev_digest_buf[0..0];
+        };
+        if (mem.eql(u8, prev_digest, &digest)) {
+            log.debug("ELF LLD digest={} match - skipping invocation", .{digest});
+            // Hot diggity dog! The output binary is already there.
+            self.base.lock = man.toOwnedLock();
+            return;
         }
+        log.debug("ELF LLD prev_digest={} new_digest={}", .{prev_digest, digest});
+
+        // We are about to change the output file to be different, so we invalidate the build hash now.
+        directory.handle.deleteFile(id_symlink_basename) catch |err| switch (err) {
+            error.FileNotFound => {},
+            else => |e| return e,
+        };
     }
-    if (is_dyn_lib) {
-        ch.hash.addOptionalBytes(self.base.options.override_soname);
-        ch.hash.addOptional(self.base.options.version);
-    }
-    ch.hash.addListOfBytes(self.base.options.system_libs);
-    ch.hash.addOptional(self.base.options.allow_shlib_undefined);
-    ch.hash.add(self.base.options.bind_global_refs_locally);
-
-    // We don't actually care whether it's a cache hit or miss; we just need the digest and the lock.
-    _ = try ch.hit();
-    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 |err| blk: {
-        log.debug("ELF LLD new_digest={} readlink error: {}", .{digest, @errorName(err)});
-        // Handle this as a cache miss.
-        break :blk prev_digest_buf[0..0];
-    };
-    if (mem.eql(u8, prev_digest, &digest)) {
-        log.debug("ELF LLD digest={} match - skipping invocation", .{digest});
-        // Hot diggity dog! The output binary is already there.
-        self.base.lock = ch.toOwnedLock();
-        return;
-    }
-    log.debug("ELF LLD prev_digest={} new_digest={}", .{prev_digest, digest});
-
-    // We are about to change the output file to be different, so we invalidate the build hash now.
-    directory.handle.deleteFile(id_symlink_basename) catch |err| switch (err) {
-        error.FileNotFound => {},
-        else => |e| return e,
-    };
 
     const target = self.base.options.target;
     const is_obj = self.base.options.output_mode == .Obj;
@@ -1620,18 +1628,20 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void {
         std.log.warn("unexpected LLD stderr: {}", .{stderr_context.data.items});
     }
 
-    // Update the dangling symlink with the digest. If it fails we can continue; it only
-    // means that the next invocation will have an unnecessary cache miss.
-    directory.handle.symLink(&digest, id_symlink_basename, .{}) catch |err| {
-        std.log.warn("failed to save linking hash digest symlink: {}", .{@errorName(err)});
-    };
-    // Again failure here only means an unnecessary cache miss.
-    ch.writeManifest() catch |err| {
-        std.log.warn("failed to write cache manifest when linking: {}", .{ @errorName(err) });
-    };
-    // We hang on to this lock so that the output file path can be used without
-    // other processes clobbering it.
-    self.base.lock = ch.toOwnedLock();
+    if (!self.base.options.disable_lld_caching) {
+        // Update the dangling symlink with the digest. If it fails we can continue; it only
+        // means that the next invocation will have an unnecessary cache miss.
+        directory.handle.symLink(&digest, id_symlink_basename, .{}) catch |err| {
+            std.log.warn("failed to save linking hash digest symlink: {}", .{@errorName(err)});
+        };
+        // Again failure here only means an unnecessary cache miss.
+        man.writeManifest() catch |err| {
+            std.log.warn("failed to write cache manifest when linking: {}", .{ @errorName(err) });
+        };
+        // We hang on to this lock so that the output file path can be used without
+        // other processes clobbering it.
+        self.base.lock = man.toOwnedLock();
+    }
 }
 
 const LLDContext = struct {
src/Compilation.zig
@@ -3,10 +3,11 @@ const Compilation = @This();
 const std = @import("std");
 const mem = std.mem;
 const Allocator = std.mem.Allocator;
-const Value = @import("value.zig").Value;
 const assert = std.debug.assert;
 const log = std.log.scoped(.compilation);
 const Target = std.Target;
+
+const Value = @import("value.zig").Value;
 const target_util = @import("target.zig");
 const Package = @import("Package.zig");
 const link = @import("link.zig");
@@ -286,6 +287,13 @@ pub const InitOptions = struct {
     emit_h: ?EmitLoc = null,
     link_mode: ?std.builtin.LinkMode = null,
     dll_export_fns: ?bool = false,
+    /// Normally when using LLD to link, Zig uses a file named "lld.id" in the
+    /// same directory as the output binary which contains the hash of the link
+    /// operation, allowing Zig to skip linking when the hash would be unchanged.
+    /// In the case that the output binary is being emitted into a directory which
+    /// is externally modified - essentially anything other than zig-cache - then
+    /// this flag would be set to disable this machinery to avoid false positives.
+    disable_lld_caching: bool = false,
     object_format: ?std.builtin.ObjectFormat = null,
     optimize_mode: std.builtin.Mode = .Debug,
     keep_source_files_loaded: bool = false,
@@ -371,6 +379,26 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation {
 
         const ofmt = options.object_format orelse options.target.getObjectFormat();
 
+        // Make a decision on whether to use LLVM or our own backend.
+        const use_llvm = if (options.use_llvm) |explicit| explicit else blk: {
+            // If we have no zig code to compile, no need for LLVM.
+            if (options.root_pkg == null)
+                break :blk false;
+
+            // If we are the stage1 compiler, we depend on the stage1 c++ llvm backend
+            // to compile zig code.
+            if (build_options.is_stage1)
+                break :blk true;
+
+            // We would want to prefer LLVM for release builds when it is available, however
+            // we don't have an LLVM backend yet :)
+            // We would also want to prefer LLVM for architectures that we don't have self-hosted support for too.
+            break :blk false;
+        };
+        if (!use_llvm and options.machine_code_model != .default) {
+            return error.MachineCodeModelNotSupported;
+        }
+
         // Make a decision on whether to use LLD or our own linker.
         const use_lld = if (options.use_lld) |explicit| explicit else blk: {
             if (!build_options.have_llvm)
@@ -393,7 +421,7 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation {
                 break :blk true;
             }
 
-            if (build_options.is_stage1) {
+            if (use_llvm) {
                 // If stage1 generates an object file, self-hosted linker is not
                 // yet sophisticated enough to handle that.
                 break :blk options.root_pkg != null;
@@ -402,25 +430,6 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation {
             break :blk false;
         };
 
-        // Make a decision on whether to use LLVM or our own backend.
-        const use_llvm = if (options.use_llvm) |explicit| explicit else blk: {
-            // If we have no zig code to compile, no need for LLVM.
-            if (options.root_pkg == null)
-                break :blk false;
-
-            // If we are the stage1 compiler, we depend on the stage1 c++ llvm backend
-            // to compile zig code.
-            if (build_options.is_stage1)
-                break :blk true;
-
-            // We would want to prefer LLVM for release builds when it is available, however
-            // we don't have an LLVM backend yet :)
-            // We would also want to prefer LLVM for architectures that we don't have self-hosted support for too.
-            break :blk false;
-        };
-        if (!use_llvm and options.machine_code_model != .default) {
-            return error.MachineCodeModelNotSupported;
-        }
 
         const link_libc = options.link_libc or
             (is_exe_or_dyn_lib and target_util.osRequiresLibC(options.target));
@@ -720,6 +729,7 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation {
             .llvm_cpu_features = llvm_cpu_features,
             .is_compiler_rt_or_libc = options.is_compiler_rt_or_libc,
             .each_lib_rpath = options.each_lib_rpath orelse false,
+            .disable_lld_caching = options.disable_lld_caching,
         });
         errdefer bin_file.destroy();
         comp.* = .{
@@ -2288,7 +2298,7 @@ pub fn updateSubCompilation(sub_compilation: *Compilation) !void {
     }
 }
 
-fn buildStaticLibFromZig(comp: *Compilation, basename: []const u8, out: *?CRTFile) !void {
+fn buildStaticLibFromZig(comp: *Compilation, src_basename: []const u8, out: *?CRTFile) !void {
     const tracy = trace(@src());
     defer tracy.end();
 
@@ -2304,12 +2314,20 @@ fn buildStaticLibFromZig(comp: *Compilation, basename: []const u8, out: *?CRTFil
             .path = special_path,
             .handle = special_dir,
         },
-        .root_src_path = basename,
+        .root_src_path = src_basename,
     };
+    const root_name = mem.split(src_basename, ".").next().?;
+    const target = comp.getTarget();
+    const bin_basename = try std.zig.binNameAlloc(comp.gpa, .{
+        .root_name = root_name,
+        .target = target,
+        .output_mode = .Lib,
+    });
+    defer comp.gpa.free(bin_basename);
 
     const emit_bin = Compilation.EmitLoc{
         .directory = null, // Put it in the cache directory.
-        .basename = basename,
+        .basename = bin_basename,
     };
     const optimize_mode: std.builtin.Mode = blk: {
         if (comp.is_test)
@@ -2323,8 +2341,8 @@ fn buildStaticLibFromZig(comp: *Compilation, basename: []const u8, out: *?CRTFil
         .global_cache_directory = comp.global_cache_directory,
         .local_cache_directory = comp.global_cache_directory,
         .zig_lib_directory = comp.zig_lib_directory,
-        .target = comp.getTarget(),
-        .root_name = mem.split(basename, ".").next().?,
+        .target = target,
+        .root_name = root_name,
         .root_pkg = &root_pkg,
         .output_mode = .Lib,
         .rand = comp.rand,
@@ -2358,7 +2376,9 @@ fn buildStaticLibFromZig(comp: *Compilation, basename: []const u8, out: *?CRTFil
 
     assert(out.* == null);
     out.* = Compilation.CRTFile{
-        .full_object_path = try sub_compilation.bin_file.options.directory.join(comp.gpa, &[_][]const u8{basename}),
+        .full_object_path = try sub_compilation.bin_file.options.directory.join(comp.gpa, &[_][]const u8{
+            sub_compilation.bin_file.options.sub_path,
+        }),
         .lock = sub_compilation.bin_file.toOwnedLock(),
     };
 }
@@ -2461,7 +2481,7 @@ fn updateStage1Module(comp: *Compilation) !void {
     ) orelse return error.OutOfMemory;
 
     const stage1_pkg = try createStage1Pkg(arena, "root", mod.root_pkg, null);
-    const output_dir = comp.bin_file.options.directory.path orelse ".";
+    const output_dir = directory.path orelse ".";
     const test_filter = comp.test_filter orelse ""[0..0];
     const test_name_prefix = comp.test_name_prefix orelse ""[0..0];
     stage1_module.* = .{
@@ -2617,13 +2637,11 @@ pub fn build_crt_file(
     try sub_compilation.updateSubCompilation();
 
     try comp.crt_files.ensureCapacity(comp.gpa, comp.crt_files.count() + 1);
-    const artifact_path = if (sub_compilation.bin_file.options.directory.path) |p|
-        try std.fs.path.join(comp.gpa, &[_][]const u8{ p, basename })
-    else
-        try comp.gpa.dupe(u8, basename);
 
     comp.crt_files.putAssumeCapacityNoClobber(basename, .{
-        .full_object_path = artifact_path,
+        .full_object_path = try sub_compilation.bin_file.options.directory.join(comp.gpa, &[_][]const u8{
+            sub_compilation.bin_file.options.sub_path,
+        }),
         .lock = sub_compilation.bin_file.toOwnedLock(),
     });
 }
src/link.zig
@@ -66,6 +66,7 @@ pub const Options = struct {
     error_return_tracing: bool,
     is_compiler_rt_or_libc: bool,
     each_lib_rpath: bool,
+    disable_lld_caching: bool,
     gc_sections: ?bool = null,
     allow_shlib_undefined: ?bool = null,
     linker_script: ?[]const u8 = null,
src/main.zig
@@ -1425,6 +1425,7 @@ pub fn buildOutputType(
         .test_evented_io = test_evented_io,
         .test_filter = test_filter,
         .test_name_prefix = test_name_prefix,
+        .disable_lld_caching = !have_enable_cache,
     }) catch |err| {
         fatal("unable to create compilation: {}", .{@errorName(err)});
     };
BRANCH_TODO
@@ -1,7 +1,6 @@
+ * make sure that `zig cc -o hello hello.c -target native-native-musl` and `zig build-exe hello.zig -lc -target native-native-musl` will share the same libc build.
+ * zig cc as a preprocessor (-E)
  * tests passing with -Dskip-non-native
- * make sure zig cc works
-   - using it as a preprocessor (-E)
-   - try building some software
  * `-ftime-report`
  *  -fstack-report               print stack size diagnostics\n"
  *  -fdump-analysis              write analysis.json file with type information\n"
@@ -15,14 +14,12 @@
  * MachO LLD linking
  * COFF LLD linking
  * WASM LLD linking
- * skip LLD caching when bin directory is not in the cache (so we don't put `id.txt` into the cwd)
-   (maybe make it an explicit option and have main.zig disable it)
-   - make sure that `zig cc -o hello hello.c -target native-native-musl` and `zig build-exe hello.zig -lc -target native-native-musl` will share the same libc build.
  * audit the CLI options for stage2
  * audit the base cache hash
  * On operating systems that support it, do an execve for `zig test` and `zig run` rather than child process.
  * restore error messages for stage2_add_link_lib
  * windows CUSTOMBUILD : error : unable to build compiler_rt: FileNotFound [D:\a\1\s\build\zig_install_lib_files.vcxproj]
+ * try building some software with zig cc
 
  * implement proper parsing of clang stderr/stdout and exposing compile errors with the Compilation API
  * implement proper parsing of LLD stderr/stdout and exposing compile errors with the Compilation API
@@ -59,3 +56,5 @@
  * close the --pkg-begin --pkg-end Package directory handles
  * make std.Progress support multithreaded
  * update musl.zig static data to use native path separator in static data rather than replacing '/' at runtime
+ * linking hello world with LLD, lld is silently calling exit(1) instead of reporting ok=false. when run standalone the error message is: ld.lld: error: section [index 3] has a sh_offset (0x57000) + sh_size (0x68) that is greater than the file size (0x57060)
+