From 819716b59f941253ae98405bb4b30049aea5f8de Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Wed, 29 Jan 2025 14:11:23 +0100 Subject: [PATCH 1/2] link: fix ambiguous names in linker scripts Currently zig fails to build while linking the system LLVM/C++ libraries on my Chimera Linux system due to the fact that libc++.so is a linker script with the following contents: INPUT(libc++.so.1 -lc++abi -lunwind) Prior to this commit, zig would try to convert "ambiguous names" in linker scripts such as libc++.so.1 in this example into -lfoo style flags. This fails in this case due to the so version number as zig checks for exactly the .so suffix. Furthermore, I do not think that this conversion is semantically correct since converting libfoo.so to -lfoo could theoretically end up resulting in libfoo.a getting linked which seems wrong when a different file is specified in the linker script. With this patch, this attempted conversion is removed. Instead, zig always first checks if the exact file/path in the linker script exists relative to the current working directory. If the file is classified as a library (including versioned shared objects such as libfoo.so.1), zig then falls back to checking if the exact file/path in the linker script exists relative to each directory in the library search path, selecting the first match or erroring out if none is found. This behavior fixes the regression that prevents building zig while linking the system LLVM/C++ libraries on Chimera Linux. --- src/link.zig | 113 ++++++++++++++++++++++++++++------------------ test/link/elf.zig | 27 +++++++---- 2 files changed, 87 insertions(+), 53 deletions(-) diff --git a/src/link.zig b/src/link.zig index ec59cce101..6518daea7b 100644 --- a/src/link.zig +++ b/src/link.zig @@ -1891,30 +1891,69 @@ pub fn resolveInputs( syslib: while (unresolved_inputs.pop()) |unresolved_input| { const name_query: UnresolvedInput.NameQuery = switch (unresolved_input) { .name_query => |nq| nq, - .ambiguous_name => |an| an: { - const lib_name, const link_mode = stripLibPrefixAndSuffix(an.name, target) orelse { - try resolvePathInput(gpa, arena, unresolved_inputs, resolved_inputs, &ld_script_bytes, target, .{ + .ambiguous_name => |an| { + // First check the path relative to the current working directory. + // If the file is a library and is not found there, check the library search paths as well. + // This is consistent with the behavior of GNU ld. + if (try resolvePathInput( + gpa, + arena, + unresolved_inputs, + resolved_inputs, + &ld_script_bytes, + target, + .{ .path = Path.initCwd(an.name), .query = an.query, - }, color); - continue; - }; - break :an .{ - .name = lib_name, - .query = .{ - .needed = an.query.needed, - .weak = an.query.weak, - .reexport = an.query.reexport, - .must_link = an.query.must_link, - .hidden = an.query.hidden, - .allow_so_scripts = an.query.allow_so_scripts, - .preferred_mode = link_mode, - .search_strategy = .no_fallback, }, - }; + color, + )) |lib_result| { + switch (lib_result) { + .ok => continue :syslib, + .no_match => { + for (lib_directories) |lib_directory| { + switch ((try resolvePathInput( + gpa, + arena, + unresolved_inputs, + resolved_inputs, + &ld_script_bytes, + target, + .{ + .path = .{ + .root_dir = lib_directory, + .sub_path = an.name, + }, + .query = an.query, + }, + color, + )).?) { + .ok => continue :syslib, + .no_match => {}, + } + } + fatal("{s}: file listed in linker script not found", .{an.name}); + }, + } + } + continue; }, .path_query => |pq| { - try resolvePathInput(gpa, arena, unresolved_inputs, resolved_inputs, &ld_script_bytes, target, pq, color); + if (try resolvePathInput( + gpa, + arena, + unresolved_inputs, + resolved_inputs, + &ld_script_bytes, + target, + pq, + color, + )) |lib_result| { + switch (lib_result) { + .ok => {}, + .no_match => fatal("{}: file not found", .{pq.path}), + } + } continue; }, .dso_exact => |dso_exact| { @@ -2176,10 +2215,10 @@ fn resolvePathInput( target: std.Target, pq: UnresolvedInput.PathQuery, color: std.zig.Color, -) Allocator.Error!void { - switch (switch (Compilation.classifyFileExt(pq.path.sub_path)) { - .static_library => try resolvePathInputLib(gpa, arena, unresolved_inputs, resolved_inputs, ld_script_bytes, target, pq, .static, color), - .shared_library => try resolvePathInputLib(gpa, arena, unresolved_inputs, resolved_inputs, ld_script_bytes, target, pq, .dynamic, color), +) Allocator.Error!?ResolveLibInputResult { + switch (Compilation.classifyFileExt(pq.path.sub_path)) { + .static_library => return try resolvePathInputLib(gpa, arena, unresolved_inputs, resolved_inputs, ld_script_bytes, target, pq, .static, color), + .shared_library => return try resolvePathInputLib(gpa, arena, unresolved_inputs, resolved_inputs, ld_script_bytes, target, pq, .dynamic, color), .object => { var file = pq.path.root_dir.handle.openFile(pq.path.sub_path, .{}) catch |err| fatal("failed to open object {}: {s}", .{ pq.path, @errorName(err) }); @@ -2190,7 +2229,7 @@ fn resolvePathInput( .must_link = pq.query.must_link, .hidden = pq.query.hidden, } }); - return; + return null; }, .res => { var file = pq.path.root_dir.handle.openFile(pq.path.sub_path, .{}) catch |err| @@ -2200,12 +2239,9 @@ fn resolvePathInput( .path = pq.path, .file = file, } }); - return; + return null; }, else => fatal("{}: unrecognized file extension", .{pq.path}), - }) { - .ok => {}, - .no_match => fatal("{}: file not found", .{pq.path}), } } @@ -2226,9 +2262,11 @@ fn resolvePathInputLib( try resolved_inputs.ensureUnusedCapacity(gpa, 1); const test_path: Path = pq.path; - // In the case of .so files, they might actually be "linker scripts" + // In the case of shared libraries, they might actually be "linker scripts" // that contain references to other libraries. - if (pq.query.allow_so_scripts and target.ofmt == .elf and mem.endsWith(u8, test_path.sub_path, ".so")) { + if (pq.query.allow_so_scripts and target.ofmt == .elf and + Compilation.classifyFileExt(test_path.sub_path) == .shared_library) + { var file = test_path.root_dir.handle.openFile(test_path.sub_path, .{}) catch |err| switch (err) { error.FileNotFound => return .no_match, else => |e| fatal("unable to search for {s} library '{'}': {s}", .{ @@ -2354,21 +2392,6 @@ pub fn openDsoInput(diags: *Diags, path: Path, needed: bool, weak: bool, reexpor } }; } -fn stripLibPrefixAndSuffix(path: []const u8, target: std.Target) ?struct { []const u8, std.builtin.LinkMode } { - const prefix = target.libPrefix(); - const static_suffix = target.staticLibSuffix(); - const dynamic_suffix = target.dynamicLibSuffix(); - const basename = fs.path.basename(path); - const unlibbed = if (mem.startsWith(u8, basename, prefix)) basename[prefix.len..] else return null; - if (mem.endsWith(u8, unlibbed, static_suffix)) return .{ - unlibbed[0 .. unlibbed.len - static_suffix.len], .static, - }; - if (mem.endsWith(u8, unlibbed, dynamic_suffix)) return .{ - unlibbed[0 .. unlibbed.len - dynamic_suffix.len], .dynamic, - }; - return null; -} - /// Returns true if and only if there is at least one input of type object, /// archive, or Windows resource file. pub fn anyObjectInputs(inputs: []const Input) bool { diff --git a/test/link/elf.zig b/test/link/elf.zig index a64da3ad20..dfa5954032 100644 --- a/test/link/elf.zig +++ b/test/link/elf.zig @@ -2123,24 +2123,35 @@ fn testLinkOrder(b: *Build, opts: Options) *Step { fn testLdScript(b: *Build, opts: Options) *Step { const test_step = addTestStep(b, "ld-script", opts); - const dso = addSharedLibrary(b, opts, .{ .name = "bar" }); - addCSourceBytes(dso, "int foo() { return 42; }", &.{}); + const bar = addSharedLibrary(b, opts, .{ .name = "bar" }); + addCSourceBytes(bar, "int bar() { return 42; }", &.{}); + + const baz = addSharedLibrary(b, opts, .{ .name = "baz" }); + addCSourceBytes(baz, "int baz() { return 42; }", &.{}); const scripts = WriteFile.create(b); - _ = scripts.add("liba.so", "INPUT(libfoo.so)"); + _ = scripts.add("liba.so", "INPUT(libfoo.so libfoo2.so.1)"); _ = scripts.add("libfoo.so", "GROUP(AS_NEEDED(-lbar))"); + // Check finding a versioned .so file that is elsewhere in the library search paths. + const scripts2 = WriteFile.create(b); + _ = scripts2.add("libfoo2.so.1", "GROUP(AS_NEEDED(-lbaz))"); + const exe = addExecutable(b, opts, .{ .name = "main" }); addCSourceBytes(exe, - \\int foo(); + \\int bar(); + \\int baz(); \\int main() { - \\ return foo() - 42; + \\ return bar() - baz(); \\} , &.{}); exe.linkSystemLibrary2("a", .{}); exe.addLibraryPath(scripts.getDirectory()); - exe.addLibraryPath(dso.getEmittedBinDirectory()); - exe.addRPath(dso.getEmittedBinDirectory()); + exe.addLibraryPath(scripts2.getDirectory()); + exe.addLibraryPath(bar.getEmittedBinDirectory()); + exe.addLibraryPath(baz.getEmittedBinDirectory()); + exe.addRPath(bar.getEmittedBinDirectory()); + exe.addRPath(baz.getEmittedBinDirectory()); exe.linkLibC(); exe.allow_so_scripts = true; @@ -2167,7 +2178,7 @@ fn testLdScriptPathError(b: *Build, opts: Options) *Step { // TODO: A future enhancement could make this error message also mention // the file that references the missing library. expectLinkErrors(exe, test_step, .{ - .stderr_contains = "error: unable to find dynamic system library 'foo' using strategy 'no_fallback'. searched paths:", + .stderr_contains = "error: libfoo.so: file listed in linker script not found", }); return test_step; From 0499c731eabd2c88928ef740b488706c51a8a6a7 Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Wed, 29 Jan 2025 14:25:13 +0100 Subject: [PATCH 2/2] link: simplify control flow This refactor was left out of the previous commit to make the diff less noisy and easier to review. There should be no change in behavior. --- src/link.zig | 219 ++++++++++++++++++++++++++------------------------- 1 file changed, 110 insertions(+), 109 deletions(-) diff --git a/src/link.zig b/src/link.zig index 6518daea7b..e3885bdbac 100644 --- a/src/link.zig +++ b/src/link.zig @@ -1889,8 +1889,116 @@ pub fn resolveInputs( mem.reverse(UnresolvedInput, unresolved_inputs.items); syslib: while (unresolved_inputs.pop()) |unresolved_input| { - const name_query: UnresolvedInput.NameQuery = switch (unresolved_input) { - .name_query => |nq| nq, + switch (unresolved_input) { + .name_query => |name_query| { + const query = name_query.query; + + // Checked in the first pass above while looking for libc libraries. + assert(!fs.path.isAbsolute(name_query.name)); + + checked_paths.clearRetainingCapacity(); + + switch (query.search_strategy) { + .mode_first, .no_fallback => { + // check for preferred mode + for (lib_directories) |lib_directory| switch (try resolveLibInput( + gpa, + arena, + unresolved_inputs, + resolved_inputs, + &checked_paths, + &ld_script_bytes, + lib_directory, + name_query, + target, + query.preferred_mode, + color, + )) { + .ok => continue :syslib, + .no_match => {}, + }; + // check for fallback mode + if (query.search_strategy == .no_fallback) { + try failed_libs.append(arena, .{ + .name = name_query.name, + .strategy = query.search_strategy, + .checked_paths = try arena.dupe(u8, checked_paths.items), + .preferred_mode = query.preferred_mode, + }); + continue :syslib; + } + for (lib_directories) |lib_directory| switch (try resolveLibInput( + gpa, + arena, + unresolved_inputs, + resolved_inputs, + &checked_paths, + &ld_script_bytes, + lib_directory, + name_query, + target, + query.fallbackMode(), + color, + )) { + .ok => continue :syslib, + .no_match => {}, + }; + try failed_libs.append(arena, .{ + .name = name_query.name, + .strategy = query.search_strategy, + .checked_paths = try arena.dupe(u8, checked_paths.items), + .preferred_mode = query.preferred_mode, + }); + continue :syslib; + }, + .paths_first => { + for (lib_directories) |lib_directory| { + // check for preferred mode + switch (try resolveLibInput( + gpa, + arena, + unresolved_inputs, + resolved_inputs, + &checked_paths, + &ld_script_bytes, + lib_directory, + name_query, + target, + query.preferred_mode, + color, + )) { + .ok => continue :syslib, + .no_match => {}, + } + + // check for fallback mode + switch (try resolveLibInput( + gpa, + arena, + unresolved_inputs, + resolved_inputs, + &checked_paths, + &ld_script_bytes, + lib_directory, + name_query, + target, + query.fallbackMode(), + color, + )) { + .ok => continue :syslib, + .no_match => {}, + } + } + try failed_libs.append(arena, .{ + .name = name_query.name, + .strategy = query.search_strategy, + .checked_paths = try arena.dupe(u8, checked_paths.items), + .preferred_mode = query.preferred_mode, + }); + continue :syslib; + }, + } + }, .ambiguous_name => |an| { // First check the path relative to the current working directory. // If the file is a library and is not found there, check the library search paths as well. @@ -1960,113 +2068,6 @@ pub fn resolveInputs( try resolved_inputs.append(gpa, .{ .dso_exact = dso_exact }); continue; }, - }; - const query = name_query.query; - - // Checked in the first pass above while looking for libc libraries. - assert(!fs.path.isAbsolute(name_query.name)); - - checked_paths.clearRetainingCapacity(); - - switch (query.search_strategy) { - .mode_first, .no_fallback => { - // check for preferred mode - for (lib_directories) |lib_directory| switch (try resolveLibInput( - gpa, - arena, - unresolved_inputs, - resolved_inputs, - &checked_paths, - &ld_script_bytes, - lib_directory, - name_query, - target, - query.preferred_mode, - color, - )) { - .ok => continue :syslib, - .no_match => {}, - }; - // check for fallback mode - if (query.search_strategy == .no_fallback) { - try failed_libs.append(arena, .{ - .name = name_query.name, - .strategy = query.search_strategy, - .checked_paths = try arena.dupe(u8, checked_paths.items), - .preferred_mode = query.preferred_mode, - }); - continue :syslib; - } - for (lib_directories) |lib_directory| switch (try resolveLibInput( - gpa, - arena, - unresolved_inputs, - resolved_inputs, - &checked_paths, - &ld_script_bytes, - lib_directory, - name_query, - target, - query.fallbackMode(), - color, - )) { - .ok => continue :syslib, - .no_match => {}, - }; - try failed_libs.append(arena, .{ - .name = name_query.name, - .strategy = query.search_strategy, - .checked_paths = try arena.dupe(u8, checked_paths.items), - .preferred_mode = query.preferred_mode, - }); - continue :syslib; - }, - .paths_first => { - for (lib_directories) |lib_directory| { - // check for preferred mode - switch (try resolveLibInput( - gpa, - arena, - unresolved_inputs, - resolved_inputs, - &checked_paths, - &ld_script_bytes, - lib_directory, - name_query, - target, - query.preferred_mode, - color, - )) { - .ok => continue :syslib, - .no_match => {}, - } - - // check for fallback mode - switch (try resolveLibInput( - gpa, - arena, - unresolved_inputs, - resolved_inputs, - &checked_paths, - &ld_script_bytes, - lib_directory, - name_query, - target, - query.fallbackMode(), - color, - )) { - .ok => continue :syslib, - .no_match => {}, - } - } - try failed_libs.append(arena, .{ - .name = name_query.name, - .strategy = query.search_strategy, - .checked_paths = try arena.dupe(u8, checked_paths.items), - .preferred_mode = query.preferred_mode, - }); - continue :syslib; - }, } @compileError("unreachable"); }