Add memory barriers to native AOT asm helpers (#106004)

jkotas · web-flow · commit ec496fed9b5f · 2024-08-09T19:39:09.000-07:00
* Add memory barriers to native AOT asm helpers Fixes #81151 * Fix pre-existing bug
diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/ClassConstructorRunner.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/ClassConstructorRunner.cs
@@ -45,7 +45,8 @@ private static unsafe IntPtr CheckStaticClassConstructionReturnNonGCStaticBase(S
         private static unsafe object CheckStaticClassConstructionReturnThreadStaticBase(TypeManagerSlot* pModuleData, int typeTlsIndex, StaticClassConstructionContext* context)
         {
             object threadStaticBase = ThreadStatics.GetThreadStaticBaseForType(pModuleData, typeTlsIndex);
-            EnsureClassConstructorRun(context);
+            if (context->cctorMethodAddress != 0)
+                EnsureClassConstructorRun(context);
             return threadStaticBase;
         }
 
diff --git a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_ARM/ARMEmitter.cs b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_ARM/ARMEmitter.cs
@@ -257,5 +257,11 @@ public void EmitRETIfNotEqual()
             EmitBEQ(4);
             EmitRET();
         }
+
+        // dmb
+        public void EmitDMB()
+        {
+            Builder.EmitUInt(0x8F5FF3BF);
+        }
     }
 }
diff --git a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_ARM64/ARM64Emitter.cs b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_ARM64/ARM64Emitter.cs
@@ -90,6 +90,15 @@ public void EmitLDR(Register regDst, Register regSrc, int offset)
             }
         }
 
+        // ldar regDst, [regAddr]
+        public void EmitLDAR(Register regDst, Register regAddr)
+        {
+            Debug.Assert((uint)regDst <= 0x1f);
+            Debug.Assert((uint)regAddr <= 0x1f);
+            uint instruction = 0xc8dffc00 | ((uint)regAddr << 5) | (uint)regDst;
+            Builder.EmitUInt(instruction);
+        }
+
         public void EmitCMP(Register reg, sbyte immediate)
         {
             if (immediate >= 0)
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunGenericHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunGenericHelperNode.cs
@@ -77,6 +77,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref ARMEmitter enco
                             // We need to trigger the cctor before returning the base. It is stored at the beginning of the non-GC statics region.
                             int cctorContextSize = NonGCStaticsNode.GetClassConstructorContextSize(factory.Target);
                             encoder.EmitLDR(encoder.TargetRegister.Arg1, encoder.TargetRegister.Arg0, (short)-cctorContextSize);
+                            encoder.EmitDMB();
                             encoder.EmitCMP(encoder.TargetRegister.Arg1, 0);
                             encoder.EmitRETIfEqual();
 
@@ -107,8 +108,9 @@ protected sealed override void EmitCode(NodeFactory factory, ref ARMEmitter enco
                             EmitDictionaryLookup(factory, ref encoder, encoder.TargetRegister.Arg1, encoder.TargetRegister.Arg2, nonGcRegionLookup, relocsOnly);
 
                             int cctorContextSize = NonGCStaticsNode.GetClassConstructorContextSize(factory.Target);
-                            encoder.EmitLDR(encoder.TargetRegister.Arg3, encoder.TargetRegister.Arg2, ((short)(factory.Target.PointerSize - cctorContextSize)));
-                            encoder.EmitCMP(encoder.TargetRegister.Arg3, 1);
+                            encoder.EmitLDR(encoder.TargetRegister.Arg3, encoder.TargetRegister.Arg2, (short)-cctorContextSize);
+                            encoder.EmitDMB();
+                            encoder.EmitCMP(encoder.TargetRegister.Arg3, 0);
                             encoder.EmitRETIfEqual();
 
                             encoder.EmitMOV(encoder.TargetRegister.Arg1, encoder.TargetRegister.Result);
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunHelperNode.cs
@@ -33,6 +33,7 @@ protected override void EmitCode(NodeFactory factory, ref ARMEmitter encoder, bo
                             encoder.EmitMOV(encoder.TargetRegister.Arg2, factory.TypeNonGCStaticsSymbol(target));
                             encoder.EmitLDR(encoder.TargetRegister.Arg3, encoder.TargetRegister.Arg2,
                                             (short)-NonGCStaticsNode.GetClassConstructorContextSize(factory.Target));
+                            encoder.EmitDMB();
                             encoder.EmitCMP(encoder.TargetRegister.Arg3, 0);
                             encoder.EmitRETIfEqual();
 
@@ -86,6 +87,7 @@ protected override void EmitCode(NodeFactory factory, ref ARMEmitter encoder, bo
                             encoder.EmitMOV(encoder.TargetRegister.Arg2, factory.TypeNonGCStaticsSymbol(target));
                             encoder.EmitLDR(encoder.TargetRegister.Arg3, encoder.TargetRegister.Arg2,
                                             (short)-NonGCStaticsNode.GetClassConstructorContextSize(factory.Target));
+                            encoder.EmitDMB();
                             encoder.EmitCMP(encoder.TargetRegister.Arg3, 0);
                             encoder.EmitRETIfEqual();
 
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM64/ARM64ReadyToRunGenericHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM64/ARM64ReadyToRunGenericHelperNode.cs
@@ -75,7 +75,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref ARM64Emitter en
                         {
                             // We need to trigger the cctor before returning the base. It is stored at the beginning of the non-GC statics region.
                             encoder.EmitSUB(encoder.TargetRegister.Arg3, encoder.TargetRegister.Arg0, NonGCStaticsNode.GetClassConstructorContextSize(factory.Target));
-                            encoder.EmitLDR(encoder.TargetRegister.Arg2, encoder.TargetRegister.Arg3);
+                            encoder.EmitLDAR(encoder.TargetRegister.Arg2, encoder.TargetRegister.Arg3);
                             encoder.EmitCMP(encoder.TargetRegister.Arg2, 0);
                             encoder.EmitRETIfEqual();
 
@@ -107,7 +107,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref ARM64Emitter en
                             EmitDictionaryLookup(factory, ref encoder, encoder.TargetRegister.Arg1, encoder.TargetRegister.Arg2, nonGcRegionLookup, relocsOnly);
 
                             encoder.EmitSUB(encoder.TargetRegister.Arg2, NonGCStaticsNode.GetClassConstructorContextSize(factory.Target));
-                            encoder.EmitLDR(encoder.TargetRegister.Arg3, encoder.TargetRegister.Arg2);
+                            encoder.EmitLDAR(encoder.TargetRegister.Arg3, encoder.TargetRegister.Arg2);
                             encoder.EmitCMP(encoder.TargetRegister.Arg3, 0);
                             encoder.EmitRETIfEqual();
 
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM64/ARM64ReadyToRunHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM64/ARM64ReadyToRunHelperNode.cs
@@ -33,7 +33,7 @@ protected override void EmitCode(NodeFactory factory, ref ARM64Emitter encoder,
                         {
                             // We need to trigger the cctor before returning the base. It is stored at the beginning of the non-GC statics region.
                             encoder.EmitSUB(encoder.TargetRegister.Arg3, encoder.TargetRegister.Result, NonGCStaticsNode.GetClassConstructorContextSize(factory.Target));
-                            encoder.EmitLDR(encoder.TargetRegister.Arg2, encoder.TargetRegister.Arg3);
+                            encoder.EmitLDAR(encoder.TargetRegister.Arg2, encoder.TargetRegister.Arg3);
                             encoder.EmitCMP(encoder.TargetRegister.Arg2, 0);
                             encoder.EmitRETIfEqual();
 
@@ -75,7 +75,7 @@ protected override void EmitCode(NodeFactory factory, ref ARM64Emitter encoder,
                                 encoder.EmitMOV(encoder.TargetRegister.Arg2, factory.TypeNonGCStaticsSymbol(target));
                                 encoder.EmitSUB(encoder.TargetRegister.Arg2, NonGCStaticsNode.GetClassConstructorContextSize(factory.Target));
 
-                                encoder.EmitLDR(encoder.TargetRegister.Arg3, encoder.TargetRegister.Arg2);
+                                encoder.EmitLDAR(encoder.TargetRegister.Arg3, encoder.TargetRegister.Arg2);
                                 encoder.EmitCMP(encoder.TargetRegister.Arg3, 0);
                                 encoder.EmitJE(helper);
 
@@ -101,7 +101,7 @@ protected override void EmitCode(NodeFactory factory, ref ARM64Emitter encoder,
                             // We need to trigger the cctor before returning the base. It is stored at the beginning of the non-GC statics region.
                             encoder.EmitMOV(encoder.TargetRegister.Arg2, factory.TypeNonGCStaticsSymbol(target));
                             encoder.EmitSUB(encoder.TargetRegister.Arg2, NonGCStaticsNode.GetClassConstructorContextSize(factory.Target));
-                            encoder.EmitLDR(encoder.TargetRegister.Arg3, encoder.TargetRegister.Arg2);
+                            encoder.EmitLDAR(encoder.TargetRegister.Arg3, encoder.TargetRegister.Arg2);
                             encoder.EmitCMP(encoder.TargetRegister.Arg3, 0);
                             encoder.EmitRETIfEqual();
 

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,8 @@ private static unsafe IntPtr CheckStaticClassConstructionReturnNonGCStaticBase(S`
`45`	`45`	`private static unsafe object CheckStaticClassConstructionReturnThreadStaticBase(TypeManagerSlot* pModuleData, int typeTlsIndex, StaticClassConstructionContext* context)`
`46`	`46`	`{`
`47`	`47`	`object threadStaticBase = ThreadStatics.GetThreadStaticBaseForType(pModuleData, typeTlsIndex);`
`48`		`- EnsureClassConstructorRun(context);`
	`48`	`+ if (context->cctorMethodAddress != 0)`
	`49`	`+ EnsureClassConstructorRun(context);`
`49`	`50`	`return threadStaticBase;`
`50`	`51`	`}`
`51`	`52`
Original file line number	Diff line number	Diff line change
`@@ -257,5 +257,11 @@ public void EmitRETIfNotEqual()`
`257`	`257`	`EmitBEQ(4);`
`258`	`258`	`EmitRET();`
`259`	`259`	`}`
	`260`	`+`
	`261`	`+ // dmb`
	`262`	`+ public void EmitDMB()`
	`263`	`+ {`
	`264`	`+ Builder.EmitUInt(0x8F5FF3BF);`
	`265`	`+ }`
`260`	`266`	`}`
`261`	`267`	`}`
Original file line number	Diff line number	Diff line change
`@@ -90,6 +90,15 @@ public void EmitLDR(Register regDst, Register regSrc, int offset)`
`90`	`90`	`}`
`91`	`91`	`}`
`92`	`92`
	`93`	`+ // ldar regDst, [regAddr]`
	`94`	`+ public void EmitLDAR(Register regDst, Register regAddr)`
	`95`	`+ {`
	`96`	`+ Debug.Assert((uint)regDst <= 0x1f);`
	`97`	`+ Debug.Assert((uint)regAddr <= 0x1f);`
	`98`	`+ uint instruction = 0xc8dffc00 \| ((uint)regAddr << 5) \| (uint)regDst;`
	`99`	`+ Builder.EmitUInt(instruction);`
	`100`	`+ }`
	`101`	`+`
`93`	`102`	`public void EmitCMP(Register reg, sbyte immediate)`
`94`	`103`	`{`
`95`	`104`	`if (immediate >= 0)`