Limit stack size on Windows like other targets

We can't use `ulimit -s` to limit stack size on Windows.  Even though Bash
accepts `ulimit -s` and the numbers change it has no effect and is not passed
to child processes.

(See https://public-inbox.org/git/alpine.DEB.2.21.1.1709131448390.4132@virtualbox/)

Instead, set it when building the test executable, following, a suggestion from
@stefantalpalaru.
https://github.com/status-im/nimbus-eth1/pull/598#discussion_r621107128

To ensure no conflict with `config.nims`, `-d:windowsNoSetStack` is used.  This
proved unnecessary in practice because the command-line option is passed to the
linker after the config file option.  But given we don't have an automated test
to verify linker behaviour, it's best not to rely on the option order, neither
how the linker treats it, or whether Nim will always send them in that order.

Testing:

This has been verified by using a smaller limit.  At 200k, all `ENABLE_EVMC=0`
OS targets passed as expected, and all `ENABLE_EVMC=1` OS targets failed with
expected kinds of errors due to stack overflow, including Windows.
(400k wasn't small enough; 32-bit x86 Windows passed on that).

Signed-off-by: Jamie Lokier <jamie@shareable.org>
This commit is contained in:
Jamie Lokier 2021-04-29 19:40:47 +01:00
parent 6b65467af7
commit 8161de1a8f
No known key found for this signature in database
GPG Key ID: CBC25C68435C30A2
2 changed files with 14 additions and 9 deletions

View File

@ -6,8 +6,9 @@ else:
if defined(windows):
# disable timestamps in Windows PE headers - https://wiki.debian.org/ReproducibleBuilds/TimestampsInPEBinaries
switch("passL", "-Wl,--no-insert-timestamp")
# increase stack size
switch("passL", "-Wl,--stack,8388608")
# increase stack size, unless something else is setting the stack size
if not defined(windowsNoSetStack):
switch("passL", "-Wl,--stack,8388608")
# https://github.com/nim-lang/Nim/issues/4057
--tlsEmulation:off
if defined(i386):

View File

@ -30,16 +30,20 @@ proc buildBinary(name: string, srcDir = "./", params = "", lang = "c") =
exec "nim " & lang & " --out:build/" & name & " " & extra_params & " " & srcDir & name & ".nim"
proc test(name: string, lang = "c") =
buildBinary name, "tests/", "-d:chronicles_log_level=ERROR"
# Verify stack usage is kept low by setting 750k stack limit in tests.
const stackLimitKiB = 750
when not defined(windows):
# Verify stack usage is kept low by setting 1024k stack limit in tests.
exec "ulimit -s 1024 && build/" & name
const (buildOption, runPrefix) = ("", "ulimit -s " & $stackLimitKiB & " && ")
else:
# Don't enforce stack limit in Windows, as we can't control it with these tools.
# No `ulimit` in Windows. `ulimit -s` in Bash is accepted but has no effect.
# See https://public-inbox.org/git/alpine.DEB.2.21.1.1709131448390.4132@virtualbox/
# When set by ulimit -s` in Bash, it's ignored. Also, the command passed to
# NimScript `exec` on Windows is not a shell script.
exec "build/" & name
# Also, the command passed to NimScript `exec` on Windows is not a shell script.
# Instead, we can set stack size at link time.
const (buildOption, runPrefix) =
(" -d:windowsNoSetStack --passL:-Wl,--stack," & $(stackLimitKiB * 1024), "")
buildBinary name, "tests/", "-d:chronicles_log_level=ERROR" & buildOption
exec runPrefix & "build/" & name
task test, "Run tests":
test "all_tests"