[NativeAOT] Move Interlocked null checks to managed, RhpLockCmpXchg64…

… to C (#100021)

* Move RhpLockCmpXchg64 implementation to C; move null checks from RhpLockCmpXchg64/RhpCheckedLockCmpXchg/RhpCheckedXchg to managed code

* Removed #ifs per feedback

* Update RhpLockCmpXchg[8/16/32] too

* Add PalInterlockedOperationBarrier

* PR feedback

* Move RhpLockCmpXchg32 to C

---------

Co-authored-by: Michał Petryka <35800402+MichalPetryka@users.noreply.github.com>

src/coreclr/nativeaot/Runtime/CMakeLists.txt

@@ -203,12 +203,6 @@ if (CLR_CMAKE_TARGET_ARCH_AMD64 AND CLR_CMAKE_TARGET_WIN32)
   )
 endif (CLR_CMAKE_TARGET_ARCH_AMD64 AND CLR_CMAKE_TARGET_WIN32)
 
-if (NOT (CLR_CMAKE_TARGET_ARCH_AMD64 OR CLR_CMAKE_TARGET_ARCH_ARM64))
-  list(APPEND RUNTIME_SOURCES_ARCH_ASM
-    ${ARCH_SOURCES_DIR}/Interlocked.${ASM_SUFFIX}
-  )
-endif (NOT (CLR_CMAKE_TARGET_ARCH_AMD64 OR CLR_CMAKE_TARGET_ARCH_ARM64))
-
 list(APPEND RUNTIME_SOURCES_ARCH_ASM
   ${ARCH_SOURCES_DIR}/AllocFast.${ASM_SUFFIX}
   ${ARCH_SOURCES_DIR}/ExceptionHandling.${ASM_SUFFIX}

src/coreclr/nativeaot/Runtime/EHHelpers.cpp

@@ -326,12 +326,6 @@ EXTERN_C CODE_LOCATION RhpCheckedAssignRefESIAVLocation;
 EXTERN_C CODE_LOCATION RhpCheckedAssignRefEDIAVLocation;
 EXTERN_C CODE_LOCATION RhpCheckedAssignRefEBPAVLocation;
 #endif
-EXTERN_C CODE_LOCATION RhpCheckedLockCmpXchgAVLocation;
-EXTERN_C CODE_LOCATION RhpCheckedXchgAVLocation;
-#if !defined(HOST_AMD64) && !defined(HOST_ARM64)
-EXTERN_C CODE_LOCATION RhpLockCmpXchg32AVLocation;
-EXTERN_C CODE_LOCATION RhpLockCmpXchg64AVLocation;
-#endif
 EXTERN_C CODE_LOCATION RhpByRefAssignRefAVLocation1;
 
 #if !defined(HOST_ARM64)
@@ -365,22 +359,10 @@ static bool InWriteBarrierHelper(uintptr_t faultingIP)
         (uintptr_t)&RhpCheckedAssignRefESIAVLocation,
         (uintptr_t)&RhpCheckedAssignRefEDIAVLocation,
         (uintptr_t)&RhpCheckedAssignRefEBPAVLocation,
-#endif
-        (uintptr_t)&RhpCheckedLockCmpXchgAVLocation,
-        (uintptr_t)&RhpCheckedXchgAVLocation,
-#if !defined(HOST_AMD64) && !defined(HOST_ARM64)
-#if !defined(HOST_X86)
-        (uintptr_t)&RhpLockCmpXchg32AVLocation,
-#endif
-        (uintptr_t)&RhpLockCmpXchg64AVLocation,
 #endif
         (uintptr_t)&RhpByRefAssignRefAVLocation1,
 #if !defined(HOST_ARM64)
         (uintptr_t)&RhpByRefAssignRefAVLocation2,
-#endif
-#if defined(HOST_ARM64) && !defined(LSE_INSTRUCTIONS_ENABLED_BY_DEFAULT)
-        (uintptr_t)&RhpCheckedLockCmpXchgAVLocation2,
-        (uintptr_t)&RhpCheckedXchgAVLocation2,
 #endif
     };
 

src/coreclr/nativeaot/Runtime/MiscHelpers.cpp

@@ -416,3 +416,15 @@ EXTERN_C void QCALLTYPE RhCpuIdEx(int* cpuInfo, int functionId, int subFunctionI
     __cpuidex(cpuInfo, functionId, subFunctionId);
 }
 #endif
+
+FCIMPL3(int32_t, RhpLockCmpXchg32, int32_t * location, int32_t value, int32_t comparand)
+{
+    return PalInterlockedCompareExchange(location, value, comparand);
+}
+FCIMPLEND
+
+FCIMPL3(int64_t, RhpLockCmpXchg64, int64_t * location, int64_t value, int64_t comparand)
+{
+    return PalInterlockedCompareExchange64(location, value, comparand);
+}
+FCIMPLEND

src/coreclr/nativeaot/Runtime/amd64/WriteBarriers.S

@@ -221,28 +221,20 @@ LEAF_END RhpCheckedAssignRef\EXPORT_REG_NAME, _TEXT
 // just one write barrier that assumes the input register is RSI.
 DEFINE_CHECKED_WRITE_BARRIER RSI, ESI
 
-// WARNING: Code in EHHelpers.cpp makes assumptions about write barrier code, in particular:
-// - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at RhpCheckedLockCmpXchgAVLocation
-// - Function "UnwindSimpleHelperToCaller" assumes the stack contains just the pushed return address
 LEAF_ENTRY RhpCheckedLockCmpXchg, _TEXT
     mov             rax, rdx
-ALTERNATE_ENTRY RhpCheckedLockCmpXchgAVLocation
     lock cmpxchg    [rdi], rsi
     jne             LOCAL_LABEL(RhpCheckedLockCmpXchg_NoBarrierRequired_RSI)
 
     DEFINE_CHECKED_WRITE_BARRIER_CORE RhpCheckedLockCmpXchg, RSI
 
 LEAF_END RhpCheckedLockCmpXchg, _TEXT
 
-// WARNING: Code in EHHelpers.cpp makes assumptions about write barrier code, in particular:
-// - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at RhpCheckedXchgAVLocation
-// - Function "UnwindSimpleHelperToCaller" assumes the stack contains just the pushed return address
 LEAF_ENTRY RhpCheckedXchg, _TEXT
 
     // Setup rax with the new object for the exchange, that way it will automatically hold the correct result
     // afterwards and we can leave rdx unaltered ready for the GC write barrier below.
     mov             rax, rsi
-ALTERNATE_ENTRY RhpCheckedXchgAVLocation
     xchg            [rdi], rax
 
     DEFINE_CHECKED_WRITE_BARRIER_CORE RhpCheckedXchg, RSI

src/coreclr/nativeaot/Runtime/amd64/WriteBarriers.asm

@@ -237,28 +237,20 @@ endm
 ;; just one write barrier that assumes the input register is RDX.
 DEFINE_CHECKED_WRITE_BARRIER RDX, EDX
 
-;; WARNING: Code in EHHelpers.cpp makes assumptions about write barrier code, in particular:
-;; - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at RhpCheckedLockCmpXchgAVLocation
-;; - Function "UnwindSimpleHelperToCaller" assumes the stack contains just the pushed return address
 LEAF_ENTRY RhpCheckedLockCmpXchg, _TEXT
     mov             rax, r8
-ALTERNATE_ENTRY RhpCheckedLockCmpXchgAVLocation
     lock cmpxchg    [rcx], rdx
     jne             RhpCheckedLockCmpXchg_NoBarrierRequired_RDX
 
     DEFINE_CHECKED_WRITE_BARRIER_CORE RhpCheckedLockCmpXchg, RDX
 
 LEAF_END RhpCheckedLockCmpXchg, _TEXT
 
-;; WARNING: Code in EHHelpers.cpp makes assumptions about write barrier code, in particular:
-;; - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at RhpCheckedXchgAVLocation
-;; - Function "UnwindSimpleHelperToCaller" assumes the stack contains just the pushed return address
 LEAF_ENTRY RhpCheckedXchg, _TEXT
 
     ;; Setup rax with the new object for the exchange, that way it will automatically hold the correct result
     ;; afterwards and we can leave rdx unaltered ready for the GC write barrier below.
     mov             rax, rdx
-ALTERNATE_ENTRY RhpCheckedXchgAVLocation
     xchg            [rcx], rax
 
     DEFINE_CHECKED_WRITE_BARRIER_CORE RhpCheckedXchg, RDX

src/coreclr/nativeaot/Runtime/arm/WriteBarriers.S

@@ -250,9 +250,6 @@ LEAF_END RhpCheckedAssignRef\EXPORT_REG_NAME, _TEXT
 // just one write barrier that assumes the input register is RSI.
 DEFINE_CHECKED_WRITE_BARRIER r1, r1
 
-// WARNING: Code in EHHelpers.cpp makes assumptions about write barrier code, in particular:
-// - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at RhpCheckedLockCmpXchgAVLocation
-// - Function "UnwindSimpleHelperToCaller" assumes no registers were pushed and LR contains the return address
 // r0 = destination address
 // r1 = value
 // r2 = comparand
@@ -261,7 +258,6 @@ LEAF_ENTRY RhpCheckedLockCmpXchg, _TEXT
           // barrier must occur before the object reference update, so we have to do it unconditionally even
           // though the update may fail below.
           dmb
-GLOBAL_LABEL RhpCheckedLockCmpXchgAVLocation
 LOCAL_LABEL(RhpCheckedLockCmpXchgRetry):
           ldrex        r3, [r0]
           cmp          r2, r3
@@ -277,16 +273,12 @@ LOCAL_LABEL(RhpCheckedLockCmpXchgRetry):
           bx           lr
 LEAF_END RhpCheckedLockCmpXchg, _TEXT
 
-// WARNING: Code in EHHelpers.cpp makes assumptions about write barrier code, in particular:
-// - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at RhpCheckedXchgAVLocation
-// - Function "UnwindSimpleHelperToCaller" assumes no registers were pushed and LR contains the return address
 // r0 = destination address
 // r1 = value
 LEAF_ENTRY RhpCheckedXchg, _TEXT
           // To implement our chosen memory model for ARM we insert a memory barrier at GC write barriers. This
           // barrier must occur before the object reference update.
           dmb
-GLOBAL_LABEL RhpCheckedXchgAVLocation
 LOCAL_LABEL(RhpCheckedXchgRetry):
           ldrex        r2, [r0]
           strex        r3, r1, [r0]

src/coreclr/nativeaot/Runtime/arm64/WriteBarriers.S

@@ -302,7 +302,6 @@ LEAF_END RhpAssignRef, _TEXT
 #endif
 
         mov    x10, x2
-    ALTERNATE_ENTRY RhpCheckedLockCmpXchgAVLocation
         casal  x10, x1, [x0]                  // exchange
         cmp    x2, x10
         bne    LOCAL_LABEL(CmpXchgNoUpdate)
@@ -311,7 +310,6 @@ LEAF_END RhpAssignRef, _TEXT
         b      LOCAL_LABEL(DoCardsCmpXchg)
 LOCAL_LABEL(CmpXchgRetry):
         // Check location value is what we expect.
-    ALTERNATE_ENTRY  RhpCheckedLockCmpXchgAVLocation2
         ldaxr   x10, [x0]
         cmp     x10, x2
         bne     LOCAL_LABEL(CmpXchgNoUpdate)
@@ -365,14 +363,12 @@ LOCAL_LABEL(NoBarrierCmpXchg):
         tbz    w16, #ARM64_ATOMICS_FEATURE_FLAG_BIT, LOCAL_LABEL(ExchangeRetry)
 #endif
 
-    ALTERNATE_ENTRY  RhpCheckedXchgAVLocation
         swpal  x1, x10, [x0]                   // exchange
 
 #ifndef LSE_INSTRUCTIONS_ENABLED_BY_DEFAULT
         b      LOCAL_LABEL(DoCardsXchg)
 LOCAL_LABEL(ExchangeRetry):
         // Read the existing memory location.
-    ALTERNATE_ENTRY  RhpCheckedXchgAVLocation2
         ldaxr   x10,  [x0]
 
         // Attempt to update with the new value.

src/coreclr/nativeaot/Runtime/arm64/WriteBarriers.asm

@@ -286,10 +286,6 @@ NotInHeap
 ;; Interlocked operation helpers where the location is an objectref, thus requiring a GC write barrier upon
 ;; successful updates.
 
-;; WARNING: Code in EHHelpers.cpp makes assumptions about write barrier code, in particular:
-;; - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at RhpCheckedLockCmpXchgAVLocation
-;; - Function "UnwindSimpleHelperToCaller" assumes no registers were pushed and LR contains the return address
-
 ;; RhpCheckedLockCmpXchg(Object** dest, Object* value, Object* comparand)
 ;;
 ;; Interlocked compare exchange on objectref.
@@ -311,7 +307,6 @@ NotInHeap
 #endif
 
         mov    x10, x2
-    ALTERNATE_ENTRY RhpCheckedLockCmpXchgAVLocation
         casal  x10, x1, [x0]                  ;; exchange
         cmp    x2, x10
         bne    CmpXchgNoUpdate
@@ -320,7 +315,6 @@ NotInHeap
         b      DoCardsCmpXchg
 CmpXchgRetry
         ;; Check location value is what we expect.
-    ALTERNATE_ENTRY  RhpCheckedLockCmpXchgAVLocation2
         ldaxr   x10, [x0]
         cmp     x10, x2
         bne     CmpXchgNoUpdate
@@ -350,10 +344,6 @@ NoBarrierCmpXchg
 
     LEAF_END RhpCheckedLockCmpXchg
 
-;; WARNING: Code in EHHelpers.cpp makes assumptions about write barrier code, in particular:
-;; - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen within at RhpCheckedXchgAVLocation
-;; - Function "UnwindSimpleHelperToCaller" assumes no registers were pushed and LR contains the return address
-
 ;; RhpCheckedXchg(Object** destination, Object* value)
 ;;
 ;; Interlocked exchange on objectref.
@@ -374,14 +364,12 @@ NoBarrierCmpXchg
         tbz    x16, #ARM64_ATOMICS_FEATURE_FLAG_BIT, ExchangeRetry
 #endif
 
-    ALTERNATE_ENTRY  RhpCheckedXchgAVLocation
         swpal  x1, x10, [x0]                   ;; exchange
 
 #ifndef LSE_INSTRUCTIONS_ENABLED_BY_DEFAULT
         b      DoCardsXchg
 ExchangeRetry
         ;; Read the existing memory location.
-    ALTERNATE_ENTRY  RhpCheckedXchgAVLocation2
         ldaxr   x10,  [x0]
 
         ;; Attempt to update with the new value.

src/coreclr/nativeaot/Runtime/i386/WriteBarriers.asm

@@ -237,28 +237,20 @@ DEFINE_CHECKED_WRITE_BARRIER EDX, ESI
 DEFINE_CHECKED_WRITE_BARRIER EDX, EDI
 DEFINE_CHECKED_WRITE_BARRIER EDX, EBP
 
-;; WARNING: Code in EHHelpers.cpp makes assumptions about write barrier code, in particular:
-;; - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at @RhpCheckedLockCmpXchgAVLocation@0
-;; - Function "UnwindSimpleHelperToCaller" assumes the stack contains just the pushed return address
 FASTCALL_FUNC RhpCheckedLockCmpXchg, 12
     mov             eax, [esp+4]
-ALTERNATE_ENTRY _RhpCheckedLockCmpXchgAVLocation
     lock cmpxchg    [ecx], edx
     jne             RhpCheckedLockCmpXchg_NoBarrierRequired_ECX_EDX
 
     DEFINE_CHECKED_WRITE_BARRIER_CORE RhpCheckedLockCmpXchg, ECX, EDX, ret 4
 
 FASTCALL_ENDFUNC
 
-;; WARNING: Code in EHHelpers.cpp makes assumptions about write barrier code, in particular:
-;; - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at @RhpCheckedXchgAVLocation@0
-;; - Function "UnwindSimpleHelperToCaller" assumes the stack contains just the pushed return address
 FASTCALL_FUNC RhpCheckedXchg, 8
 
     ;; Setup eax with the new object for the exchange, that way it will automatically hold the correct result
     ;; afterwards and we can leave edx unaltered ready for the GC write barrier below.
     mov             eax, edx
-ALTERNATE_ENTRY _RhpCheckedXchgAVLocation
     xchg            [ecx], eax
 
     DEFINE_CHECKED_WRITE_BARRIER_CORE RhpCheckedXchg, ECX, EDX, ret

src/coreclr/nativeaot/Runtime/portable.cpp

@@ -374,7 +374,6 @@ FCIMPLEND
 
 FCIMPL3(Object *, RhpCheckedLockCmpXchg, Object ** location, Object * value, Object * comparand)
 {
-    // @TODO: USE_PORTABLE_HELPERS - Null check
     Object * ret = (Object *)PalInterlockedCompareExchangePointer((void * volatile *)location, value, comparand);
     InlineCheckedWriteBarrier(location, value);
     return ret;
@@ -390,20 +389,6 @@ FCIMPL2(Object *, RhpCheckedXchg, Object ** location, Object * value)
 }
 FCIMPLEND
 
-FCIMPL3(int32_t, RhpLockCmpXchg32, int32_t * location, int32_t value, int32_t comparand)
-{
-    // @TODO: USE_PORTABLE_HELPERS - Null check
-    return PalInterlockedCompareExchange(location, value, comparand);
-}
-FCIMPLEND
-
-FCIMPL3(int64_t, RhpLockCmpXchg64, int64_t * location, int64_t value, int64_t comparand)
-{
-    // @TODO: USE_PORTABLE_HELPERS - Null check
-    return PalInterlockedCompareExchange64(location, value, comparand);
-}
-FCIMPLEND
-
 FCIMPL0(void*, RhAllocateThunksMapping)
 {
     return NULL;