From bddf138e7285eefb86ef880e1200a929193da40b Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Wed, 8 Mar 2023 06:27:03 +0100 Subject: [PATCH 1/2] wasm-link: fix storing decls in the right segment When a decl is `undefined` is must be stored in the data segment when the build mode is safe. For unsafe optimize modes, it must be stored in the bss segment instead. For mutable decls where the atom contains all zeroes, it must always be stored in the bss segment. All other values will result in the atom being stored in the data segment. --- src/link/Wasm.zig | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index e998a8d50e..01100add4b 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -2828,22 +2828,31 @@ pub fn flushModule(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Nod const decl = mod.declPtr(entry.key_ptr.*); if (decl.isExtern()) continue; const atom_index = entry.value_ptr.*; + const atom = wasm.getAtomPtr(atom_index); if (decl.ty.zigTypeTag() == .Fn) { try wasm.parseAtom(atom_index, .function); } else if (decl.getVariable()) |variable| { if (!variable.is_mutable) { try wasm.parseAtom(atom_index, .{ .data = .read_only }); } else if (variable.init.isUndefDeep()) { - try wasm.parseAtom(atom_index, .{ .data = .uninitialized }); + // for safe build modes, we store the atom in the data segment, + // whereas for unsafe build modes we store it in bss. + const is_initialized = wasm.base.options.optimize_mode == .Debug or + wasm.base.options.optimize_mode == .ReleaseSafe; + try wasm.parseAtom(atom_index, .{ .data = if (is_initialized) .initialized else .uninitialized }); } else { - try wasm.parseAtom(atom_index, .{ .data = .initialized }); + // when the decl is all zeroes, we store the atom in the bss segment, + // in all other cases it will be in the data segment. + const is_zeroes = for (atom.code.items) |byte| { + if (byte != 0) break false; + } else true; + try wasm.parseAtom(atom_index, .{ .data = if (is_zeroes) .uninitialized else .initialized }); } } else { try wasm.parseAtom(atom_index, .{ .data = .read_only }); } // also parse atoms for a decl's locals - const atom = wasm.getAtomPtr(atom_index); for (atom.locals.items) |local_atom_index| { try wasm.parseAtom(local_atom_index, .{ .data = .read_only }); } From d0fb1ef9625b55cc71b41438265f596a00d63749 Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Wed, 8 Mar 2023 06:42:23 +0100 Subject: [PATCH 2/2] wasm-link: update bss linker test Updates the linker test to verify the various cases where we must store the data in the bss segment. --- test/link/wasm/bss/build.zig | 108 ++++++++++++++++++++++++----------- test/link/wasm/bss/lib2.zig | 5 ++ 2 files changed, 79 insertions(+), 34 deletions(-) create mode 100644 test/link/wasm/bss/lib2.zig diff --git a/test/link/wasm/bss/build.zig b/test/link/wasm/bss/build.zig index bba2e7c602..4a26e78a12 100644 --- a/test/link/wasm/bss/build.zig +++ b/test/link/wasm/bss/build.zig @@ -6,38 +6,78 @@ pub fn build(b: *std.Build) void { const test_step = b.step("test", "Test"); b.default_step = test_step; - const lib = b.addSharedLibrary(.{ - .name = "lib", - .root_source_file = .{ .path = "lib.zig" }, - .target = .{ .cpu_arch = .wasm32, .os_tag = .freestanding }, - .optimize = .Debug, - }); - lib.use_llvm = false; - lib.use_lld = false; - lib.strip = false; - // to make sure the bss segment is emitted, we must import memory - lib.import_memory = true; - lib.install(); - - const check_lib = lib.checkObject(); - - // since we import memory, make sure it exists with the correct naming - check_lib.checkStart("Section import"); - check_lib.checkNext("entries 1"); - check_lib.checkNext("module env"); // default module name is "env" - check_lib.checkNext("name memory"); // as per linker specification - - // since we are importing memory, ensure it's not exported - check_lib.checkNotPresent("Section export"); - - // validate the name of the stack pointer - check_lib.checkStart("Section custom"); - check_lib.checkNext("type data_segment"); - check_lib.checkNext("names 2"); - check_lib.checkNext("index 0"); - check_lib.checkNext("name .rodata"); - check_lib.checkNext("index 1"); // bss section always last - check_lib.checkNext("name .bss"); - - test_step.dependOn(&check_lib.step); + add(b, test_step, .Debug, true); + add(b, test_step, .ReleaseFast, false); + add(b, test_step, .ReleaseSmall, false); + add(b, test_step, .ReleaseSafe, true); +} + +fn add(b: *std.Build, test_step: *std.Build.Step, optimize_mode: std.builtin.OptimizeMode, is_safe: bool) void { + { + const lib = b.addSharedLibrary(.{ + .name = "lib", + .root_source_file = .{ .path = "lib.zig" }, + .target = .{ .cpu_arch = .wasm32, .os_tag = .freestanding }, + .optimize = optimize_mode, + }); + lib.use_llvm = false; + lib.use_lld = false; + lib.strip = false; + // to make sure the bss segment is emitted, we must import memory + lib.import_memory = true; + + const check_lib = lib.checkObject(); + + // since we import memory, make sure it exists with the correct naming + check_lib.checkStart("Section import"); + check_lib.checkNext("entries 1"); + check_lib.checkNext("module env"); // default module name is "env" + check_lib.checkNext("name memory"); // as per linker specification + + // since we are importing memory, ensure it's not exported + check_lib.checkNotPresent("Section export"); + + // validate the name of the stack pointer + check_lib.checkStart("Section custom"); + check_lib.checkNext("type data_segment"); + check_lib.checkNext("names 2"); + check_lib.checkNext("index 0"); + check_lib.checkNext("name .rodata"); + // for safe optimization modes `undefined` is stored in data instead of bss. + if (is_safe) { + check_lib.checkNext("index 1"); + check_lib.checkNext("name .data"); + check_lib.checkNotPresent("name .bss"); + } else { + check_lib.checkNext("index 1"); // bss section always last + check_lib.checkNext("name .bss"); + } + test_step.dependOn(&check_lib.step); + } + + // verify zero'd declaration is stored in bss for all optimization modes. + { + const lib = b.addSharedLibrary(.{ + .name = "lib", + .root_source_file = .{ .path = "lib2.zig" }, + .target = .{ .cpu_arch = .wasm32, .os_tag = .freestanding }, + .optimize = optimize_mode, + }); + lib.use_llvm = false; + lib.use_lld = false; + lib.strip = false; + // to make sure the bss segment is emitted, we must import memory + lib.import_memory = true; + + const check_lib = lib.checkObject(); + check_lib.checkStart("Section custom"); + check_lib.checkNext("type data_segment"); + check_lib.checkNext("names 2"); + check_lib.checkNext("index 0"); + check_lib.checkNext("name .rodata"); + check_lib.checkNext("index 1"); + check_lib.checkNext("name .bss"); + + test_step.dependOn(&check_lib.step); + } } diff --git a/test/link/wasm/bss/lib2.zig b/test/link/wasm/bss/lib2.zig new file mode 100644 index 0000000000..9f43128880 --- /dev/null +++ b/test/link/wasm/bss/lib2.zig @@ -0,0 +1,5 @@ +pub var bss: u32 = 0; + +export fn foo() void { + _ = bss; +}