Commit 196ba706a0

Luuk de Gram <luuk@degram.dev>
2024-02-28 06:31:26
wasm: gc fixes and re-enable linker tests
Certain symbols were left unmarked, meaning they would not be emit into the final binary incorrectly. We now mark the synthetic symbols to ensure they are emit as they are already created under the circumstance they're needed for. This also re-enables disabled tests that were left disabled in a previous merge conflict. Lastly, this adds the shared-memory test to the test harnass as it was previously forgotten and therefore regressed.
1 parent 5ba5a2c
Changed files (8)
src
test
link
wasm
archive
export-data
shared-memory
type
src/link/Wasm/Object.zig
@@ -15,7 +15,7 @@ const Allocator = std.mem.Allocator;
 const leb = std.leb;
 const meta = std.meta;
 
-const log = std.log.scoped(.link);
+const log = std.log.scoped(.object);
 
 /// Index into the list of relocatable object files within the linker driver.
 index: File.Index = .null,
src/link/Wasm/ZigObject.zig
@@ -315,7 +315,7 @@ fn finishUpdateDecl(
     const atom_index = decl_info.atom;
     const atom = wasm_file.getAtomPtr(atom_index);
     const sym = zig_object.symbol(atom.sym_index);
-    const full_name = mod.intern_pool.stringToSlice(try decl.getFullyQualifiedName(mod));
+    const full_name = mod.intern_pool.stringToSlice(try decl.fullyQualifiedName(mod));
     sym.name = try zig_object.string_table.insert(gpa, full_name);
     try atom.code.appendSlice(gpa, code);
     atom.size = @intCast(code.len);
@@ -401,7 +401,7 @@ pub fn getOrCreateAtomForDecl(zig_object: *ZigObject, wasm_file: *Wasm, decl_ind
         gop.value_ptr.* = .{ .atom = try wasm_file.createAtom(sym_index, zig_object.index) };
         const mod = wasm_file.base.comp.module.?;
         const decl = mod.declPtr(decl_index);
-        const full_name = mod.intern_pool.stringToSlice(try decl.getFullyQualifiedName(mod));
+        const full_name = mod.intern_pool.stringToSlice(try decl.fullyQualifiedName(mod));
         const sym = zig_object.symbol(sym_index);
         sym.name = try zig_object.string_table.insert(gpa, full_name);
     }
@@ -455,7 +455,7 @@ pub fn lowerUnnamedConst(zig_object: *ZigObject, wasm_file: *Wasm, tv: TypedValu
     const parent_atom_index = try zig_object.getOrCreateAtomForDecl(wasm_file, decl_index);
     const parent_atom = wasm_file.getAtom(parent_atom_index);
     const local_index = parent_atom.locals.items.len;
-    const fqn = mod.intern_pool.stringToSlice(try decl.getFullyQualifiedName(mod));
+    const fqn = mod.intern_pool.stringToSlice(try decl.fullyQualifiedName(mod));
     const name = try std.fmt.allocPrintZ(gpa, "__unnamed_{s}_{d}", .{
         fqn, local_index,
     });
@@ -838,6 +838,7 @@ pub fn updateExports(
     const atom = wasm_file.getAtom(atom_index);
     const atom_sym = atom.symbolLoc().getSymbol(wasm_file).*;
     const gpa = mod.gpa;
+    log.debug("Updating exports for decl '{s}'", .{mod.intern_pool.stringToSlice(decl.name)});
 
     for (exports) |exp| {
         if (mod.intern_pool.stringToSliceUnwrap(exp.opts.section)) |section| {
@@ -888,6 +889,7 @@ pub fn updateExports(
         if (exp.opts.visibility == .hidden) {
             sym.setFlag(.WASM_SYM_VISIBILITY_HIDDEN);
         }
+        log.debug("  with name '{s}' - {}", .{ export_string, sym });
         try zig_object.global_syms.put(gpa, export_name, sym_index);
         try wasm_file.symbol_atom.put(gpa, .{ .file = zig_object.index, .index = sym_index }, atom_index);
     }
@@ -1061,7 +1063,7 @@ pub fn createDebugSectionForIndex(zig_object: *ZigObject, wasm_file: *Wasm, inde
 pub fn updateDeclLineNumber(zig_object: *ZigObject, mod: *Module, decl_index: InternPool.DeclIndex) !void {
     if (zig_object.dwarf) |*dw| {
         const decl = mod.declPtr(decl_index);
-        const decl_name = mod.intern_pool.stringToSlice(try decl.getFullyQualifiedName(mod));
+        const decl_name = mod.intern_pool.stringToSlice(try decl.fullyQualifiedName(mod));
 
         log.debug("updateDeclLineNumber {s}{*}", .{ decl_name, decl });
         try dw.updateDeclLineNumber(mod, decl_index);
src/link/Wasm.zig
@@ -11,6 +11,7 @@ const leb = std.leb;
 const link = @import("../link.zig");
 const lldMain = @import("../main.zig").lldMain;
 const log = std.log.scoped(.link);
+const gc_log = std.log.scoped(.gc);
 const mem = std.mem;
 const trace = @import("../tracy.zig").trace;
 const types = @import("Wasm/types.zig");
@@ -525,6 +526,7 @@ pub fn createEmpty(
             const symbol = loc.getSymbol(wasm);
             symbol.setFlag(.WASM_SYM_VISIBILITY_HIDDEN);
             symbol.index = @intCast(wasm.imported_globals_count + wasm.wasm_globals.items.len);
+            symbol.mark();
             try wasm.wasm_globals.append(gpa, .{
                 .global_type = .{ .valtype = .i32, .mutable = true },
                 .init = .{ .i32_const = undefined },
@@ -535,6 +537,7 @@ pub fn createEmpty(
             const symbol = loc.getSymbol(wasm);
             symbol.setFlag(.WASM_SYM_VISIBILITY_HIDDEN);
             symbol.index = @intCast(wasm.imported_globals_count + wasm.wasm_globals.items.len);
+            symbol.mark();
             try wasm.wasm_globals.append(gpa, .{
                 .global_type = .{ .valtype = .i32, .mutable = false },
                 .init = .{ .i32_const = undefined },
@@ -545,6 +548,7 @@ pub fn createEmpty(
             const symbol = loc.getSymbol(wasm);
             symbol.setFlag(.WASM_SYM_VISIBILITY_HIDDEN);
             symbol.index = @intCast(wasm.imported_globals_count + wasm.wasm_globals.items.len);
+            symbol.mark();
             try wasm.wasm_globals.append(gpa, .{
                 .global_type = .{ .valtype = .i32, .mutable = false },
                 .init = .{ .i32_const = undefined },
@@ -968,6 +972,8 @@ fn setupInitMemoryFunction(wasm: *Wasm) !void {
     if (!wasm.hasPassiveInitializationSegments()) {
         return;
     }
+    const sym_loc = try wasm.createSyntheticSymbol("__wasm_init_memory", .function);
+    sym_loc.getSymbol(wasm).mark();
 
     const flag_address: u32 = if (shared_memory) address: {
         // when we have passive initialization segments and shared memory
@@ -1130,7 +1136,8 @@ fn setupTLSRelocationsFunction(wasm: *Wasm) !void {
         return;
     }
 
-    // const loc = try wasm.createSyntheticSymbol("__wasm_apply_global_tls_relocs");
+    const loc = try wasm.createSyntheticSymbol("__wasm_apply_global_tls_relocs", .function);
+    loc.getSymbol(wasm).mark();
     var function_body = std.ArrayList(u8).init(gpa);
     defer function_body.deinit();
     const writer = function_body.writer();
@@ -1833,8 +1840,7 @@ fn createSyntheticFunction(
     function_body: *std.ArrayList(u8),
 ) !void {
     const gpa = wasm.base.comp.gpa;
-    const loc = wasm.findGlobalSymbol(symbol_name) orelse
-        try wasm.createSyntheticSymbol(symbol_name, .function);
+    const loc = wasm.findGlobalSymbol(symbol_name).?; // forgot to create symbol?
     const symbol = loc.getSymbol(wasm);
     if (symbol.isDead()) {
         return;
@@ -1884,6 +1890,9 @@ fn initializeTLSFunction(wasm: *Wasm) !void {
 
     if (!shared_memory) return;
 
+    // ensure function is marked as we must emit it
+    wasm.findGlobalSymbol("__wasm_init_tls").?.getSymbol(wasm).mark();
+
     var function_body = std.ArrayList(u8).init(gpa);
     defer function_body.deinit();
     const writer = function_body.writer();
@@ -1932,6 +1941,7 @@ fn initializeTLSFunction(wasm: *Wasm) !void {
     if (wasm.findGlobalSymbol("__wasm_apply_global_tls_relocs")) |loc| {
         try writer.writeByte(std.wasm.opcode(.call));
         try leb.writeULEB128(writer, loc.getSymbol(wasm).index);
+        loc.getSymbol(wasm).mark();
     }
 
     try writer.writeByte(std.wasm.opcode(.end));
@@ -2039,14 +2049,19 @@ fn mergeSections(wasm: *Wasm) !void {
                     // We found an alias to the same function, discard this symbol in favor of
                     // the original symbol and point the discard function to it. This ensures
                     // we only emit a single function, instead of duplicates.
-                    symbol.unmark();
-                    try wasm.discarded.putNoClobber(
-                        gpa,
-                        sym_loc,
-                        .{ .file = gop.key_ptr.*.file, .index = gop.value_ptr.*.sym_index },
-                    );
-                    try removed_duplicates.append(sym_loc);
-                    continue;
+                    // we favor keeping the global over a local.
+                    const original_loc: SymbolLoc = .{ .file = gop.key_ptr.file, .index = gop.value_ptr.sym_index };
+                    const original_sym = original_loc.getSymbol(wasm);
+                    if (original_sym.isLocal() and symbol.isGlobal()) {
+                        original_sym.unmark();
+                        try wasm.discarded.put(gpa, original_loc, sym_loc);
+                        try removed_duplicates.append(original_loc);
+                    } else {
+                        symbol.unmark();
+                        try wasm.discarded.putNoClobber(gpa, sym_loc, original_loc);
+                        try removed_duplicates.append(sym_loc);
+                        continue;
+                    }
                 }
                 gop.value_ptr.* = .{ .func = obj_file.function(sym_loc.index), .sym_index = sym_loc.index };
                 symbol.index = @as(u32, @intCast(gop.index)) + wasm.imported_functions_count;
@@ -2073,6 +2088,7 @@ fn mergeSections(wasm: *Wasm) !void {
     // For any removed duplicates, remove them from the resolved symbols list
     for (removed_duplicates.items) |sym_loc| {
         assert(wasm.resolved_symbols.swapRemove(sym_loc));
+        gc_log.debug("Removed duplicate for function '{s}'", .{sym_loc.getName(wasm)});
     }
 
     log.debug("Merged ({d}) functions", .{wasm.functions.count()});
@@ -2119,12 +2135,7 @@ fn mergeTypes(wasm: *Wasm) !void {
     log.debug("Completed merging and deduplicating types. Total count: ({d})", .{wasm.func_types.items.len});
 }
 
-fn setupExports(wasm: *Wasm) !void {
-    const comp = wasm.base.comp;
-    const gpa = comp.gpa;
-    if (comp.config.output_mode == .Obj) return;
-    log.debug("Building exports from symbols", .{});
-
+fn checkExportNames(wasm: *Wasm) !void {
     const force_exp_names = wasm.export_symbol_names;
     if (force_exp_names.len > 0) {
         var failed_exports = false;
@@ -2144,6 +2155,13 @@ fn setupExports(wasm: *Wasm) !void {
             return error.FlushFailure;
         }
     }
+}
+
+fn setupExports(wasm: *Wasm) !void {
+    const comp = wasm.base.comp;
+    const gpa = comp.gpa;
+    if (comp.config.output_mode == .Obj) return;
+    log.debug("Building exports from symbols", .{});
 
     for (wasm.resolved_symbols.keys()) |sym_loc| {
         const symbol = sym_loc.getSymbol(wasm);
@@ -2272,6 +2290,7 @@ fn setupMemory(wasm: *Wasm) !void {
         memory_ptr = mem.alignForward(u64, memory_ptr, 4);
         const loc = try wasm.createSyntheticSymbol("__wasm_init_memory_flag", .data);
         const sym = loc.getSymbol(wasm);
+        sym.mark();
         sym.virtual_address = @as(u32, @intCast(memory_ptr));
         memory_ptr += 4;
     }
@@ -2561,6 +2580,7 @@ pub fn flushModule(wasm: *Wasm, arena: Allocator, prog_node: *std.Progress.Node)
     if (comp.link_errors.items.len > 0) return error.FlushFailure;
     try wasm.resolveLazySymbols();
     try wasm.checkUndefinedSymbols();
+    try wasm.checkExportNames();
 
     try wasm.setupInitFunctions();
     if (comp.link_errors.items.len > 0) return error.FlushFailure;
@@ -4044,6 +4064,7 @@ fn mark(wasm: *Wasm, loc: SymbolLoc) !void {
         return;
     }
     symbol.mark();
+    gc_log.debug("Marked symbol '{s}'", .{loc.getName(wasm)});
     if (symbol.isUndefined()) {
         // undefined symbols do not have an associated `Atom` and therefore also
         // do not contain relocations.
test/link/wasm/archive/build.zig
@@ -19,12 +19,13 @@ fn add(b: *std.Build, test_step: *std.Build.Step, optimize: std.builtin.Optimize
         .name = "main",
         .root_source_file = .{ .path = "main.zig" },
         .optimize = optimize,
-        .target = .{ .cpu_arch = .wasm32, .os_tag = .freestanding },
+        .target = b.resolveTargetQuery(.{ .cpu_arch = .wasm32, .os_tag = .freestanding }),
         .strip = false,
     });
     lib.entry = .disabled;
     lib.use_llvm = false;
     lib.use_lld = false;
+    lib.root_module.export_symbol_names = &.{"foo"};
 
     const check = lib.checkObject();
     check.checkInHeaders();
test/link/wasm/export-data/build.zig
@@ -13,7 +13,7 @@ pub fn build(b: *std.Build) void {
         .name = "lib",
         .root_source_file = .{ .path = "lib.zig" },
         .optimize = .ReleaseSafe, // to make the output deterministic in address positions
-        .target = .{ .cpu_arch = .wasm32, .os_tag = .freestanding },
+        .target = b.resolveTargetQuery(.{ .cpu_arch = .wasm32, .os_tag = .freestanding }),
     });
     lib.entry = .disabled;
     lib.use_lld = false;
test/link/wasm/shared-memory/build.zig
@@ -11,37 +11,39 @@ pub fn build(b: *std.Build) void {
 }
 
 fn add(b: *std.Build, test_step: *std.Build.Step, optimize_mode: std.builtin.OptimizeMode) void {
-    const lib = b.addExecutable(.{
+    const exe = b.addExecutable(.{
         .name = "lib",
         .root_source_file = .{ .path = "lib.zig" },
-        .target = .{
+        .target = b.resolveTargetQuery(.{
             .cpu_arch = .wasm32,
             .cpu_model = .{ .explicit = &std.Target.wasm.cpu.mvp },
             .cpu_features_add = std.Target.wasm.featureSet(&.{ .atomics, .bulk_memory }),
             .os_tag = .freestanding,
-        },
+        }),
         .optimize = optimize_mode,
         .strip = false,
         .single_threaded = false,
     });
-    lib.entry = .disabled;
-    lib.use_lld = false;
-    lib.import_memory = true;
-    lib.export_memory = true;
-    lib.shared_memory = true;
-    lib.max_memory = 67108864;
-    lib.root_module.export_symbol_names = &.{"foo"};
+    exe.entry = .disabled;
+    exe.use_lld = false;
+    exe.import_memory = true;
+    exe.export_memory = true;
+    exe.shared_memory = true;
+    exe.max_memory = 67108864;
+    exe.root_module.export_symbol_names = &.{"foo"};
 
-    const check_lib = lib.checkObject();
+    const check_exe = exe.checkObject();
 
-    check_lib.checkStart("Section import");
-    check_lib.checkNext("entries 1");
-    check_lib.checkNext("module env");
-    check_lib.checkNext("name memory"); // ensure we are importing memory
+    check_exe.checkInHeaders();
+    check_exe.checkExact("Section import");
+    check_exe.checkExact("entries 1");
+    check_exe.checkExact("module env");
+    check_exe.checkExact("name memory"); // ensure we are importing memory
 
-    check_lib.checkStart("Section export");
-    check_lib.checkNext("entries 2");
-    check_lib.checkNext("name memory"); // ensure we also export memory again
+    check_exe.checkInHeaders();
+    check_exe.checkExact("Section export");
+    check_exe.checkExact("entries 2");
+    check_exe.checkExact("name memory"); // ensure we also export memory again
 
     // This section *must* be emit as the start function is set to the index
     // of __wasm_init_memory
@@ -49,49 +51,42 @@ fn add(b: *std.Build, test_step: *std.Build.Step, optimize_mode: std.builtin.Opt
     // This means we won't have __wasm_init_memory in such case, and therefore
     // should also not have a section "start"
     if (optimize_mode == .Debug) {
-        check_lib.checkStart("Section start");
+        check_exe.checkInHeaders();
+        check_exe.checkExact("Section start");
     }
 
     // This section is only and *must* be emit when shared-memory is enabled
     // release modes will have the TLS segment optimized out in our test-case.
     if (optimize_mode == .Debug) {
-        check_lib.checkStart("Section data_count");
-        check_lib.checkNext("count 3");
+        check_exe.checkInHeaders();
+        check_exe.checkExact("Section data_count");
+        check_exe.checkExact("count 1");
     }
 
-    check_lib.checkStart("Section custom");
-    check_lib.checkNext("name name");
-    check_lib.checkNext("type function");
+    check_exe.checkInHeaders();
+    check_exe.checkExact("Section custom");
+    check_exe.checkExact("name name");
+    check_exe.checkExact("type function");
     if (optimize_mode == .Debug) {
-        check_lib.checkNext("name __wasm_init_memory");
+        check_exe.checkExact("name __wasm_init_memory");
     }
-    check_lib.checkNext("name __wasm_init_tls");
-    check_lib.checkNext("type global");
+    check_exe.checkExact("name __wasm_init_tls");
+    check_exe.checkExact("type global");
 
     // In debug mode the symbol __tls_base is resolved to an undefined symbol
     // from the object file, hence its placement differs than in release modes
     // where the entire tls segment is optimized away, and tls_base will have
     // its original position.
-    if (optimize_mode == .Debug) {
-        check_lib.checkNext("name __tls_size");
-        check_lib.checkNext("name __tls_align");
-        check_lib.checkNext("name __tls_base");
-    } else {
-        check_lib.checkNext("name __tls_base");
-        check_lib.checkNext("name __tls_size");
-        check_lib.checkNext("name __tls_align");
-    }
+    check_exe.checkExact("name __tls_base");
+    check_exe.checkExact("name __tls_size");
+    check_exe.checkExact("name __tls_align");
 
-    check_lib.checkNext("type data_segment");
+    check_exe.checkExact("type data_segment");
     if (optimize_mode == .Debug) {
-        check_lib.checkNext("names 3");
-        check_lib.checkNext("index 0");
-        check_lib.checkNext("name .rodata");
-        check_lib.checkNext("index 1");
-        check_lib.checkNext("name .bss");
-        check_lib.checkNext("index 2");
-        check_lib.checkNext("name .tdata");
+        check_exe.checkExact("names 1");
+        check_exe.checkExact("index 0");
+        check_exe.checkExact("name .tdata");
     }
 
-    test_step.dependOn(&check_lib.step);
+    test_step.dependOn(&check_exe.step);
 }
test/link/wasm/type/build.zig
@@ -13,31 +13,32 @@ pub fn build(b: *std.Build) void {
 }
 
 fn add(b: *std.Build, test_step: *std.Build.Step, optimize: std.builtin.OptimizeMode) void {
-    const lib = b.addExecutable(.{
+    const exe = b.addExecutable(.{
         .name = "lib",
         .root_source_file = .{ .path = "lib.zig" },
         .target = b.resolveTargetQuery(.{ .cpu_arch = .wasm32, .os_tag = .freestanding }),
         .optimize = optimize,
         .strip = false,
     });
-    lib.entry = .disabled;
-    lib.use_llvm = false;
-    lib.use_lld = false;
-    b.installArtifact(lib);
+    exe.entry = .disabled;
+    exe.use_llvm = false;
+    exe.use_lld = false;
+    exe.root_module.export_symbol_names = &.{"foo"};
+    b.installArtifact(exe);
 
-    const check_lib = lib.checkObject();
-    check_lib.checkInHeaders();
-    check_lib.checkExact("Section type");
+    const check_exe = exe.checkObject();
+    check_exe.checkInHeaders();
+    check_exe.checkExact("Section type");
     // only 2 entries, although we have more functions.
     // This is to test functions with the same function signature
     // have their types deduplicated.
-    check_lib.checkExact("entries 2");
-    check_lib.checkExact("params 1");
-    check_lib.checkExact("type i32");
-    check_lib.checkExact("returns 1");
-    check_lib.checkExact("type i64");
-    check_lib.checkExact("params 0");
-    check_lib.checkExact("returns 0");
+    check_exe.checkExact("entries 2");
+    check_exe.checkExact("params 1");
+    check_exe.checkExact("type i32");
+    check_exe.checkExact("returns 1");
+    check_exe.checkExact("type i64");
+    check_exe.checkExact("params 0");
+    check_exe.checkExact("returns 0");
 
-    test_step.dependOn(&check_lib.step);
+    test_step.dependOn(&check_exe.step);
 }
test/link.zig
@@ -35,11 +35,10 @@ pub const cases = [_]Case{
     },
 
     // WASM Cases
-    // https://github.com/ziglang/zig/issues/16938
-    //.{
-    //    .build_root = "test/link/wasm/archive",
-    //    .import = @import("link/wasm/archive/build.zig"),
-    //},
+    .{
+        .build_root = "test/link/wasm/archive",
+        .import = @import("link/wasm/archive/build.zig"),
+    },
     .{
         .build_root = "test/link/wasm/basic-features",
         .import = @import("link/wasm/basic-features/build.zig"),
@@ -52,11 +51,10 @@ pub const cases = [_]Case{
         .build_root = "test/link/wasm/export",
         .import = @import("link/wasm/export/build.zig"),
     },
-    // https://github.com/ziglang/zig/issues/16937
-    //.{
-    //    .build_root = "test/link/wasm/export-data",
-    //    .import = @import("link/wasm/export-data/build.zig"),
-    //},
+    .{
+        .build_root = "test/link/wasm/export-data",
+        .import = @import("link/wasm/export-data/build.zig"),
+    },
     .{
         .build_root = "test/link/wasm/extern",
         .import = @import("link/wasm/extern/build.zig"),
@@ -81,6 +79,10 @@ pub const cases = [_]Case{
         .build_root = "test/link/wasm/segments",
         .import = @import("link/wasm/segments/build.zig"),
     },
+    .{
+        .build_root = "test/link/wasm/shared-memory",
+        .import = @import("link/wasm/shared-memory/build.zig"),
+    },
     .{
         .build_root = "test/link/wasm/stack_pointer",
         .import = @import("link/wasm/stack_pointer/build.zig"),