From ff66a18555dc6c00bd928f48462c3fc44f86ab6c Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Mon, 3 Jan 2022 20:03:22 -0700 Subject: [PATCH] linker: fix build-obj and -fno-emit-bin This commit fixes two problems: * `zig build-obj` regressed from the cache-mode branch. It would crash because it assumed that dirname on the emit bin path would not be null. This assumption was invalid when outputting to the current working directory - a pretty common use case for `zig build-obj`. * When using the LLVM backend, `-fno-emit-bin` combined with any other kind of emitting, such as `-femit-asm`, emitted nothing. Both issues are now fixed. --- src/codegen/llvm.zig | 32 ++++++++++------------- src/glibc.zig | 4 +-- src/link.zig | 18 +++++++++++-- src/link/Coff.zig | 26 +++++++++++++------ src/link/Elf.zig | 61 ++++++++++++++++++++++++++------------------ src/link/MachO.zig | 29 ++++++++++++++++----- src/link/Wasm.zig | 32 +++++++++++++++-------- 7 files changed, 129 insertions(+), 73 deletions(-) diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index e29116476d..070c667e6b 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -181,8 +181,6 @@ pub const Object = struct { /// The backing memory for `type_map`. Periodically garbage collected after flush(). /// The code for doing the periodical GC is not yet implemented. type_map_arena: std.heap.ArenaAllocator, - /// Where to put the output object file, relative to bin_file.options.emit directory. - sub_path: []const u8, pub const TypeMap = std.HashMapUnmanaged( Type, @@ -191,14 +189,14 @@ pub const Object = struct { std.hash_map.default_max_load_percentage, ); - pub fn create(gpa: Allocator, sub_path: []const u8, options: link.Options) !*Object { + pub fn create(gpa: Allocator, options: link.Options) !*Object { const obj = try gpa.create(Object); errdefer gpa.destroy(obj); - obj.* = try Object.init(gpa, sub_path, options); + obj.* = try Object.init(gpa, options); return obj; } - pub fn init(gpa: Allocator, sub_path: []const u8, options: link.Options) !Object { + pub fn init(gpa: Allocator, options: link.Options) !Object { const context = llvm.Context.create(); errdefer context.dispose(); @@ -271,7 +269,6 @@ pub const Object = struct { .decl_map = .{}, .type_map = .{}, .type_map_arena = std.heap.ArenaAllocator.init(gpa), - .sub_path = sub_path, }; } @@ -324,19 +321,22 @@ pub const Object = struct { const mod = comp.bin_file.options.module.?; const cache_dir = mod.zig_cache_artifact_directory; - const emit_bin_path: ?[*:0]const u8 = if (comp.bin_file.options.emit) |emit| blk: { - const full_out_path = try emit.directory.join(arena, &[_][]const u8{emit.sub_path}); - break :blk try std.fs.path.joinZ(arena, &.{ - std.fs.path.dirname(full_out_path).?, self.sub_path, - }); - } else null; + const emit_bin_path: ?[*:0]const u8 = if (comp.bin_file.options.emit) |emit| + try emit.basenamePath(arena, try arena.dupeZ(u8, comp.bin_file.intermediary_basename.?)) + else + null; const emit_asm_path = try locPath(arena, comp.emit_asm, cache_dir); const emit_llvm_ir_path = try locPath(arena, comp.emit_llvm_ir, cache_dir); const emit_llvm_bc_path = try locPath(arena, comp.emit_llvm_bc, cache_dir); - const debug_emit_path = emit_bin_path orelse "(none)"; - log.debug("emit LLVM object to {s}", .{debug_emit_path}); + const emit_asm_msg = emit_asm_path orelse "(none)"; + const emit_bin_msg = emit_bin_path orelse "(none)"; + const emit_llvm_ir_msg = emit_llvm_ir_path orelse "(none)"; + const emit_llvm_bc_msg = emit_llvm_bc_path orelse "(none)"; + log.debug("emit LLVM object asm={s} bin={s} ir={s} bc={s}", .{ + emit_asm_msg, emit_bin_msg, emit_llvm_ir_msg, emit_llvm_bc_msg, + }); var error_message: [*:0]const u8 = undefined; if (self.target_machine.emitToFile( @@ -354,10 +354,6 @@ pub const Object = struct { )) { defer llvm.disposeMessage(error_message); - const emit_asm_msg = emit_asm_path orelse "(none)"; - const emit_bin_msg = emit_bin_path orelse "(none)"; - const emit_llvm_ir_msg = emit_llvm_ir_path orelse "(none)"; - const emit_llvm_bc_msg = emit_llvm_bc_path orelse "(none)"; log.err("LLVM failed to emit asm={s} bin={s} ir={s} bc={s}: {s}", .{ emit_asm_msg, emit_bin_msg, emit_llvm_ir_msg, emit_llvm_bc_msg, error_message, diff --git a/src/glibc.zig b/src/glibc.zig index 4424cb9b90..65ee88ed81 100644 --- a/src/glibc.zig +++ b/src/glibc.zig @@ -225,7 +225,7 @@ pub fn buildCRTFile(comp: *Compilation, crt_file: CRTFile) !void { }); }, .scrt1_o => { - const start_os: Compilation.CSourceFile = blk: { + const start_o: Compilation.CSourceFile = blk: { var args = std.ArrayList([]const u8).init(arena); try add_include_dirs(comp, arena, &args); try args.appendSlice(&[_][]const u8{ @@ -266,7 +266,7 @@ pub fn buildCRTFile(comp: *Compilation, crt_file: CRTFile) !void { .extra_flags = args.items, }; }; - return comp.build_crt_file("Scrt1", .Obj, &[_]Compilation.CSourceFile{ start_os, abi_note_o }); + return comp.build_crt_file("Scrt1", .Obj, &[_]Compilation.CSourceFile{ start_o, abi_note_o }); }, .libc_nonshared_a => { const target = comp.getTarget(); diff --git a/src/link.zig b/src/link.zig index 5c33ce3b70..422d86d4b3 100644 --- a/src/link.zig +++ b/src/link.zig @@ -43,6 +43,21 @@ pub const Emit = struct { directory: Compilation.Directory, /// Path to the output file, relative to `directory`. sub_path: []const u8, + + /// Returns the full path to `basename` if it were in the same directory as the + /// `Emit` sub_path. + pub fn basenamePath(emit: Emit, arena: Allocator, basename: [:0]const u8) ![:0]const u8 { + const full_path = if (emit.directory.path) |p| + try fs.path.join(arena, &[_][]const u8{ p, emit.sub_path }) + else + emit.sub_path; + + if (fs.path.dirname(full_path)) |dirname| { + return try fs.path.joinZ(arena, &.{ dirname, basename }); + } else { + return basename; + } + } }; pub const Options = struct { @@ -533,9 +548,8 @@ pub const File = struct { /// Commit pending changes and write headers. Takes into account final output mode /// and `use_lld`, not only `effectiveOutputMode`. pub fn flush(base: *File, comp: *Compilation) !void { - const emit = base.options.emit orelse return; // -fno-emit-bin - if (comp.clang_preprocessor_mode == .yes) { + const emit = base.options.emit orelse return; // -fno-emit-bin // TODO: avoid extra link step when it's just 1 object file (the `zig cc -c` case) // Until then, we do `lld -r -o output.o input.o` even though the output is the same // as the input. For the preprocessing case (`zig cc -E -o foo`) we copy the file diff --git a/src/link/Coff.zig b/src/link/Coff.zig index ec06c28b44..01a200e9f6 100644 --- a/src/link/Coff.zig +++ b/src/link/Coff.zig @@ -129,11 +129,7 @@ pub fn openPath(allocator: Allocator, sub_path: []const u8, options: link.Option assert(options.object_format == .coff); if (build_options.have_llvm and options.use_llvm) { - const self = try createEmpty(allocator, options); - errdefer self.base.destroy(); - - self.llvm_object = try LlvmObject.create(allocator, sub_path, options); - return self; + return createEmpty(allocator, options); } const file = try options.emit.?.directory.handle.createFile(sub_path, .{ @@ -403,6 +399,7 @@ pub fn createEmpty(gpa: Allocator, options: link.Options) !*Coff { else => return error.UnsupportedCOFFArchitecture, }; const self = try gpa.create(Coff); + errdefer gpa.destroy(self); self.* = .{ .base = .{ .tag = .coff, @@ -412,6 +409,9 @@ pub fn createEmpty(gpa: Allocator, options: link.Options) !*Coff { }, .ptr_width = ptr_width, }; + if (build_options.have_llvm and options.use_llvm) { + self.llvm_object = try LlvmObject.create(gpa, options); + } return self; } @@ -817,6 +817,14 @@ pub fn updateDeclExports( } pub fn flush(self: *Coff, comp: *Compilation) !void { + if (self.base.options.emit == null) { + if (build_options.have_llvm) { + if (self.llvm_object) |llvm_object| { + return try llvm_object.flushModule(comp); + } + } + return; + } if (build_options.have_llvm and self.base.options.use_lld) { return self.linkWithLLD(comp); } else { @@ -905,9 +913,11 @@ fn linkWithLLD(self: *Coff, comp: *Compilation) !void { try self.flushModule(comp); - break :blk try fs.path.join(arena, &.{ - fs.path.dirname(full_out_path).?, self.base.intermediary_basename.?, - }); + if (fs.path.dirname(full_out_path)) |dirname| { + break :blk try fs.path.join(arena, &.{ dirname, self.base.intermediary_basename.? }); + } else { + break :blk self.base.intermediary_basename.?; + } } else null; const is_lib = self.base.options.output_mode == .Lib; diff --git a/src/link/Elf.zig b/src/link/Elf.zig index 105b012fbf..78a0b9d3a5 100644 --- a/src/link/Elf.zig +++ b/src/link/Elf.zig @@ -241,11 +241,7 @@ pub fn openPath(allocator: Allocator, sub_path: []const u8, options: link.Option assert(options.object_format == .elf); if (build_options.have_llvm and options.use_llvm) { - const self = try createEmpty(allocator, options); - errdefer self.base.destroy(); - - self.llvm_object = try LlvmObject.create(allocator, sub_path, options); - return self; + return createEmpty(allocator, options); } const file = try options.emit.?.directory.handle.createFile(sub_path, .{ @@ -298,6 +294,7 @@ pub fn createEmpty(gpa: Allocator, options: link.Options) !*Elf { }; const self = try gpa.create(Elf); errdefer gpa.destroy(self); + self.* = .{ .base = .{ .tag = .elf, @@ -307,9 +304,9 @@ pub fn createEmpty(gpa: Allocator, options: link.Options) !*Elf { }, .ptr_width = ptr_width, }; - // TODO get rid of the sub_path parameter to LlvmObject.create - // and create the llvm_object here. Also openPath needs to - // not override this field or there will be a memory leak. + if (build_options.have_llvm and options.use_llvm) { + self.llvm_object = try LlvmObject.create(gpa, options); + } return self; } @@ -788,14 +785,21 @@ pub const abbrev_pad1 = 5; pub const abbrev_parameter = 6; pub fn flush(self: *Elf, comp: *Compilation) !void { - if (build_options.have_llvm and self.base.options.use_lld) { - return self.linkWithLLD(comp); - } else { - switch (self.base.options.effectiveOutputMode()) { - .Exe, .Obj => {}, - .Lib => return error.TODOImplementWritingLibFiles, + if (self.base.options.emit == null) { + if (build_options.have_llvm) { + if (self.llvm_object) |llvm_object| { + return try llvm_object.flushModule(comp); + } } - return self.flushModule(comp); + return; + } + const use_lld = build_options.have_llvm and self.base.options.use_lld; + if (use_lld) { + return self.linkWithLLD(comp); + } + switch (self.base.options.output_mode) { + .Exe, .Obj => return self.flushModule(comp), + .Lib => return error.TODOImplementWritingLibFiles, } } @@ -803,8 +807,11 @@ pub fn flushModule(self: *Elf, comp: *Compilation) !void { const tracy = trace(@src()); defer tracy.end(); - if (build_options.have_llvm) - if (self.llvm_object) |llvm_object| return try llvm_object.flushModule(comp); + if (build_options.have_llvm) { + if (self.llvm_object) |llvm_object| { + return try llvm_object.flushModule(comp); + } + } // TODO This linker code currently assumes there is only 1 compilation unit and it // corresponds to the Zig source code. @@ -1327,9 +1334,11 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void { try self.flushModule(comp); - break :blk try fs.path.join(arena, &.{ - fs.path.dirname(full_out_path).?, self.base.intermediary_basename.?, - }); + if (fs.path.dirname(full_out_path)) |dirname| { + break :blk try fs.path.join(arena, &.{ dirname, self.base.intermediary_basename.? }); + } else { + break :blk self.base.intermediary_basename.?; + } } else null; const is_obj = self.base.options.output_mode == .Obj; @@ -1446,10 +1455,13 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void { }; } - // Due to a deficiency in LLD, we need to special-case BPF to a simple file copy when generating - // relocatables. Normally, we would expect `lld -r` to work. However, because LLD wants to resolve - // BPF relocations which it shouldn't, it fails before even generating the relocatable. - if (self.base.options.output_mode == .Obj and (self.base.options.lto or target.isBpfFreestanding())) { + // Due to a deficiency in LLD, we need to special-case BPF to a simple file + // copy when generating relocatables. Normally, we would expect `lld -r` to work. + // However, because LLD wants to resolve BPF relocations which it shouldn't, it fails + // before even generating the relocatable. + if (self.base.options.output_mode == .Obj and + (self.base.options.lto or target.isBpfFreestanding())) + { // In this case we must do a simple file copy // here. TODO: think carefully about how we can avoid this redundant operation when doing // build-obj. See also the corresponding TODO in linkAsArchive. @@ -1473,7 +1485,6 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void { try fs.cwd().copyFile(the_object_path, fs.cwd(), full_out_path, .{}); } } else { - // Create an LLD command line and invoke it. var argv = std.ArrayList([]const u8).init(self.base.allocator); defer argv.deinit(); diff --git a/src/link/MachO.zig b/src/link/MachO.zig index ed720080af..aec4fae1d8 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -313,11 +313,10 @@ pub fn openPath(allocator: Allocator, options: link.Options) !*MachO { // TODO this intermediary_basename isn't enough; in the case of `zig build-exe`, // we also want to put the intermediary object file in the cache while the // main emit directory is the cwd. - const sub_path = try std.fmt.allocPrint(allocator, "{s}{s}", .{ + self.llvm_object = try LlvmObject.create(allocator, options); + self.base.intermediary_basename = try std.fmt.allocPrint(allocator, "{s}{s}", .{ emit.sub_path, options.object_format.fileExt(options.target.cpu.arch), }); - self.llvm_object = try LlvmObject.create(allocator, sub_path, options); - self.base.intermediary_basename = sub_path; } if (options.output_mode == .Lib and @@ -373,7 +372,6 @@ pub fn openPath(allocator: Allocator, options: link.Options) !*MachO { } pub fn createEmpty(gpa: Allocator, options: link.Options) !*MachO { - const self = try gpa.create(MachO); const cpu_arch = options.target.cpu.arch; const os_tag = options.target.os.tag; const abi = options.target.abi; @@ -383,6 +381,9 @@ pub fn createEmpty(gpa: Allocator, options: link.Options) !*MachO { const requires_adhoc_codesig = cpu_arch == .aarch64 and (os_tag == .macos or abi == .simulator); const needs_prealloc = !(build_options.is_stage1 and options.use_stage1); + const self = try gpa.create(MachO); + errdefer gpa.destroy(self); + self.* = .{ .base = .{ .tag = .macho, @@ -395,10 +396,22 @@ pub fn createEmpty(gpa: Allocator, options: link.Options) !*MachO { .needs_prealloc = needs_prealloc, }; + if (build_options.have_llvm and options.use_llvm) { + self.llvm_object = try LlvmObject.create(gpa, options); + } + return self; } pub fn flush(self: *MachO, comp: *Compilation) !void { + if (self.base.options.emit == null) { + if (build_options.have_llvm) { + if (self.llvm_object) |llvm_object| { + return try llvm_object.flushModule(comp); + } + } + return; + } if (self.base.options.output_mode == .Lib and self.base.options.link_mode == .Static) { if (build_options.have_llvm) { return self.base.linkAsArchive(comp); @@ -449,9 +462,11 @@ pub fn flushModule(self: *MachO, comp: *Compilation) !void { try self.flushObject(comp); - break :blk try fs.path.join(arena, &.{ - fs.path.dirname(full_out_path).?, obj_basename, - }); + if (fs.path.dirname(full_out_path)) |dirname| { + break :blk try fs.path.join(arena, &.{ dirname, obj_basename }); + } else { + break :blk obj_basename; + } } else null; const is_lib = self.base.options.output_mode == .Lib; diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index 220ab2f53c..e63b99d567 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -101,11 +101,7 @@ pub fn openPath(allocator: Allocator, sub_path: []const u8, options: link.Option assert(options.object_format == .wasm); if (build_options.have_llvm and options.use_llvm) { - const self = try createEmpty(allocator, options); - errdefer self.base.destroy(); - - self.llvm_object = try LlvmObject.create(allocator, sub_path, options); - return self; + return createEmpty(allocator, options); } // TODO: read the file and keep valid parts instead of truncating @@ -139,8 +135,9 @@ pub fn openPath(allocator: Allocator, sub_path: []const u8, options: link.Option } pub fn createEmpty(gpa: Allocator, options: link.Options) !*Wasm { - const wasm_bin = try gpa.create(Wasm); - wasm_bin.* = .{ + const self = try gpa.create(Wasm); + errdefer gpa.destroy(self); + self.* = .{ .base = .{ .tag = .wasm, .options = options, @@ -148,7 +145,10 @@ pub fn createEmpty(gpa: Allocator, options: link.Options) !*Wasm { .allocator = gpa, }, }; - return wasm_bin; + if (build_options.have_llvm and options.use_llvm) { + self.llvm_object = try LlvmObject.create(gpa, options); + } + return self; } pub fn deinit(self: *Wasm) void { @@ -576,6 +576,14 @@ fn resetState(self: *Wasm) void { } pub fn flush(self: *Wasm, comp: *Compilation) !void { + if (self.base.options.emit == null) { + if (build_options.have_llvm) { + if (self.llvm_object) |llvm_object| { + return try llvm_object.flushModule(comp); + } + } + return; + } if (build_options.have_llvm and self.base.options.use_lld) { return self.linkWithLLD(comp); } else { @@ -1075,9 +1083,11 @@ fn linkWithLLD(self: *Wasm, comp: *Compilation) !void { try self.flushModule(comp); - break :blk try fs.path.join(arena, &.{ - fs.path.dirname(full_out_path).?, self.base.intermediary_basename.?, - }); + if (fs.path.dirname(full_out_path)) |dirname| { + break :blk try fs.path.join(arena, &.{ dirname, self.base.intermediary_basename.? }); + } else { + break :blk self.base.intermediary_basename.?; + } } else null; const is_obj = self.base.options.output_mode == .Obj;