From 7802c26449657b205dcaa47bb6575bb26171c024 Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Sun, 25 Dec 2022 22:04:01 +0100 Subject: WebAssembly: do not link with --allow-undefined unconditionally In #1622, when targeting WebAsembly, the --allow-undefined flag became unconditionally added to the linker. This is not always desirable. First, this is error prone. Code with references to unkown symbols will link just fine, but then fail at run-time. This behavior is inconsistent with all other targets. For freestanding wasm applications, and applications that only use WASI, undefined references are better reported at compile-time. This behavior is also inconsistent with clang itself. Autoconf and cmake scripts checking for function presence think that all tested functions exist, but then resulting application cannot run. For example, this is one of the reasons compilation of Ruby 3.2.0 to WASI fails with zig cc, while it works out of the box with clang. But all applications checking for symbol existence before compilation are affected. This reverts the behavior to the one Zig had before #1622, and introduces an `import_symbols` flag to ignore undefined symbols, assuming that the webassembly runtime will define them. --- src/Compilation.zig | 2 ++ src/link.zig | 1 + src/link/Wasm.zig | 4 +++- src/main.zig | 7 +++++++ 4 files changed, 13 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/Compilation.zig b/src/Compilation.zig index b385fa5f72..ab91e3cbb3 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -954,6 +954,7 @@ pub const InitOptions = struct { linker_allow_shlib_undefined: ?bool = null, linker_bind_global_refs_locally: ?bool = null, linker_import_memory: ?bool = null, + linker_import_symbols: bool = false, linker_import_table: bool = false, linker_export_table: bool = false, linker_initial_memory: ?u64 = null, @@ -1811,6 +1812,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { .bind_global_refs_locally = options.linker_bind_global_refs_locally orelse false, .compress_debug_sections = options.linker_compress_debug_sections orelse .none, .import_memory = options.linker_import_memory orelse false, + .import_symbols = options.linker_import_symbols, .import_table = options.linker_import_table, .export_table = options.linker_export_table, .initial_memory = options.linker_initial_memory, diff --git a/src/link.zig b/src/link.zig index 450763ac18..1dadacc2fe 100644 --- a/src/link.zig +++ b/src/link.zig @@ -128,6 +128,7 @@ pub const Options = struct { compress_debug_sections: CompressDebugSections, bind_global_refs_locally: bool, import_memory: bool, + import_symbols: bool, import_table: bool, export_table: bool, initial_memory: ?u64, diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index 346c92ebbe..1f929da150 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -3461,8 +3461,10 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) ! } else if (wasm.base.options.entry == null) { try argv.append("--no-entry"); // So lld doesn't look for _start. } + if (wasm.base.options.import_symbols) { + try argv.appendSlice(&[_][]const u8{"--allow-undefined"}); + } try argv.appendSlice(&[_][]const u8{ - "--allow-undefined", "-o", full_out_path, }); diff --git a/src/main.zig b/src/main.zig index ec0eb74e93..d9cc147604 100644 --- a/src/main.zig +++ b/src/main.zig @@ -517,6 +517,7 @@ const usage_build_generic = \\ -dead_strip (Darwin) remove functions and data that are unreachable by the entry point or exported symbols \\ -dead_strip_dylibs (Darwin) remove dylibs that are unreachable by the entry point or exported symbols \\ --import-memory (WebAssembly) import memory from the environment + \\ --import-symbols (WebAssembly) import missing symbols from the host environment \\ --import-table (WebAssembly) import function table from the host environment \\ --export-table (WebAssembly) export function table to the host environment \\ --initial-memory=[bytes] (WebAssembly) initial size of the linear memory @@ -718,6 +719,7 @@ fn buildOutputType( var linker_allow_shlib_undefined: ?bool = null; var linker_bind_global_refs_locally: ?bool = null; var linker_import_memory: ?bool = null; + var linker_import_symbols: bool = false; var linker_import_table: bool = false; var linker_export_table: bool = false; var linker_initial_memory: ?u64 = null; @@ -1316,6 +1318,8 @@ fn buildOutputType( } } else if (mem.eql(u8, arg, "--import-memory")) { linker_import_memory = true; + } else if (mem.eql(u8, arg, "--import-symbols")) { + linker_import_symbols = true; } else if (mem.eql(u8, arg, "--import-table")) { linker_import_table = true; } else if (mem.eql(u8, arg, "--export-table")) { @@ -1837,6 +1841,8 @@ fn buildOutputType( linker_bind_global_refs_locally = true; } else if (mem.eql(u8, arg, "--import-memory")) { linker_import_memory = true; + } else if (mem.eql(u8, arg, "--import-symbols")) { + linker_import_symbols = true; } else if (mem.eql(u8, arg, "--import-table")) { linker_import_table = true; } else if (mem.eql(u8, arg, "--export-table")) { @@ -2977,6 +2983,7 @@ fn buildOutputType( .linker_allow_shlib_undefined = linker_allow_shlib_undefined, .linker_bind_global_refs_locally = linker_bind_global_refs_locally, .linker_import_memory = linker_import_memory, + .linker_import_symbols = linker_import_symbols, .linker_import_table = linker_import_table, .linker_export_table = linker_export_table, .linker_initial_memory = linker_initial_memory, -- cgit v1.2.3 From 4aab8118a771e37566c0c3b1c40c175ce1e98285 Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Wed, 28 Dec 2022 14:41:27 +0100 Subject: WebAssembly: don't append `--export` for functions No longer automatically append the `--export` flag for each exported function unconditionally. This was essentially a hack to prevent binary bloat caused by compiler-rt symbols being always included in the final binary as they were exported and therefore not garbage- collected. This is no longer needed as we now support the ability to set the visibility of exports. This essentially reverts 6d951aff7e32b1b0252d341e66517a9a9ee98a2d --- src/Compilation.zig | 9 --------- src/link/Wasm.zig | 31 ++++++------------------------- 2 files changed, 6 insertions(+), 34 deletions(-) (limited to 'src') diff --git a/src/Compilation.zig b/src/Compilation.zig index ab91e3cbb3..5b80e5c7a7 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -181,10 +181,6 @@ emit_docs: ?EmitLoc, work_queue_wait_group: WaitGroup = .{}, astgen_wait_group: WaitGroup = .{}, -/// Exported symbol names. This is only for when the target is wasm. -/// TODO: Remove this when Stage2 becomes the default compiler as it will already have this information. -export_symbol_names: std.ArrayListUnmanaged([]const u8) = .{}, - pub const default_stack_protector_buffer_size = 4; pub const SemaError = Module.SemaError; @@ -2168,11 +2164,6 @@ pub fn destroy(self: *Compilation) void { self.cache_parent.manifest_dir.close(); if (self.owned_link_dir) |*dir| dir.close(); - for (self.export_symbol_names.items) |symbol_name| { - gpa.free(symbol_name); - } - self.export_symbol_names.deinit(gpa); - // This destroys `self`. self.arena_state.promote(gpa).deinit(); } diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index 1f929da150..7bc09c137c 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -3406,39 +3406,14 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) ! try argv.append("--stack-first"); } - var auto_export_symbols = true; // Users are allowed to specify which symbols they want to export to the wasm host. for (wasm.base.options.export_symbol_names) |symbol_name| { const arg = try std.fmt.allocPrint(arena, "--export={s}", .{symbol_name}); try argv.append(arg); - auto_export_symbols = false; } if (wasm.base.options.rdynamic) { try argv.append("--export-dynamic"); - auto_export_symbols = false; - } - - if (auto_export_symbols) { - if (wasm.base.options.module) |mod| { - // when we use stage1, we use the exports that stage1 provided us. - // For stage2, we can directly retrieve them from the module. - const skip_export_non_fn = target.os.tag == .wasi and - wasm.base.options.wasi_exec_model == .command; - for (mod.decl_exports.values()) |exports| { - for (exports.items) |exprt| { - const exported_decl = mod.declPtr(exprt.exported_decl); - if (skip_export_non_fn and exported_decl.ty.zigTypeTag() != .Fn) { - // skip exporting symbols when we're building a WASI command - // and the symbol is not a function - continue; - } - const symbol_name = exported_decl.name; - const arg = try std.fmt.allocPrint(arena, "--export={s}", .{symbol_name}); - try argv.append(arg); - } - } - } } if (wasm.base.options.entry) |entry| { @@ -3457,6 +3432,12 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) ! if (wasm.base.options.wasi_exec_model == .reactor) { // Reactor execution model does not have _start so lld doesn't look for it. try argv.append("--no-entry"); + // Make sure "_initialize" and other used-defined functions are exported if this is WASI reactor. + // If rdynamic is true, it will already be appended, so only verify if the user did not specify + // the flag in which case, we ensure `--export-dynamic` is called. + if (!wasm.base.options.rdynamic) { + try argv.append("--export-dynamic"); + } } } else if (wasm.base.options.entry == null) { try argv.append("--no-entry"); // So lld doesn't look for _start. -- cgit v1.2.3 From 8403612adc67d6398b3664b20eb453475688bd46 Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Wed, 28 Dec 2022 16:36:46 +0100 Subject: test/link: update linker tests Force importing symbols to show the correct functions are being imported from the host environment. --- src/link/Wasm.zig | 2 +- test/link/wasm/extern-mangle/build.zig | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index 7bc09c137c..9beb40e418 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -3443,7 +3443,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) ! try argv.append("--no-entry"); // So lld doesn't look for _start. } if (wasm.base.options.import_symbols) { - try argv.appendSlice(&[_][]const u8{"--allow-undefined"}); + try argv.append("--allow-undefined"); } try argv.appendSlice(&[_][]const u8{ "-o", diff --git a/test/link/wasm/extern-mangle/build.zig b/test/link/wasm/extern-mangle/build.zig index f2916c35a7..ae46117f18 100644 --- a/test/link/wasm/extern-mangle/build.zig +++ b/test/link/wasm/extern-mangle/build.zig @@ -10,6 +10,8 @@ pub fn build(b: *Builder) void { const lib = b.addSharedLibrary("lib", "lib.zig", .unversioned); lib.setBuildMode(mode); lib.setTarget(.{ .cpu_arch = .wasm32, .os_tag = .freestanding }); + lib.import_symbols = true; // import `a` and `b` + lib.rdynamic = true; // export `foo` lib.install(); const check_lib = lib.checkObject(.wasm); -- cgit v1.2.3