Commit b7c3ebcb9e

Jakub Konka <kubkon@jakubkonka.com>
2020-11-06 10:58:49
Rely on ZIG_SYSTEM_LINKER_HACK instead of input flags
1 parent e0e3cea
lib/std/special/build_runner.zig
@@ -112,8 +112,6 @@ pub fn main() !void {
                 builder.verbose_cc = true;
             } else if (mem.eql(u8, arg, "--verbose-llvm-cpu-features")) {
                 builder.verbose_llvm_cpu_features = true;
-            } else if (mem.eql(u8, arg, "--system-linker-hack")) {
-                builder.system_linker_hack = true;
             } else if (mem.eql(u8, arg, "--")) {
                 builder.args = argsRest(args, arg_idx);
                 break;
@@ -215,7 +213,6 @@ fn usage(builder: *Builder, already_ran_build: bool, out_stream: anytype) !void
         \\  --verbose-cimport           Enable compiler debug output for C imports
         \\  --verbose-cc                Enable compiler debug output for C compilation
         \\  --verbose-llvm-cpu-features Enable compiler debug output for LLVM CPU features
-        \\  --system-linker-hack        Use system linker LD instead of LLD (requires LLVM enabled)
         \\
     );
 }
lib/std/build.zig
@@ -67,7 +67,6 @@ pub const Builder = struct {
     vcpkg_root: VcpkgRoot,
     pkg_config_pkg_list: ?(PkgConfigError![]const PkgConfigPkg) = null,
     args: ?[][]const u8 = null,
-    system_linker_hack: bool,
 
     const PkgConfigError = error{
         PkgConfigCrashed,
@@ -174,7 +173,6 @@ pub const Builder = struct {
             .install_path = undefined,
             .vcpkg_root = VcpkgRoot{ .Unattempted = {} },
             .args = null,
-            .system_linker_hack = false,
         };
         try self.top_level_steps.append(&self.install_tls);
         try self.top_level_steps.append(&self.uninstall_tls);
@@ -1239,7 +1237,6 @@ pub const LibExeObjStep = struct {
     packages: ArrayList(Pkg),
     build_options_contents: std.ArrayList(u8),
     build_options_artifact_args: std.ArrayList(BuildOptionArtifactArg),
-    system_linker_hack: bool = false,
 
     object_src: []const u8,
 
@@ -1900,10 +1897,6 @@ pub const LibExeObjStep = struct {
         self.exec_cmd_args = args;
     }
 
-    pub fn enableSystemLinkerHack(self: *LibExeObjStep) void {
-        self.system_linker_hack = true;
-    }
-
     fn linkLibraryOrObject(self: *LibExeObjStep, other: *LibExeObjStep) void {
         self.step.dependOn(&other.step);
         self.link_objects.append(LinkObject{ .OtherStep = other }) catch unreachable;
@@ -2076,7 +2069,6 @@ pub const LibExeObjStep = struct {
         if (builder.verbose_link or self.verbose_link) zig_args.append("--verbose-link") catch unreachable;
         if (builder.verbose_cc or self.verbose_cc) zig_args.append("--verbose-cc") catch unreachable;
         if (builder.verbose_llvm_cpu_features) zig_args.append("--verbose-llvm-cpu-features") catch unreachable;
-        if (builder.system_linker_hack or self.system_linker_hack) zig_args.append("--system-linker-hack") catch unreachable;
 
         if (self.emit_llvm_ir) try zig_args.append("-femit-llvm-ir");
         if (self.emit_asm) try zig_args.append("-femit-asm");
src/link/Elf.zig
@@ -1353,20 +1353,14 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void {
     // Create an LLD command line and invoke it.
     var argv = std.ArrayList([]const u8).init(self.base.allocator);
     defer argv.deinit();
-
-    if (self.base.options.is_native_os and self.base.options.system_linker_hack) {
-        try argv.append("ld");
-    } else {
-        // Even though we're calling LLD as a library it thinks the first argument is its own exe name.
-        try argv.append("lld");
-
-        try argv.append("-error-limit=0");
-    }
-
+    // Even though we're calling LLD as a library it thinks the first argument is its own exe name.
+    try argv.append("lld");
     if (is_obj) {
         try argv.append("-r");
     }
 
+    try argv.append("-error-limit=0");
+
     if (self.base.options.output_mode == .Exe) {
         try argv.append("-z");
         try argv.append(try std.fmt.allocPrint(arena, "stack-size={}", .{stack_size}));
@@ -1622,63 +1616,43 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void {
         Compilation.dump_argv(argv.items);
     }
 
-    if (self.base.options.is_native_os and self.base.options.system_linker_hack) {
-        const result = try std.ChildProcess.exec(.{ .allocator = self.base.allocator, .argv = argv.items });
-        defer {
-            self.base.allocator.free(result.stdout);
-            self.base.allocator.free(result.stderr);
-        }
-        if (result.stdout.len != 0) {
-            std.log.warn("unexpected LD stdout: {}", .{result.stdout});
-        }
-        if (result.stderr.len != 0) {
-            std.log.warn("unexpected LD stderr: {}", .{result.stderr});
-        }
-        if (result.term.Exited != 0) {
-            // TODO parse this output and surface with the Compilation API rather than
-            // directly outputting to stderr here.
-            std.debug.print("{}", .{result.stderr});
-            return error.LDReportedFailure;
-        }
-    } else {
-        // Oh, snapplesauce! We need null terminated argv.
-        const new_argv = try arena.allocSentinel(?[*:0]const u8, argv.items.len, null);
-        for (argv.items) |arg, i| {
-            new_argv[i] = try arena.dupeZ(u8, arg);
-        }
+    // Oh, snapplesauce! We need null terminated argv.
+    const new_argv = try arena.allocSentinel(?[*:0]const u8, argv.items.len, null);
+    for (argv.items) |arg, i| {
+        new_argv[i] = try arena.dupeZ(u8, arg);
+    }
 
-        var stderr_context: LLDContext = .{
-            .elf = self,
-            .data = std.ArrayList(u8).init(self.base.allocator),
-        };
-        defer stderr_context.data.deinit();
-        var stdout_context: LLDContext = .{
-            .elf = self,
-            .data = std.ArrayList(u8).init(self.base.allocator),
-        };
-        defer stdout_context.data.deinit();
-        const llvm = @import("../llvm.zig");
-        const ok = llvm.Link(
-            .ELF,
-            new_argv.ptr,
-            new_argv.len,
-            append_diagnostic,
-            @ptrToInt(&stdout_context),
-            @ptrToInt(&stderr_context),
-        );
-        if (stderr_context.oom or stdout_context.oom) return error.OutOfMemory;
-        if (stdout_context.data.items.len != 0) {
-            std.log.warn("unexpected LLD stdout: {}", .{stdout_context.data.items});
-        }
-        if (!ok) {
-            // TODO parse this output and surface with the Compilation API rather than
-            // directly outputting to stderr here.
-            std.debug.print("{}", .{stderr_context.data.items});
-            return error.LLDReportedFailure;
-        }
-        if (stderr_context.data.items.len != 0) {
-            std.log.warn("unexpected LLD stderr: {}", .{stderr_context.data.items});
-        }
+    var stderr_context: LLDContext = .{
+        .elf = self,
+        .data = std.ArrayList(u8).init(self.base.allocator),
+    };
+    defer stderr_context.data.deinit();
+    var stdout_context: LLDContext = .{
+        .elf = self,
+        .data = std.ArrayList(u8).init(self.base.allocator),
+    };
+    defer stdout_context.data.deinit();
+    const llvm = @import("../llvm.zig");
+    const ok = llvm.Link(
+        .ELF,
+        new_argv.ptr,
+        new_argv.len,
+        append_diagnostic,
+        @ptrToInt(&stdout_context),
+        @ptrToInt(&stderr_context),
+    );
+    if (stderr_context.oom or stdout_context.oom) return error.OutOfMemory;
+    if (stdout_context.data.items.len != 0) {
+        std.log.warn("unexpected LLD stdout: {}", .{stdout_context.data.items});
+    }
+    if (!ok) {
+        // TODO parse this output and surface with the Compilation API rather than
+        // directly outputting to stderr here.
+        std.debug.print("{}", .{stderr_context.data.items});
+        return error.LLDReportedFailure;
+    }
+    if (stderr_context.data.items.len != 0) {
+        std.log.warn("unexpected LLD stderr: {}", .{stderr_context.data.items});
     }
 
     if (!self.base.options.disable_lld_caching) {
src/link/MachO.zig
@@ -534,7 +534,9 @@ fn linkWithLLD(self: *MachO, comp: *Compilation) !void {
         defer argv.deinit();
 
         // TODO https://github.com/ziglang/zig/issues/6971
-        if (self.base.options.is_native_os and self.base.options.system_linker_hack) {
+        // Note that there is no need to check if running natively since we do that already
+        // when setting `system_linker_hack` in Compilation struct.
+        if (self.base.options.system_linker_hack) {
             try argv.append("ld");
         } else {
             // Even though we're calling LLD as a library it thinks the first argument is its own exe name.
@@ -710,7 +712,9 @@ fn linkWithLLD(self: *MachO, comp: *Compilation) !void {
         }
 
         // TODO https://github.com/ziglang/zig/issues/6971
-        if (self.base.options.is_native_os and self.base.options.system_linker_hack) {
+        // Note that there is no need to check if running natively since we do that already
+        // when setting `system_linker_hack` in Compilation struct.
+        if (self.base.options.system_linker_hack) {
             const result = try std.ChildProcess.exec(.{ .allocator = self.base.allocator, .argv = argv.items });
             defer {
                 self.base.allocator.free(result.stdout);
src/Compilation.zig
@@ -348,8 +348,6 @@ pub const InitOptions = struct {
     want_valgrind: ?bool = null,
     use_llvm: ?bool = null,
     use_lld: ?bool = null,
-    /// When set, this option will overwrite LLD with the system linker LD.
-    system_linker_hack: bool = false,
     use_clang: ?bool = null,
     rdynamic: bool = false,
     strip: bool = false,
@@ -474,13 +472,22 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation {
             break :blk false;
         };
 
-        const syslibroot = if (build_options.have_llvm and comptime std.Target.current.isDarwin()) outer: {
-            const path = if (use_lld and options.is_native_os and options.target.isDarwin()) inner: {
-                const syslibroot_path = try std.zig.system.getSDKPath(arena);
-                break :inner syslibroot_path;
-            } else null;
-            break :outer path;
-        } else null;
+        const DarwinOptions = struct {
+            syslibroot: ?[]const u8 = null,
+            system_linker_hack: bool = false,
+        };
+
+        const darwin_options: DarwinOptions = if (build_options.have_llvm and comptime std.Target.current.isDarwin()) outer: {
+            const opts: DarwinOptions = if (use_lld and options.is_native_os and options.target.isDarwin()) inner: {
+                const syslibroot = try std.zig.system.getSDKPath(arena);
+                const system_linker_hack = if (std.os.getenv("ZIG_SYSTEM_LINKER_HACK")) |_| true else false;
+                break :inner .{
+                    .syslibroot = syslibroot,
+                    .system_linker_hack = system_linker_hack,
+                };
+            } else .{};
+            break :outer opts;
+        } else .{};
 
         const link_libc = options.link_libc or target_util.osRequiresLibC(options.target);
 
@@ -777,14 +784,14 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation {
             .optimize_mode = options.optimize_mode,
             .use_lld = use_lld,
             .use_llvm = use_llvm,
-            .system_linker_hack = options.system_linker_hack,
+            .system_linker_hack = darwin_options.system_linker_hack,
             .link_libc = link_libc,
             .link_libcpp = options.link_libcpp,
             .objects = options.link_objects,
             .frameworks = options.frameworks,
             .framework_dirs = options.framework_dirs,
             .system_libs = system_libs,
-            .syslibroot = syslibroot,
+            .syslibroot = darwin_options.syslibroot,
             .lib_dirs = options.lib_dirs,
             .rpath_list = options.rpath_list,
             .strip = strip,
src/link.zig
@@ -57,8 +57,8 @@ pub const Options = struct {
     /// other objects.
     /// Otherwise (depending on `use_lld`) this link code directly outputs and updates the final binary.
     use_llvm: bool,
-    /// If this is true and `use_llvm` is true, this link code will use system linker `ld` instead of 
-    /// the LLD.
+    /// Darwin-only. If this is true, `use_llvm` is true, and `is_native_os` is true, this link code will
+    /// use system linker `ld` instead of the LLD.
     system_linker_hack: bool,
     link_libc: bool,
     link_libcpp: bool,
src/main.zig
@@ -317,7 +317,6 @@ const usage_build_generic =
     \\  --image-base [addr]            Set base address for executable image
     \\  -framework [name]              (darwin) link against framework
     \\  -F[dir]                        (darwin) add search path for frameworks
-    \\  --system-linker-hack           Use system linker LD instead of LLD (requires LLVM enabled)
     \\
     \\Test Options:
     \\  --test-filter [text]           Skip tests that do not match filter
@@ -475,7 +474,6 @@ fn buildOutputType(
     var image_base_override: ?u64 = null;
     var use_llvm: ?bool = null;
     var use_lld: ?bool = null;
-    var system_linker_hack = false;
     var use_clang: ?bool = null;
     var link_eh_frame_hdr = false;
     var link_emit_relocs = false;
@@ -917,8 +915,6 @@ fn buildOutputType(
                         mem.startsWith(u8, arg, "-I"))
                     {
                         try clang_argv.append(arg);
-                    } else if (mem.startsWith(u8, arg, "--system-linker-hack")) {
-                        system_linker_hack = true;
                     } else {
                         fatal("unrecognized parameter: '{}'", .{arg});
                     }
@@ -1643,7 +1639,6 @@ fn buildOutputType(
         .want_valgrind = want_valgrind,
         .use_llvm = use_llvm,
         .use_lld = use_lld,
-        .system_linker_hack = system_linker_hack,
         .use_clang = use_clang,
         .rdynamic = rdynamic,
         .linker_script = linker_script,
@@ -2165,7 +2160,6 @@ pub fn cmdBuild(gpa: *Allocator, arena: *Allocator, args: []const []const u8) !v
         var override_global_cache_dir: ?[]const u8 = null;
         var override_local_cache_dir: ?[]const u8 = null;
         var child_argv = std.ArrayList([]const u8).init(arena);
-        var system_linker_hack = false;
 
         const argv_index_exe = child_argv.items.len;
         _ = try child_argv.addOne();
@@ -2206,10 +2200,6 @@ pub fn cmdBuild(gpa: *Allocator, arena: *Allocator, args: []const []const u8) !v
                         override_global_cache_dir = args[i];
                         try child_argv.appendSlice(&[_][]const u8{ arg, args[i] });
                         continue;
-                    } else if (mem.eql(u8, arg, "--system-linker-hack")) {
-                        system_linker_hack = true;
-                        try child_argv.append(arg);
-                        continue;
                     }
                 }
                 try child_argv.append(arg);
@@ -2345,7 +2335,6 @@ pub fn cmdBuild(gpa: *Allocator, arena: *Allocator, args: []const []const u8) !v
             .optimize_mode = .Debug,
             .self_exe_path = self_exe_path,
             .rand = &default_prng.random,
-            .system_linker_hack = system_linker_hack,
         }) catch |err| {
             fatal("unable to create compilation: {}", .{@errorName(err)});
         };
build.zig
@@ -396,8 +396,8 @@ fn configureStage2(b: *Builder, exe: anytype, ctx: Context, need_cpp_includes: b
                 try addCxxKnownPath(b, ctx, exe, "libstdc++.a", null, need_cpp_includes);
                 exe.linkSystemLibrary("pthread");
                 // TODO LLD cannot perform this link.
+                // Set ZIG_SYSTEM_LINKER_HACK env var to use system linker ld instead.
                 // See https://github.com/ziglang/zig/issues/1535
-                exe.enableSystemLinkerHack();
             } else |err| switch (err) {
                 error.RequiredLibraryNotFound => {
                     // System compiler, not gcc.
CMakeLists.txt
@@ -505,24 +505,13 @@ add_dependencies(zig zig_build_zig1)
 
 install(TARGETS zig DESTINATION bin)
 
-if(NOT SYSTEM_LINKER_HACK)
-  set(ZIG_INSTALL_ARGS "build"
-      --override-lib-dir "${CMAKE_SOURCE_DIR}/lib"
-      "-Dlib-files-only"
-      --prefix "${CMAKE_INSTALL_PREFIX}"
-      "-Dconfig_h=${ZIG_CONFIG_H_OUT}"
-      install
-  )
-else()
-  set(ZIG_INSTALL_ARGS "build"
-      --override-lib-dir "${CMAKE_SOURCE_DIR}/lib"
-      "-Dlib-files-only"
-      --prefix "${CMAKE_INSTALL_PREFIX}"
-      "-Dconfig_h=${ZIG_CONFIG_H_OUT}"
-      --system-linker-hack
-      install
-  )
-endif()
+set(ZIG_INSTALL_ARGS "build"
+    --override-lib-dir "${CMAKE_SOURCE_DIR}/lib"
+    "-Dlib-files-only"
+    --prefix "${CMAKE_INSTALL_PREFIX}"
+    "-Dconfig_h=${ZIG_CONFIG_H_OUT}"
+    install
+)
 
 # CODE has no effect with Visual Studio build system generator, therefore
 # when using Visual Studio build system generator we resort to running