authorgravatar for kubkon@jakubkonka.comJakub Konka <kubkon@jakubkonka.com> 2021-07-31 16:01:02+02:00
committergravatar for kubkon@jakubkonka.comJakub Konka <kubkon@jakubkonka.com> 2021-08-01 09:06:56+02:00
log0b15ba8334bc03b59a975e579aac01a6b3fc2109
tree267ab3306af2b4277b933770de1d3fd696260372
parentf023cdad7ca676977d9b5abd3d38677779aab211

macho: don't allocate Dylib on the heap

instead, immediately transfer ownership to MachO struct. Also, revert back to try-ok-fail parsing approach of objects, archives, and dylibs. It seems easier to try and fail than check if the file *is* of a certain type given that a dylib may be a stub and parsing yaml twice in a row seems very wasteful. Hint for the future: if we optimise yaml/TAPI parsing, this approach may be rethought!

4 files changed, 85 insertions(+), 77 deletions(-)

src/link/MachO.zig+13-34
...@@ -63,7 +63,7 @@ entry_addr: ?u64 = null,...@@ -63,7 +63,7 @@ entry_addr: ?u64 = null,
6363
64objects: std.ArrayListUnmanaged(Object) = .{},64objects: std.ArrayListUnmanaged(Object) = .{},
65archives: std.ArrayListUnmanaged(Archive) = .{},65archives: std.ArrayListUnmanaged(Archive) = .{},
66dylibs: std.ArrayListUnmanaged(*Dylib) = .{},66dylibs: std.ArrayListUnmanaged(Dylib) = .{},
6767
68next_dylib_ordinal: u16 = 1,68next_dylib_ordinal: u16 = 1,
6969
...@@ -994,25 +994,15 @@ fn parseInputFiles(self: *MachO, files: []const []const u8, syslibroot: ?[]const...@@ -994,25 +994,15 @@ fn parseInputFiles(self: *MachO, files: []const []const u8, syslibroot: ?[]const
994 const path = try std.fs.realpath(file_name, &buffer);994 const path = try std.fs.realpath(file_name, &buffer);
995 break :full_path try self.base.allocator.dupe(u8, path);995 break :full_path try self.base.allocator.dupe(u8, path);
996 };996 };
997 const file = try fs.cwd().openFile(full_path, .{});997 defer self.base.allocator.free(full_path);
998998
999 if (try Object.isObject(file)) {999 if (try Object.createAndParseFromPath(self.base.allocator, arch, full_path)) |object| {
1000 const object = try self.objects.addOne(self.base.allocator);1000 try self.objects.append(self.base.allocator, object);
1001 object.* = .{
1002 .name = full_path,
1003 .file = file,
1004 };
1005 try object.parse(self.base.allocator, arch);
1006 continue;1001 continue;
1007 }1002 }
10081003
1009 if (try Archive.isArchive(file, arch)) {1004 if (try Archive.createAndParseFromPath(self.base.allocator, arch, full_path)) |archive| {
1010 const archive = try self.archives.addOne(self.base.allocator);1005 try self.archives.append(self.base.allocator, archive);
1011 archive.* = .{
1012 .name = full_path,
1013 .file = file,
1014 };
1015 try archive.parse(self.base.allocator, arch);
1016 continue;1006 continue;
1017 }1007 }
10181008
...@@ -1024,8 +1014,6 @@ fn parseInputFiles(self: *MachO, files: []const []const u8, syslibroot: ?[]const...@@ -1024,8 +1014,6 @@ fn parseInputFiles(self: *MachO, files: []const []const u8, syslibroot: ?[]const
1024 continue;1014 continue;
1025 }1015 }
10261016
1027 self.base.allocator.free(full_path);
1028 file.close();
1029 log.warn("unknown filetype for positional input file: '{s}'", .{file_name});1017 log.warn("unknown filetype for positional input file: '{s}'", .{file_name});
1030 }1018 }
1031}1019}
...@@ -1033,8 +1021,6 @@ fn parseInputFiles(self: *MachO, files: []const []const u8, syslibroot: ?[]const...@@ -1033,8 +1021,6 @@ fn parseInputFiles(self: *MachO, files: []const []const u8, syslibroot: ?[]const
1033fn parseLibs(self: *MachO, libs: []const []const u8, syslibroot: ?[]const u8) !void {1021fn parseLibs(self: *MachO, libs: []const []const u8, syslibroot: ?[]const u8) !void {
1034 const arch = self.base.options.target.cpu.arch;1022 const arch = self.base.options.target.cpu.arch;
1035 for (libs) |lib| {1023 for (libs) |lib| {
1036 const file = try fs.cwd().openFile(lib, .{});
1037
1038 if (try Dylib.createAndParseFromPath(self.base.allocator, arch, lib, .{1024 if (try Dylib.createAndParseFromPath(self.base.allocator, arch, lib, .{
1039 .syslibroot = syslibroot,1025 .syslibroot = syslibroot,
1040 })) |dylibs| {1026 })) |dylibs| {
...@@ -1043,17 +1029,11 @@ fn parseLibs(self: *MachO, libs: []const []const u8, syslibroot: ?[]const u8) !v...@@ -1043,17 +1029,11 @@ fn parseLibs(self: *MachO, libs: []const []const u8, syslibroot: ?[]const u8) !v
1043 continue;1029 continue;
1044 }1030 }
10451031
1046 if (try Archive.isArchive(file, arch)) {1032 if (try Archive.createAndParseFromPath(self.base.allocator, arch, lib)) |archive| {
1047 const archive = try self.archives.addOne(self.base.allocator);1033 try self.archives.append(self.base.allocator, archive);
1048 archive.* = .{
1049 .name = try self.base.allocator.dupe(u8, lib),
1050 .file = file,
1051 };
1052 try archive.parse(self.base.allocator, arch);
1053 continue;1034 continue;
1054 }1035 }
10551036
1056 file.close();
1057 log.warn("unknown filetype for a library: '{s}'", .{lib});1037 log.warn("unknown filetype for a library: '{s}'", .{lib});
1058 }1038 }
1059}1039}
...@@ -2351,17 +2331,17 @@ fn resolveSymbols(self: *MachO) !void {...@@ -2351,17 +2331,17 @@ fn resolveSymbols(self: *MachO) !void {
2351 });2331 });
2352 }2332 }
23532333
2354 var referenced = std.AutoHashMap(*Dylib, void).init(self.base.allocator);2334 var referenced = std.AutoHashMap(u16, void).init(self.base.allocator);
2355 defer referenced.deinit();2335 defer referenced.deinit();
23562336
2357 loop: for (self.undefs.items) |sym| {2337 loop: for (self.undefs.items) |sym| {
2358 if (symbolIsNull(sym)) continue;2338 if (symbolIsNull(sym)) continue;
23592339
2360 const sym_name = self.getString(sym.n_strx);2340 const sym_name = self.getString(sym.n_strx);
2361 for (self.dylibs.items) |dylib| {2341 for (self.dylibs.items) |*dylib, id| {
2362 if (!dylib.symbols.contains(sym_name)) continue;2342 if (!dylib.symbols.contains(sym_name)) continue;
23632343
2364 if (!referenced.contains(dylib)) {2344 if (!referenced.contains(@intCast(u16, id))) {
2365 // Add LC_LOAD_DYLIB load command for each referenced dylib/stub.2345 // Add LC_LOAD_DYLIB load command for each referenced dylib/stub.
2366 dylib.ordinal = self.next_dylib_ordinal;2346 dylib.ordinal = self.next_dylib_ordinal;
2367 const dylib_id = dylib.id orelse unreachable;2347 const dylib_id = dylib.id orelse unreachable;
...@@ -2375,7 +2355,7 @@ fn resolveSymbols(self: *MachO) !void {...@@ -2375,7 +2355,7 @@ fn resolveSymbols(self: *MachO) !void {
2375 errdefer dylib_cmd.deinit(self.base.allocator);2355 errdefer dylib_cmd.deinit(self.base.allocator);
2376 try self.load_commands.append(self.base.allocator, .{ .Dylib = dylib_cmd });2356 try self.load_commands.append(self.base.allocator, .{ .Dylib = dylib_cmd });
2377 self.next_dylib_ordinal += 1;2357 self.next_dylib_ordinal += 1;
2378 try referenced.putNoClobber(dylib, {});2358 try referenced.putNoClobber(@intCast(u16, id), {});
2379 }2359 }
23802360
2381 const resolv = self.symbol_resolver.getPtr(sym.n_strx) orelse unreachable;2361 const resolv = self.symbol_resolver.getPtr(sym.n_strx) orelse unreachable;
...@@ -3365,9 +3345,8 @@ pub fn deinit(self: *MachO) void {...@@ -3365,9 +3345,8 @@ pub fn deinit(self: *MachO) void {
3365 }3345 }
3366 self.archives.deinit(self.base.allocator);3346 self.archives.deinit(self.base.allocator);
33673347
3368 for (self.dylibs.items) |dylib| {3348 for (self.dylibs.items) |*dylib| {
3369 dylib.deinit(self.base.allocator);3349 dylib.deinit(self.base.allocator);
3370 self.base.allocator.destroy(dylib);
3371 }3350 }
3372 self.dylibs.deinit(self.base.allocator);3351 self.dylibs.deinit(self.base.allocator);
33733352
src/link/MachO/Archive.zig+35-21
...@@ -104,38 +104,52 @@ pub fn deinit(self: *Archive, allocator: *Allocator) void {...@@ -104,38 +104,52 @@ pub fn deinit(self: *Archive, allocator: *Allocator) void {
104 allocator.free(self.name);104 allocator.free(self.name);
105}105}
106106
107pub fn isArchive(file: fs.File, arch: Arch) !bool {107pub fn createAndParseFromPath(allocator: *Allocator, arch: Arch, path: []const u8) !?Archive {
108 const Internal = struct {108 const file = fs.cwd().openFile(path, .{}) catch |err| switch (err) {
109 fn isArchive(reader: anytype, a: Arch) !bool {109 error.FileNotFound => return null,
110 const offset = try fat.getLibraryOffset(reader, a);110 else => |e| return e,
111 try reader.context.seekTo(offset);111 };
112 const magic = try reader.readBytesNoEof(SARMAG);112 errdefer file.close();
113 if (!mem.eql(u8, &magic, ARMAG)) return false;113
114 const header = try reader.readStruct(ar_hdr);114 const name = try allocator.dupe(u8, path);
115 return mem.eql(u8, &header.ar_fmag, ARFMAG);115 errdefer allocator.free(name);
116 }116
117 var archive = Archive{
118 .name = name,
119 .file = file,
117 };120 };
118 const is_archive = if (Internal.isArchive(file.reader(), arch)) |res|121
119 res122 archive.parse(allocator, arch) catch |err| switch (err) {
120 else |err| switch (err) {123 error.EndOfStream, error.NotArchive => {
121 error.EndOfStream => false,124 archive.deinit(allocator);
122 error.MismatchedCpuArchitecture => true, // TODO maybe this check should be done differently?125 return null;
126 },
123 else => |e| return e,127 else => |e| return e,
124 };128 };
125 try file.seekTo(0);129
126 return is_archive;130 return archive;
127}131}
128132
129pub fn parse(self: *Archive, allocator: *Allocator, arch: Arch) !void {133pub fn parse(self: *Archive, allocator: *Allocator, arch: Arch) !void {
130 self.library_offset = try fat.getLibraryOffset(self.file.reader(), arch);
131 try self.file.seekTo(self.library_offset);
132 const reader = self.file.reader();134 const reader = self.file.reader();
135 self.library_offset = try fat.getLibraryOffset(reader, arch);
136 try self.file.seekTo(self.library_offset);
137
133 const magic = try reader.readBytesNoEof(SARMAG);138 const magic = try reader.readBytesNoEof(SARMAG);
139 if (!mem.eql(u8, &magic, ARMAG)) {
140 log.debug("invalid magic: expected '{s}', found '{s}'", .{ ARMAG, magic });
141 return error.NotArchive;
142 }
143
134 self.header = try reader.readStruct(ar_hdr);144 self.header = try reader.readStruct(ar_hdr);
135 var embedded_name = try parseName(allocator, self.header.?, reader);145 if (!mem.eql(u8, &self.header.?.ar_fmag, ARFMAG)) {
136 defer allocator.free(embedded_name);146 log.debug("invalid header delimiter: expected '{s}', found '{s}'", .{ ARFMAG, self.header.?.ar_fmag });
147 return error.NotArchive;
148 }
137149
150 var embedded_name = try parseName(allocator, self.header.?, reader);
138 log.debug("parsing archive '{s}' at '{s}'", .{ embedded_name, self.name });151 log.debug("parsing archive '{s}' at '{s}'", .{ embedded_name, self.name });
152 defer allocator.free(embedded_name);
139153
140 try self.parseTableOfContents(allocator, reader);154 try self.parseTableOfContents(allocator, reader);
141 try reader.context.seekTo(0);155 try reader.context.seekTo(0);
src/link/MachO/Dylib.zig+5-9
...@@ -148,20 +148,17 @@ pub fn createAndParseFromPath(...@@ -148,20 +148,17 @@ pub fn createAndParseFromPath(
148 arch: Arch,148 arch: Arch,
149 path: []const u8,149 path: []const u8,
150 opts: CreateOpts,150 opts: CreateOpts,
151) Error!?[]*Dylib {151) Error!?[]Dylib {
152 const file = fs.cwd().openFile(path, .{}) catch |err| switch (err) {152 const file = fs.cwd().openFile(path, .{}) catch |err| switch (err) {
153 error.FileNotFound => return null,153 error.FileNotFound => return null,
154 else => |e| return e,154 else => |e| return e,
155 };155 };
156 errdefer file.close();156 errdefer file.close();
157157
158 const dylib = try allocator.create(Dylib);
159 errdefer allocator.destroy(dylib);
160
161 const name = try allocator.dupe(u8, path);158 const name = try allocator.dupe(u8, path);
162 errdefer allocator.free(name);159 errdefer allocator.free(name);
163160
164 dylib.* = .{161 var dylib = Dylib{
165 .name = name,162 .name = name,
166 .file = file,163 .file = file,
167 };164 };
...@@ -172,7 +169,6 @@ pub fn createAndParseFromPath(...@@ -172,7 +169,6 @@ pub fn createAndParseFromPath(
172169
173 var lib_stub = LibStub.loadFromFile(allocator, file) catch {170 var lib_stub = LibStub.loadFromFile(allocator, file) catch {
174 dylib.deinit(allocator);171 dylib.deinit(allocator);
175 allocator.destroy(dylib);
176 return null;172 return null;
177 };173 };
178 defer lib_stub.deinit();174 defer lib_stub.deinit();
...@@ -191,12 +187,11 @@ pub fn createAndParseFromPath(...@@ -191,12 +187,11 @@ pub fn createAndParseFromPath(
191187
192 // TODO maybe this should be an error and facilitate auto-cleanup?188 // TODO maybe this should be an error and facilitate auto-cleanup?
193 dylib.deinit(allocator);189 dylib.deinit(allocator);
194 allocator.destroy(dylib);
195 return null;190 return null;
196 }191 }
197 }192 }
198193
199 var dylibs = std.ArrayList(*Dylib).init(allocator);194 var dylibs = std.ArrayList(Dylib).init(allocator);
200 defer dylibs.deinit();195 defer dylibs.deinit();
201196
202 try dylibs.append(dylib);197 try dylibs.append(dylib);
...@@ -449,7 +444,7 @@ pub fn parseDependentLibs(...@@ -449,7 +444,7 @@ pub fn parseDependentLibs(
449 self: *Dylib,444 self: *Dylib,
450 allocator: *Allocator,445 allocator: *Allocator,
451 arch: Arch,446 arch: Arch,
452 out: *std.ArrayList(*Dylib),447 out: *std.ArrayList(Dylib),
453 syslibroot: ?[]const u8,448 syslibroot: ?[]const u8,
454) !void {449) !void {
455 outer: for (self.dependent_libs.items) |id| {450 outer: for (self.dependent_libs.items) |id| {
...@@ -489,6 +484,7 @@ pub fn parseDependentLibs(...@@ -489,6 +484,7 @@ pub fn parseDependentLibs(
489 )) orelse {484 )) orelse {
490 continue;485 continue;
491 };486 };
487 defer allocator.free(dylibs);
492488
493 try out.appendSlice(dylibs);489 try out.appendSlice(dylibs);
494490
src/link/MachO/Object.zig+32-13
...@@ -154,29 +154,47 @@ pub fn deinit(self: *Object, allocator: *Allocator) void {...@@ -154,29 +154,47 @@ pub fn deinit(self: *Object, allocator: *Allocator) void {
154 }154 }
155}155}
156156
157pub fn isObject(file: fs.File) !bool {157pub fn createAndParseFromPath(allocator: *Allocator, arch: Arch, path: []const u8) !?Object {
158 const Internal = struct {158 const file = fs.cwd().openFile(path, .{}) catch |err| switch (err) {
159 fn isObject(reader: anytype) !bool {159 error.FileNotFound => return null,
160 const header = try reader.readStruct(macho.mach_header_64);160 else => |e| return e,
161 return header.filetype == macho.MH_OBJECT;
162 }
163 };161 };
164 const is_object = if (Internal.isObject(file.reader())) |res|162 errdefer file.close();
165 res163
166 else |err| switch (err) {164 const name = try allocator.dupe(u8, path);
167 error.EndOfStream => false,165 errdefer allocator.free(name);
166
167 var object = Object{
168 .name = name,
169 .file = file,
170 };
171
172 object.parse(allocator, arch) catch |err| switch (err) {
173 error.EndOfStream, error.NotObject => {
174 object.deinit(allocator);
175 return null;
176 },
168 else => |e| return e,177 else => |e| return e,
169 };178 };
170 try file.seekTo(0);179
171 return is_object;180 return object;
172}181}
173182
174pub fn parse(self: *Object, allocator: *Allocator, arch: Arch) !void {183pub fn parse(self: *Object, allocator: *Allocator, arch: Arch) !void {
175 var reader = self.file.reader();184 const reader = self.file.reader();
176 if (self.file_offset) |offset| {185 if (self.file_offset) |offset| {
177 try reader.context.seekTo(offset);186 try reader.context.seekTo(offset);
178 }187 }
188
179 const header = try reader.readStruct(macho.mach_header_64);189 const header = try reader.readStruct(macho.mach_header_64);
190 if (header.filetype != macho.MH_OBJECT) {
191 log.debug("invalid filetype: expected 0x{x}, found 0x{x}", .{
192 macho.MH_OBJECT,
193 header.filetype,
194 });
195 return error.NotObject;
196 }
197
180 const this_arch: Arch = switch (header.cputype) {198 const this_arch: Arch = switch (header.cputype) {
181 macho.CPU_TYPE_ARM64 => .aarch64,199 macho.CPU_TYPE_ARM64 => .aarch64,
182 macho.CPU_TYPE_X86_64 => .x86_64,200 macho.CPU_TYPE_X86_64 => .x86_64,
...@@ -189,6 +207,7 @@ pub fn parse(self: *Object, allocator: *Allocator, arch: Arch) !void {...@@ -189,6 +207,7 @@ pub fn parse(self: *Object, allocator: *Allocator, arch: Arch) !void {
189 log.err("mismatched cpu architecture: expected {s}, found {s}", .{ arch, this_arch });207 log.err("mismatched cpu architecture: expected {s}, found {s}", .{ arch, this_arch });
190 return error.MismatchedCpuArchitecture;208 return error.MismatchedCpuArchitecture;
191 }209 }
210
192 self.header = header;211 self.header = header;
193212
194 try self.readLoadCommands(allocator, reader);213 try self.readLoadCommands(allocator, reader);