diff --git a/src/coreclr/nativeaot/Runtime/CMakeLists.txt b/src/coreclr/nativeaot/Runtime/CMakeLists.txt index cb9a63c197377..2d163ea27d78b 100644 --- a/src/coreclr/nativeaot/Runtime/CMakeLists.txt +++ b/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} diff --git a/src/coreclr/nativeaot/Runtime/EHHelpers.cpp b/src/coreclr/nativeaot/Runtime/EHHelpers.cpp index 52357a187ddd4..325128c4e01fc 100644 --- a/src/coreclr/nativeaot/Runtime/EHHelpers.cpp +++ b/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 }; diff --git a/src/coreclr/nativeaot/Runtime/MiscHelpers.cpp b/src/coreclr/nativeaot/Runtime/MiscHelpers.cpp index 6189b60688010..d6c734bc0118a 100644 --- a/src/coreclr/nativeaot/Runtime/MiscHelpers.cpp +++ b/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 diff --git a/src/coreclr/nativeaot/Runtime/amd64/WriteBarriers.S b/src/coreclr/nativeaot/Runtime/amd64/WriteBarriers.S index 8078be2a9a80d..e55e682653b51 100644 --- a/src/coreclr/nativeaot/Runtime/amd64/WriteBarriers.S +++ b/src/coreclr/nativeaot/Runtime/amd64/WriteBarriers.S @@ -221,12 +221,8 @@ 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) @@ -234,15 +230,11 @@ ALTERNATE_ENTRY RhpCheckedLockCmpXchgAVLocation 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 diff --git a/src/coreclr/nativeaot/Runtime/amd64/WriteBarriers.asm b/src/coreclr/nativeaot/Runtime/amd64/WriteBarriers.asm index 7fdf76f0fd832..302b9e0a8b1fe 100644 --- a/src/coreclr/nativeaot/Runtime/amd64/WriteBarriers.asm +++ b/src/coreclr/nativeaot/Runtime/amd64/WriteBarriers.asm @@ -237,12 +237,8 @@ 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 @@ -250,15 +246,11 @@ ALTERNATE_ENTRY RhpCheckedLockCmpXchgAVLocation 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 diff --git a/src/coreclr/nativeaot/Runtime/arm/Interlocked.S b/src/coreclr/nativeaot/Runtime/arm/Interlocked.S deleted file mode 100644 index 47b8bbeff00e9..0000000000000 --- a/src/coreclr/nativeaot/Runtime/arm/Interlocked.S +++ /dev/null @@ -1,57 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -.syntax unified -.thumb - -#include // generated by the build from AsmOffsets.cpp -#include - -// WARNING: Code in EHHelpers.cpp makes assumptions about this helper, in particular: -// - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at RhpLockCmpXchg32AVLocation -// - Function "UnwindSimpleHelperToCaller" assumes no registers were pushed and LR contains the return address -// r0 = destination address -// r1 = value -// r2 = comparand -LEAF_ENTRY RhpLockCmpXchg32, _TEXT - dmb -GLOBAL_LABEL RhpLockCmpXchg32AVLocation -LOCAL_LABEL(CmpXchg32Retry): - ldrex r3, [r0] - cmp r2, r3 - bne LOCAL_LABEL(CmpXchg32Exit) - strex r12, r1, [r0] - cmp r12, #0 - bne LOCAL_LABEL(CmpXchg32Retry) -LOCAL_LABEL(CmpXchg32Exit): - mov r0, r3 - dmb - bx lr -LEAF_END RhpLockCmpXchg32, _TEXT - -// WARNING: Code in EHHelpers.cpp makes assumptions about this helper, in particular: -// - Function "InWriteBarrierHelper" assumes an AV due to passed in null pointer will happen at RhpLockCmpXchg64AVLocation -// - Function "UnwindSimpleHelperToCaller" assumes no registers were pushed and LR contains the return address -// r0 = destination address -// {r2,r3} = value -// sp[0+8] = comparand -LEAF_ENTRY RhpLockCmpXchg64, _TEXT -GLOBAL_LABEL RhpLockCmpXchg64AVLocation - ldr r12, [r0] // dummy read for null check - PROLOG_PUSH "{r4-r6,lr}" - dmb - ldrd r4, r5, [sp,#0x10] -LOCAL_LABEL(CmpXchg64Retry): - ldrexd r6, r1, [r0] - cmp r6, r4 - bne LOCAL_LABEL(CmpXchg64Exit) - cmp r1, r5 - bne LOCAL_LABEL(CmpXchg64Exit) - strexd r12, r2, r3, [r0] - cmp r12, #0 - bne LOCAL_LABEL(CmpXchg64Retry) -LOCAL_LABEL(CmpXchg64Exit): - mov r0, r6 - dmb - EPILOG_POP "{r4-r6,pc}" -LEAF_END RhpLockCmpXchg64, _TEXT diff --git a/src/coreclr/nativeaot/Runtime/arm/WriteBarriers.S b/src/coreclr/nativeaot/Runtime/arm/WriteBarriers.S index 3f7a10a859255..3bb862231a347 100644 --- a/src/coreclr/nativeaot/Runtime/arm/WriteBarriers.S +++ b/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] diff --git a/src/coreclr/nativeaot/Runtime/arm64/WriteBarriers.S b/src/coreclr/nativeaot/Runtime/arm64/WriteBarriers.S index 275ae9401dca7..474509ea587f6 100644 --- a/src/coreclr/nativeaot/Runtime/arm64/WriteBarriers.S +++ b/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. diff --git a/src/coreclr/nativeaot/Runtime/arm64/WriteBarriers.asm b/src/coreclr/nativeaot/Runtime/arm64/WriteBarriers.asm index 1389921eff544..5ccccd2a301ec 100644 --- a/src/coreclr/nativeaot/Runtime/arm64/WriteBarriers.asm +++ b/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. diff --git a/src/coreclr/nativeaot/Runtime/i386/Interlocked.S b/src/coreclr/nativeaot/Runtime/i386/Interlocked.S deleted file mode 100644 index 876f2dfbcb80d..0000000000000 --- a/src/coreclr/nativeaot/Runtime/i386/Interlocked.S +++ /dev/null @@ -1,4 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -// TODO: Implement diff --git a/src/coreclr/nativeaot/Runtime/i386/Interlocked.asm b/src/coreclr/nativeaot/Runtime/i386/Interlocked.asm deleted file mode 100644 index d862d4ddc0d88..0000000000000 --- a/src/coreclr/nativeaot/Runtime/i386/Interlocked.asm +++ /dev/null @@ -1,35 +0,0 @@ -;; Licensed to the .NET Foundation under one or more agreements. -;; The .NET Foundation licenses this file to you under the MIT license. - - .586 - .xmm - .model flat - option casemap:none - .code - -include AsmMacros.inc - -FASTCALL_FUNC RhpLockCmpXchg64, 20 - -_value$ = 16 -_comparand$ = 8 - -ALTERNATE_ENTRY _RhpLockCmpXchg64AVLocation - ;; Null check - cmp dword ptr [ecx], ecx - - mov eax, DWORD PTR _comparand$[esp-4] - mov edx, DWORD PTR _comparand$[esp] - push ebx - mov ebx, DWORD PTR _value$[esp] - push esi - mov esi, ecx - mov ecx, DWORD PTR _value$[esp+8] - lock cmpxchg8b QWORD PTR [esi] - pop esi - pop ebx - ret 16 - -FASTCALL_ENDFUNC - -end diff --git a/src/coreclr/nativeaot/Runtime/i386/WriteBarriers.asm b/src/coreclr/nativeaot/Runtime/i386/WriteBarriers.asm index 953ea11b74de6..133081bee8319 100644 --- a/src/coreclr/nativeaot/Runtime/i386/WriteBarriers.asm +++ b/src/coreclr/nativeaot/Runtime/i386/WriteBarriers.asm @@ -237,12 +237,8 @@ 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 @@ -250,15 +246,11 @@ ALTERNATE_ENTRY _RhpCheckedLockCmpXchgAVLocation 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 diff --git a/src/coreclr/nativeaot/Runtime/portable.cpp b/src/coreclr/nativeaot/Runtime/portable.cpp index 11922ac274e64..bcd99d051198d 100644 --- a/src/coreclr/nativeaot/Runtime/portable.cpp +++ b/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; diff --git a/src/coreclr/nativeaot/Runtime/unix/PalRedhawkInline.h b/src/coreclr/nativeaot/Runtime/unix/PalRedhawkInline.h index 51c3a85727183..983f17a36aba0 100644 --- a/src/coreclr/nativeaot/Runtime/unix/PalRedhawkInline.h +++ b/src/coreclr/nativeaot/Runtime/unix/PalRedhawkInline.h @@ -5,52 +5,86 @@ #include +FORCEINLINE void PalInterlockedOperationBarrier() +{ +#if (defined(HOST_ARM64) && !defined(LSE_INSTRUCTIONS_ENABLED_BY_DEFAULT) && !defined(__clang__)) || defined(HOST_LOONGARCH64) || defined(HOST_RISCV64) + // On arm64, most of the __sync* functions generate a code sequence like: + // loop: + // ldaxr (load acquire exclusive) + // ... + // stlxr (store release exclusive) + // cbnz loop + // + // It is possible for a load following the code sequence above to be reordered to occur prior to the store above due to the + // release barrier, this is substantiated by https://github.com/dotnet/coreclr/pull/17508. Interlocked operations in the PAL + // require the load to occur after the store. This memory barrier should be used following a call to a __sync* function to + // prevent that reordering. Code generated for arm32 includes a 'dmb' after 'cbnz', so no issue there at the moment. + __sync_synchronize(); +#endif +} + FORCEINLINE int32_t PalInterlockedIncrement(_Inout_ int32_t volatile *pDst) { - return __sync_add_and_fetch(pDst, 1); + int32_t result = __sync_add_and_fetch(pDst, 1); + PalInterlockedOperationBarrier(); + return result; } FORCEINLINE int32_t PalInterlockedDecrement(_Inout_ int32_t volatile *pDst) { - return __sync_sub_and_fetch(pDst, 1); + int32_t result = __sync_sub_and_fetch(pDst, 1); + PalInterlockedOperationBarrier(); + return result; } FORCEINLINE uint32_t PalInterlockedOr(_Inout_ uint32_t volatile *pDst, uint32_t iValue) { - return __sync_or_and_fetch(pDst, iValue); + int32_t result = __sync_or_and_fetch(pDst, iValue); + PalInterlockedOperationBarrier(); + return result; } FORCEINLINE uint32_t PalInterlockedAnd(_Inout_ uint32_t volatile *pDst, uint32_t iValue) { - return __sync_and_and_fetch(pDst, iValue); + int32_t result = __sync_and_and_fetch(pDst, iValue); + PalInterlockedOperationBarrier(); + return result; } FORCEINLINE int32_t PalInterlockedExchange(_Inout_ int32_t volatile *pDst, int32_t iValue) { #ifdef __clang__ - return __sync_swap(pDst, iValue); + int32_t result =__sync_swap(pDst, iValue); #else - return __atomic_exchange_n(pDst, iValue, __ATOMIC_ACQ_REL); + int32_t result =__atomic_exchange_n(pDst, iValue, __ATOMIC_ACQ_REL); #endif + PalInterlockedOperationBarrier(); + return result; } FORCEINLINE int64_t PalInterlockedExchange64(_Inout_ int64_t volatile *pDst, int64_t iValue) { #ifdef __clang__ - return __sync_swap(pDst, iValue); + int32_t result =__sync_swap(pDst, iValue); #else - return __atomic_exchange_n(pDst, iValue, __ATOMIC_ACQ_REL); + int32_t result =__atomic_exchange_n(pDst, iValue, __ATOMIC_ACQ_REL); #endif + PalInterlockedOperationBarrier(); + return result; } FORCEINLINE int32_t PalInterlockedCompareExchange(_Inout_ int32_t volatile *pDst, int32_t iValue, int32_t iComparand) { - return __sync_val_compare_and_swap(pDst, iComparand, iValue); + int32_t result = __sync_val_compare_and_swap(pDst, iComparand, iValue); + PalInterlockedOperationBarrier(); + return result; } FORCEINLINE int64_t PalInterlockedCompareExchange64(_Inout_ int64_t volatile *pDst, int64_t iValue, int64_t iComparand) { - return __sync_val_compare_and_swap(pDst, iComparand, iValue); + int64_t result = __sync_val_compare_and_swap(pDst, iComparand, iValue); + PalInterlockedOperationBarrier(); + return result; } #if defined(HOST_AMD64) || defined(HOST_ARM64) @@ -58,6 +92,7 @@ FORCEINLINE uint8_t PalInterlockedCompareExchange128(_Inout_ int64_t volatile *p { __int128_t iComparand = ((__int128_t)pComparandAndResult[1] << 64) + (uint64_t)pComparandAndResult[0]; __int128_t iResult = __sync_val_compare_and_swap((__int128_t volatile*)pDst, iComparand, ((__int128_t)iValueHigh << 64) + (uint64_t)iValueLow); + PalInterlockedOperationBarrier(); pComparandAndResult[0] = (int64_t)iResult; pComparandAndResult[1] = (int64_t)(iResult >> 64); return iComparand == iResult; } diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Interlocked.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Interlocked.cs index 6b351011a00b2..09b3609b45699 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Interlocked.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Interlocked.cs @@ -15,18 +15,23 @@ public static partial class Interlocked public static int CompareExchange(ref int location1, int value, int comparand) { #if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 || TARGET_RISCV64 - return CompareExchange(ref location1, value, comparand); + return CompareExchange(ref location1, value, comparand); // Must expand intrinsic #else + if (Unsafe.IsNullRef(ref location1)) + ThrowHelper.ThrowNullReferenceException(); return RuntimeImports.InterlockedCompareExchange(ref location1, value, comparand); #endif } [Intrinsic] + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static long CompareExchange(ref long location1, long value, long comparand) { #if TARGET_AMD64 || TARGET_ARM64 || TARGET_RISCV64 - return CompareExchange(ref location1, value, comparand); + return CompareExchange(ref location1, value, comparand); // Must expand intrinsic #else + if (Unsafe.IsNullRef(ref location1)) + ThrowHelper.ThrowNullReferenceException(); return RuntimeImports.InterlockedCompareExchange(ref location1, value, comparand); #endif } @@ -36,13 +41,16 @@ public static long CompareExchange(ref long location1, long value, long comparan [MethodImpl(MethodImplOptions.AggressiveInlining)] public static T CompareExchange(ref T location1, T value, T comparand) where T : class? { - return Unsafe.As(RuntimeImports.InterlockedCompareExchange(ref Unsafe.As(ref location1), value, comparand)); + return Unsafe.As(CompareExchange(ref Unsafe.As(ref location1), value, comparand)); } [Intrinsic] + [MethodImpl(MethodImplOptions.AggressiveInlining)] [return: NotNullIfNotNull(nameof(location1))] public static object? CompareExchange(ref object? location1, object? value, object? comparand) { + if (Unsafe.IsNullRef(ref location1)) + ThrowHelper.ThrowNullReferenceException(); return RuntimeImports.InterlockedCompareExchange(ref location1, value, comparand); } @@ -54,7 +62,7 @@ public static T CompareExchange(ref T location1, T value, T comparand) where public static int Exchange(ref int location1, int value) { #if TARGET_X86 || TARGET_AMD64 || TARGET_ARM64 || TARGET_RISCV64 - return Exchange(ref location1, value); + return Exchange(ref location1, value); // Must expand intrinsic #else int oldValue; @@ -71,7 +79,7 @@ public static int Exchange(ref int location1, int value) public static long Exchange(ref long location1, long value) { #if TARGET_AMD64 || TARGET_ARM64 || TARGET_RISCV64 - return Exchange(ref location1, value); + return Exchange(ref location1, value); // Must expand intrinsic #else long oldValue; @@ -85,17 +93,22 @@ public static long Exchange(ref long location1, long value) } [Intrinsic] - [return: NotNullIfNotNull(nameof(location1))] [MethodImpl(MethodImplOptions.AggressiveInlining)] + [return: NotNullIfNotNull(nameof(location1))] public static T Exchange([NotNullIfNotNull(nameof(value))] ref T location1, T value) where T : class? { + if (Unsafe.IsNullRef(ref location1)) + ThrowHelper.ThrowNullReferenceException(); return Unsafe.As(RuntimeImports.InterlockedExchange(ref Unsafe.As(ref location1), value)); } [Intrinsic] + [MethodImpl(MethodImplOptions.AggressiveInlining)] [return: NotNullIfNotNull(nameof(location1))] public static object? Exchange([NotNullIfNotNull(nameof(value))] ref object? location1, object? value) { + if (Unsafe.IsNullRef(ref location1)) + ThrowHelper.ThrowNullReferenceException(); return RuntimeImports.InterlockedExchange(ref location1, value); } diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h index cb60519e727e8..0016424977006 100644 --- a/src/coreclr/pal/inc/pal.h +++ b/src/coreclr/pal/inc/pal.h @@ -3448,7 +3448,7 @@ BitScanReverse64( FORCEINLINE void PAL_InterlockedOperationBarrier() { -#if defined(HOST_ARM64) || defined(HOST_LOONGARCH64) || defined(HOST_RISCV64) +#if (defined(HOST_ARM64) && !defined(LSE_INSTRUCTIONS_ENABLED_BY_DEFAULT) && !defined(__clang__)) || defined(HOST_LOONGARCH64) || defined(HOST_RISCV64) // On arm64, most of the __sync* functions generate a code sequence like: // loop: // ldaxr (load acquire exclusive)