Another take at eliminating UB in patchedcode.S with Apple linkers

Author: filipnavara

src/coreclr/pal/inc/unixasmmacrosamd64.inc

@@ -52,25 +52,6 @@ C_FUNC(\Name):
         .cfi_endproc
 .endm
 
-// Special version of the LEAF_ENTRY macro that should be used inside
-// patchable code region to prevent the linker from relocating the code.
-// On most Unix platforms this is identical to LEAF_ENTRY. However, on
-// Apple platforms it produces the function entry points at .alt_entry
-// symbols that create external labels but keep the whole patched region
-// as single atom for the linker.
-.macro LEAF_ENTRY_PATCHABLE Name, Section
-#if defined(__APPLE__)
-        .text
-        .p2align 4
-        .alt_entry C_FUNC(\Name)
-        .private_extern C_FUNC(\Name)
-C_FUNC(\Name):
-        .cfi_startproc
-#else
-        LEAF_ENTRY \Name, \Section
-#endif
-.endm
-
 .macro LEAF_END_MARKED Name, Section
 #if defined(__APPLE__)
         .alt_entry C_FUNC(\Name\()_End)

src/coreclr/pal/inc/unixasmmacrosarm64.inc

@@ -63,25 +63,6 @@ C_FUNC(\Name):
         .cfi_endproc
 .endm
 
-// Special version of the LEAF_ENTRY macro that should be used inside
-// patchable code region to prevent the linker from relocating the code.
-// On most Unix platforms this is identical to LEAF_ENTRY. However, on
-// Apple platforms it produces the function entry points at .alt_entry
-// symbols that create external labels but keep the whole patched region
-// as single atom for the linker.
-.macro LEAF_ENTRY_PATCHABLE Name, Section
-#if defined(__APPLE__)
-        .text
-        .p2align        2
-        .alt_entry C_FUNC(\Name)
-        .private_extern C_FUNC(\Name)
-C_FUNC(\Name):
-        .cfi_startproc
-#else
-        LEAF_ENTRY \Name, \Section
-#endif
-.endm
-
 .macro LEAF_END_MARKED Name, Section
 #if defined(__APPLE__)
         .alt_entry C_FUNC(\Name\()_End)

src/coreclr/vm/amd64/patchedcode.S

@@ -5,10 +5,58 @@
 #include "unixasmmacros.inc"
 #include "asmconstants.h"
 
+// On Apple platforms we emit the whole patched region as single function
+// with .alt_entry labels for individual write barrier helpers. This ensures
+// the linker doesn't relocate or split the code and treats it as single
+// atom. We also need to be careful to produce the correct unwinding
+// information.
+
+//-----------------------------------------------------------------------------
+// The following Macros help in WRITE_BARRIER Implementations
+// WRITE_BARRIER_ENTRY
+//
+// Declare the start of a write barrier function. Use similarly to NESTED_ENTRY. This is the only legal way
+// to declare a write barrier function.
+//
+.macro WRITE_BARRIER_ENTRY name
+#if defined(__APPLE__)
+    .text
+    .p2align        4
+    .alt_entry C_FUNC(\name)
+    .private_extern C_FUNC(\name)
+C_FUNC(\name):
+#else
+    LEAF_ENTRY \name, _TEXT
+#endif
+.endm
+
+// WRITE_BARRIER_END
+//
+// The partner to WRITE_BARRIER_ENTRY, used like NESTED_END.
+//
+.macro WRITE_BARRIER_END name
+#if defined(__APPLE__)
+    .alt_entry C_FUNC(\name\()_End)
+    .private_extern C_FUNC(\name\()_End)
+C_FUNC(\name\()_End):
+    // make sure this symbol gets its own address
+    nop
+#else
+    LEAF_END_MARKED \name, _TEXT
+#endif
+.endm
+
 // Mark start of the code region that we patch at runtime
+#if defined(__APPLE__)
+        .global C_FUNC(JIT_PatchedCodeStart)
+C_FUNC(JIT_PatchedCodeStart):
+        .cfi_startproc
+        ret
+#else
 LEAF_ENTRY JIT_PatchedCodeStart, _TEXT
         ret
 LEAF_END JIT_PatchedCodeStart, _TEXT
+#endif
 
 
 // There is an even more optimized version of these helpers possible which takes
@@ -24,7 +72,7 @@ LEAF_END JIT_PatchedCodeStart, _TEXT
 // it needs to have it's card updated
 //
 // void JIT_CheckedWriteBarrier(Object** dst, Object* src)
-LEAF_ENTRY_PATCHABLE JIT_CheckedWriteBarrier, _TEXT
+WRITE_BARRIER_ENTRY JIT_CheckedWriteBarrier
 
         // When WRITE_BARRIER_CHECK is defined _NotInHeap will write the reference
         // but if it isn't then it will just return.
@@ -45,7 +93,7 @@ LEAF_ENTRY_PATCHABLE JIT_CheckedWriteBarrier, _TEXT
         // See comment above about possible AV
         mov     [rdi], rsi
         ret
-LEAF_END_MARKED JIT_CheckedWriteBarrier, _TEXT
+WRITE_BARRIER_END JIT_CheckedWriteBarrier
 
 
 // This is used by the mechanism to hold either the JIT_WriteBarrier_PreGrow
@@ -54,7 +102,7 @@ LEAF_END_MARKED JIT_CheckedWriteBarrier, _TEXT
 // larger of the two functions (JIT_WriteBarrier_PostGrow) to ensure we have created
 // enough space to copy that code in.
 .balign 16
-LEAF_ENTRY_PATCHABLE JIT_WriteBarrier, _TEXT
+WRITE_BARRIER_ENTRY JIT_WriteBarrier
 #ifdef _DEBUG
         // In debug builds, this just contains jump to the debug version of the write barrier by default
         jmp C_FUNC(JIT_WriteBarrier_Debug)
@@ -237,9 +285,17 @@ LEAF_ENTRY_PATCHABLE JIT_WriteBarrier, _TEXT
     // make sure this is bigger than any of the others
     .balign 16
         nop
-LEAF_END_MARKED JIT_WriteBarrier, _TEXT
+WRITE_BARRIER_END JIT_WriteBarrier
 
 // Mark start of the code region that we patch at runtime
-LEAF_ENTRY_PATCHABLE JIT_PatchedCodeLast, _TEXT
+#if defined(__APPLE__)
+        .private_extern C_FUNC(JIT_PatchedCodeLast)
+        .alt_entry C_FUNC(JIT_PatchedCodeLast)
+C_FUNC(JIT_PatchedCodeLast):
+        .cfi_endproc
+        ret
+#else
+LEAF_ENTRY JIT_PatchedCodeLast, _TEXT
         ret
 LEAF_END JIT_PatchedCodeLast, _TEXT
+#endif

src/coreclr/vm/arm64/patchedcode.S

@@ -5,6 +5,12 @@
 #include "unixasmmacros.inc"
 #include "patchedcodeconstants.h"
 
+// On Apple platforms we emit the whole patched region as single function
+// with .alt_entry labels for individual write barrier helpers. This ensures
+// the linker doesn't relocate or split the code and treats it as single
+// atom. We also need to be careful to produce the correct unwinding
+// information.
+
 //-----------------------------------------------------------------------------
 // The following Macros help in WRITE_BARRIER Implementations
 // WRITE_BARRIER_ENTRY
@@ -13,23 +19,46 @@
 // to declare a write barrier function.
 //
 .macro WRITE_BARRIER_ENTRY name
-    LEAF_ENTRY_PATCHABLE \name, _TEXT
+#if defined(__APPLE__)
+    .text
+    .p2align        2
+    .alt_entry C_FUNC(\name)
+    .private_extern C_FUNC(\name)
+C_FUNC(\name):
+#else
+    LEAF_ENTRY \name, _TEXT
+#endif
 .endm
 
 // WRITE_BARRIER_END
 //
 // The partner to WRITE_BARRIER_ENTRY, used like NESTED_END.
 //
 .macro WRITE_BARRIER_END name
+#if defined(__APPLE__)
+    .alt_entry C_FUNC(\name\()_End)
+    .private_extern C_FUNC(\name\()_End)
+C_FUNC(\name\()_End):
+    // make sure this symbol gets its own address
+    nop
+#else
     LEAF_END_MARKED \name, _TEXT
+#endif
 .endm
 
 .balign 64  // Align to power of two at least as big as patchable literal pool so that it fits optimally in cache line
 //------------------------------------------
 // Start of the writeable code region
+#if defined(__APPLE__)
+    .global C_FUNC(JIT_PatchedCodeStart)
+C_FUNC(JIT_PatchedCodeStart):
+    .cfi_startproc
+    ret  lr
+#else
 LEAF_ENTRY JIT_PatchedCodeStart, _TEXT
     ret  lr
 LEAF_END JIT_PatchedCodeStart, _TEXT
+#endif
 
 //-----------------------------------------------------------------------------
 // void JIT_ByRefWriteBarrier
@@ -145,10 +174,17 @@ WRITE_BARRIER_END JIT_WriteBarrier_Table
 
 // ------------------------------------------------------------------
 // End of the writeable code region
-LEAF_ENTRY_PATCHABLE JIT_PatchedCodeLast, _TEXT
+#if defined(__APPLE__)
+    .private_extern C_FUNC(JIT_PatchedCodeLast)
+    .alt_entry C_FUNC(JIT_PatchedCodeLast)
+C_FUNC(JIT_PatchedCodeLast):
+    .cfi_endproc
+    ret  lr
+#else
+LEAF_ENTRY JIT_PatchedCodeLast, _TEXT
     ret  lr
 LEAF_END JIT_PatchedCodeLast, _TEXT
-
+#endif
 
 
 //-----------------------------------------------------------------------------