Commit ff66a18555

Andrew Kelley <andrew@ziglang.org>
2022-01-04 04:03:22
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.
1 parent 81fa31c
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,
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;
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();
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;
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;
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();
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