From e0e3ceac19c0de0f8b3e408f9d3728496bc051f7 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Thu, 5 Nov 2020 11:29:58 +0100 Subject: [PATCH 1/3] Re-enable system linker hack It is now possible to force linking with system linker `ld` instead of the LLVM `lld` linker when building natively on the target. This can be done at each stage by specifying `--system-linker-hack` flag, and can be useful on platforms where `lld` fails to operate properly such as macOS 11 Big Sur on ARM64 where every binary/dylib is expected to be codesigned. Some example invocations for each stage of compilation of Zig toolchain: ``` cmake .. -DCMAKE_PREFIX_PATH=/path/to/llvm -DSYSTEM_LINKER_HACK=1 ``` ``` build/zig build test --system-linker-hack ``` ``` build/zig build --prefix $(pwd)/stage2 -Denable-llvm --system-linker-hack ``` ``` build/zig build-exe hello.zig --system-linker-hack ``` --- CMakeLists.txt | 25 ++++++-- lib/std/build.zig | 7 +- lib/std/special/build_runner.zig | 3 + src/Compilation.zig | 3 + src/link.zig | 3 + src/link/Elf.zig | 106 +++++++++++++++++++------------ src/link/MachO.zig | 105 ++++++++++++++++++------------ src/main.zig | 11 ++++ 8 files changed, 173 insertions(+), 90 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 473e0da7e0..8ca9747819 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -505,13 +505,24 @@ add_dependencies(zig zig_build_zig1) install(TARGETS zig DESTINATION bin) -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 -) +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() # CODE has no effect with Visual Studio build system generator, therefore # when using Visual Studio build system generator we resort to running diff --git a/lib/std/build.zig b/lib/std/build.zig index a1ac3f88f2..e9388b7bec 100644 --- a/lib/std/build.zig +++ b/lib/std/build.zig @@ -67,6 +67,7 @@ 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, @@ -173,6 +174,7 @@ 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); @@ -2074,6 +2076,7 @@ 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"); @@ -2283,10 +2286,6 @@ pub const LibExeObjStep = struct { } } - if (self.system_linker_hack) { - try zig_args.append("--system-linker-hack"); - } - if (self.valgrind_support) |valgrind_support| { if (valgrind_support) { try zig_args.append("-fvalgrind"); diff --git a/lib/std/special/build_runner.zig b/lib/std/special/build_runner.zig index f861d3159b..17c28b9ca6 100644 --- a/lib/std/special/build_runner.zig +++ b/lib/std/special/build_runner.zig @@ -112,6 +112,8 @@ 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; @@ -213,6 +215,7 @@ 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) \\ ); } diff --git a/src/Compilation.zig b/src/Compilation.zig index d4289fad34..961f8c08d5 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -348,6 +348,8 @@ 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, @@ -775,6 +777,7 @@ 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, .link_libc = link_libc, .link_libcpp = options.link_libcpp, .objects = options.link_objects, diff --git a/src/link.zig b/src/link.zig index 02433ecde7..65074924d1 100644 --- a/src/link.zig +++ b/src/link.zig @@ -57,6 +57,9 @@ 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. + system_linker_hack: bool, link_libc: bool, link_libcpp: bool, function_sections: bool, diff --git a/src/link/Elf.zig b/src/link/Elf.zig index 7d4662c68d..886d74cd95 100644 --- a/src/link/Elf.zig +++ b/src/link/Elf.zig @@ -1353,14 +1353,20 @@ 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(); - // Even though we're calling LLD as a library it thinks the first argument is its own exe name. - try argv.append("lld"); + + 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"); + } + 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})); @@ -1616,43 +1622,63 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void { Compilation.dump_argv(argv.items); } - // 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); - } + 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); + } - 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) { diff --git a/src/link/MachO.zig b/src/link/MachO.zig index 01830c1561..9352669eba 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -532,11 +532,17 @@ fn linkWithLLD(self: *MachO, comp: *Compilation) !void { // Create an LLD command line and invoke it. var argv = std.ArrayList([]const u8).init(self.base.allocator); defer argv.deinit(); - // 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"); - try argv.append("0"); + // TODO https://github.com/ziglang/zig/issues/6971 + 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"); + try argv.append("0"); + } try argv.append("-demangle"); @@ -703,42 +709,63 @@ fn linkWithLLD(self: *MachO, comp: *Compilation) !void { Compilation.dump_argv(argv.items); } - 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); - } + // TODO https://github.com/ziglang/zig/issues/6971 + 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 { + 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 = .{ - .macho = self, - .data = std.ArrayList(u8).init(self.base.allocator), - }; - defer stderr_context.data.deinit(); - var stdout_context: LLDContext = .{ - .macho = self, - .data = std.ArrayList(u8).init(self.base.allocator), - }; - defer stdout_context.data.deinit(); - const llvm = @import("../llvm.zig"); - const ok = llvm.Link( - .MachO, - 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 = .{ + .macho = self, + .data = std.ArrayList(u8).init(self.base.allocator), + }; + defer stderr_context.data.deinit(); + var stdout_context: LLDContext = .{ + .macho = self, + .data = std.ArrayList(u8).init(self.base.allocator), + }; + defer stdout_context.data.deinit(); + const llvm = @import("../llvm.zig"); + const ok = llvm.Link( + .MachO, + 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}); + } } } diff --git a/src/main.zig b/src/main.zig index 7df8cb1eda..5bb9a7afdd 100644 --- a/src/main.zig +++ b/src/main.zig @@ -317,6 +317,7 @@ 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 @@ -474,6 +475,7 @@ 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; @@ -915,6 +917,8 @@ 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}); } @@ -1639,6 +1643,7 @@ 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, @@ -2160,6 +2165,7 @@ 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(); @@ -2200,6 +2206,10 @@ 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); @@ -2335,6 +2345,7 @@ 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)}); }; From b7c3ebcb9e3aefbdd82a43c7ab122931787c7805 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Fri, 6 Nov 2020 10:58:49 +0100 Subject: [PATCH 2/3] Rely on ZIG_SYSTEM_LINKER_HACK instead of input flags --- CMakeLists.txt | 25 ++------ build.zig | 2 +- lib/std/build.zig | 8 --- lib/std/special/build_runner.zig | 3 - src/Compilation.zig | 29 +++++---- src/link.zig | 4 +- src/link/Elf.zig | 106 ++++++++++++------------------- src/link/MachO.zig | 8 ++- src/main.zig | 11 ---- 9 files changed, 74 insertions(+), 122 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8ca9747819..473e0da7e0 100644 --- a/CMakeLists.txt +++ b/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 diff --git a/build.zig b/build.zig index b4d8b71f79..ae6073f8be 100644 --- a/build.zig +++ b/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. diff --git a/lib/std/build.zig b/lib/std/build.zig index e9388b7bec..5666005fa4 100644 --- a/lib/std/build.zig +++ b/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"); diff --git a/lib/std/special/build_runner.zig b/lib/std/special/build_runner.zig index 17c28b9ca6..f861d3159b 100644 --- a/lib/std/special/build_runner.zig +++ b/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) \\ ); } diff --git a/src/Compilation.zig b/src/Compilation.zig index 961f8c08d5..b6f380ae60 100644 --- a/src/Compilation.zig +++ b/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, diff --git a/src/link.zig b/src/link.zig index 65074924d1..7b0271f978 100644 --- a/src/link.zig +++ b/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, diff --git a/src/link/Elf.zig b/src/link/Elf.zig index 886d74cd95..7d4662c68d 100644 --- a/src/link/Elf.zig +++ b/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) { diff --git a/src/link/MachO.zig b/src/link/MachO.zig index 9352669eba..391f490c1f 100644 --- a/src/link/MachO.zig +++ b/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); diff --git a/src/main.zig b/src/main.zig index 5bb9a7afdd..7df8cb1eda 100644 --- a/src/main.zig +++ b/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)}); }; From ab69b89d528c828e949bb2d2f3632401a2d382fe Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Fri, 6 Nov 2020 11:57:53 +0100 Subject: [PATCH 3/3] Address review comments --- src/Compilation.zig | 2 +- src/link/MachO.zig | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilation.zig b/src/Compilation.zig index b6f380ae60..36847f0437 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -480,7 +480,7 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation { 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; + const system_linker_hack = std.os.getenv("ZIG_SYSTEM_LINKER_HACK") != null; break :inner .{ .syslibroot = syslibroot, .system_linker_hack = system_linker_hack, diff --git a/src/link/MachO.zig b/src/link/MachO.zig index 391f490c1f..6fbd1265d5 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -726,7 +726,7 @@ fn linkWithLLD(self: *MachO, comp: *Compilation) !void { if (result.stderr.len != 0) { std.log.warn("unexpected LD stderr: {}", .{result.stderr}); } - if (result.term.Exited != 0) { + if (result.term != .Exited or 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});