From b9224c172fea2399623bd707a10e021e776329bc Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Mon, 2 Jan 2023 15:45:35 +0100 Subject: [PATCH] wasm-linker: Fix & mangle symbol name of imports When outputting the names section, we should output the actual symbol name rather than the import name. This makes sure that symbols with an explicit name set have the correct name but retain the import name too. We also now correctly mangle the name of an extern function with an explicit library name. This ensures that functions that have a different library name, but the same import/function name, can be resolved correctly with other modules and don't resolve to the same symbol. --- src/link/Wasm.zig | 34 ++++++++++++++++++++-------- test/link.zig | 7 +++--- test/link/wasm/export-data/build.zig | 8 +++---- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index e03c8b1ba0..d62d5adb25 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -813,7 +813,12 @@ fn checkUndefinedSymbols(wasm: *const Wasm) !void { const file_name = if (undef.file) |file_index| name: { break :name wasm.objects.items[file_index].name; } else wasm.name; - log.err("could not resolve undefined symbol '{s}'", .{undef.getName(wasm)}); + const import_name = if (undef.file) |file_index| name: { + const obj = wasm.objects.items[file_index]; + const name_index = obj.findImport(symbol.tag.externalType(), symbol.index).name; + break :name obj.string_table.get(name_index); + } else wasm.string_table.get(wasm.imports.get(undef).?.name); + log.err("could not resolve undefined symbol '{s}'", .{import_name}); log.err(" defined in '{s}'", .{file_name}); } } @@ -1430,18 +1435,31 @@ pub fn addOrUpdateImport( type_index: ?u32, ) !void { assert(symbol_index != 0); - // For the import name itwasm, we use the decl's name, rather than the fully qualified name - const decl_name_index = try wasm.string_table.put(wasm.base.allocator, name); + // For the import name, we use the decl's name, rather than the fully qualified name + // Also mangle the name when the lib name is set and not equal to "C" so imports with the same + // name but different module can be resolved correctly. + const mangle_name = lib_name != null and + !std.mem.eql(u8, std.mem.sliceTo(lib_name.?, 0), "c"); + const full_name = if (mangle_name) full_name: { + break :full_name try std.fmt.allocPrint(wasm.base.allocator, "{s}|{s}", .{ name, lib_name.? }); + } else name; + defer if (mangle_name) wasm.base.allocator.free(full_name); + + const decl_name_index = try wasm.string_table.put(wasm.base.allocator, full_name); const symbol: *Symbol = &wasm.symbols.items[symbol_index]; symbol.setUndefined(true); symbol.setGlobal(true); symbol.name = decl_name_index; + if (mangle_name) { + // we specified a specific name for the symbol that does not match the import name + symbol.setFlag(.WASM_SYM_EXPLICIT_NAME); + } const global_gop = try wasm.globals.getOrPut(wasm.base.allocator, decl_name_index); if (!global_gop.found_existing) { const loc: SymbolLoc = .{ .file = null, .index = symbol_index }; global_gop.value_ptr.* = loc; try wasm.resolved_symbols.put(wasm.base.allocator, loc, {}); - try wasm.undefs.putNoClobber(wasm.base.allocator, name, loc); + try wasm.undefs.putNoClobber(wasm.base.allocator, full_name, loc); } if (type_index) |ty_index| { @@ -1452,7 +1470,7 @@ pub fn addOrUpdateImport( if (!gop.found_existing) { gop.value_ptr.* = .{ .module_name = try wasm.string_table.put(wasm.base.allocator, module_name), - .name = decl_name_index, + .name = try wasm.string_table.put(wasm.base.allocator, name), .kind = .{ .function = ty_index }, }; } @@ -3130,11 +3148,7 @@ fn emitNameSection(wasm: *Wasm, binary_bytes: *std.ArrayList(u8), arena: std.mem for (wasm.resolved_symbols.keys()) |sym_loc| { const symbol = sym_loc.getSymbol(wasm).*; - const name = if (symbol.isUndefined()) blk: { - if (symbol.tag == .data) continue; - const imp = wasm.imports.get(sym_loc) orelse continue; - break :blk wasm.string_table.get(imp.name); - } else sym_loc.getName(wasm); + const name = sym_loc.getName(wasm); switch (symbol.tag) { .function => { const gop = funcs.getOrPutAssumeCapacity(symbol.index); diff --git a/test/link.zig b/test/link.zig index 2fe62b8068..5e26ae728d 100644 --- a/test/link.zig +++ b/test/link.zig @@ -47,9 +47,10 @@ fn addWasmCases(cases: *tests.StandaloneContext) void { .requires_stage2 = true, }); - cases.addBuildFile("test/link/wasm/export-data/build.zig", .{ - .build_modes = true, - }); + // TODO: Fix open handle in wasm-linker refraining rename from working on Windows. + if (builtin.os.tag != .windows) { + cases.addBuildFile("test/link/wasm/export-data/build.zig", .{}); + } cases.addBuildFile("test/link/wasm/extern/build.zig", .{ .build_modes = true, diff --git a/test/link/wasm/export-data/build.zig b/test/link/wasm/export-data/build.zig index 3df5c1790c..283566dab3 100644 --- a/test/link/wasm/export-data/build.zig +++ b/test/link/wasm/export-data/build.zig @@ -2,13 +2,11 @@ const std = @import("std"); const Builder = std.build.Builder; pub fn build(b: *Builder) void { - const mode = b.standardReleaseOptions(); - const test_step = b.step("test", "Test"); test_step.dependOn(b.getInstallStep()); const lib = b.addSharedLibrary("lib", "lib.zig", .unversioned); - lib.setBuildMode(mode); + lib.setBuildMode(.ReleaseSafe); // to make the output deterministic in address positions lib.setTarget(.{ .cpu_arch = .wasm32, .os_tag = .freestanding }); lib.use_lld = false; lib.export_symbol_names = &.{ "foo", "bar" }; @@ -25,8 +23,8 @@ pub fn build(b: *Builder) void { check_lib.checkNext("type i32"); check_lib.checkNext("mutable false"); check_lib.checkNext("i32.const {bar_address}"); - check_lib.checkComputeCompare("foo_address", .{ .op = .eq, .value = .{ .literal = 0x0c } }); - check_lib.checkComputeCompare("bar_address", .{ .op = .eq, .value = .{ .literal = 0x10 } }); + check_lib.checkComputeCompare("foo_address", .{ .op = .eq, .value = .{ .literal = 0 } }); + check_lib.checkComputeCompare("bar_address", .{ .op = .eq, .value = .{ .literal = 4 } }); check_lib.checkStart("Section export"); check_lib.checkNext("entries 3");