[NativeAOT] Missing memory fence before bulk move of objects (dotnet#…

…90890) * Memory fence before bulk move of objects * deleted GCMemoryHelpers.h * Introduced a GCHeapMemoryBarrier helper.
kotlarmilos · Aug 22, 2023 · 3bda6e0 · 3bda6e0
1 parent 5fea131
commit 3bda6e0
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 18 deletions.
diff --git a/src/coreclr/nativeaot/Runtime/GCMemoryHelpers.cpp b/src/coreclr/nativeaot/Runtime/GCMemoryHelpers.cpp
@@ -10,7 +10,6 @@
 #include "PalRedhawkCommon.h"
 #include "CommonMacros.inl"
 
-#include "GCMemoryHelpers.h"
 #include "GCMemoryHelpers.inl"
 
 // This function clears a piece of memory in a GC safe way.
@@ -31,20 +30,30 @@ COOP_PINVOKE_CDECL_HELPER(void *, RhpGcSafeZeroMemory, (void * mem, size_t size)
     return mem;
 }
 
+#if defined(TARGET_X86) || defined(TARGET_AMD64) 
+    // 
+    // Memory writes are already ordered
+    // 
+    #define GCHeapMemoryBarrier() 
+#else
+    #define GCHeapMemoryBarrier() MemoryBarrier() 
+#endif 
+
 // Move memory, in a way that is compatible with a move onto the heap, but
 // does not require the destination pointer to be on the heap.
 
 COOP_PINVOKE_HELPER(void, RhBulkMoveWithWriteBarrier, (uint8_t* pDest, uint8_t* pSrc, size_t cbDest))
 {
+    // It is possible that the bulk write is publishing object references accessible so far only
+    // by the current thread to shared memory.
+    // The memory model requires that writes performed by current thread are observable no later
+    // than the writes that will actually publish the references.
+    GCHeapMemoryBarrier();
+
     if (pDest <= pSrc || pSrc + cbDest <= pDest)
         InlineForwardGCSafeCopy(pDest, pSrc, cbDest);
     else
         InlineBackwardGCSafeCopy(pDest, pSrc, cbDest);
 
     InlinedBulkWriteBarrier(pDest, cbDest);
 }
-
-void REDHAWK_CALLCONV RhpBulkWriteBarrier(void* pMemStart, uint32_t cbMemSize)
-{
-    InlinedBulkWriteBarrier(pMemStart, cbMemSize);
-}
diff --git a/src/coreclr/nativeaot/Runtime/GCMemoryHelpers.h b/src/coreclr/nativeaot/Runtime/GCMemoryHelpers.h
diff --git a/src/coreclr/nativeaot/Runtime/MiscHelpers.cpp b/src/coreclr/nativeaot/Runtime/MiscHelpers.cpp
@@ -35,7 +35,6 @@
 #include "MethodTable.inl"
 #include "CommonMacros.inl"
 #include "volatile.h"
-#include "GCMemoryHelpers.h"
 #include "GCMemoryHelpers.inl"
 #include "yieldprocessornormalized.h"
 #include "RhConfig.h"

diff --git a/src/coreclr/nativeaot/Runtime/gcrhenv.cpp b/src/coreclr/nativeaot/Runtime/gcrhenv.cpp
@@ -42,7 +42,6 @@
 
 #include "daccess.h"
 
-#include "GCMemoryHelpers.h"
 #include "interoplibinterface.h"
 
 #include "holder.h"

diff --git a/src/coreclr/nativeaot/Runtime/portable.cpp b/src/coreclr/nativeaot/Runtime/portable.cpp
@@ -31,7 +31,6 @@
 #include "MethodTable.inl"
 #include "ObjectLayout.h"
 
-#include "GCMemoryHelpers.h"
 #include "GCMemoryHelpers.inl"
 
 #if defined(USE_PORTABLE_HELPERS)

diff --git a/src/coreclr/nativeaot/Runtime/threadstore.cpp b/src/coreclr/nativeaot/Runtime/threadstore.cpp
@@ -24,7 +24,6 @@
 #include "yieldprocessornormalized.h"
 
 #include "slist.inl"
-#include "GCMemoryHelpers.h"
 
 EXTERN_C volatile uint32_t RhpTrapThreads;
 volatile uint32_t RhpTrapThreads = (uint32_t)TrapThreadsFlags::None;