authorgravatar for kubkon@jakubkonka.comJakub Konka <kubkon@jakubkonka.com> 2023-08-30 19:02:25+02:00
committergravatar for kubkon@jakubkonka.comJakub Konka <kubkon@jakubkonka.com> 2023-08-30 19:02:25+02:00
log5806e761bb676cdd537308f6c4a197e42228416d
treef7d0c2d7cc95695dfb07113a8e5bcba452daf595
parent22c81740ef611fe6e3b7ac2390fa9cf058f0ac6b

macho: improve error reporting for re-exports mismatch

4 files changed, 71 insertions(+), 40 deletions(-)

src/link.zig-2
...@@ -734,8 +734,6 @@ pub const File = struct {...@@ -734,8 +734,6 @@ pub const File = struct {
734 MissingEndForBody,734 MissingEndForBody,
735 MissingEndForExpression,735 MissingEndForExpression,
736 /// TODO: this should be removed from the error set in favor of using ErrorFlags736 /// TODO: this should be removed from the error set in favor of using ErrorFlags
737 MissingMainEntrypoint,
738 /// TODO: this should be removed from the error set in favor of using ErrorFlags
739 MissingSection,737 MissingSection,
740 MissingSymbol,738 MissingSymbol,
741 MissingTableSymbols,739 MissingTableSymbols,
src/link/MachO.zig+67-32
...@@ -399,10 +399,7 @@ pub fn flushModule(self: *MachO, comp: *Compilation, prog_node: *std.Progress.No...@@ -399,10 +399,7 @@ pub fn flushModule(self: *MachO, comp: *Compilation, prog_node: *std.Progress.No
399 self.dylibs_map.clearRetainingCapacity();399 self.dylibs_map.clearRetainingCapacity();
400 self.referenced_dylibs.clearRetainingCapacity();400 self.referenced_dylibs.clearRetainingCapacity();
401401
402 var dependent_libs = std.fifo.LinearFifo(struct {402 var dependent_libs = std.fifo.LinearFifo(DylibReExportInfo, .Dynamic).init(arena);
403 id: Dylib.Id,
404 parent: u16,
405 }, .Dynamic).init(arena);
406403
407 for (libs.keys(), libs.values()) |path, lib| {404 for (libs.keys(), libs.values()) |path, lib| {
408 const in_file = try std.fs.cwd().openFile(path, .{});405 const in_file = try std.fs.cwd().openFile(path, .{});
...@@ -417,6 +414,7 @@ pub fn flushModule(self: *MachO, comp: *Compilation, prog_node: *std.Progress.No...@@ -417,6 +414,7 @@ pub fn flushModule(self: *MachO, comp: *Compilation, prog_node: *std.Progress.No
417 lib,414 lib,
418 false,415 false,
419 false,416 false,
417 null,
420 &dependent_libs,418 &dependent_libs,
421 &parse_ctx,419 &parse_ctx,
422 ) catch |err| try self.handleAndReportParseError(path, err, &parse_ctx);420 ) catch |err| try self.handleAndReportParseError(path, err, &parse_ctx);
...@@ -749,7 +747,7 @@ pub fn parsePositional(...@@ -749,7 +747,7 @@ pub fn parsePositional(
749 .path = null,747 .path = null,
750 .needed = false,748 .needed = false,
751 .weak = false,749 .weak = false,
752 }, must_link, false, dependent_libs, ctx);750 }, must_link, false, null, dependent_libs, ctx);
753 }751 }
754}752}
755753
...@@ -806,6 +804,7 @@ pub fn parseLibrary(...@@ -806,6 +804,7 @@ pub fn parseLibrary(
806 lib: link.SystemLib,804 lib: link.SystemLib,
807 must_link: bool,805 must_link: bool,
808 is_dependent: bool,806 is_dependent: bool,
807 reexport_info: ?DylibReExportInfo,
809 dependent_libs: anytype,808 dependent_libs: anytype,
810 ctx: *ParseErrorCtx,809 ctx: *ParseErrorCtx,
811) ParseError!void {810) ParseError!void {
...@@ -823,6 +822,7 @@ pub fn parseLibrary(...@@ -823,6 +822,7 @@ pub fn parseLibrary(
823 .needed = lib.needed,822 .needed = lib.needed,
824 .weak = lib.weak,823 .weak = lib.weak,
825 .dependent = is_dependent,824 .dependent = is_dependent,
825 .reexport_info = reexport_info,
826 }, ctx);826 }, ctx);
827 } else return error.UnknownFileType;827 } else return error.UnknownFileType;
828 } else if (Archive.isArchive(file, 0)) {828 } else if (Archive.isArchive(file, 0)) {
...@@ -832,12 +832,14 @@ pub fn parseLibrary(...@@ -832,12 +832,14 @@ pub fn parseLibrary(
832 .needed = lib.needed,832 .needed = lib.needed,
833 .weak = lib.weak,833 .weak = lib.weak,
834 .dependent = is_dependent,834 .dependent = is_dependent,
835 .reexport_info = reexport_info,
835 }, ctx);836 }, ctx);
836 } else {837 } else {
837 self.parseLibStub(file, path, dependent_libs, .{838 self.parseLibStub(file, path, dependent_libs, .{
838 .needed = lib.needed,839 .needed = lib.needed,
839 .weak = lib.weak,840 .weak = lib.weak,
840 .dependent = is_dependent,841 .dependent = is_dependent,
842 .reexport_info = reexport_info,
841 }, ctx) catch |err| switch (err) {843 }, ctx) catch |err| switch (err) {
842 error.NotLibStub, error.UnexpectedToken => return error.UnknownFileType,844 error.NotLibStub, error.UnexpectedToken => return error.UnknownFileType,
843 else => |e| return e,845 else => |e| return e,
...@@ -935,8 +937,13 @@ fn parseArchive(...@@ -935,8 +937,13 @@ fn parseArchive(
935 }937 }
936}938}
937939
940pub const DylibReExportInfo = struct {
941 id: Dylib.Id,
942 parent: u16,
943};
944
938const DylibOpts = struct {945const DylibOpts = struct {
939 id: ?Dylib.Id = null,946 reexport_info: ?DylibReExportInfo = null,
940 dependent: bool = false,947 dependent: bool = false,
941 needed: bool = false,948 needed: bool = false,
942 weak: bool = false,949 weak: bool = false,
...@@ -986,10 +993,7 @@ fn parseDylib(...@@ -986,10 +993,7 @@ fn parseDylib(
986 return error.InvalidTarget;993 return error.InvalidTarget;
987 }994 }
988995
989 try self.addDylib(dylib, .{996 try self.addDylib(dylib, dylib_options, ctx);
990 .needed = dylib_options.needed,
991 .weak = dylib_options.weak,
992 });
993}997}
994998
995fn parseLibStub(999fn parseLibStub(
...@@ -1038,20 +1042,17 @@ fn parseLibStub(...@@ -1038,20 +1042,17 @@ fn parseLibStub(
1038 path,1042 path,
1039 );1043 );
10401044
1041 try self.addDylib(dylib, .{1045 try self.addDylib(dylib, dylib_options, ctx);
1042 .needed = dylib_options.needed,
1043 .weak = dylib_options.weak,
1044 });
1045}1046}
10461047
1047fn addDylib(self: *MachO, dylib: Dylib, dylib_options: DylibOpts) ParseError!void {1048fn addDylib(self: *MachO, dylib: Dylib, dylib_options: DylibOpts, ctx: *ParseErrorCtx) ParseError!void {
1048 if (dylib_options.id) |id| {1049 if (dylib_options.reexport_info) |reexport_info| {
1049 if (dylib.id.?.current_version < id.compatibility_version) {1050 if (dylib.id.?.current_version < reexport_info.id.compatibility_version) {
1050 // TODO convert into an error1051 ctx.detected_dylib_id = .{
1051 log.warn("found dylib is incompatible with the required minimum version", .{});1052 .parent = reexport_info.parent,
1052 log.warn(" dylib: {s}", .{id.name});1053 .required_version = reexport_info.id.compatibility_version,
1053 log.warn(" required minimum version: {}", .{id.compatibility_version});1054 .found_version = dylib.id.?.current_version,
1054 log.warn(" dylib version: {}", .{dylib.id.?.current_version});1055 };
1055 return error.IncompatibleDylibVersion;1056 return error.IncompatibleDylibVersion;
1056 }1057 }
1057 }1058 }
...@@ -1119,14 +1120,9 @@ pub fn parseDependentLibs(self: *MachO, dependent_libs: anytype) !void {...@@ -1119,14 +1120,9 @@ pub fn parseDependentLibs(self: *MachO, dependent_libs: anytype) !void {
1119 };1120 };
11201121
1121 const full_path = maybe_full_path orelse {1122 const full_path = maybe_full_path orelse {
1122 try self.misc_errors.ensureUnusedCapacity(gpa, 1);
1123 var notes = try gpa.alloc(File.ErrorMsg, 1);
1124 errdefer gpa.free(notes);
1125 const parent_name = if (parent.id) |id| id.name else parent.path;1123 const parent_name = if (parent.id) |id| id.name else parent.path;
1126 notes[0] = .{ .msg = try std.fmt.allocPrint(gpa, "a dependency of {s}", .{parent_name}) };1124 try self.reportDependencyError(parent_name, null, "missing dynamic library dependency: '{s}'", .{
1127 self.misc_errors.appendAssumeCapacity(.{1125 dep_id.id.name,
1128 .msg = try std.fmt.allocPrint(gpa, "missing dynamic library dependency: '{s}'", .{dep_id.id.name}),
1129 .notes = notes,
1130 });1126 });
1131 continue;1127 continue;
1132 };1128 };
...@@ -1143,7 +1139,7 @@ pub fn parseDependentLibs(self: *MachO, dependent_libs: anytype) !void {...@@ -1143,7 +1139,7 @@ pub fn parseDependentLibs(self: *MachO, dependent_libs: anytype) !void {
1143 .path = null,1139 .path = null,
1144 .needed = false,1140 .needed = false,
1145 .weak = weak,1141 .weak = weak,
1146 }, false, true, dependent_libs, &parse_ctx) catch |err|1142 }, false, true, dep_id, dependent_libs, &parse_ctx) catch |err|
1147 try self.handleAndReportParseError(full_path, err, &parse_ctx);1143 try self.handleAndReportParseError(full_path, err, &parse_ctx);
11481144
1149 // TODO I think that it would be nice to rewrite this error to include metadata for failed dependency1145 // TODO I think that it would be nice to rewrite this error to include metadata for failed dependency
...@@ -4498,7 +4494,7 @@ pub fn writeHeader(self: *MachO, ncmds: u32, sizeofcmds: u32) !void {...@@ -4498,7 +4494,7 @@ pub fn writeHeader(self: *MachO, ncmds: u32, sizeofcmds: u32) !void {
4498 header.cputype = macho.CPU_TYPE_X86_64;4494 header.cputype = macho.CPU_TYPE_X86_64;
4499 header.cpusubtype = macho.CPU_SUBTYPE_X86_64_ALL;4495 header.cpusubtype = macho.CPU_SUBTYPE_X86_64_ALL;
4500 },4496 },
4501 else => return error.UnsupportedCpuArchitecture,4497 else => unreachable,
4502 }4498 }
45034499
4504 switch (self.base.options.output_mode) {4500 switch (self.base.options.output_mode) {
...@@ -4866,11 +4862,17 @@ pub fn getSectionPrecedence(header: macho.section_64) u8 {...@@ -4866,11 +4862,17 @@ pub fn getSectionPrecedence(header: macho.section_64) u8 {
48664862
4867pub const ParseErrorCtx = struct {4863pub const ParseErrorCtx = struct {
4868 arena_allocator: std.heap.ArenaAllocator,4864 arena_allocator: std.heap.ArenaAllocator,
4865 detected_dylib_id: struct {
4866 parent: u16,
4867 required_version: u32,
4868 found_version: u32,
4869 },
4869 detected_targets: std.ArrayList([]const u8),4870 detected_targets: std.ArrayList([]const u8),
48704871
4871 pub fn init(gpa: Allocator) ParseErrorCtx {4872 pub fn init(gpa: Allocator) ParseErrorCtx {
4872 return .{4873 return .{
4873 .arena_allocator = std.heap.ArenaAllocator.init(gpa),4874 .arena_allocator = std.heap.ArenaAllocator.init(gpa),
4875 .detected_dylib_id = undefined,
4874 .detected_targets = std.ArrayList([]const u8).init(gpa),4876 .detected_targets = std.ArrayList([]const u8).init(gpa),
4875 };4877 };
4876 }4878 }
...@@ -4894,6 +4896,18 @@ pub fn handleAndReportParseError(...@@ -4894,6 +4896,18 @@ pub fn handleAndReportParseError(
4894 const cpu_arch = self.base.options.target.cpu.arch;4896 const cpu_arch = self.base.options.target.cpu.arch;
4895 switch (err) {4897 switch (err) {
4896 error.DylibAlreadyExists => {},4898 error.DylibAlreadyExists => {},
4899 error.IncompatibleDylibVersion => {
4900 const parent = &self.dylibs.items[ctx.detected_dylib_id.parent];
4901 try self.reportDependencyError(
4902 if (parent.id) |id| id.name else parent.path,
4903 path,
4904 "incompatible dylib version: expected at least '{}', but found '{}'",
4905 .{
4906 load_commands.appleVersionToSemanticVersion(ctx.detected_dylib_id.required_version),
4907 load_commands.appleVersionToSemanticVersion(ctx.detected_dylib_id.found_version),
4908 },
4909 );
4910 },
4897 error.UnknownFileType => try self.reportParseError(path, "unknown file type", .{}),4911 error.UnknownFileType => try self.reportParseError(path, "unknown file type", .{}),
4898 error.InvalidTarget, error.InvalidTargetFatLibrary => {4912 error.InvalidTarget, error.InvalidTargetFatLibrary => {
4899 var targets_string = std.ArrayList(u8).init(self.base.allocator);4913 var targets_string = std.ArrayList(u8).init(self.base.allocator);
...@@ -4923,7 +4937,28 @@ pub fn handleAndReportParseError(...@@ -4923,7 +4937,28 @@ pub fn handleAndReportParseError(
4923 }4937 }
4924}4938}
49254939
4926pub fn reportParseError(self: *MachO, path: []const u8, comptime format: []const u8, args: anytype) !void {4940fn reportDependencyError(
4941 self: *MachO,
4942 parent: []const u8,
4943 path: ?[]const u8,
4944 comptime format: []const u8,
4945 args: anytype,
4946) !void {
4947 const gpa = self.base.allocator;
4948 try self.misc_errors.ensureUnusedCapacity(gpa, 1);
4949 var notes = try std.ArrayList(File.ErrorMsg).initCapacity(gpa, 2);
4950 defer notes.deinit();
4951 if (path) |p| {
4952 notes.appendAssumeCapacity(.{ .msg = try std.fmt.allocPrint(gpa, "while parsing {s}", .{p}) });
4953 }
4954 notes.appendAssumeCapacity(.{ .msg = try std.fmt.allocPrint(gpa, "a dependency of {s}", .{parent}) });
4955 self.misc_errors.appendAssumeCapacity(.{
4956 .msg = try std.fmt.allocPrint(gpa, format, args),
4957 .notes = try notes.toOwnedSlice(),
4958 });
4959}
4960
4961fn reportParseError(self: *MachO, path: []const u8, comptime format: []const u8, args: anytype) !void {
4927 const gpa = self.base.allocator;4962 const gpa = self.base.allocator;
4928 try self.misc_errors.ensureUnusedCapacity(gpa, 1);4963 try self.misc_errors.ensureUnusedCapacity(gpa, 1);
4929 var notes = try gpa.alloc(File.ErrorMsg, 1);4964 var notes = try gpa.alloc(File.ErrorMsg, 1);
src/link/MachO/load_commands.zig+2-2
...@@ -440,14 +440,14 @@ const supported_platforms = [_]SupportedPlatforms{...@@ -440,14 +440,14 @@ const supported_platforms = [_]SupportedPlatforms{
440};440};
441// zig fmt: on441// zig fmt: on
442442
443pub inline fn semanticVersionToAppleVersion(version: std.SemanticVersion) u32 {443inline fn semanticVersionToAppleVersion(version: std.SemanticVersion) u32 {
444 const major = version.major;444 const major = version.major;
445 const minor = version.minor;445 const minor = version.minor;
446 const patch = version.patch;446 const patch = version.patch;
447 return (@as(u32, @intCast(major)) << 16) | (@as(u32, @intCast(minor)) << 8) | @as(u32, @intCast(patch));447 return (@as(u32, @intCast(major)) << 16) | (@as(u32, @intCast(minor)) << 8) | @as(u32, @intCast(patch));
448}448}
449449
450inline fn appleVersionToSemanticVersion(version: u32) std.SemanticVersion {450pub inline fn appleVersionToSemanticVersion(version: u32) std.SemanticVersion {
451 return .{451 return .{
452 .major = @as(u16, @truncate(version >> 16)),452 .major = @as(u16, @truncate(version >> 16)),
453 .minor = @as(u8, @truncate(version >> 8)),453 .minor = @as(u8, @truncate(version >> 8)),
src/link/MachO/zld.zig+2-4
...@@ -340,10 +340,7 @@ pub fn linkWithZld(...@@ -340,10 +340,7 @@ pub fn linkWithZld(
340 Compilation.dump_argv(argv.items);340 Compilation.dump_argv(argv.items);
341 }341 }
342342
343 var dependent_libs = std.fifo.LinearFifo(struct {343 var dependent_libs = std.fifo.LinearFifo(MachO.DylibReExportInfo, .Dynamic).init(arena);
344 id: Dylib.Id,
345 parent: u16,
346 }, .Dynamic).init(arena);
347344
348 for (positionals.items) |obj| {345 for (positionals.items) |obj| {
349 const in_file = try std.fs.cwd().openFile(obj.path, .{});346 const in_file = try std.fs.cwd().openFile(obj.path, .{});
...@@ -374,6 +371,7 @@ pub fn linkWithZld(...@@ -374,6 +371,7 @@ pub fn linkWithZld(
374 lib,371 lib,
375 false,372 false,
376 false,373 false,
374 null,
377 &dependent_libs,375 &dependent_libs,
378 &parse_ctx,376 &parse_ctx,
379 ) catch |err| try macho_file.handleAndReportParseError(path, err, &parse_ctx);377 ) catch |err| try macho_file.handleAndReportParseError(path, err, &parse_ctx);