From 53d8003be6e31b060dc84a03068460a6df02e742 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Fri, 2 Feb 2024 05:09:18 +0100 Subject: [PATCH] [NativeAOT/ARM] Prevent misoptimization by clang (#97811) * Fix *AVLocation to be global labels without the Thumb bit to prevent misoptimization * Remove Thumb bit from assembly location constants and its masking * Introduce CODE_LOCATION type and use it instead of "void*". --- src/coreclr/nativeaot/Runtime/CommonMacros.h | 3 ++ src/coreclr/nativeaot/Runtime/EHHelpers.cpp | 44 +++++++++---------- .../nativeaot/Runtime/StackFrameIterator.cpp | 2 +- .../nativeaot/Runtime/arm/Interlocked.S | 8 ++-- .../nativeaot/Runtime/arm/StubDispatch.S | 2 +- .../nativeaot/Runtime/arm/WriteBarriers.S | 16 +++---- .../Runtime/unix/unixasmmacrosarm.inc | 7 ++- 7 files changed, 45 insertions(+), 37 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/CommonMacros.h b/src/coreclr/nativeaot/Runtime/CommonMacros.h index b6d96af686785..9c762216dd7f1 100644 --- a/src/coreclr/nativeaot/Runtime/CommonMacros.h +++ b/src/coreclr/nativeaot/Runtime/CommonMacros.h @@ -170,6 +170,9 @@ inline bool IS_ALIGNED(T* val, uintptr_t alignment); #define THUMB_CODE 1 #endif +// Type for external code location references inside the assembly code. +typedef uint8_t CODE_LOCATION; + // // Define an unmanaged function called from managed code that needs to execute in co-operative GC mode. (There // should be very few of these, most such functions will be simply p/invoked). diff --git a/src/coreclr/nativeaot/Runtime/EHHelpers.cpp b/src/coreclr/nativeaot/Runtime/EHHelpers.cpp index 242acc2f99e65..28cb9e617f09b 100644 --- a/src/coreclr/nativeaot/Runtime/EHHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/EHHelpers.cpp @@ -281,25 +281,25 @@ EXTERN_C void* RhpThrowHwEx2 = NULL; EXTERN_C void* RhpRethrow2 = NULL; #endif -EXTERN_C void * RhpAssignRefAVLocation; -EXTERN_C void * RhpCheckedAssignRefAVLocation; -EXTERN_C void * RhpCheckedLockCmpXchgAVLocation; -EXTERN_C void * RhpCheckedXchgAVLocation; +EXTERN_C CODE_LOCATION RhpAssignRefAVLocation; +EXTERN_C CODE_LOCATION RhpCheckedAssignRefAVLocation; +EXTERN_C CODE_LOCATION RhpCheckedLockCmpXchgAVLocation; +EXTERN_C CODE_LOCATION RhpCheckedXchgAVLocation; #if !defined(HOST_AMD64) && !defined(HOST_ARM64) -EXTERN_C void * RhpLockCmpXchg8AVLocation; -EXTERN_C void * RhpLockCmpXchg16AVLocation; -EXTERN_C void * RhpLockCmpXchg32AVLocation; -EXTERN_C void * RhpLockCmpXchg64AVLocation; +EXTERN_C CODE_LOCATION RhpLockCmpXchg8AVLocation; +EXTERN_C CODE_LOCATION RhpLockCmpXchg16AVLocation; +EXTERN_C CODE_LOCATION RhpLockCmpXchg32AVLocation; +EXTERN_C CODE_LOCATION RhpLockCmpXchg64AVLocation; #endif -EXTERN_C void * RhpByRefAssignRefAVLocation1; +EXTERN_C CODE_LOCATION RhpByRefAssignRefAVLocation1; #if !defined(HOST_ARM64) -EXTERN_C void * RhpByRefAssignRefAVLocation2; +EXTERN_C CODE_LOCATION RhpByRefAssignRefAVLocation2; #endif #if defined(HOST_ARM64) && !defined(LSE_INSTRUCTIONS_ENABLED_BY_DEFAULT) -EXTERN_C void* RhpCheckedLockCmpXchgAVLocation2; -EXTERN_C void* RhpCheckedXchgAVLocation2; +EXTERN_C CODE_LOCATION RhpCheckedLockCmpXchgAVLocation2; +EXTERN_C CODE_LOCATION RhpCheckedXchgAVLocation2; #endif static bool InWriteBarrierHelper(uintptr_t faultingIP) @@ -336,7 +336,7 @@ static bool InWriteBarrierHelper(uintptr_t faultingIP) ASSERT(*(uint8_t*)writeBarrierAVLocations[i] != 0xE9); // jmp XXXXXXXX #endif - if (PCODEToPINSTR(writeBarrierAVLocations[i]) == faultingIP) + if (writeBarrierAVLocations[i] == faultingIP) return true; } #endif // USE_PORTABLE_HELPERS @@ -344,14 +344,14 @@ static bool InWriteBarrierHelper(uintptr_t faultingIP) return false; } -EXTERN_C void* RhpInitialInterfaceDispatch; -EXTERN_C void* RhpInterfaceDispatchAVLocation1; -EXTERN_C void* RhpInterfaceDispatchAVLocation2; -EXTERN_C void* RhpInterfaceDispatchAVLocation4; -EXTERN_C void* RhpInterfaceDispatchAVLocation8; -EXTERN_C void* RhpInterfaceDispatchAVLocation16; -EXTERN_C void* RhpInterfaceDispatchAVLocation32; -EXTERN_C void* RhpInterfaceDispatchAVLocation64; +EXTERN_C CODE_LOCATION RhpInitialInterfaceDispatch; +EXTERN_C CODE_LOCATION RhpInterfaceDispatchAVLocation1; +EXTERN_C CODE_LOCATION RhpInterfaceDispatchAVLocation2; +EXTERN_C CODE_LOCATION RhpInterfaceDispatchAVLocation4; +EXTERN_C CODE_LOCATION RhpInterfaceDispatchAVLocation8; +EXTERN_C CODE_LOCATION RhpInterfaceDispatchAVLocation16; +EXTERN_C CODE_LOCATION RhpInterfaceDispatchAVLocation32; +EXTERN_C CODE_LOCATION RhpInterfaceDispatchAVLocation64; static bool InInterfaceDispatchHelper(uintptr_t faultingIP) { @@ -377,7 +377,7 @@ static bool InInterfaceDispatchHelper(uintptr_t faultingIP) ASSERT(*(uint8_t*)interfaceDispatchAVLocations[i] != 0xE9); // jmp XXXXXXXX #endif - if (PCODEToPINSTR(interfaceDispatchAVLocations[i]) == faultingIP) + if (interfaceDispatchAVLocations[i] == faultingIP) return true; } #endif // USE_PORTABLE_HELPERS diff --git a/src/coreclr/nativeaot/Runtime/StackFrameIterator.cpp b/src/coreclr/nativeaot/Runtime/StackFrameIterator.cpp index 31ed351ff098a..117c31fb99a99 100644 --- a/src/coreclr/nativeaot/Runtime/StackFrameIterator.cpp +++ b/src/coreclr/nativeaot/Runtime/StackFrameIterator.cpp @@ -76,7 +76,7 @@ GVAL_IMPL_INIT(PTR_VOID, g_RhpRethrow2Addr, PointerToRhpRethrow2); #ifdef DACCESS_COMPILE #define EQUALS_RETURN_ADDRESS(x, func_name) ((x) == g_ ## func_name ## Addr) #else -#define EQUALS_RETURN_ADDRESS(x, func_name) (((x)) == (PTR_VOID)PCODEToPINSTR((PCODE)PointerTo ## func_name)) +#define EQUALS_RETURN_ADDRESS(x, func_name) (((x)) == (PointerTo ## func_name)) #endif #ifdef DACCESS_COMPILE diff --git a/src/coreclr/nativeaot/Runtime/arm/Interlocked.S b/src/coreclr/nativeaot/Runtime/arm/Interlocked.S index 67a079863d986..631731c7e3a32 100644 --- a/src/coreclr/nativeaot/Runtime/arm/Interlocked.S +++ b/src/coreclr/nativeaot/Runtime/arm/Interlocked.S @@ -15,7 +15,7 @@ // r2 = comparand LEAF_ENTRY RhpLockCmpXchg8, _TEXT dmb -ALTERNATE_ENTRY RhpLockCmpXchg8AVLocation +GLOBAL_LABEL RhpLockCmpXchg8AVLocation LOCAL_LABEL(CmpXchg8Retry): ldrexb r3, [r0] cmp r2, r3 @@ -38,7 +38,7 @@ LEAF_END RhpLockCmpXchg8, _TEXT LEAF_ENTRY RhpLockCmpXchg16, _TEXT uxth r2, r2 dmb -ALTERNATE_ENTRY RhpLockCmpXchg16AVLocation +GLOBAL_LABEL RhpLockCmpXchg16AVLocation LOCAL_LABEL(CmpXchg16Retry): ldrexh r3, [r0] cmp r2, r3 @@ -60,7 +60,7 @@ LEAF_END RhpLockCmpXchg16, _TEXT // r2 = comparand LEAF_ENTRY RhpLockCmpXchg32, _TEXT dmb -ALTERNATE_ENTRY RhpLockCmpXchg32AVLocation +GLOBAL_LABEL RhpLockCmpXchg32AVLocation LOCAL_LABEL(CmpXchg32Retry): ldrex r3, [r0] cmp r2, r3 @@ -81,7 +81,7 @@ LEAF_END RhpLockCmpXchg32, _TEXT // {r2,r3} = value // sp[0+8] = comparand LEAF_ENTRY RhpLockCmpXchg64, _TEXT -ALTERNATE_ENTRY RhpLockCmpXchg64AVLocation +GLOBAL_LABEL RhpLockCmpXchg64AVLocation ldr r12, [r0] // dummy read for null check PROLOG_PUSH "{r4-r6,lr}" dmb diff --git a/src/coreclr/nativeaot/Runtime/arm/StubDispatch.S b/src/coreclr/nativeaot/Runtime/arm/StubDispatch.S index 05cf4f919817d..9ff29b9989b7d 100644 --- a/src/coreclr/nativeaot/Runtime/arm/StubDispatch.S +++ b/src/coreclr/nativeaot/Runtime/arm/StubDispatch.S @@ -22,7 +22,7 @@ NESTED_ENTRY RhpInterfaceDispatch\entries, _TEXT, NoHandler ldr r2, [r12, #OFFSETOF__InterfaceDispatchCell__m_pCache] // Load the MethodTable from the object instance in r0. - ALTERNATE_ENTRY RhpInterfaceDispatchAVLocation\entries + GLOBAL_LABEL RhpInterfaceDispatchAVLocation\entries ldr r1, [r0] CurrentOffset = OFFSETOF__InterfaceDispatchCache__m_rgEntries diff --git a/src/coreclr/nativeaot/Runtime/arm/WriteBarriers.S b/src/coreclr/nativeaot/Runtime/arm/WriteBarriers.S index 2d22c9bf1f18b..3f7a10a859255 100644 --- a/src/coreclr/nativeaot/Runtime/arm/WriteBarriers.S +++ b/src/coreclr/nativeaot/Runtime/arm/WriteBarriers.S @@ -164,9 +164,9 @@ ALTERNATE_ENTRY RhpAssignRef // Write the reference into the location. Note that we rely on the fact that no GC can occur between here // and the card table update we may perform below. -ALTERNATE_ENTRY "RhpAssignRefAvLocation"\EXPORT_REG_NAME // WriteBarrierFunctionAvLocation +GLOBAL_LABEL "RhpAssignRefAvLocation"\EXPORT_REG_NAME // WriteBarrierFunctionAvLocation .ifc \REFREG, r1 -ALTERNATE_ENTRY RhpAssignRefAVLocation +GLOBAL_LABEL RhpAssignRefAVLocation .endif str \REFREG, [r0] @@ -234,9 +234,9 @@ ALTERNATE_ENTRY RhpCheckedAssignRef dmb // Write the reference into the location. Note that we rely on the fact that no GC can occur between here // and the card table update we may perform below. -ALTERNATE_ENTRY "RhpCheckedAssignRefAvLocation"\EXPORT_REG_NAME // WriteBarrierFunctionAvLocation +GLOBAL_LABEL "RhpCheckedAssignRefAvLocation"\EXPORT_REG_NAME // WriteBarrierFunctionAvLocation .ifc \REFREG, r1 -ALTERNATE_ENTRY RhpCheckedAssignRefAVLocation +GLOBAL_LABEL RhpCheckedAssignRefAVLocation .endif str \REFREG, [r0] @@ -261,7 +261,7 @@ 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 -ALTERNATE_ENTRY RhpCheckedLockCmpXchgAVLocation +GLOBAL_LABEL RhpCheckedLockCmpXchgAVLocation LOCAL_LABEL(RhpCheckedLockCmpXchgRetry): ldrex r3, [r0] cmp r2, r3 @@ -286,7 +286,7 @@ 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 -ALTERNATE_ENTRY RhpCheckedXchgAVLocation +GLOBAL_LABEL RhpCheckedXchgAVLocation LOCAL_LABEL(RhpCheckedXchgRetry): ldrex r2, [r0] strex r3, r1, [r0] @@ -320,9 +320,9 @@ LEAF_ENTRY RhpByRefAssignRef, _TEXT // See comment in RhpAssignRef dmb -ALTERNATE_ENTRY RhpByRefAssignRefAVLocation1 +GLOBAL_LABEL RhpByRefAssignRefAVLocation1 ldr r2, [r1] -ALTERNATE_ENTRY RhpByRefAssignRefAVLocation2 +GLOBAL_LABEL RhpByRefAssignRefAVLocation2 str r2, [r0] // Check whether the writes were even into the heap. If not there's no card update required. diff --git a/src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm.inc b/src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm.inc index eaf96c70609ee..0c5fe62c5b199 100644 --- a/src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm.inc +++ b/src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm.inc @@ -56,6 +56,11 @@ C_FUNC(\Name): C_FUNC(\Name): .endm +.macro GLOBAL_LABEL Name + .global C_FUNC(\Name) +C_FUNC(\Name): +.endm + .macro LEAF_ENTRY Name, Section .thumb_func .global C_FUNC(\Name) @@ -213,7 +218,7 @@ C_FUNC(\Name): .data .align 4 C_FUNC(\Name): - .word 1b + 1 // Add 1 to indicate thumb code + .word 1b .global C_FUNC(\Name) .text