Skip to content

Commit

Permalink
[NativeAOT/ARM] Prevent misoptimization by clang (#97811)
Browse files Browse the repository at this point in the history
* 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*".
  • Loading branch information
filipnavara committed Feb 2, 2024
1 parent e9539aa commit 53d8003
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 37 deletions.
3 changes: 3 additions & 0 deletions src/coreclr/nativeaot/Runtime/CommonMacros.h
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
44 changes: 22 additions & 22 deletions src/coreclr/nativeaot/Runtime/EHHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -336,22 +336,22 @@ 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

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)
{
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/nativeaot/Runtime/StackFrameIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/nativeaot/Runtime/arm/Interlocked.S
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/nativeaot/Runtime/arm/StubDispatch.S
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions src/coreclr/nativeaot/Runtime/arm/WriteBarriers.S
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down Expand Up @@ -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]

Expand All @@ -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
Expand All @@ -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]
Expand Down Expand Up @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down

0 comments on commit 53d8003

Please sign in to comment.