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 ++ 1 file changed, 2 insertions(+) (limited to 'src/Compilation.zig') 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, -- 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/Compilation.zig') 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