Put stack first when not optimizing for size

[sbc100]

Also, when not using santizers (which currently rely on using the start
of memory).

This allows STACK_OVERFLOW_CHECK=1 to detect stack overflow conditions
better without relying on the STACK_OVERFLOW_CHECK=2 binaryen pass.

This works because when stack is placed first in memory stack overflow
results in SP dropping below zero which is a runtime error. We can
then distinguish such runtime errors by looking at the SP value at the
time of the crash.

This change is part of a sequence of the effort to reduce the default
stack size. The hope here is that we will be able to accurately catch
overflows in debug builds even without the STACK_OVERFLOW_CHECK=2
binaryen pass, thus minimizing the impact of reducing the stack size.

See #14177

[dschuff]

Is there anything useful we can say in the changelog? Maybe just highlighting the user-visible difference in stack overflow behavior in unoptimized builds?

[sbc100]

Added a ChangeLog entry

[kripken]

(the ptrToString changes are from the other PR I think?)

[sbc100]

(the ptrToString changes are from the other PR I think?)

yes sorry, when I stack PRs like that and then one of them gets merged, it confuses github. Please ignore.

[dschuff]

I just found out that one of our users is doing something like the following in JS: EM_ASM_DOUBLE({"return HEAP8.length - STACK_SIZE"}). If you ignore the fact that it misses the static data, the intent is to give the total memory available to the app (i.e. the heap size). That obviously only works with globals-first and not with stack-first. Is there a portable way to get the heap size? On the emscripten side I see STACK_HIGH, STACK_LOW, HEAP_BASE, and GLOBAL_BASE but that's not enough to portably get the stack size. Is there maybe a linker-generated symbol?

[dschuff]

actually, answering my own question: it looks like https://reviews.llvm.org/D136110 will allow that.
Nevermind; I think there is a way, because the heap is always last, right? So HEAP8.length - HEAP_BASE will always give that size, right?
And in C you could use the linker-generated symbol and do __builtin_wasm_memory_size() - __heap_base

[sbc100]

actually, answering my own question: it looks like https://reviews.llvm.org/D136110 will allow that.

Using HEAP8.length - __heap_base is probably the quickest fix, and it works for growable memory too.

Also, the value of HEAP8.length - STACK_SIZE will be the same both before and after this change.. what makes you think this change will break this? HEAP8.length - STACK_SIZE is the size of linear memory minus the stack size, which is the combined size of the heap and static data... this is true regardless of whether the stack comes first.

[dschuff]

oh, yeah, you're right. I was looking for a way that also correctly accounts for the globals size.

[juj]

When a function needs to push its stack frame to accommodate for new items, does LLVM not do a stack overflow check right there and then in debug/other suitable builds? Needing to rely on the stack being in front of the memory address to get stack overflow to work seems silly and brittle, since it complicates build modes, and the same strategy can't extend to pthreads.

[sbc100]

When a function needs to push its stack frame to accommodate for new items, does LLVM not do a stack overflow check right there and then in debug/other suitable builds? Needing to rely on the stack being in front of the memory address to get stack overflow to work seems silly and brittle, since it complicates build modes, and the same strategy can't extend to pthreads.

No, by default in debug builds you don't get precise stack checking like that. In emscripten we have -sSTACK_OVERFLOW_CHECK=2 which does precise overflow checking, but that requires a binaryen pass.

I had initially proposed that we make -sSTACK_OVERFLOW_CHECK=2 the default in debug builds (currently its the cheaper -sSTACK_OVERFLOW_CHECK=1 that we get in debug builds). My PR to make that happen is here: #18040. Turns out it doesn't cost a lot a runtime, but forcing all debug biulds to run a binaryen pass is undesirable, because that can slow down time significantly. We are trying hard to eliminate the need to binaryen passes in debug builds for this reason.

There may also be other llvm options to enable stack checking, but I think that would require specific command line flags for all compilation units (not just a link flag), and I don't think we use them all in emscripten today. We only use the non-llvm STACK_OVERFLOW_CHECK options currently.

[juj]

Thanks - yeah, I was thinking LLVM would be able to do this out of the box, seems odd if it doesn't.

Though if/since it doesn't, then relying on the Wasm VM trap behavior does make sense.