Skip to content

Commit

Permalink
stage2: default dynamic libraries to be linked as needed
Browse files Browse the repository at this point in the history 
After this change, the default for dynamic libraries (`-l` or
`--library`) is to only link them if they end up being actually used.

With the Zig CLI, the new options `-needed-l` or `--needed-library` can
be used to force link against a dynamic library.

With `zig cc`, this behavior can be overridden with `-Wl,--no-as-needed`
(and restored with `-Wl,--as-needed`).

Closes #10164
  • Loading branch information
@andrewrk
andrewrk committed Nov 21, 2021
1 parent a699d67 commit 4e5a88b
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 47 deletions.
8 changes: 0 additions & 8 deletions src/Cache.zig
View file
Expand Up @@ -90,14 +90,6 @@ pub const HashHelper = struct {
for (list_of_bytes) |bytes| hh.addBytes(bytes);
}

pub fn addStringSet(hh: *HashHelper, hm: std.StringArrayHashMapUnmanaged(void)) void {
const keys = hm.keys();
hh.add(keys.len);
for (keys) |key| {
hh.addBytes(key);
}
}

/// Convert the input value into bytes and record it as a dependency of the process being cached.
pub fn add(hh: *HashHelper, x: anytype) void {
switch (@TypeOf(x)) {
Expand Down
19 changes: 11 additions & 8 deletions src/Compilation.zig
View file
Expand Up @@ -627,6 +627,8 @@ pub const ClangPreprocessorMode = enum {
stdout,
};

pub const SystemLib = link.SystemLib;

pub const InitOptions = struct {
zig_lib_directory: Directory,
local_cache_directory: Directory,
Expand Down Expand Up @@ -672,7 +674,8 @@ pub const InitOptions = struct {
link_objects: []const []const u8 = &[0][]const u8{},
framework_dirs: []const []const u8 = &[0][]const u8{},
frameworks: []const []const u8 = &[0][]const u8{},
system_libs: []const []const u8 = &[0][]const u8{},
system_lib_names: []const []const u8 = &.{},
system_lib_infos: []const SystemLib = &.{},
/// These correspond to the WASI libc emulated subcomponents including:
/// * process clocks
/// * getpid
Expand Down Expand Up @@ -935,7 +938,7 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation {
if (options.link_objects.len != 0 or
options.c_source_files.len != 0 or
options.frameworks.len != 0 or
options.system_libs.len != 0 or
options.system_lib_names.len != 0 or
options.link_libc or options.link_libcpp or
link_eh_frame_hdr or
options.link_emit_relocs or
Expand Down Expand Up @@ -1003,7 +1006,7 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation {
break :dl true;
}
const any_dyn_libs: bool = x: {
if (options.system_libs.len != 0)
if (options.system_lib_names.len != 0)
break :x true;
for (options.link_objects) |obj| {
switch (classifyFileExt(obj)) {
Expand Down Expand Up @@ -1050,7 +1053,7 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation {
options.target,
options.is_native_abi,
link_libc,
options.system_libs.len != 0 or options.frameworks.len != 0,
options.system_lib_names.len != 0 or options.frameworks.len != 0,
options.libc_installation,
);

Expand Down Expand Up @@ -1372,11 +1375,11 @@ pub fn create(gpa: *Allocator, options: InitOptions) !*Compilation {
};
};

var system_libs: std.StringArrayHashMapUnmanaged(void) = .{};
var system_libs: std.StringArrayHashMapUnmanaged(SystemLib) = .{};
errdefer system_libs.deinit(gpa);
try system_libs.ensureTotalCapacity(gpa, options.system_libs.len);
for (options.system_libs) |lib_name| {
system_libs.putAssumeCapacity(lib_name, {});
try system_libs.ensureTotalCapacity(gpa, options.system_lib_names.len);
for (options.system_lib_names) |lib_name, i| {
system_libs.putAssumeCapacity(lib_name, options.system_lib_infos[i]);
}

const bin_file = try link.File.openPath(gpa, .{
Expand Down
18 changes: 17 additions & 1 deletion src/link.zig
View file
Expand Up @@ -18,6 +18,22 @@ const wasi_libc = @import("wasi_libc.zig");
const Air = @import("Air.zig");
const Liveness = @import("Liveness.zig");

pub const SystemLib = struct {
needed: bool = false,
};

pub fn hashAddSystemLibs(
hh: *Cache.HashHelper,
hm: std.StringArrayHashMapUnmanaged(SystemLib),
) void {
const keys = hm.keys();
hh.add(keys.len);
hh.addListOfBytes(keys);
for (hm.values()) |value| {
hh.add(value.needed);
}
}

pub const producer_string = if (builtin.is_test) "zig test" else "zig " ++ build_options.version;

pub const Emit = struct {
Expand Down Expand Up @@ -121,7 +137,7 @@ pub const Options = struct {
objects: []const []const u8,
framework_dirs: []const []const u8,
frameworks: []const []const u8,
system_libs: std.StringArrayHashMapUnmanaged(void),
system_libs: std.StringArrayHashMapUnmanaged(SystemLib),
wasi_emulated_libs: []const wasi_libc.CRTFile,
lib_dirs: []const []const u8,
rpath_list: []const []const u8,
Expand Down
2 changes: 1 addition & 1 deletion src/link/Coff.zig
View file
Expand Up @@ -940,7 +940,7 @@ fn linkWithLLD(self: *Coff, comp: *Compilation) !void {
}
}
}
man.hash.addStringSet(self.base.options.system_libs);
link.hashAddSystemLibs(&man.hash, self.base.options.system_libs);
man.hash.addOptional(self.base.options.subsystem);
man.hash.add(self.base.options.is_test);
man.hash.add(self.base.options.tsaware);
Expand Down
42 changes: 35 additions & 7 deletions src/link/Elf.zig
View file
Expand Up @@ -1345,7 +1345,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void {
}
man.hash.addOptionalBytes(self.base.options.soname);
man.hash.addOptional(self.base.options.version);
man.hash.addStringSet(self.base.options.system_libs);
link.hashAddSystemLibs(&man.hash, self.base.options.system_libs);
man.hash.add(allow_shlib_undefined);
man.hash.add(self.base.options.bind_global_refs_locally);
man.hash.add(self.base.options.tsan);
Expand Down Expand Up @@ -1550,7 +1550,9 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void {
for (self.base.options.system_libs.keys()) |link_lib| {
test_path.shrinkRetainingCapacity(0);
const sep = fs.path.sep_str;
try test_path.writer().print("{s}" ++ sep ++ "lib{s}.so", .{ lib_dir_path, link_lib });
try test_path.writer().print("{s}" ++ sep ++ "lib{s}.so", .{
lib_dir_path, link_lib,
});
fs.cwd().access(test_path.items, .{}) catch |err| switch (err) {
error.FileNotFound => continue,
else => |e| return e,
Expand Down Expand Up @@ -1627,16 +1629,42 @@ fn linkWithLLD(self: *Elf, comp: *Compilation) !void {
// Shared libraries.
if (is_exe_or_dyn_lib) {
const system_libs = self.base.options.system_libs.keys();
try argv.ensureUnusedCapacity(system_libs.len);
for (system_libs) |link_lib| {
// By this time, we depend on these libs being dynamically linked libraries and not static libraries
// (the check for that needs to be earlier), but they could be full paths to .so files, in which
// case we want to avoid prepending "-l".
const system_libs_values = self.base.options.system_libs.values();

// Worst-case, we need an --as-needed argument for every lib, as well
// as one before and one after.
try argv.ensureUnusedCapacity(system_libs.len * 2 + 2);
argv.appendAssumeCapacity("--as-needed");
var as_needed = true;

for (system_libs) |link_lib, i| {
const lib_as_needed = !system_libs_values[i].needed;
switch ((@as(u2, @boolToInt(lib_as_needed)) << 1) | @boolToInt(as_needed)) {
0b00, 0b11 => {},
0b01 => {
argv.appendAssumeCapacity("--no-as-needed");
as_needed = false;
},
0b10 => {
argv.appendAssumeCapacity("--as-needed");
as_needed = true;
},
}

// By this time, we depend on these libs being dynamically linked
// libraries and not static libraries (the check for that needs to be earlier),
// but they could be full paths to .so files, in which case we
// want to avoid prepending "-l".
const ext = Compilation.classifyFileExt(link_lib);
const arg = if (ext == .shared_library) link_lib else try std.fmt.allocPrint(arena, "-l{s}", .{link_lib});
argv.appendAssumeCapacity(arg);
}

if (!as_needed) {
argv.appendAssumeCapacity("--as-needed");
as_needed = true;
}

// libc++ dep
if (self.base.options.link_libcpp) {
try argv.append(comp.libcxxabi_static_lib.?.full_object_path);
Expand Down
2 changes: 1 addition & 1 deletion src/link/MachO.zig
View file
Expand Up @@ -471,7 +471,7 @@ pub fn flushModule(self: *MachO, comp: *Compilation) !void {
if (is_dyn_lib) {
man.hash.addOptional(self.base.options.version);
}
man.hash.addStringSet(self.base.options.system_libs);
link.hashAddSystemLibs(&man.hash, self.base.options.system_libs);
man.hash.addOptionalBytes(self.base.options.sysroot);

// We don't actually care whether it's a cache hit or miss; we just need the digest and the lock.
Expand Down
58 changes: 37 additions & 21 deletions src/main.zig
View file
Expand Up @@ -387,7 +387,9 @@ const usage_build_generic =
\\ -ffunction-sections Places each function in a separate section
\\
\\Link Options:
\\ -l[lib], --library [lib] Link against system library
\\ -l[lib], --library [lib] Link against system library (only if actually used)
\\ -needed-l[lib], Link against system library (even if unused)
\\ --needed-library [lib]
\\ -L[d], --library-directory [d] Add a directory to the library search path
\\ -T[script], --script [script] Use a custom linker script
\\ --version-script [path] Provide a version .map file
Expand Down Expand Up @@ -655,7 +657,7 @@ fn buildOutputType(
var wasi_exec_model: ?std.builtin.WasiExecModel = null;
var enable_link_snapshots: bool = false;

var system_libs = std.ArrayList([]const u8).init(gpa);
var system_libs = std.StringArrayHashMap(Compilation.SystemLib).init(gpa);
defer system_libs.deinit();

var wasi_emulated_libs = std.ArrayList(wasi_libc.CRTFile).init(gpa);
Expand Down Expand Up @@ -860,10 +862,14 @@ fn buildOutputType(
version_script = args[i];
} else if (mem.eql(u8, arg, "--library") or mem.eql(u8, arg, "-l")) {
if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg});
// We don't know whether this library is part of libc or libc++ until we resolve the target.
// So we simply append to the list for now.
// We don't know whether this library is part of libc or libc++ until
// we resolve the target, so we simply append to the list for now.
i += 1;
try system_libs.append(args[i]);
try system_libs.put(args[i], .{ .needed = false });
} else if (mem.eql(u8, arg, "--needed-library") or mem.eql(u8, arg, "-needed-l")) {
if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg});
i += 1;
try system_libs.put(args[i], .{ .needed = true });
} else if (mem.eql(u8, arg, "-D") or
mem.eql(u8, arg, "-isystem") or
mem.eql(u8, arg, "-I") or
Expand Down Expand Up @@ -1164,9 +1170,11 @@ fn buildOutputType(
} else if (mem.startsWith(u8, arg, "-F")) {
try framework_dirs.append(arg[2..]);
} else if (mem.startsWith(u8, arg, "-l")) {
// We don't know whether this library is part of libc or libc++ until we resolve the target.
// So we simply append to the list for now.
try system_libs.append(arg[2..]);
// We don't know whether this library is part of libc or libc++ until
// we resolve the target, so we simply append to the list for now.
try system_libs.put(arg["-l".len..], .{ .needed = false });
} else if (mem.startsWith(u8, arg, "-needed-l")) {
try system_libs.put(arg["-needed-l".len..], .{ .needed = true });
} else if (mem.startsWith(u8, arg, "-D") or
mem.startsWith(u8, arg, "-I"))
{
Expand Down Expand Up @@ -1230,6 +1238,7 @@ fn buildOutputType(
var linker_args = std.ArrayList([]const u8).init(arena);
var it = ClangArgIterator.init(arena, all_args);
var emit_llvm = false;
var needed = false;
while (it.has_next) {
it.next() catch |err| {
fatal("unable to parse command line parameters: {s}", .{@errorName(err)});
Expand Down Expand Up @@ -1262,9 +1271,9 @@ fn buildOutputType(
},
.l => {
// -l
// We don't know whether this library is part of libc or libc++ until we resolve the target.
// So we simply append to the list for now.
try system_libs.append(it.only_arg);
// We don't know whether this library is part of libc or libc++ until
// we resolve the target, so we simply append to the list for now.
try system_libs.put(it.only_arg, .{ .needed = needed });
},
.ignore => {},
.driver_punt => {
Expand Down Expand Up @@ -1302,8 +1311,13 @@ fn buildOutputType(
continue;
}
}

try linker_args.append(linker_arg);
if (mem.eql(u8, linker_arg, "--as-needed")) {
needed = false;
} else if (mem.eql(u8, linker_arg, "--no-as-needed")) {
needed = true;
} else {
try linker_args.append(linker_arg);
}
}
},
.optimize => {
Expand Down Expand Up @@ -1725,21 +1739,22 @@ fn buildOutputType(
// existence via flags instead.
{
var i: usize = 0;
while (i < system_libs.items.len) {
const lib_name = system_libs.items[i];
while (i < system_libs.count()) {
const lib_name = system_libs.keys()[i];

if (target_util.is_libc_lib_name(target_info.target, lib_name)) {
link_libc = true;
_ = system_libs.orderedRemove(i);
_ = system_libs.orderedRemove(lib_name);
continue;
}
if (target_util.is_libcpp_lib_name(target_info.target, lib_name)) {
link_libcpp = true;
_ = system_libs.orderedRemove(i);
_ = system_libs.orderedRemove(lib_name);
continue;
}
if (mem.eql(u8, lib_name, "unwind")) {
link_libunwind = true;
_ = system_libs.orderedRemove(i);
_ = system_libs.orderedRemove(lib_name);
continue;
}
if (std.fs.path.isAbsolute(lib_name)) {
Expand All @@ -1748,7 +1763,7 @@ fn buildOutputType(
if (target_info.target.os.tag == .wasi) {
if (wasi_libc.getEmulatedLibCRTFile(lib_name)) |crt_file| {
try wasi_emulated_libs.append(crt_file);
_ = system_libs.orderedRemove(i);
_ = system_libs.orderedRemove(lib_name);
continue;
}
}
Expand Down Expand Up @@ -1777,7 +1792,7 @@ fn buildOutputType(
const is_darwin_on_darwin = (comptime builtin.target.isDarwin()) and cross_target.isDarwin();

if (sysroot == null and (cross_target.isNativeOs() or is_darwin_on_darwin) and
(system_libs.items.len != 0 or want_native_include_dirs))
(system_libs.count() != 0 or want_native_include_dirs))
{
const paths = std.zig.system.NativePaths.detect(arena, target_info) catch |err| {
fatal("unable to detect native system paths: {s}", .{@errorName(err)});
Expand Down Expand Up @@ -2144,7 +2159,8 @@ fn buildOutputType(
.link_objects = link_objects.items,
.framework_dirs = framework_dirs.items,
.frameworks = frameworks.items,
.system_libs = system_libs.items,
.system_lib_names = system_libs.keys(),
.system_lib_infos = system_libs.values(),
.wasi_emulated_libs = wasi_emulated_libs.items,
.link_libc = link_libc,
.link_libcpp = link_libcpp,
Expand Down

0 comments on commit 4e5a88b

Please sign in to comment.