From 27c5c7fb23fceb0a333444408a1dea4188a14c32 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 24 Nov 2021 18:08:56 -0700 Subject: [PATCH] stage2: proper `-femit-implib` frontend support * Improve the logic for determining whether emitting an import lib is eligible, and improve the error message when the user provides contradictory arguments. * Integrate with the EmitLoc / Emit system that already exists, and use the `-femit-implib[=path]`/`-fno-emit-implib` convention that already exists. * Proper integration with the caching system. * CLI: fix bug in error reporting for resolving EmitLoc values for other parameters. --- src/Compilation.zig | 30 +++++++++-- src/link.zig | 3 +- src/link/Coff.zig | 11 ++-- src/main.zig | 123 ++++++++++++++++++++++++++++++-------------- 4 files changed, 117 insertions(+), 50 deletions(-) diff --git a/src/Compilation.zig b/src/Compilation.zig index c84fd2c96d..2ff390f196 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -654,6 +654,8 @@ pub const InitOptions = struct { emit_analysis: ?EmitLoc = null, /// `null` means to not emit docs. emit_docs: ?EmitLoc = null, + /// `null` means to not emit an import lib. + emit_implib: ?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 @@ -766,8 +768,6 @@ pub const InitOptions = struct { test_filter: ?[]const u8 = null, test_name_prefix: ?[]const u8 = null, subsystem: ?std.Target.SubSystem = null, - /// Windows/PE only. Where to output the import library, can contain directories. - out_implib: ?[]const u8 = null, /// WASI-only. Type of WASI execution model ("command" or "reactor"). wasi_exec_model: ?std.builtin.WasiExecModel = null, /// (Zig compiler development) Enable dumping linker's state as JSON. @@ -947,7 +947,7 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation { options.output_mode == .Lib or options.image_base_override != null or options.linker_script != null or options.version_script != null or - options.out_implib != null) + options.emit_implib != null) { break :blk true; } @@ -1183,6 +1183,7 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation { cache.hash.add(options.output_mode); cache.hash.add(options.machine_code_model); cache.hash.addOptionalEmitLoc(options.emit_bin); + cache.hash.addOptionalEmitLoc(options.emit_implib); cache.hash.addBytes(options.root_name); if (options.target.os.tag == .wasi) cache.hash.add(wasi_exec_model); // TODO audit this and make sure everything is in it @@ -1339,18 +1340,21 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation { const bin_file_emit: ?link.Emit = blk: { const emit_bin = options.emit_bin orelse break :blk null; + if (emit_bin.directory) |directory| { break :blk link.Emit{ .directory = directory, .sub_path = emit_bin.basename, }; } + if (module) |zm| { break :blk link.Emit{ .directory = zm.zig_cache_artifact_directory, .sub_path = emit_bin.basename, }; } + // We could use the cache hash as is no problem, however, we increase // the likelihood of cache hits by adding the first C source file // path name (not contents) to the hash. This way if the user is compiling @@ -1377,6 +1381,24 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation { }; }; + const implib_emit: ?link.Emit = blk: { + const emit_implib = options.emit_implib orelse break :blk null; + + if (emit_implib.directory) |directory| { + break :blk link.Emit{ + .directory = directory, + .sub_path = emit_implib.basename, + }; + } + + // Use the same directory as the bin. The CLI already emits an + // error if -fno-emit-bin is combined with -femit-implib. + break :blk link.Emit{ + .directory = bin_file_emit.?.directory, + .sub_path = emit_implib.basename, + }; + }; + var system_libs: std.StringArrayHashMapUnmanaged(SystemLib) = .{}; errdefer system_libs.deinit(gpa); try system_libs.ensureTotalCapacity(gpa, options.system_lib_names.len); @@ -1386,6 +1408,7 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation { const bin_file = try link.File.openPath(gpa, .{ .emit = bin_file_emit, + .implib_emit = implib_emit, .root_name = root_name, .module = module, .target = options.target, @@ -1461,7 +1484,6 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation { .each_lib_rpath = options.each_lib_rpath orelse options.is_native_os, .disable_lld_caching = options.disable_lld_caching, .subsystem = options.subsystem, - .out_implib = options.out_implib, .is_test = options.is_test, .wasi_exec_model = wasi_exec_model, .use_stage1 = use_stage1, diff --git a/src/link.zig b/src/link.zig index f00d7809f4..3657397500 100644 --- a/src/link.zig +++ b/src/link.zig @@ -47,6 +47,8 @@ pub const Options = struct { /// This is `null` when -fno-emit-bin is used. When `openPath` or `flush` is called, /// it will have already been null-checked. emit: ?Emit, + /// This is `null` not building a Windows DLL, or when -fno-emit-implib is used. + implib_emit: ?Emit, target: std.Target, output_mode: std.builtin.OutputMode, link_mode: std.builtin.LinkMode, @@ -127,7 +129,6 @@ pub const Options = struct { gc_sections: ?bool = null, allow_shlib_undefined: ?bool, subsystem: ?std.Target.SubSystem, - out_implib: ?[]const u8, linker_script: ?[]const u8, version_script: ?[]const u8, soname: ?[]const u8, diff --git a/src/link/Coff.zig b/src/link/Coff.zig index 19a99a2e32..55bd912bef 100644 --- a/src/link/Coff.zig +++ b/src/link/Coff.zig @@ -947,7 +947,6 @@ fn linkWithLLD(self: *Coff, comp: *Compilation) !void { man.hash.add(self.base.options.dynamicbase); man.hash.addOptional(self.base.options.major_subsystem_version); man.hash.addOptional(self.base.options.minor_subsystem_version); - man.hash.addOptionalBytes(self.base.options.out_implib); // We don't actually care whether it's a cache hit or miss; we just need the digest and the lock. _ = try man.hit(); @@ -978,7 +977,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 @@ -1070,6 +1068,11 @@ fn linkWithLLD(self: *Coff, comp: *Compilation) !void { try argv.append(try allocPrint(arena, "-OUT:{s}", .{full_out_path})); + if (self.base.options.implib_emit) |emit| { + const implib_out_path = try emit.directory.join(arena, &[_][]const u8{emit.sub_path}); + try argv.append(try allocPrint(arena, "-IMPLIB:{s}", .{implib_out_path})); + } + if (self.base.options.link_libc) { if (self.base.options.libc_installation) |libc_installation| { try argv.append(try allocPrint(arena, "-LIBPATH:{s}", .{libc_installation.crt_dir.?})); @@ -1095,10 +1098,6 @@ fn linkWithLLD(self: *Coff, comp: *Compilation) !void { try argv.append(p); } - if (self.base.options.out_implib != null) { - try argv.append(try allocPrint(arena, "-IMPLIB:{s}.lib", .{full_out_path})); - } - const resolved_subsystem: ?std.Target.SubSystem = blk: { if (self.base.options.subsystem) |explicit| break :blk explicit; switch (target.os.tag) { diff --git a/src/main.zig b/src/main.zig index 7d8b9b1265..2c99f508d4 100644 --- a/src/main.zig +++ b/src/main.zig @@ -317,6 +317,8 @@ const usage_build_generic = \\ -fno-emit-docs (default) Do not produce docs/ dir with html documentation \\ -femit-analysis[=path] Write analysis JSON file with type information \\ -fno-emit-analysis (default) Do not write analysis JSON file with type information + \\ -femit-implib[=path] (default) Produce an import .lib when building a Windows DLL + \\ -fno-emit-implib Do not produce an import .lib when building a Windows DLL \\ --show-builtin Output the source of @import("builtin") then exit \\ --cache-dir [path] Override the local cache directory \\ --global-cache-dir [path] Override the global cache directory @@ -585,6 +587,8 @@ fn buildOutputType( var emit_llvm_bc: Emit = .no; var emit_docs: Emit = .no; var emit_analysis: Emit = .no; + var emit_implib: Emit = .yes_default_path; + var emit_implib_arg_provided = false; var target_arch_os_abi: []const u8 = "native"; var target_mcpu: ?[]const u8 = null; var target_dynamic_linker: ?[]const u8 = null; @@ -654,7 +658,6 @@ fn buildOutputType( var main_pkg_path: ?[]const u8 = null; var clang_preprocessor_mode: Compilation.ClangPreprocessorMode = .no; var subsystem: ?std.Target.SubSystem = null; - var out_implib: ?[]const u8 = null; var major_subsystem_version: ?u32 = null; var minor_subsystem_version: ?u32 = null; var wasi_exec_model: ?std.builtin.WasiExecModel = null; @@ -1091,6 +1094,15 @@ fn buildOutputType( emit_analysis = .{ .yes = arg["-femit-analysis=".len..] }; } else if (mem.eql(u8, arg, "-fno-emit-analysis")) { emit_analysis = .no; + } else if (mem.eql(u8, arg, "-femit-implib")) { + emit_implib = .yes_default_path; + emit_implib_arg_provided = true; + } else if (mem.startsWith(u8, arg, "-femit-implib=")) { + emit_implib = .{ .yes = arg["-femit-implib=".len..] }; + emit_implib_arg_provided = true; + } else if (mem.eql(u8, arg, "-fno-emit-implib")) { + emit_implib = .no; + emit_implib_arg_provided = true; } else if (mem.eql(u8, arg, "-dynamic")) { link_mode = .Dynamic; } else if (mem.eql(u8, arg, "-static")) { @@ -1650,7 +1662,8 @@ fn buildOutputType( if (i >= linker_args.items.len) { fatal("expected linker arg after '{s}'", .{arg}); } - out_implib = linker_args.items[i]; + emit_implib = .{ .yes = linker_args.items[i] }; + emit_implib_arg_provided = true; } else { warn("unsupported linker arg: {s}", .{arg}); } @@ -1998,11 +2011,15 @@ fn buildOutputType( const default_h_basename = try std.fmt.allocPrint(arena, "{s}.h", .{root_name}); var emit_h_resolved = emit_h.resolve(default_h_basename) catch |err| { switch (emit_h) { - .yes => { - fatal("unable to open directory from argument '-femit-h', '{s}': {s}", .{ emit_h.yes, @errorName(err) }); + .yes => |p| { + fatal("unable to open directory from argument '-femit-h', '{s}': {s}", .{ + p, @errorName(err), + }); }, .yes_default_path => { - fatal("unable to open directory from arguments '--name' or '-fsoname', '{s}': {s}", .{ default_h_basename, @errorName(err) }); + fatal("unable to open directory from arguments '--name' or '-fsoname', '{s}': {s}", .{ + default_h_basename, @errorName(err), + }); }, .no => unreachable, } @@ -2012,11 +2029,15 @@ fn buildOutputType( const default_asm_basename = try std.fmt.allocPrint(arena, "{s}.s", .{root_name}); var emit_asm_resolved = emit_asm.resolve(default_asm_basename) catch |err| { switch (emit_asm) { - .yes => { - fatal("unable to open directory from argument '-femit-asm', '{s}': {s}", .{ emit_asm.yes, @errorName(err) }); + .yes => |p| { + fatal("unable to open directory from argument '-femit-asm', '{s}': {s}", .{ + p, @errorName(err), + }); }, .yes_default_path => { - fatal("unable to open directory from arguments '--name' or '-fsoname', '{s}': {s}", .{ default_asm_basename, @errorName(err) }); + fatal("unable to open directory from arguments '--name' or '-fsoname', '{s}': {s}", .{ + default_asm_basename, @errorName(err), + }); }, .no => unreachable, } @@ -2026,11 +2047,15 @@ fn buildOutputType( const default_llvm_ir_basename = try std.fmt.allocPrint(arena, "{s}.ll", .{root_name}); var emit_llvm_ir_resolved = emit_llvm_ir.resolve(default_llvm_ir_basename) catch |err| { switch (emit_llvm_ir) { - .yes => { - fatal("unable to open directory from argument '-femit-llvm-ir', '{s}': {s}", .{ emit_llvm_ir.yes, @errorName(err) }); + .yes => |p| { + fatal("unable to open directory from argument '-femit-llvm-ir', '{s}': {s}", .{ + p, @errorName(err), + }); }, .yes_default_path => { - fatal("unable to open directory from arguments '--name' or '-fsoname', '{s}': {s}", .{ default_llvm_ir_basename, @errorName(err) }); + fatal("unable to open directory from arguments '--name' or '-fsoname', '{s}': {s}", .{ + default_llvm_ir_basename, @errorName(err), + }); }, .no => unreachable, } @@ -2040,11 +2065,15 @@ fn buildOutputType( const default_llvm_bc_basename = try std.fmt.allocPrint(arena, "{s}.bc", .{root_name}); var emit_llvm_bc_resolved = emit_llvm_bc.resolve(default_llvm_bc_basename) catch |err| { switch (emit_llvm_bc) { - .yes => { - fatal("unable to open directory from argument '-femit-llvm-bc', '{s}': {s}", .{ emit_llvm_bc.yes, @errorName(err) }); + .yes => |p| { + fatal("unable to open directory from argument '-femit-llvm-bc', '{s}': {s}", .{ + p, @errorName(err), + }); }, .yes_default_path => { - fatal("unable to open directory from arguments '--name' or '-fsoname', '{s}': {s}", .{ default_llvm_bc_basename, @errorName(err) }); + fatal("unable to open directory from arguments '--name' or '-fsoname', '{s}': {s}", .{ + default_llvm_bc_basename, @errorName(err), + }); }, .no => unreachable, } @@ -2054,11 +2083,15 @@ fn buildOutputType( const default_analysis_basename = try std.fmt.allocPrint(arena, "{s}-analysis.json", .{root_name}); var emit_analysis_resolved = emit_analysis.resolve(default_analysis_basename) catch |err| { switch (emit_analysis) { - .yes => { - fatal("unable to open directory from argument 'femit-analysis', '{s}': {s}", .{ emit_analysis.yes, @errorName(err) }); + .yes => |p| { + fatal("unable to open directory from argument '-femit-analysis', '{s}': {s}", .{ + p, @errorName(err), + }); }, .yes_default_path => { - fatal("unable to open directory from arguments 'name' or 'soname', '{s}': {s}", .{ default_analysis_basename, @errorName(err) }); + fatal("unable to open directory from arguments 'name' or 'soname', '{s}': {s}", .{ + default_analysis_basename, @errorName(err), + }); }, .no => unreachable, } @@ -2067,8 +2100,10 @@ fn buildOutputType( var emit_docs_resolved = emit_docs.resolve("docs") catch |err| { switch (emit_docs) { - .yes => { - fatal("unable to open directory from argument 'femit-docs', '{s}': {s}", .{ emit_h.yes, @errorName(err) }); + .yes => |p| { + fatal("unable to open directory from argument '-femit-docs', '{s}': {s}", .{ + p, @errorName(err), + }); }, .yes_default_path => { fatal("unable to open directory 'docs': {s}", .{@errorName(err)}); @@ -2078,6 +2113,35 @@ fn buildOutputType( }; defer emit_docs_resolved.deinit(); + const is_dyn_lib = switch (output_mode) { + .Obj, .Exe => false, + .Lib => (link_mode orelse .Static) == .Dynamic, + }; + const implib_eligible = is_dyn_lib and + emit_bin_loc != null and target_info.target.os.tag == .windows; + if (!implib_eligible) { + if (!emit_implib_arg_provided) { + emit_implib = .no; + } else if (emit_implib != .no) { + fatal("the argument -femit-implib is allowed only when building a Windows DLL", .{}); + } + } + const default_implib_basename = try std.fmt.allocPrint(arena, "{s}.lib", .{root_name}); + var emit_implib_resolved = emit_implib.resolve(default_implib_basename) catch |err| { + switch (emit_implib) { + .yes => |p| { + fatal("unable to open directory from argument '-femit-implib', '{s}': {s}", .{ + p, @errorName(err), + }); + }, + .yes_default_path => { + fatal("unable to open directory 'docs': {s}", .{@errorName(err)}); + }, + .no => unreachable, + } + }; + defer emit_implib_resolved.deinit(); + const main_pkg: ?*Package = if (root_src_file) |src_path| blk: { if (main_pkg_path) |p| { const rel_src_path = try fs.path.relative(gpa, p, src_path); @@ -2168,16 +2232,6 @@ fn buildOutputType( else => false, }; - // Always output import libraries (.lib) when building for msvc to replicate - // `link` behavior. lld does not always output import libraries so on the - // gnu abi users must set out_implib. - if (output_mode == .Lib and emit_bin == .yes and target_info.target.abi == .msvc and out_implib == null) { - const emit_bin_ext = fs.path.extension(emit_bin.yes); - out_implib = try std.fmt.allocPrint(gpa, "{s}.lib", .{ - emit_bin.yes[0 .. emit_bin.yes.len - emit_bin_ext.len], - }); - } - gimmeMoreOfThoseSweetSweetFileDescriptors(); const comp = Compilation.create(gpa, .{ @@ -2199,6 +2253,7 @@ fn buildOutputType( .emit_llvm_bc = emit_llvm_bc_resolved.data, .emit_docs = emit_docs_resolved.data, .emit_analysis = emit_analysis_resolved.data, + .emit_implib = emit_implib_resolved.data, .link_mode = link_mode, .dll_export_fns = dll_export_fns, .object_format = object_format, @@ -2286,7 +2341,6 @@ fn buildOutputType( .test_name_prefix = test_name_prefix, .disable_lld_caching = !have_enable_cache, .subsystem = subsystem, - .out_implib = out_implib, .wasi_exec_model = wasi_exec_model, .debug_compile_errors = debug_compile_errors, .enable_link_snapshots = enable_link_snapshots, @@ -2685,15 +2739,6 @@ fn updateModule(gpa: *Allocator, comp: *Compilation, hook: AfterUpdateHook) !voi _ = try cache_dir.updateFile(src_pdb_path, cwd, dst_pdb_path, .{}); } - - if (comp.bin_file.options.out_implib) |out_implib| { - const src_implib_path = try std.fmt.allocPrint(gpa, "{s}.lib", .{bin_sub_path}); - defer gpa.free(src_implib_path); - if (std.fs.path.dirname(out_implib)) |implib_dir| { - try cwd.makePath(implib_dir); - } - _ = try cache_dir.updateFile(src_implib_path, cwd, out_implib, .{}); - } }, } }