Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macho linking uses incorrect value for headerpad_max_install_names #13229

Closed
andrewrk opened this issue Oct 19, 2022 · 1 comment
Closed

macho linking uses incorrect value for headerpad_max_install_names #13229

andrewrk opened this issue Oct 19, 2022 · 1 comment
Labels
bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals. linking os-macos
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Oct 19, 2022

This value, std.os.PATH_MAX applies to the host target. However the linker is supposed to be linking for the target given to the compiler at runtime.

const name_len = if (assume_max_path_len) std.os.PATH_MAX else std.mem.len(name) + 1;

This bug is revealed clearly when compiling for WASI which has no std.os.PATH_MAX value, yet should still be able to cross-compile macos binaries:

$ stage3/bin/zig build -p wasi -Dskip-install-lib-files -Dtarget=wasm32-wasi
/home/andy/Downloads/zig/lib/std/os.zig:110:28: error: root struct of file 'os.wasi' has no member named 'PATH_MAX'
pub const PATH_MAX = system.PATH_MAX;
                     ~~~~~~^~~~~~~~~
referenced by:
    calcInstallNameLen: /home/andy/Downloads/zig/src/link/MachO.zig:3540:53
    calcLCsSize: /home/andy/Downloads/zig/src/link/MachO.zig:3577:24
    remaining reference traces hidden; use '-freference-trace' to see all reference traces

In theory we should be able to use instead something like std.os.darwin.PATH_MAX however the std lib is organized in a way that only makes that work if the host matches.

A quickfix instead would look something like this:

--- a/src/link/MachO.zig
+++ b/src/link/MachO.zig
@@ -3537,7 +3537,8 @@ pub fn populateMissingMetadata(self: *MachO) !void {
 }
 
 inline fn calcInstallNameLen(cmd_size: u64, name: []const u8, assume_max_path_len: bool) u64 {
-    const name_len = if (assume_max_path_len) std.os.PATH_MAX else std.mem.len(name) + 1;
+    const darwin_path_max = 1024;
+    const name_len = if (assume_max_path_len) darwin_path_max else std.mem.len(name) + 1;
     return mem.alignForwardGeneric(u64, cmd_size + name_len, @alignOf(u64));
 }
@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals. os-macos linking labels Oct 19, 2022
@andrewrk andrewrk modified the milestones: 0.10.1, 0.10.0 Oct 19, 2022
@andrewrk
Copy link
Member Author

Moving to 0.10.0 milestone so I can say in the release notes that Zig compiles for WASI now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals. linking os-macos
Projects
None yet
Development

No branches or pull requests

1 participant