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);
}
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...
Possible solutions:
@stackSize() https://github.com/ziglang/zig/issues/1639The 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.
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?
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.
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.
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.
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.
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.
Sorry, I meant that there exists plumbing for the elf linker to overwrite the stack size. This is only exposed via
zig cc --stack 1048576right 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!
@andrewrk merged a couple of changes addressing this issue (increased default stack to 1MB and added 鈥攕tack flag to the compiler), however, I鈥檇 be tempted to leave this issue open as a subtracker of #1639. What do you think @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!