runtime: Fix sigaltstack call with musl on certain Intel CPUs - #21
Conversation
|
I'll port this to our 5.4.0 branch, our tsan branches, and I'll put an upstream PR up once we've validated it a bit with Semgrep. |
dijkstracula
left a comment
There was a problem hiding this comment.
Looks great, nice job debugging this.
| size_t size = SIGSTKSZ; | ||
| #ifdef _SC_MINSIGSTKSZ | ||
| /* glibc/musl only */ | ||
| long min = sysconf(_SC_MINSIGSTKSZ); |
There was a problem hiding this comment.
Do we want/need to handle sysconf returning -1 (and setting errno)? I think this would only happen if _SC_MINSIGSTKSZ is not a valid system limit, which you gate via the macro, so maybe not a concern but just throwing it out there.
There was a problem hiding this comment.
Yes, and we already do (see the line below this one). I believe that it's possible for _SC_MINSIGSTKSZ to be a valid constant at build time but for the built binary to link against a different libc for which it is not a valid constant. In that case, sysconf would error, but this is handled by the min > 0 check below.
…63) (semgrep/semgrep-proprietary#6752) This pulls in semgrep/ocaml#19 and semgrep/ocaml#21. See the latter for a description of the fatal error in the runtime that this will fix. Test plan: CI is green on this PR and the relevant jobs are green on the [test PR](semgrep/semgrep-proprietary#6753), which forces intel Depot runners, e.g. https://github.com/semgrep/semgrep-proprietary/actions/runs/29363621357/job/87191257620 synced from Pro 49302cf1304af25e9391bb02b17c121af97b45c9
The problem
On Friday we started sporadically seeing some test failures in CI with the error message
Fatal error: Failed to allocate signal stack for domain 0. It turned out that in order to mitigate a capacity incident, Depot added Intel runners to their fleet, which was previously AMD only. These errors occurred consistently when we were allocated an Intel runner.The OCaml runtime calls
sigaltstackwith the size of the stack set toSIGSTKSZ. Musl libc defines that as a build-time constant.Musl 1.2.6 includes a change that errors if the size passed to
sigaltstackis less thansysconf(_SC_MINSIGSTKSZ). This value is determined at runtime based onAT_MINSIGSTKSZ. The kernel determinesAT_MINSIGSTKSZbased in part on constants provided by the CPU. As a result,sysconf(_SC_MINSIGSTKSZ)can be greater thanSIGSTKSZ, leading to a fatal error in the OCaml runtime when linked against musl 1.2.6 or later.The production Semgrep image is currently based on Alpine 3.23, which ships with musl 1.2.5, which lacks the enforcement of the minimum. Thus, we are currently safe from this issue in production. Alpine 3.24 ships with musl 1.2.6, so we need to address this issue before upgrading.
The fix
The fix is fairly straightforward. We dynamically choose the size of the signal stack based on
sysconf(_SC_MINSIGSTKSZ), when available. This logic is gated behind an#ifdefbecause on some platforms (e.g. macOS) it is not available.Test plan
I put together a set of CI jobs that illustrates the issue and tests this change (Semgrep-internal): https://github.com/semgrep/semgrep-proprietary/actions/runs/29355721766
-4runners were hardcoded by Depot for us to always land on Intel, and-8were hardcoded to always land on AMD.Intel(R) Xeon(R) Platinum 8488CCPU,sysconf(_SC_MINSIGSTKSZ) = 12976andSIGSTKSZ=8192. CallingsigaltstackwithSIGSTKSZsucceeds on Alpine 3.23 (musl 1.2.5), but fails on Alpine 3.24 (musl 1.2.6) because the size is below the minimum.Fatal error: Failed to allocate signal stack for domain 0: Out of memoryusing OCaml 5.3.0-semgrep.