Commit 372e9709ad

Jakub Konka <kubkon@jakubkonka.com>
2021-10-18 21:09:03
macho: fix LLVM codepaths in self-hosted linker
* do not add linkage scope to aliased exported symbols - this is not respected on macOS * special-case `MachO.openPath` in `link.File.openPath` as on macOS we always link with zld * redirect to `MachO.flushObject` when linking relocatable objects in MachO linker whereas move the entire linking logic into `MachO.flushModule`
1 parent 2d7b55a
Changed files (3)
src
src/codegen/llvm.zig
@@ -527,19 +527,11 @@ pub const Object = struct {
                 if (self.llvm_module.getNamedGlobalAlias(exp_name_z.ptr, exp_name_z.len)) |alias| {
                     alias.setAliasee(llvm_global);
                 } else {
-                    const alias = self.llvm_module.addAlias(llvm_global.typeOf(), llvm_global, exp_name_z);
-                    switch (exp.options.linkage) {
-                        .Internal => alias.setLinkage(.Internal),
-                        .Strong => alias.setLinkage(.External),
-                        .Weak => {
-                            if (is_extern) {
-                                alias.setLinkage(.ExternalWeak);
-                            } else {
-                                alias.setLinkage(.WeakODR);
-                            }
-                        },
-                        .LinkOnce => alias.setLinkage(.LinkOnceODR),
-                    }
+                    _ = self.llvm_module.addAlias(
+                        llvm_global.typeOf(),
+                        llvm_global,
+                        exp_name_z,
+                    );
                 }
             }
         } else {
src/link/MachO.zig
@@ -275,18 +275,15 @@ pub const SrcFn = struct {
     };
 };
 
-pub fn openPath(allocator: *Allocator, sub_path: []const u8, options: link.Options) !*MachO {
+pub fn openPath(allocator: *Allocator, options: link.Options) !*MachO {
     assert(options.object_format == .macho);
 
-    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;
+    const use_stage1 = build_options.is_stage1 and options.use_stage1;
+    if (use_stage1 or options.emit == null) {
+        return createEmpty(allocator, options);
     }
-
-    const file = try options.emit.?.directory.handle.createFile(sub_path, .{
+    const emit = options.emit.?;
+    const file = try emit.directory.handle.createFile(emit.sub_path, .{
         .truncate = false,
         .read = true,
         .mode = link.determineMode(options),
@@ -301,7 +298,20 @@ pub fn openPath(allocator: *Allocator, sub_path: []const u8, options: link.Optio
 
     self.base.file = file;
 
-    if (options.output_mode == .Lib and options.link_mode == .Static) {
+    if (build_options.have_llvm and options.use_llvm and options.module != null) {
+        // 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}", .{
+            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
+        options.link_mode == .Static and self.base.intermediary_basename != null)
+    {
         return self;
     }
 
@@ -384,16 +394,22 @@ pub fn flush(self: *MachO, comp: *Compilation) !void {
             return error.TODOImplementWritingStaticLibFiles;
         }
     }
+    try self.flushModule(comp);
+}
 
+pub fn flushModule(self: *MachO, comp: *Compilation) !void {
     const tracy = trace(@src());
     defer tracy.end();
 
+    const use_stage1 = build_options.is_stage1 and self.base.options.use_stage1;
+    if (!use_stage1 and self.base.options.output_mode == .Obj)
+        return self.flushObject(comp);
+
     var arena_allocator = std.heap.ArenaAllocator.init(self.base.allocator);
     defer arena_allocator.deinit();
     const arena = &arena_allocator.allocator;
 
     const directory = self.base.options.emit.?.directory; // Just an alias to make it shorter to type.
-    const use_stage1 = build_options.is_stage1 and self.base.options.use_stage1;
 
     // If there is no Zig code to compile, then we should skip flushing the output file because it
     // will not be part of the linker line anyway.
@@ -410,7 +426,7 @@ pub fn flush(self: *MachO, comp: *Compilation) !void {
         }
 
         const obj_basename = self.base.intermediary_basename orelse break :blk null;
-        try self.flushModule(comp);
+        try self.flushObject(comp);
         const full_obj_path = try directory.join(arena, &[_][]const u8{obj_basename});
         break :blk full_obj_path;
     } else null;
@@ -534,15 +550,16 @@ pub fn flush(self: *MachO, comp: *Compilation) !void {
                 .read = true,
                 .mode = link.determineMode(self.base.options),
             });
-            try self.populateMissingMetadata();
+            // Index 0 is always a null symbol.
             try self.locals.append(self.base.allocator, .{
                 .n_strx = 0,
-                .n_type = macho.N_UNDF,
+                .n_type = 0,
                 .n_sect = 0,
                 .n_desc = 0,
                 .n_value = 0,
             });
             try self.strtab.append(self.base.allocator, 0);
+            try self.populateMissingMetadata();
         }
 
         if (needs_full_relink) {
@@ -887,7 +904,40 @@ pub fn flush(self: *MachO, comp: *Compilation) !void {
             sect.offset = 0;
         }
 
-        try self.flushModule(comp);
+        try self.setEntryPoint();
+        try self.updateSectionOrdinals();
+        try self.writeLinkeditSegment();
+
+        if (self.d_sym) |*ds| {
+            // Flush debug symbols bundle.
+            try ds.flushModule(self.base.allocator, self.base.options);
+        }
+
+        if (self.requires_adhoc_codesig) {
+            // Preallocate space for the code signature.
+            // We need to do this at this stage so that we have the load commands with proper values
+            // written out to the file.
+            // The most important here is to have the correct vm and filesize of the __LINKEDIT segment
+            // where the code signature goes into.
+            try self.writeCodeSignaturePadding();
+        }
+
+        try self.writeLoadCommands();
+        try self.writeHeader();
+
+        if (self.entry_addr == null and self.base.options.output_mode == .Exe) {
+            log.debug("flushing. no_entry_point_found = true", .{});
+            self.error_flags.no_entry_point_found = true;
+        } else {
+            log.debug("flushing. no_entry_point_found = false", .{});
+            self.error_flags.no_entry_point_found = false;
+        }
+
+        assert(!self.load_commands_dirty);
+
+        if (self.requires_adhoc_codesig) {
+            try self.writeCodeSignature(); // code signing always comes last
+        }
     }
 
     cache: {
@@ -909,46 +959,14 @@ pub fn flush(self: *MachO, comp: *Compilation) !void {
     self.cold_start = false;
 }
 
-pub fn flushModule(self: *MachO, comp: *Compilation) !void {
-    _ = comp;
-
+pub fn flushObject(self: *MachO, comp: *Compilation) !void {
     const tracy = trace(@src());
     defer tracy.end();
 
-    try self.setEntryPoint();
-    try self.updateSectionOrdinals();
-    try self.writeLinkeditSegment();
-
-    if (self.d_sym) |*ds| {
-        // Flush debug symbols bundle.
-        try ds.flushModule(self.base.allocator, self.base.options);
-    }
-
-    if (self.requires_adhoc_codesig) {
-        // Preallocate space for the code signature.
-        // We need to do this at this stage so that we have the load commands with proper values
-        // written out to the file.
-        // The most important here is to have the correct vm and filesize of the __LINKEDIT segment
-        // where the code signature goes into.
-        try self.writeCodeSignaturePadding();
-    }
-
-    try self.writeLoadCommands();
-    try self.writeHeader();
+    if (build_options.have_llvm)
+        if (self.llvm_object) |llvm_object| return llvm_object.flushModule(comp);
 
-    if (self.entry_addr == null and self.base.options.output_mode == .Exe) {
-        log.debug("flushing. no_entry_point_found = true", .{});
-        self.error_flags.no_entry_point_found = true;
-    } else {
-        log.debug("flushing. no_entry_point_found = false", .{});
-        self.error_flags.no_entry_point_found = false;
-    }
-
-    assert(!self.load_commands_dirty);
-
-    if (self.requires_adhoc_codesig) {
-        try self.writeCodeSignature(); // code signing always comes last
-    }
+    return error.TODOImplementWritingObjFiles;
 }
 
 fn resolveSearchDir(
@@ -3035,6 +3053,7 @@ fn growAtom(self: *MachO, atom: *Atom, new_atom_size: u64, alignment: u64, match
 }
 
 pub fn allocateDeclIndexes(self: *MachO, decl: *Module.Decl) !void {
+    if (self.llvm_object) |_| return;
     if (decl.link.macho.local_sym_index != 0) return;
 
     try self.locals.ensureUnusedCapacity(self.base.allocator, 1);
@@ -3458,6 +3477,7 @@ pub fn updateDeclExports(
 }
 
 pub fn deleteExport(self: *MachO, exp: Export) void {
+    if (self.llvm_object) |_| return;
     const sym_index = exp.sym_index orelse return;
     self.globals_free_list.append(self.base.allocator, sym_index) catch {};
     const global = &self.globals.items[sym_index];
src/link.zig
@@ -192,12 +192,16 @@ pub const File = struct {
     /// rewriting it. A malicious file is detected as incremental link failure
     /// and does not cause Illegal Behavior. This operation is not atomic.
     pub fn openPath(allocator: *Allocator, options: Options) !*File {
+        if (options.object_format == .macho) {
+            return &(try MachO.openPath(allocator, options)).base;
+        }
+
         const use_stage1 = build_options.is_stage1 and options.use_stage1;
         if (use_stage1 or options.emit == null) {
             return switch (options.object_format) {
                 .coff => &(try Coff.createEmpty(allocator, options)).base,
                 .elf => &(try Elf.createEmpty(allocator, options)).base,
-                .macho => &(try MachO.createEmpty(allocator, options)).base,
+                .macho => unreachable,
                 .wasm => &(try Wasm.createEmpty(allocator, options)).base,
                 .plan9 => return &(try Plan9.createEmpty(allocator, options)).base,
                 .c => unreachable, // Reported error earlier.
@@ -215,7 +219,7 @@ pub const File = struct {
                 return switch (options.object_format) {
                     .coff => &(try Coff.createEmpty(allocator, options)).base,
                     .elf => &(try Elf.createEmpty(allocator, options)).base,
-                    .macho => &(try MachO.createEmpty(allocator, options)).base,
+                    .macho => unreachable,
                     .plan9 => &(try Plan9.createEmpty(allocator, options)).base,
                     .wasm => &(try Wasm.createEmpty(allocator, options)).base,
                     .c => unreachable, // Reported error earlier.
@@ -235,7 +239,7 @@ pub const File = struct {
         const file: *File = switch (options.object_format) {
             .coff => &(try Coff.openPath(allocator, sub_path, options)).base,
             .elf => &(try Elf.openPath(allocator, sub_path, options)).base,
-            .macho => &(try MachO.openPath(allocator, sub_path, options)).base,
+            .macho => unreachable,
             .plan9 => &(try Plan9.openPath(allocator, sub_path, options)).base,
             .wasm => &(try Wasm.openPath(allocator, sub_path, options)).base,
             .c => &(try C.openPath(allocator, sub_path, options)).base,
@@ -576,7 +580,11 @@ pub const File = struct {
                 const full_obj_path = try o_directory.join(arena, &[_][]const u8{obj_basename});
                 break :blk full_obj_path;
             }
-            try base.flushModule(comp);
+            if (base.options.object_format == .macho) {
+                try base.cast(MachO).?.flushObject(comp);
+            } else {
+                try base.flushModule(comp);
+            }
             const obj_basename = base.intermediary_basename.?;
             const full_obj_path = try directory.join(arena, &[_][]const u8{obj_basename});
             break :blk full_obj_path;