[PATCH] Cygwin: gendef: fix AArch64 _sigfe_maybe control flow

[Aswin Kalies Ramkumar Mangayarkarasi]

Hi Jon,

This is a fix for the _sigfe_maybe stub you reviewed in
https://cygwin.com/pipermail/cygwin-patches/2026q3/015305.html

You wrote:

>> Hmmm... while staring at this code, trying to refresh my memory
>> of how this it all works... this seems wrong.
>>
>> Comparing this to the x86_64 version of sigfe_maybe, shouldn't
>> the branch here be to forward to label 1? Otherwise this
>> function effectively does nothing.

You are right, thank you for catching it.  The patch adds a
.L_sigfe_entry label inside the AArch64 _sigfe at the position
matching x86_64's label 1:, and branches to it with b.eq.

Fixing that turned up two more defects in the same stub.

The bypass path could not work either.  On x86_64 the generated
wrapper pushes the address of the real function, so 'ret' pops it and
jumps there.  AArch64 'ret' branches through x30, and the generated
wrapper is

  sub sp, sp, 16
  str x9, [sp, 0]     // x9 = &real_function
  br  x9              // br, not bl: x30 still holds the original
                      //   caller's return address

so 'ret' returned straight to that caller without calling the real
function, leaving sp 16 bytes low on every call.  It is replaced with
'ldr x9, [sp], #16' and 'br x9', which is what _sigfe itself does on
exit.

The magic value was also built with its halves reversed: movz #0xc763
/ movk #0x173f assembles 0x173fc763, not CYGTLS_INITIALIZED
(0xc763173f).  The first defect masked this, since the comparison
result was discarded.

>> (Of course, this isn't very important since sigfe_maybe is only
>> used by cygwin_detach_dll())

Agreed, and that is why none of it was noticed.

Validation: the generated sigfe.s and the linked aarch64 cygwin1.dll
were both checked to confirm the branch and the constant came out as
intended.  At runtime on a Windows ARM64 host, a test program
dlopen()/dlclose()d a Cygwin DLL 10000 times against the patched
cygwin1.dll; all cycles completed without fault, and a marker byte
written by the test DLL's destructor was counted 10000 times,
confirming each dlclose really reached the detach path.

Two limitations: the loop was not run against a pre-fix build, so it
has not been shown to fail on the original code; and it cannot tell
which of the two _sigfe_maybe paths is taken, since a thread that
wrongly took the bypass would still call cygwin_detach_dll and still
return 0 from dlclose, losing only the signal-frame bookkeeping.

Thanks and Regards,
Aswin Kalies
Inline patch

---
 winsup/cygwin/scripts/gendef | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/winsup/cygwin/scripts/gendef b/winsup/cygwin/scripts/gendef
index 5f25c543e..ddbe50d58 100755
--- a/winsup/cygwin/scripts/gendef
+++ b/winsup/cygwin/scripts/gendef
@@ -368,25 +368,32 @@ EOF
      .seh_proc _sigfe_maybe
 _sigfe_maybe:                            # stack is aligned on entry!
      .seh_endprologue
-     ldr     x10, [x18, #0x8]            // Load TEB pointer in x10
-     ldr     x11, =_cygtls.initialized   // Load relative offset of _cygtls.initialized
-     add     x11, x10, x11                     // compute absolute address and store in x11
-     cmp     sp, x11                     // Compare current stack pointer with TLS location
-     b.hs    0f                                // if sp >= tls, skip TLS logic
-     ldr     w12, [x11]                        // Load the value at _cygtls.initialized (32-bit)
-     movz    w13, #0xc763                // Prepare magic value(0xc763173f) lower 16 bits
-     movk    w13, #0x173f, lsl #16       // Add upper 16 bits, full value now in w13
-     cmp     w12, w13              // Compare loaded value with magic
-     b.ne    0f                              // If not equal, not initialized, skip TLS logic
-     ret
+     // x18 is the Windows TEB pointer.  Its field at offset 8 is the base
+     // used to resolve offsets within Cygwin's _cygtls block.
+     ldr     x10, [x18, #0x8]            // Load the Cygwin TLS base into x10
+     ldr     x11, =_cygtls.initialized   // Load the _cygtls.initialized offset
+     add     x11, x10, x11                     // Compute &_cygtls.initialized
+     cmp     sp, x11                     // Compare SP with the TLS location
+     b.hs    0f                                // No usable Cygwin TLS if SP >= it
+     ldr     w12, [x11]                        // Load the initialization marker
+     movz    w13, #0x173f                // Prepare magic value 0xc763173f, low half
+     movk    w13, #0xc763, lsl #16       // Add high half: 0xc763173f
+     cmp     w12, w13              // Compare the marker with the expected magic value
+     b.eq    .L_sigfe_entry              // Valid TLS: use the normal signal front end
 0:
-     ret
+     // The wrapper reserved 16 bytes and stored the real function at [sp].
+     // AArch64 ret uses x30 and does not pop that wrapper slot.
+     ldr     x9, [sp], #16               // Remove the wrapper slot and load the target
+     br      x9                          // Enter the target while preserving x30
      .seh_endproc

     .seh_proc _sigfe
 _sigfe:
     .seh_endprologue
     ldr     x10, [x18, #0x8]       // Load TLS base into x10
+.L_sigfe_entry:
+     // _sigfe_maybe reaches this point after loading x10; direct _sigfe calls
+     // reach it through the TLS-base load above.
     mov     w9, #1                 // constant value for lock acquisition
 0:  ldr     x11, =_cygtls.stacklock      // Load offset of stacklock
     add     x12, x10, x11          // Compute final address of stacklock
--
2.49.0.windows.1


-------------- next part --------------
A non-text attachment was scrubbed...
Name: Cygwin-gendef-fix-AArch64-_sigfe_maybe-control-flow.patch
Type: application/octet-stream
Size: 3813 bytes
Desc: Cygwin-gendef-fix-AArch64-_sigfe_maybe-control-flow.patch
URL: <https://cygwin.com/pipermail/cygwin-patches/attachments/20260921/80f18b8f/attachment.obj>