From 3bda6e0013ddb5b48a7b2a89fd84bf4fbbed0e37 Mon Sep 17 00:00:00 2001
From: Vladimir Sadov <vsadov@microsoft.com>
Date: Mon, 21 Aug 2023 20:00:25 -0700
Subject: [PATCH] [NativeAOT] Missing memory fence before bulk move of objects
 (#90890)

* Memory fence before bulk move of objects

* deleted GCMemoryHelpers.h

* Introduced a GCHeapMemoryBarrier helper.
---
 .../nativeaot/Runtime/GCMemoryHelpers.cpp     | 21 +++++++++++++------
 .../nativeaot/Runtime/GCMemoryHelpers.h       |  8 -------
 src/coreclr/nativeaot/Runtime/MiscHelpers.cpp |  1 -
 src/coreclr/nativeaot/Runtime/gcrhenv.cpp     |  1 -
 src/coreclr/nativeaot/Runtime/portable.cpp    |  1 -
 src/coreclr/nativeaot/Runtime/threadstore.cpp |  1 -
 6 files changed, 15 insertions(+), 18 deletions(-)
 delete mode 100644 src/coreclr/nativeaot/Runtime/GCMemoryHelpers.h

diff --git a/src/coreclr/nativeaot/Runtime/GCMemoryHelpers.cpp b/src/coreclr/nativeaot/Runtime/GCMemoryHelpers.cpp
index 27126acbdb839..30f2c5c5fd3e9 100644
--- 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,11 +30,26 @@ 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
@@ -43,8 +57,3 @@ COOP_PINVOKE_HELPER(void, RhBulkMoveWithWriteBarrier, (uint8_t* pDest, uint8_t*
 
     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
deleted file mode 100644
index 127b4d772040a..0000000000000
--- a/src/coreclr/nativeaot/Runtime/GCMemoryHelpers.h
+++ /dev/null
@@ -1,8 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-
-//
-// Unmanaged GC memory helpers
-//
-
-EXTERN_C void REDHAWK_CALLCONV RhpBulkWriteBarrier(void* pMemStart, uint32_t cbMemSize);
diff --git a/src/coreclr/nativeaot/Runtime/MiscHelpers.cpp b/src/coreclr/nativeaot/Runtime/MiscHelpers.cpp
index ec2fabcc540f1..6df37cf23b9d3 100644
--- 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
index 3d0990962b7c9..3ec488605c1b3 100644
--- 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
index d45b3d062d00e..8b425bfe2dff1 100644
--- 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
index 67a6949fd7fb0..2e8369f9175fc 100644
--- 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;