Thanks decoder for sending quite an obscure bug my way :). From looking at the code a while in RR and trying to understand what happened, I could then craft a test case, attached in incoming patch.
When we set an interrupt, it sets TlsData::stackLimit to uintptr max. A stack limit check is emitted as follows for Ion/Cranelift:
- put SP in a temp register
- subtract Tls->stackLimit from temp (so temp := SP - stack limit, which is guaranteed to be positive since we maintain the invariant that SP >= stack limit, in a world where allocating stack memory means subtracting from SP)
- if temp >(signed) allocation size, that is, if SP - stack limit > allocation size, we're all good. Otherwise, we generate a software trap.
Now, when the interrupt flag is set, TlsData::stackLimit is set to uintptr max, so SP - stack limit === SP + 1, and this flag is never honored. It means that both the interrupt check is never done, and that the stack overflow check is meaningless.
The baseline compiler isn't affected, because it does something different:
- subtract amount from SP
- then check if the resulting SP >(unsigned) stack limit.
(There might be a cleanup to do there: the stack allocation for Ion/Cranelift mentions that doing so might result in a borked SP value when we end up in the signal handler.)
I think Ion/Cranelift want the same as baseline (still using the temp). If we want to make sure that SP - amount doesn't overflow, we'd need a supplementary check; but it doesn't seem likely we'd ever need it.
(decoder told me this isn't sec-sensitive on IRC)