From 16daf3f3bcb27b2e0b0f45ee48c69824a4804981 Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Thu, 26 May 2022 16:25:26 +0200 Subject: [PATCH] wasm-link: Discard old symbols correctly When a new symbol is resolved to an existing symbol where it doesn't overwrite the existing symbol, we now add this symbol to the discarded list. This is required so when any relocation points to the symbol, we can retrieve the correct symbol it's resolved by instead. --- src/link/Wasm.zig | 17 ++++++++++------- src/link/Wasm/Atom.zig | 1 - src/link/Wasm/Symbol.zig | 7 +++++-- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index f734b228aecc6fb0da2427ef35624f0837a28b02..74d3ab36aaa947773b1585ce65d45d614cd45eeb 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -491,6 +491,7 @@ fn resolveSymbolsInObject(self: *Wasm, object_index: u16) !void { return error.SymbolCollision; } + try self.discarded.put(self.base.allocator, location, existing_loc); continue; // Do not overwrite defined symbols with undefined symbols } @@ -503,6 +504,7 @@ fn resolveSymbolsInObject(self: *Wasm, object_index: u16) !void { // when both symbols are weak, we skip overwriting if (existing_sym.isWeak() and symbol.isWeak()) { + try self.discarded.put(self.base.allocator, location, existing_loc); continue; } @@ -510,15 +512,13 @@ fn resolveSymbolsInObject(self: *Wasm, object_index: u16) !void { log.debug("Overwriting symbol '{s}'", .{sym_name}); log.debug(" old definition in '{s}'", .{existing_file_path}); log.debug(" new definition in '{s}'", .{object.name}); - try self.discarded.putNoClobber(self.base.allocator, maybe_existing.value_ptr.*, location); + try self.discarded.putNoClobber(self.base.allocator, existing_loc, location); maybe_existing.value_ptr.* = location; try self.globals.put(self.base.allocator, sym_name_index, location); try self.resolved_symbols.put(self.base.allocator, location, {}); assert(self.resolved_symbols.swapRemove(existing_loc)); if (existing_sym.isUndefined()) { - // ensure order remains intact in case we later - // resolve symbols again in a loop - assert(self.undefs.orderedRemove(sym_name)); + assert(self.undefs.swapRemove(sym_name)); } } } @@ -1004,7 +1004,6 @@ pub fn updateDeclExports( switch (exp.options.linkage) { .Internal => { symbol.setFlag(.WASM_SYM_VISIBILITY_HIDDEN); - symbol.setFlag(.WASM_SYM_BINDING_WEAK); }, .Weak => { symbol.setFlag(.WASM_SYM_BINDING_WEAK); @@ -1026,6 +1025,7 @@ pub fn updateDeclExports( } symbol.setGlobal(true); + symbol.setUndefined(false); try self.globals.put( self.base.allocator, export_name, @@ -1034,6 +1034,7 @@ pub fn updateDeclExports( // if the symbol was previously undefined, remove it as an import _ = self.imports.remove(sym_loc); + _ = self.undefs.swapRemove(exp.options.name); exp.link.wasm.sym_index = sym_index; } } @@ -1103,11 +1104,13 @@ pub fn addOrUpdateImport( /// is asserted instead. type_index: ?u32, ) !void { + assert(symbol_index != 0); // For the import name itself, we use the decl's name, rather than the fully qualified name const decl_name_index = try self.string_table.put(self.base.allocator, name); const symbol: *Symbol = &self.symbols.items[symbol_index]; symbol.setUndefined(true); symbol.setGlobal(true); + symbol.name = decl_name_index; const global_gop = try self.globals.getOrPut(self.base.allocator, decl_name_index); if (!global_gop.found_existing) { const loc: SymbolLoc = .{ .file = null, .index = symbol_index }; @@ -2113,7 +2116,7 @@ pub fn flushModule(self: *Wasm, comp: *Compilation, prog_node: *std.Progress.Nod while (true) { if (!is_obj) { - try atom.resolveRelocs(self); + atom.resolveRelocs(self); } sorted_atoms.appendAssumeCapacity(atom); atom = atom.next orelse break; @@ -2172,7 +2175,7 @@ pub fn flushModule(self: *Wasm, comp: *Compilation, prog_node: *std.Progress.Nod var current_offset: u32 = 0; while (true) { if (!is_obj) { - try atom.resolveRelocs(self); + atom.resolveRelocs(self); } // Pad with zeroes to ensure all segments are aligned diff --git a/src/link/Wasm/Atom.zig b/src/link/Wasm/Atom.zig index edef52004db08e1612c0e9f930f6c3bcb5e603a8..588a9a3eddd39dce87bb3c2e91c2f3cf7ce03e9b 100644 --- a/src/link/Wasm/Atom.zig +++ b/src/link/Wasm/Atom.zig @@ -147,7 +147,6 @@ pub fn resolveRelocs(self: *Atom, wasm_bin: *const Wasm) void { fn relocationValue(self: Atom, relocation: types.Relocation, wasm_bin: *const Wasm) u64 { const target_loc: Wasm.SymbolLoc = .{ .file = self.file, .index = relocation.index }; const symbol = target_loc.getSymbol(wasm_bin).*; - switch (relocation.relocation_type) { .R_WASM_FUNCTION_INDEX_LEB => return symbol.index, .R_WASM_TABLE_NUMBER_LEB => return symbol.index, diff --git a/src/link/Wasm/Symbol.zig b/src/link/Wasm/Symbol.zig index 94548efe315d856e29bff16d3dd38a0f023103c1..fa6ea89d69d9a8b110db9b43519595462f2a8cfd 100644 --- a/src/link/Wasm/Symbol.zig +++ b/src/link/Wasm/Symbol.zig @@ -142,6 +142,8 @@ pub fn isNoStrip(self: Symbol) bool { pub fn isExported(self: Symbol) bool { if (self.isUndefined() or self.isLocal()) return false; if (self.isHidden()) return false; + if (self.hasFlag(.WASM_SYM_EXPORTED)) return true; + if (self.hasFlag(.WASM_SYM_BINDING_WEAK)) return false; return true; } @@ -165,9 +167,10 @@ pub fn format(self: Symbol, comptime fmt: []const u8, options: std.fmt.FormatOpt }; const visible: []const u8 = if (self.isVisible()) "yes" else "no"; const binding: []const u8 = if (self.isLocal()) "local" else "global"; + const undef: []const u8 = if (self.isUndefined()) "undefined" else ""; try writer.print( - "{c} binding={s} visible={s} id={d} name_offset={d}", - .{ kind_fmt, binding, visible, self.index, self.name }, + "{c} binding={s} visible={s} id={d} name_offset={d} {s}", + .{ kind_fmt, binding, visible, self.index, self.name, undef }, ); } -- 2.54.0