wasm has a max stack size of ~66K

[fengb]

Easiest to reproduce with the WASI test patches:

$ zig test wasi.zig -target wasm32-wasi --test-cmd wasmtime --test-cmd-bin

const std = @import("std");

test "safe" {
    var buf: [64000]u8 = undefined;
    std.debug.warn("{*}\n", &buf);
}

test "broken in debug" {
    var buf: [65000]u8 = undefined;
    std.debug.warn("{*}\n", &buf);
}

test "broken in release-safe" {
    var buf: [66000]u8 = undefined;
    std.debug.warn("{*}\n", &buf);
}

test "broken in release-fast" {
    var buf: [68000]u8 = undefined;
    std.debug.warn("{*}\n", &buf);
}

[fengb]

I found this in the outputted wasm:

(memory (;0;) 2)
(global (;0;) (mut i32) (i32.const 68752))

Manually dialing up memory=3 and global=69000 fixes this.

Edit: global (;0;) == __heap_base (aka stack end). This should be configurable by the linker, but it's also a surprisingly low default...

[fengb]

Possible solutions:

1. expose a compile flag to set the stack-size
2. bump the default to something less silly
3. implement @stackSize() eliminate stack overflow #1639

[fengb]

The LLVM stack is actually not the same as the wasm stack. It's only being for local variables that do not have native representation in the wasm machine (i32, i64, f32, f64) so only arrays and structs. This explains why certain types of tests (ones with large stack buffers) blow out the stack but stdlib tests mostly run fine.

[kubkon]

I was going to file a new issue, but this one seems on point, or at least related to stack problems with libstd tests, so instead, I'll pile it on in here. If you feel this warrants a new issue, please feel free to speak up.

Currently, in std.packed_int_array you'll notice we have "PackedIntArray" and "PackedIntSlice" tests skipped if arch is wasm32. If we re-enable any one of those two, we'll be greeted by a nasty runtime "call stack exhausted" trap in Wasmtime:

143/1411 linked_list.test "std-wasm32-wasi-Debug-bare-single TailQueue concatenation"...OK
144/1411 packed_int_array.test "std-wasm32-wasi-Debug-bare-single PackedIntArray"...Error: failed to run main module `/Users/kubkon/dev/zig/zig-cache/o/ixxWdKlUup-uWom93BNqeUrzWJ74VkkY4GXDijhrqbh-         W3FwEh_23kjYQTWrMJKx/test.wasm`

Caused by:
    0: failed to invoke command default
    1: wasm trap: call stack exhausted
       wasm backtrace:
         0: 0xffffffff - <unknown>!panic
         1: 0xcc45c7 - <unknown>!panic
         ...
         32089: 0xcc45c7 - <unknown>!panic
         32090: 0xcc85c8 - <unknown>!__lshrti3
         32091: 0x64f475 - <unknown>!packed_int_array.PackedIntIo(i120,builtin.Endian.Little).setBits.3604
         32092: 0x43062d - <unknown>!packed_int_array.PackedIntIo(i120,builtin.Endian.Little).set
         32093: 0x9fde1 - <unknown>!packed_int_array.PackedIntArrayEndian(i120,builtin.Endian.Little,19).set
         32094: 0x4c85a - <unknown>!packed_int_array.test "std-wasm32-wasi-Debug-bare-single PackedIntArray"
         32095: 0x3beadf - <unknown>!std.special.main
         32096: 0x3be40f - <unknown>!_start

Increasing the stack size by passing an additional extension option to lld in the form of -z stack-size=value doesn't actually fix or change the rt panic here in any way. Interestingly enough however running the tests in the problematic module works just fine:

zig test lib/std/packed_int_array.zig

@fengb if you have any ideas about this, feel free to shout out!

The issue aside, I think it'd still be a good idea to increase the default stack size to something more reasonable than just 1 Wasm page of memory as @fengb suggested. If we take cue from Rust, Alex increased the default to approx 1MB (relevant PR in rustlang). This won't fix the problem I mentioned, but will hopefully prevent some silly overflows in the future. @andrewrk @fengb lemme know what you guys think, and if you're happy with this, I'll supply a patch.

Taking this a bit further, I think that acting on option 1. is also worth doing. Perhaps we could even give the Zig's programmer "raw" access to the linker, and pass whatever flags they want, much like it's done in Rust with -Clink-args='...'? This way, if the programmer wanted to have a large stack, they'd be free to adjust accordingly. What do y'all think about this?

Finally, option 3. seems really interesting, but also definitely non-trivial, so in the meantime, I'd suggest we go with the above, and when 3. lands, we can rethink our strategy. Thoughts?

[fengb]

Option 3 is the longterm goal to eliminate stack overflow, but I don't think we're there yet.

I think we can expose a flag to set the stack value. There's a --stack [n] flag for zig cc and we can probably carry that over to build-lib and build-exe. Arbitrary linker flags aren't available yet so -z stack-size=[n] wouldn't work and I don't think andrewrk wants more ties to LLVM.

[pixelherodev]

My two cents:

There is no reason for the default stack size to be so low, regardless of these issues. I'll open a separate proposal if you want, but I honestly think we should, at minimum, double it. Different platforms should have different defaults, based on reasonable memory expectations. on x64 for instance, we can assume a bare minimum of 2GiB of RAM, and so using even 1MiB for the stack is just not a big deal.

[kubkon]

OK, since we seem to agree that a larger default stack size is not a bad idea, I've now submitted #5529 which increases it from 1 Wasm page to 16 Wasm pages, or roughly 1MB.

IMHO, the next thing worth following up here with is exposing a flag to set the stack value as @fengb suggested.

[kubkon]

I've had a poke around zig cc and now I see that it's a drop-in replacement for clang essentially which implies we can pass additional linker args like we'd normally do with clang. So, wrt to increasing the stack size, we could do:

zig cc -Wl,stack-size -Wl,65536

However, because it is a drop-in replacement for clang, it requires an installed version of libc for the specified target, am I right? In other words, AFAICS, to compile to WASI I'd need wasi-libc installed on my system, which for me personally is less than ideal since I'd like to steer away from wasi-libc and use Zig's libstd instead. Is there any way around it? If not, how could we expose stack size manipulation arg to the end-user? I guess what I'm really asking here is, are we going to replace lld with our own linker down the line, or will we still depend on it in the future? If the latter, even if we don't want to expose the precise lld arg for manipulating stack sizes, I guess it'd be good to expose some sort of a flag to the end-user of zig proper anyway should they decide their app needs it.

[fengb]

Sorry, I meant that there exists plumbing for the elf linker to overwrite the stack size. This is only exposed via zig cc --stack 1048576 right now, but it would make sense to add the same flag for build-lib and build-exe, and then add it to the wasm linker.

[kubkon]

Sorry, I meant that there exists plumbing for the elf linker to overwrite the stack size. This is only exposed via zig cc --stack 1048576 right now, but it would make sense to add the same flag for build-lib and build-exe, and then add it to the wasm linker.

Makes sense! Thanks for clarifying!

[kubkon]

@andrewrk merged a couple of changes addressing this issue (increased default stack to 1MB and added —stack flag to the compiler), however, I’d be tempted to leave this issue open as a subtracker of #1639. What do you think @fengb?

[fengb]

I think this can be closed. 1 + 2 have been implemented and 3 is a longstanding issue that'll be tackled by the umbrella issue. Thanks!

[williamstein]

The stack size is 1MB when using build-exe, but it is still only 64KB when using build-lib. It's possible that this is a bug (?). I'm working around this by explicitly passing --stack 1048576. I did hit random crashes due to stack overflow which I didn't solve until I found this issue via google. I then dived into the wasm2wat output to find that this problem remains when using build-lib. Here's how to reproduce this:

~/test/stack$ zig version
0.10.0-dev.3629+2ccaa5414
~/test/stack$ uname -a
Darwin max.local 21.6.0 Darwin Kernel Version 21.6.0: Wed Aug 10 14:28:23 PDT 2022; root:xnu-8020.141.5~2/RELEASE_ARM64_T6000 arm64

~/test/stack$ cat b.zig

const std = @import("std");
pub fn main() void {
    std.debug.print("hello world\n", .{});
}

~/test/stack$ zig build-exe -target wasm32-wasi b.zig
~/test/stack$ wasm2wat b.wasm |grep const |grep stack_pointer
  (global $__stack_pointer (mut i32) (i32.const 1048576))
~/test/stack$ zig build-lib -dynamic -rdynamic -target wasm32-wasi b.zig
~/test/stack$ wasm2wat b.wasm |grep const |grep stack_pointer
  (global $__stack_pointer (mut i32) (i32.const 65536))
~/test/stack$ zig build-lib -dynamic -rdynamic -target wasm32-wasi --stack 1048576 b.zig
~/test/stack$ wasm2wat b.wasm |grep const |grep stack_pointer
  (global $__stack_pointer (mut i32) (i32.const 1048576))

I also tested with build from zig-bootstrap, so self-hosted (?) and the output is identical.

~/test/stack$ zig version
0.10.0-dev.3653+7152a58c1

[kubkon]

Could you take a look @Luukdegram?

[Luukdegram]

It seems this was deliberately only implemented for build-exe. I cannot figure out the reasoning behind it, and neither does git blame tell me much. In my opinion, we should set this 1MB default regardless of build type when using the LLVM backend or building from c-code. (The native backend can calculate the required size easily). If the user doesn't require that 1MB size, they can always still override the stack size, but at least the behavior will be consistent.

[kubkon]

Sounds good! Could you look into adding this missing bit then please? In the meantime, I will reopen this issue.

[Luukdegram]

Will do!