From f767f8e3dc158a74debdd288884458b860d2ebc9 Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Tue, 11 Jan 2022 20:21:22 +0100 Subject: [PATCH 1/2] wasm: Place the stack at the start of memory --- src/link/Wasm.zig | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index f1dca0afe083..2dfe0dd5d9c2 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -494,9 +494,21 @@ fn setupMemory(self: *Wasm) !void { log.debug("Setting up memory layout", .{}); const page_size = 64 * 1024; const stack_size = self.base.options.stack_size_override orelse page_size * 1; - const stack_alignment = 16; - var memory_ptr: u64 = self.base.options.global_base orelse 1024; - memory_ptr = std.mem.alignForwardGeneric(u64, memory_ptr, stack_alignment); + const stack_alignment = 16; // wasm's stack alignment as specified by tool-convention + // Always place the stack at the start by default + // unless the user specified the global-base flag + var place_stack_first = true; + var memory_ptr: u64 = if (self.base.options.global_base) |base| blk: { + place_stack_first = false; + break :blk base; + } else 0; + + if (place_stack_first) { + memory_ptr = std.mem.alignForwardGeneric(u64, memory_ptr, stack_alignment); + memory_ptr += stack_size; + // We always put the stack pointer global at index 0 + self.globals.items[0].init.i32_const = @bitCast(i32, @intCast(u32, memory_ptr)); + } var offset: u32 = @intCast(u32, memory_ptr); for (self.segments.items) |*segment, i| { @@ -510,8 +522,11 @@ fn setupMemory(self: *Wasm) !void { offset += segment.size; } - memory_ptr = std.mem.alignForwardGeneric(u64, memory_ptr, stack_alignment); - memory_ptr += stack_size; + if (!place_stack_first) { + memory_ptr = std.mem.alignForwardGeneric(u64, memory_ptr, stack_alignment); + memory_ptr += stack_size; + self.globals.items[0].init.i32_const = @bitCast(i32, @intCast(u32, memory_ptr)); + } // Setup the max amount of pages // For now we only support wasm32 by setting the maximum allowed memory size 2^32-1 @@ -554,9 +569,6 @@ fn setupMemory(self: *Wasm) !void { self.memories.limits.max = @intCast(u32, max_memory / page_size); log.debug("Maximum memory pages: {d}", .{self.memories.limits.max}); } - - // We always put the stack pointer global at index 0 - self.globals.items[0].init.i32_const = @bitCast(i32, @intCast(u32, memory_ptr)); } fn resetState(self: *Wasm) void { From 975049e96e2245022a90368360fe7f3617e5f194 Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Tue, 11 Jan 2022 20:35:44 +0100 Subject: [PATCH 2/2] wasm-ld: Append `--stack-first` by default By placing the stack at the start of the memory section, we prevent the runtime from silently overwriting the global declarations and instead trap. We do however, allow users to overwrite this behavior by setting the global-base, which puts the stack at the end of the memory section and the static data at the base that was specified. The reason a user would want to do this, is when they are sure the stack will not overflow and they want to decrease the binary size as the offsets to the static memory are generally smaller. (Having the stack in front, means that accessing the memory after the stack has a bigger offset when loading/storing from memory). --- src/link/Wasm.zig | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index 2dfe0dd5d9c2..99311b844100 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -1243,6 +1243,12 @@ fn linkWithLLD(self: *Wasm, comp: *Compilation) !void { if (self.base.options.global_base) |global_base| { const arg = try std.fmt.allocPrint(arena, "--global-base={d}", .{global_base}); try argv.append(arg); + } else { + // We prepend it by default, so when a stack overflow happens the runtime will trap correctly, + // rather than silently overwrite all global declarations. See https://github.com/ziglang/zig/issues/4496 + // + // The user can overwrite this behavior by setting the global-base + try argv.append("--stack-first"); } var auto_export_symbols = true; @@ -1294,10 +1300,6 @@ fn linkWithLLD(self: *Wasm, comp: *Compilation) !void { const arg = try std.fmt.allocPrint(arena, "stack-size={d}", .{stack_size}); try argv.append(arg); - // Put stack before globals so that stack overflow results in segfault immediately - // before corrupting globals. See https://github.com/ziglang/zig/issues/4496 - try argv.append("--stack-first"); - if (self.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");