macho: further improve error reporting

src/MachO.zig

@@ -281,9 +281,20 @@ pub fn flush(self: *MachO) !void {
         }
     }
 
+    var has_parse_error = false;
     for (resolved_objects.items) |obj| {
-        try self.parsePositional(arena, obj);
+        self.parsePositional(arena, obj) catch |err| {
+            has_parse_error = true;
+            switch (err) {
+                error.ParseFailed => {}, // already reported
+                else => |e| self.base.fatal("{s}: unexpected error occurred while parsing input file: {s}", .{
+                    obj.path, @errorName(e),
+                }),
+            }
+        };
     }
+    if (has_parse_error) return error.LinkFailed;
+
     for (self.dylibs.items) |index| {
         self.getFile(index).?.dylib.umbrella = index;
     }
@@ -661,14 +672,21 @@ fn parseArchive(self: *MachO, arena: Allocator, obj: LinkObject) !bool {
     defer archive.deinit(gpa);
     try archive.parse(arena, self);
 
+    var has_parse_error = false;
     for (archive.objects.items) |extracted| {
         const index = @as(File.Index, @intCast(try self.files.addOne(gpa)));
         self.files.set(index, .{ .object = extracted });
         const object = &self.files.items(.data)[index].object;
         object.index = index;
         object.alive = obj.must_link or obj.needed or self.options.all_load;
         object.hidden = obj.hidden;
-        try object.parse(self);
+        object.parse(self) catch |err| switch (err) {
+            error.ParseFailed => {
+                has_parse_error = true;
+                continue;
+            },
+            else => |e| return e,
+        };
         try self.objects.append(gpa, index);
         self.validateCpuArch(index);
         self.validatePlatform(index);
@@ -677,6 +695,7 @@ fn parseArchive(self: *MachO, arena: Allocator, obj: LinkObject) !bool {
         // anyhow.
         object.alive = object.alive or (self.options.force_load_objc and object.hasObjc());
     }
+    if (has_parse_error) return error.ParselFailed;
 
     return true;
 }
@@ -886,11 +905,11 @@ fn parseDependentDylibs(
                         break :full_path full_path;
                     }
                 } else if (eatPrefix(id.name, "@loader_path/")) |_| {
-                    self.base.fatal("fatal linker error: {s}: TODO handle install_name '{s}'", .{
+                    return self.base.fatal("{s}: TODO handle install_name '{s}'", .{
                         self.getFile(dylib_index).?.dylib.path, id.name,
                     });
                 } else if (eatPrefix(id.name, "@executable_path/")) |_| {
-                    self.base.fatal("fatal linker error: {s}: TODO handle install_name '{s}'", .{
+                    return self.base.fatal("{s}: TODO handle install_name '{s}'", .{
                         self.getFile(dylib_index).?.dylib.path, id.name,
                     });
                 }
@@ -1258,6 +1277,7 @@ fn reportUndefs(self: *MachO) !void {
 
     const max_notes = 4;
 
+    var has_undefs = false;
     var it = self.undefs.iterator();
     while (it.next()) |entry| {
         const undef_sym = self.getSymbol(entry.key_ptr.*);
@@ -1266,6 +1286,7 @@ fn reportUndefs(self: *MachO) !void {
 
         const err = try addFn(&self.base, nnotes);
         try err.addMsg("undefined symbol: {s}", .{undef_sym.getName(self)});
+        has_undefs = true;
 
         var inote: usize = 0;
         while (inote < @min(notes.items.len, max_notes)) : (inote += 1) {
@@ -1283,6 +1304,7 @@ fn reportUndefs(self: *MachO) !void {
     for (self.undefined_symbols.items) |index| {
         const sym = self.getSymbol(index);
         if (sym.getFile(self) != null) continue; // If undefined in an object file, will be reported above
+        has_undefs = true;
         const err = try addFn(&self.base, 1);
         try err.addMsg("undefined symbol: {s}", .{sym.getName(self)});
         try err.addNote("-u command line option", .{});
@@ -1291,6 +1313,7 @@ fn reportUndefs(self: *MachO) !void {
     if (self.entry_index) |index| {
         const sym = self.getSymbol(index);
         if (sym.getFile(self) == null) {
+            has_undefs = true;
             const err = try addFn(&self.base, 1);
             try err.addMsg("undefined symbol: {s}", .{sym.getName(self)});
             try err.addNote("implicit entry/start for main executable", .{});
@@ -1300,6 +1323,7 @@ fn reportUndefs(self: *MachO) !void {
     if (self.dyld_stub_binder_index) |index| {
         const sym = self.getSymbol(index);
         if (sym.getFile(self) == null and self.stubs_sect_index != null) {
+            has_undefs = true;
             const err = try addFn(&self.base, 1);
             try err.addMsg("undefined symbol: {s}", .{sym.getName(self)});
             try err.addNote("implicit -u command line option", .{});
@@ -1309,11 +1333,14 @@ fn reportUndefs(self: *MachO) !void {
     if (self.objc_msg_send_index) |index| {
         const sym = self.getSymbol(index);
         if (sym.getFile(self) == null and self.objc_stubs_sect_index != null) {
+            has_undefs = true;
             const err = try addFn(&self.base, 1);
             try err.addMsg("undefined symbol: {s}", .{sym.getName(self)});
             try err.addNote("implicit -u command line option", .{});
         }
     }
+
+    if (has_undefs) return error.LinkFailed;
 }
 
 fn initSyntheticSections(self: *MachO) !void {

src/MachO/Archive.zig

@@ -78,11 +78,10 @@ pub fn parse(self: *Archive, arena: Allocator, macho_file: *MachO) !void {
         const hdr = try reader.readStruct(ar_hdr);
 
         if (!mem.eql(u8, &hdr.ar_fmag, ARFMAG)) {
-            // TODO convert into a proper error
-            log.debug("{s}: invalid header delimiter: expected '{s}', found '{s}'", .{
+            macho_file.base.fatal("{s}: invalid header delimiter: expected '{s}', found '{s}'", .{
                 self.path, std.fmt.fmtSliceEscapeLower(ARFMAG), std.fmt.fmtSliceEscapeLower(&hdr.ar_fmag),
             });
-            return;
+            return error.ParseFailed;
         }
 
         var size = try hdr.size();

src/MachO/Dylib.zig

@@ -44,8 +44,10 @@ pub fn parse(self: *Dylib, macho_file: *MachO) !void {
 
     self.header = try reader.readStruct(macho.mach_header_64);
 
-    const lc_id = self.getLoadCommand(.ID_DYLIB) orelse
-        return macho_file.base.fatal("{s}: missing LC_ID_DYLIB load command", .{self.path});
+    const lc_id = self.getLoadCommand(.ID_DYLIB) orelse {
+        macho_file.base.fatal("{s}: missing LC_ID_DYLIB load command", .{self.path});
+        return error.ParseFailed;
+    };
     self.id = try Id.fromLoadCommand(gpa, lc_id.cast(macho.dylib_command).?, lc_id.getDylibPathName());
 
     var it = LoadCommandIterator{

src/MachO/Object.zig

@@ -348,9 +348,12 @@ fn linkNlistToAtom(self: *Object, macho_file: *MachO) !void {
             // if (sect.attrs() & macho.S_ATTR_DEBUG != 0) continue;
             if (self.findAtomInSection(nlist.n_value, nlist.n_sect - 1)) |atom_index| {
                 atom.* = atom_index;
-            } else return macho_file.base.fatalFatal("{}: symbol {s} not attached to any (sub)section", .{
-                self.fmtPath(), self.getString(nlist.n_strx),
-            });
+            } else {
+                macho_file.base.fatal("{}: symbol {s} not attached to any (sub)section", .{
+                    self.fmtPath(), self.getString(nlist.n_strx),
+                });
+                return error.ParseFailed;
+            }
         }
     }
 }
@@ -445,10 +448,12 @@ fn initRelocs(self: *Object, macho_file: *MachO) !void {
                     @as(i64, @intCast(sect.addr)) + rel.r_address + addend + 4
                 else
                     addend;
-                const target = self.findAtomInSection(@intCast(taddr), @intCast(nsect)) orelse
-                    return macho_file.base.fatalFatal("{}: {s},{s}: 0x{x}: bad relocation", .{
-                    self.fmtPath(), sect.segName(), sect.sectName(), rel.r_address,
-                });
+                const target = self.findAtomInSection(@intCast(taddr), @intCast(nsect)) orelse {
+                    macho_file.base.fatal("{}: {s},{s}: 0x{x}: bad relocation", .{
+                        self.fmtPath(), sect.segName(), sect.sectName(), rel.r_address,
+                    });
+                    return error.ParseFailed;
+                };
                 addend = taddr - @as(i64, @intCast(macho_file.getAtom(target).?.getInputSection(macho_file).addr));
                 break :blk target;
             } else self.symbols.items[rel.r_symbolnum];
@@ -588,9 +593,12 @@ fn initUnwindRecords(self: *Object, sect_id: u8, macho_file: *MachO) !void {
                         out.atom = atom_index;
                         const atom = out.getAtom(macho_file);
                         out.atom_offset = @intCast(rec.rangeStart - atom.getInputAddress(macho_file));
-                    } else return macho_file.base.fatalFatal("{}: {s},{s}: 0x{x}: bad relocation", .{
-                        self.fmtPath(), header.segName(), header.sectName(), rel.offset,
-                    }),
+                    } else {
+                        macho_file.base.fatal("{}: {s},{s}: 0x{x}: bad relocation", .{
+                            self.fmtPath(), header.segName(), header.sectName(), rel.offset,
+                        });
+                        return error.ParseFailed;
+                    },
                 },
                 16 => { // personality function
                     assert(rel.tag == .@"extern"); // TODO error
@@ -605,9 +613,12 @@ fn initUnwindRecords(self: *Object, sect_id: u8, macho_file: *MachO) !void {
                         out.lsda = atom_index;
                         const atom = out.getLsdaAtom(macho_file).?;
                         out.lsda_offset = @intCast(rec.lsda - atom.getInputAddress(macho_file));
-                    } else return macho_file.base.fatalFatal("{}: {s},{s}: 0x{x}: bad relocation", .{
-                        self.fmtPath(), header.segName(), header.sectName(), rel.offset,
-                    }),
+                    } else {
+                        macho_file.base.fatal("{}: {s},{s}: 0x{x}: bad relocation", .{
+                            self.fmtPath(), header.segName(), header.sectName(), rel.offset,
+                        });
+                        return error.ParseFailed;
+                    },
                 },
                 else => {},
             }

src/MachO/eh_frame.zig

@@ -148,10 +148,12 @@ pub const Fde = struct {
         // Parse target atom index
         const pc_begin = std.mem.readInt(i64, data[8..][0..8], .little);
         const taddr: u64 = @intCast(@as(i64, @intCast(sect.addr + fde.offset + 8)) + pc_begin);
-        fde.atom = object.findAtom(taddr) orelse
-            return macho_file.base.fatalFatal("{}: {s},{s}: 0x{x}: invalid function reference in FDE", .{
-            object.fmtPath(), sect.segName(), sect.sectName(), fde.offset + 8,
-        });
+        fde.atom = object.findAtom(taddr) orelse {
+            macho_file.base.fatal("{}: {s},{s}: 0x{x}: invalid function reference in FDE", .{
+                object.fmtPath(), sect.segName(), sect.sectName(), fde.offset + 8,
+            });
+            return error.ParseFailed;
+        };
         const atom = fde.getAtom(macho_file);
         fde.atom_offset = @intCast(taddr - atom.getInputAddress(macho_file));
 
@@ -163,10 +165,13 @@ pub const Fde = struct {
         } else null;
         if (cie_index) |cie| {
             fde.cie = cie;
-        } else return macho_file.base.fatalFatal("{}: no matching CIE found for FDE at offset {x}", .{
-            object.fmtPath(),
-            fde.offset,
-        });
+        } else {
+            macho_file.base.fatal("{}: no matching CIE found for FDE at offset {x}", .{
+                object.fmtPath(),
+                fde.offset,
+            });
+            return error.ParseFailed;
+        }
 
         const cie = fde.getCie(macho_file);
 
@@ -182,10 +187,12 @@ pub const Fde = struct {
                 .p64 => try reader.readInt(i64, .little),
             };
             const lsda_addr: u64 = @intCast(@as(i64, @intCast(sect.addr + 24 + offset + fde.offset)) + lsda_ptr);
-            fde.lsda = object.findAtom(lsda_addr) orelse
-                return macho_file.base.fatalFatal("{}: {s},{s}: 0x{x}: invalid LSDA reference in FDE", .{
-                object.fmtPath(), sect.segName(), sect.sectName(), fde.offset + offset + 24,
-            });
+            fde.lsda = object.findAtom(lsda_addr) orelse {
+                macho_file.base.fatal("{}: {s},{s}: 0x{x}: invalid LSDA reference in FDE", .{
+                    object.fmtPath(), sect.segName(), sect.sectName(), fde.offset + offset + 24,
+                });
+                return error.ParseFailed;
+            };
             const lsda_atom = fde.getLsdaAtom(macho_file).?;
             fde.lsda_offset = @intCast(lsda_addr - lsda_atom.getInputAddress(macho_file));
         }

src/Zld.zig

@@ -249,20 +249,12 @@ pub fn deinit(base: *Zld) void {
 }
 
 pub fn flush(base: *Zld) !void {
-    const res = switch (base.tag) {
-        .elf => @fieldParentPtr(Elf, "base", base).flush(),
-        .macho => @fieldParentPtr(MachO, "base", base).flush(),
-        .coff => @fieldParentPtr(Coff, "base", base).flush(),
-        .wasm => @fieldParentPtr(Wasm, "base", base).flush(),
-    };
-    _ = res catch |err| switch (err) {
-        error.Fatal => {
-            base.reportErrors();
-            base.deinit();
-            process.exit(1);
-        },
-        else => |e| return e,
-    };
+    switch (base.tag) {
+        .elf => try @fieldParentPtr(Elf, "base", base).flush(),
+        .macho => try @fieldParentPtr(MachO, "base", base).flush(),
+        .coff => try @fieldParentPtr(Coff, "base", base).flush(),
+        .wasm => try @fieldParentPtr(Wasm, "base", base).flush(),
+    }
 }
 
 pub fn warn(base: *Zld, comptime format: []const u8, args: anytype) void {
@@ -277,11 +269,6 @@ pub fn fatal(base: *Zld, comptime format: []const u8, args: anytype) void {
     base.errors.appendAssumeCapacity(.{ .msg = msg });
 }
 
-pub fn fatalFatal(base: *Zld, comptime format: []const u8, args: anytype) error{Fatal} {
-    base.fatal("fatal linker error: " ++ format, args);
-    return error.Fatal;
-}
-
 pub const ErrorWithNotes = struct {
     err_index: usize,
     allocator: Allocator,
@@ -434,15 +421,15 @@ pub fn reportWarningsAndErrors(base: *Zld) !void {
     }
 }
 
-fn reportErrors(base: *Zld) void {
+pub fn reportErrors(base: *Zld) void {
     var errors = base.getAllErrorsAlloc() catch @panic("OOM");
     defer errors.deinit(base.allocator);
     if (errors.errorMessageCount() > 0) {
         errors.renderToStdErr(.{ .ttyconf = std.io.tty.detectConfig(std.io.getStdErr()) });
     }
 }
 
-fn reportWarnings(base: *Zld) void {
+pub fn reportWarnings(base: *Zld) void {
     var warnings = base.getAllWarningsAlloc() catch @panic("OOM");
     defer warnings.deinit(base.allocator);
     if (warnings.errorMessageCount() > 0) {

src/main.zig

@@ -105,6 +105,16 @@ pub fn main() !void {
     defer thread_pool.deinit();
 
     const zld = try Zld.openPath(gpa, tag, opts, &thread_pool);
-    defer zld.deinit();
-    try zld.flush();
+    zld.flush() catch |err| switch (err) {
+        error.LinkFailed => {
+            zld.reportWarnings();
+            zld.reportErrors();
+            std.process.exit(1);
+        },
+        else => |e| {
+            fatal("unexpected linker error: {s}", .{@errorName(e)});
+            return e;
+        },
+    };
+    zld.reportWarnings();
 }