Merge pull request #16553 from ziglang/issue-11896

macho: fix parsing of TBDv3 dylib stubs

6 files changed, 114 insertions, 18 deletions

diff --git a/src/link/MachO/Dylib.zig b/src/link/MachO/Dylib.zig
index ee8f34f756..2aacf4009b 100644
--- a/src/link/MachO/Dylib.zig
+++ b/src/link/MachO/Dylib.zig
@@ -9,12 +9,14 @@ const macho = std.macho;
 const math = std.math;
 const mem = std.mem;
 const fat = @import("fat.zig");
+const tapi = @import("../tapi.zig");
 
 const Allocator = mem.Allocator;
 const CrossTarget = std.zig.CrossTarget;
-const LibStub = @import("../tapi.zig").LibStub;
+const LibStub = tapi.LibStub;
 const LoadCommandIterator = macho.LoadCommandIterator;
 const MachO = @import("../MachO.zig");
+const Tbd = tapi.Tbd;
 
 id: ?Id = null,
 weak: bool = false,
@@ -247,7 +249,8 @@ const TargetMatcher = struct {
             .allocator = allocator,
             .target = target,
         };
-        try self.target_strings.append(allocator, try targetToAppleString(allocator, target));
+        const apple_string = try targetToAppleString(allocator, target);
+        try self.target_strings.append(allocator, apple_string);
 
         const abi = target.abi orelse .none;
         if (abi == .simulator) {
@@ -270,22 +273,29 @@ const TargetMatcher = struct {
         self.target_strings.deinit(self.allocator);
     }
 
-    fn targetToAppleString(allocator: Allocator, target: CrossTarget) ![]const u8 {
-        const cpu_arch = switch (target.cpu_arch.?) {
+    inline fn cpuArchToAppleString(cpu_arch: std.Target.Cpu.Arch) []const u8 {
+        return switch (cpu_arch) {
             .aarch64 => "arm64",
             .x86_64 => "x86_64",
             else => unreachable,
         };
-        const os_tag = @tagName(target.os_tag.?);
-        const target_abi = target.abi orelse .none;
-        const abi: ?[]const u8 = switch (target_abi) {
+    }
+
+    inline fn abiToAppleString(abi: std.Target.Abi) ?[]const u8 {
+        return switch (abi) {
             .none => null,
             .simulator => "simulator",
             .macabi => "maccatalyst",
             else => unreachable,
         };
-        if (abi) |x| {
-            return std.fmt.allocPrint(allocator, "{s}-{s}-{s}", .{ cpu_arch, os_tag, x });
+    }
+
+    fn targetToAppleString(allocator: Allocator, target: CrossTarget) ![]const u8 {
+        const cpu_arch = cpuArchToAppleString(target.cpu_arch.?);
+        const os_tag = @tagName(target.os_tag.?);
+        const target_abi = abiToAppleString(target.abi orelse .none);
+        if (target_abi) |abi| {
+            return std.fmt.allocPrint(allocator, "{s}-{s}-{s}", .{ cpu_arch, os_tag, abi });
         }
         return std.fmt.allocPrint(allocator, "{s}-{s}", .{ cpu_arch, os_tag });
     }
@@ -305,7 +315,26 @@ const TargetMatcher = struct {
     }
 
     fn matchesArch(self: TargetMatcher, archs: []const []const u8) bool {
-        return hasValue(archs, @tagName(self.target.cpu_arch.?));
+        return hasValue(archs, cpuArchToAppleString(self.target.cpu_arch.?));
+    }
+
+    fn matchesTargetTbd(self: TargetMatcher, tbd: Tbd) !bool {
+        var arena = std.heap.ArenaAllocator.init(self.allocator);
+        defer arena.deinit();
+
+        const targets = switch (tbd) {
+            .v3 => |v3| blk: {
+                var targets = std.ArrayList([]const u8).init(arena.allocator());
+                for (v3.archs) |arch| {
+                    const target = try std.fmt.allocPrint(arena.allocator(), "{s}-{s}", .{ arch, v3.platform });
+                    try targets.append(target);
+                }
+                break :blk targets.items;
+            },
+            .v4 => |v4| v4.targets,
+        };
+
+        return self.matchesTarget(targets);
     }
 };
 
@@ -348,11 +377,7 @@ pub fn parseFromStub(
     defer matcher.deinit();
 
     for (lib_stub.inner, 0..) |elem, stub_index| {
-        const is_match = switch (elem) {
-            .v3 => |stub| matcher.matchesArch(stub.archs),
-            .v4 => |stub| matcher.matchesTarget(stub.targets),
-        };
-        if (!is_match) continue;
+        if (!(try matcher.matchesTargetTbd(elem))) continue;
 
         if (stub_index > 0) {
             // TODO I thought that we could switch on presence of `parent-umbrella` map;
diff --git a/src/link/tapi/yaml.zig b/src/link/tapi/yaml.zig
index 0fc165003a..5e3602f620 100644
--- a/src/link/tapi/yaml.zig
+++ b/src/link/tapi/yaml.zig
@@ -197,7 +197,7 @@ pub const Value = union(enum) {
 
             return Value{ .string = try arena.dupe(u8, value.string_value.items) };
         } else {
-            log.err("Unexpected node type: {}", .{node.tag});
+            log.debug("Unexpected node type: {}", .{node.tag});
             return error.UnexpectedNodeType;
         }
     }
@@ -270,7 +270,7 @@ pub const Value = union(enum) {
                         if (try encode(arena, elem)) |value| {
                             list.appendAssumeCapacity(value);
                         } else {
-                            log.err("Could not encode value in a list: {any}", .{elem});
+                            log.debug("Could not encode value in a list: {any}", .{elem});
                             return error.CannotEncodeValue;
                         }
                     }
@@ -432,7 +432,7 @@ pub const Yaml = struct {
             }
 
             const unwrapped = value orelse {
-                log.err("missing struct field: {s}: {s}", .{ field.name, @typeName(field.type) });
+                log.debug("missing struct field: {s}: {s}", .{ field.name, @typeName(field.type) });
                 return error.StructFieldMissing;
             };
             @field(parsed, field.name) = try self.parseValue(field.type, unwrapped);
diff --git a/test/link.zig b/test/link.zig
index c877e6c357..f79b4a478c 100644
--- a/test/link.zig
+++ b/test/link.zig
@@ -165,6 +165,10 @@ pub const cases = [_]Case{
         .import = @import("link/macho/strict_validation/build.zig"),
     },
     .{
+        .build_root = "test/link/macho/tbdv3",
+        .import = @import("link/macho/tbdv3/build.zig"),
+    },
+    .{
         .build_root = "test/link/macho/tls",
         .import = @import("link/macho/tls/build.zig"),
     },
diff --git a/test/link/macho/tbdv3/a.c b/test/link/macho/tbdv3/a.c
new file mode 100644
index 0000000000..ddd18ee4b6
--- /dev/null
+++ b/test/link/macho/tbdv3/a.c
@@ -0,0 +1,3 @@
+int getFoo() {
+  return 42;
+}
diff --git a/test/link/macho/tbdv3/build.zig b/test/link/macho/tbdv3/build.zig
new file mode 100644
index 0000000000..c2eb6df626
--- /dev/null
+++ b/test/link/macho/tbdv3/build.zig
@@ -0,0 +1,57 @@
+const std = @import("std");
+const builtin = @import("builtin");
+
+pub const requires_symlinks = true;
+pub const requires_macos_sdk = false;
+
+pub fn build(b: *std.Build) void {
+    const test_step = b.step("test", "Test it");
+    b.default_step = test_step;
+
+    add(b, test_step, .Debug);
+    add(b, test_step, .ReleaseFast);
+    add(b, test_step, .ReleaseSmall);
+    add(b, test_step, .ReleaseSafe);
+}
+
+fn add(b: *std.Build, test_step: *std.Build.Step, optimize: std.builtin.OptimizeMode) void {
+    const target: std.zig.CrossTarget = .{ .os_tag = .macos };
+
+    const lib = b.addSharedLibrary(.{
+        .name = "a",
+        .version = .{ .major = 1, .minor = 0, .patch = 0 },
+        .optimize = optimize,
+        .target = target,
+    });
+    lib.addCSourceFile("a.c", &.{});
+    lib.linkLibC();
+
+    const tbd_file = b.addWriteFile("liba.tbd",
+        \\--- !tapi-tbd-v3
+        \\archs:           [ arm64, x86_64 ]
+        \\uuids:           [ 'arm64: DEADBEEF', 'x86_64: BEEFDEAD' ]
+        \\platform:        macos
+        \\install-name:    @rpath/liba.dylib
+        \\current-version: 0
+        \\exports:         
+        \\  - archs:           [ arm64, x86_64 ]
+        \\    symbols:         [ _getFoo ]
+    );
+
+    const exe = b.addExecutable(.{
+        .name = "test",
+        .optimize = optimize,
+        .target = target,
+    });
+    exe.addCSourceFile("main.c", &[0][]const u8{});
+    exe.linkSystemLibrary("a");
+    exe.addLibraryPathDirectorySource(tbd_file.getDirectorySource());
+    exe.addRPathDirectorySource(lib.getOutputDirectorySource());
+    exe.linkLibC();
+
+    const run = b.addRunArtifact(exe);
+    run.skip_foreign_checks = true;
+    run.expectExitCode(0);
+
+    test_step.dependOn(&run.step);
+}
diff --git a/test/link/macho/tbdv3/main.c b/test/link/macho/tbdv3/main.c
new file mode 100644
index 0000000000..3cf37ae590
--- /dev/null
+++ b/test/link/macho/tbdv3/main.c
@@ -0,0 +1,7 @@
+#include <stdio.h>
+
+int getFoo();
+
+int main() {
+  return getFoo() - 42;
+}