On 22/07/2026 13:51, Aswin Kalies Ramkumar Mangayarkarasi wrote:
Hi Jon,
Thanks for the review.
No problem! Apologies again for the long delay in getting around to
looking at this.
This is a follow-up patch to:
https://cygwin.com/pipermail/cygwin-patches/2026q3/015206.html
Responses inline below; v3 addresses all of the review points.
For testing, we built a standalone ucontext/altstack test suite and ran it
against a freshly built cygwin1.dll on Windows ARM64. The tests cover
getcontext/setcontext/swapcontext/makecontext round-trips, odd argument counts
(9–13), NULL uc_link exit behaviour, a 5000-iteration swapcontext loop, and an
SA_ONSTACK signal handler.
We also ran a four-way isolation matrix where each branch reverted exactly one
of the three fixes to verify that each change is independently required.
Reverting the setcontext hand-restore causes every makecontext/swapcontext test
to crash with STATUS_STACK_BUFFER_OVERRUN, matching the pre-port baseline.
Reverting the trampoline (including the sp,x19 fix and mov w0,#0xff) causes the
odd-argc tests to fail with STATUS_DATATYPE_MISALIGNMENT, and the NULL uc_link
test exits with 0 instead of 0xff.
I agree there's nothing in the upstream testsuite covering this yet. I'm happy
to contribute these tests as a follow-up if that would be useful.
Great! That's very impressive work.
Yes, please! It would be good to have these in our testsuite. Please ask
if you have any questions about doing that.
Regarding the clobber list in call_signal_handler, the previous comment was misleading. The clobber list is serving two distinct purposes, and I've rewritten the comment to explain that. x0-x7, x9, x10, and x29 are explicitly used by the assembly as scratch/argument registers and are listed as clobbers to prevent GCC from allocating "r" input operands into registers that the assembly overwrites before reading. The x86_64 version doesn't need this because its inputs are "o" memory operands. x8, x11-x17, x30 (clobbered by blr), and v0-v7/v16-v31 are the normal caller-saved registers clobbered by the C call to altstack_wrapper. x18 (TEB) and v8-v15 (callee-saved on Windows) are preserved and therefore omitted.> So the registers you pointed out aren't redundant with the explicit
save/restore sequence. Removing them would allow GCC to place an input
operand into a register that the assembly overwrites before reading it.
That said, I'm also happy to convert this path to "o" memory operands
like the x86_64 implementation if you think that's the cleaner approach.
I kept "r" operands simply to minimise churn in the already-tested assembly.
No, that's fine. I hadn't considered the input registers at all. So it
makes sense now.
For RtlRestoreContext, unfortunately I haven't found any Microsoft documentation describing this behaviour. The STATUS_ILLEGAL_INSTRUCTION failure on a synthetic context was discovered empirically. I also tried setting ContextFlags = CONTEXT_FULL on the makecontext-generated context in case RtlRestoreContext required a particular flag mask, but it made no difference. The hand-restore is genuinely required. I've added a comment noting this.> For the 272 constant, I've added a comment explaining that it's the
byte offset of the V-register array within struct __mcontext, whose
layout mirrors the Windows ARM64 CONTEXT structure. The comment now also
documents all offsets used by the routine.
Thanks.
Regarding x17 and CPSR not being restored in setcontext, this is the same
asymmetry as in the earlier incyg fix, but you're right that it should be
documented explicitly rather than left implicit. The omission is intentional.
x16 (IP0) and x17 (IP1) are the ABI-defined intra-procedure-call scratch
registers. This implementation uses x16 as the base register and x17 as the
branch target, and the ABI already permits linker-inserted veneers to clobber
both registers, so a ucontext consumer cannot rely on either surviving a
setcontext() regardless. Their saved values are therefore intentionally not
restored. CPSR is likewise not restored because there is no EL0 instruction
that restores the entire register, and the only architecturally visible state
that user code could reasonably care about (NZCV) is caller-clobbered across
the makecontext()/swapcontext() boundary that this code serves. glibc and musl
take the same approach on AArch64. I've added comments explaining this.
Finally, I've fixed both the "return address on the stack" comment and the
missing makecontext #error cases. The header comment now states that the continuation
trampoline is reached via a stack return slot on x86_64 but via lr on AArch64. I also
added the missing #else / #error unimplemented for this target to each of the four #if
defined(x86_64) / #elif defined(aarch64) blocks in makecontext.
Great. Thanks for addressing all my little quibbles!
I applied this patch.
