Allow initonly field during ldflda and stfld import

dotnet · Aug 16, 2021 · e66f149 · e66f149
1 parent 6acb72b
commit e66f149
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 41 deletions.
diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
@@ -711,7 +711,7 @@ private bool TryGetUnmanagedCallingConventionFromModOpt(MethodSignature signatur
             // Default to managed since in the modopt case we need to differentiate explicitly using a calling convention that matches the default
             // and not specifying a calling convention at all and using the implicit default case in P/Invoke stub inlining.
             callConv = CorInfoCallConvExtension.Managed;
-            if (!signature.HasEmbeddedSignatureData || signature.GetEmbeddedSignatureData() == null)
+            if (!signature.HasEmbeddedSignatureData)
                 return false;
 
             bool found = false;

diff --git a/src/coreclr/tools/ILVerification/ILImporter.Verify.cs b/src/coreclr/tools/ILVerification/ILImporter.Verify.cs
@@ -267,9 +267,9 @@ private void FindEnclosingExceptionRegions()
                         }
                     }
                     // Check if offset is within the range [FilterOffset, HandlerOffset[
-                    if (r.FilterOffset != -1 && r.FilterOffset <= offset && offset < r.HandlerOffset )
+                    if (r.FilterOffset != -1 && r.FilterOffset <= offset && offset < r.HandlerOffset)
                     {
-                        if(!basicBlock.FilterIndex.HasValue)
+                        if (!basicBlock.FilterIndex.HasValue)
                         {
                             basicBlock.FilterIndex = j;
                         }
@@ -301,7 +301,7 @@ private void InitialPass()
                 ILOpcode opCode = (ILOpcode)ReadILByte();
 
                 previousWasPrefix = false;
-again:
+            again:
                 switch (opCode)
                 {
                     // Check this pointer modification
@@ -1601,7 +1601,7 @@ void ImportCall(ILOpcode opcode, int token)
                 }
 
                 // To support direct calls on readonly byrefs, just pretend declaredThis is readonly too
-                if(declaredThis.Kind == StackValueKind.ByRef && (actualThis.Kind == StackValueKind.ByRef && actualThis.IsReadOnly))
+                if (declaredThis.Kind == StackValueKind.ByRef && (actualThis.Kind == StackValueKind.ByRef && actualThis.IsReadOnly))
                 {
                     declaredThis.SetIsReadOnly();
                 }
@@ -2107,8 +2107,9 @@ void ImportAddressOfField(int token, bool isStatic)
                 isPermanentHome = actualThis.Kind == StackValueKind.ObjRef || actualThis.IsPermanentHome;
                 instance = actualThis.Type;
 
-                if (field.IsInitOnly)
-                    Check(_method.IsConstructor && field.OwningType == _method.OwningType && actualThis.IsThisPtr, VerifierError.InitOnly);
+                // TODO: verification of readonly references https://github.com/dotnet/runtime/issues/57444
+                // if (field.IsInitOnly)
+                //    Check(_method.IsConstructor && field.OwningType == _method.OwningType && actualThis.IsThisPtr, VerifierError.InitOnly);
             }
 
             Check(_method.OwningType.CanAccess(field, instance), VerifierError.FieldAccess);
@@ -2122,10 +2123,9 @@ void ImportStoreField(int token, bool isStatic)
             ClearPendingPrefix(Prefix.Volatile);
 
             var value = Pop();
-
             var field = ResolveFieldToken(token);
-
             TypeDesc instance;
+
             if (isStatic)
             {
                 Check(field.IsStatic, VerifierError.ExpectedStaticField);
@@ -2154,7 +2154,8 @@ void ImportStoreField(int token, bool isStatic)
                 instance = actualThis.Type;
 
                 if (field.IsInitOnly)
-                    Check(_method.IsConstructor && field.OwningType == _method.OwningType && actualThis.IsThisPtr, VerifierError.InitOnly);
+                    Check(field.OwningType == _method.OwningType && actualThis.IsThisPtr &&
+                        (_method.IsConstructor || HasIsExternalInit(_method.Signature)), VerifierError.InitOnly);
             }
 
             // Check any constraints on the fields' class --- accessing the field might cause a class constructor to run.
@@ -2647,6 +2648,21 @@ void CheckPendingPrefix(Prefix mask)
             Check((mask & Prefix.Constrained) == 0, VerifierError.Constrained);
         }
 
+        static bool HasIsExternalInit(MethodSignature signature)
+        {
+            if (signature.HasEmbeddedSignatureData)
+            {
+                foreach (var data in signature.GetEmbeddedSignatureData())
+                {
+                    if (data.type is MetadataType mdType && mdType.Namespace == "System.Runtime.CompilerServices" && mdType.Name == "IsExternalInit" &&
+                        data.index == MethodSignature.IndexOfCustomModifiersOnReturnType)
+                        return true;
+                }
+            }
+
+            return false;
+        }
+
         bool HasPendingPrefix(Prefix prefix)
         {
             return (_pendingPrefix & prefix) != 0;

diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/SignatureTests.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/SignatureTests.cs
@@ -36,8 +36,8 @@ public SignatureTests(ITestOutputHelper output)
 
         private static string GetModOptMethodSignatureInfo(MethodSignature signature)
         {
-            if (!signature.HasEmbeddedSignatureData || signature.GetEmbeddedSignatureData() == null)
-                return "";
+            if (!signature.HasEmbeddedSignatureData)
+                return string.Empty;
 
             StringBuilder sb = new StringBuilder();
             foreach (EmbeddedSignatureData data in signature.GetEmbeddedSignatureData())

diff --git a/src/tests/ilverify/ILTests/FieldTests.il b/src/tests/ilverify/ILTests/FieldTests.il
@@ -28,6 +28,17 @@
     .field public initonly int32 InstanceInitonlyField
     .field public static initonly int32 StaticInitonlyField
 
+    .field private initonly int32 InstanceExternalInitField
+
+    .method public hidebysig specialname instance void modreq([System.Runtime]System.Runtime.CompilerServices.IsExternalInit) 'special.StoreExternalInitField.set_MyProperty_Valid'(int32 'value') cil managed { ret }
+    .method public hidebysig specialname instance void modreq([System.Runtime]System.Runtime.CompilerServices.IsExternalInit) set_MyProperty(int32 'value') cil managed
+    {
+        ldarg.0
+        ldarg.1
+        stfld int32 FieldTestsType::InstanceExternalInitField
+        ret
+    }
+
     .method public instance void Stsfld.UnsatisfiedParentConstraints_Invalid_UnsatisfiedFieldParentInst() cil managed
     {
         ldc.i4.0
@@ -43,7 +54,8 @@
         ret
     }
 
-    .method public instance void Ldflda.InitonlyFieldOutsideCtor_Invalid_InitOnly() cil managed
+    // TODO: verification of readonly references https://github.com/dotnet/runtime/issues/57444
+    .method public instance void Ldflda.InitonlyFieldOutsideCtor_Valid() cil managed
     {
         ldarg.0
         ldflda  int32 FieldTestsType::InstanceInitonlyField
@@ -52,7 +64,7 @@
     }
 
     .method public hidebysig instance void 'special.StoreInitonlyField..ctor_Valid'() cil managed { ret }
-    .method public hidebysig specialname rtspecialname instance void .ctor() cil managed 
+    .method public hidebysig specialname rtspecialname instance void .ctor() cil managed
     {
         ldarg.0
         call    instance void [System.Runtime]System.Object::.ctor()
@@ -63,7 +75,7 @@
     }
 
     .method public hidebysig instance void 'special.LoadAddrInitonlyField..ctor_Valid'(int32) cil managed { ret }
-    .method public hidebysig specialname rtspecialname instance void .ctor(int32) cil managed 
+    .method public hidebysig specialname rtspecialname instance void .ctor(int32) cil managed
     {
         ldarg.0
         call    instance void [System.Runtime]System.Object::.ctor()
@@ -74,7 +86,7 @@
     }
 
     .method public hidebysig instance void 'special.LoadAddrInitonlyField..ctor_Valid'(int64) cil managed { ret }
-    .method public hidebysig specialname rtspecialname instance void .ctor(int64) cil managed 
+    .method public hidebysig specialname rtspecialname instance void .ctor(int64) cil managed
     {
         ldarg.0
         call    instance void [System.Runtime]System.Object::.ctor()
@@ -85,7 +97,7 @@
     }
 
     .method public hidebysig instance void 'special.StoreInitonlyFieldOtherType..ctor_Invalid_InitOnly'(class OtherType c) cil managed { ret }
-    .method public hidebysig specialname rtspecialname instance void .ctor(class OtherType c) cil managed 
+    .method public hidebysig specialname rtspecialname instance void .ctor(class OtherType c) cil managed
     {
         ldarg.0
         call    instance void [System.Runtime]System.Object::.ctor()
@@ -96,7 +108,7 @@
     }
 
     .method public hidebysig instance void 'special.StoreInitonlyFieldOtherInstance..ctor_Invalid_InitOnly'(class FieldTestsType c) cil managed { ret }
-    .method public hidebysig specialname rtspecialname instance void .ctor(class FieldTestsType c) cil managed 
+    .method public hidebysig specialname rtspecialname instance void .ctor(class FieldTestsType c) cil managed
     {
         ldarg.0
         call    instance void [System.Runtime]System.Object::.ctor()
@@ -107,7 +119,7 @@
     }
 
     .method public hidebysig instance void 'special.StsfldInitonlyInCtor..ctor_Invalid_InitOnly'(bool) cil managed { ret }
-    .method public hidebysig specialname rtspecialname instance void .ctor(bool) cil managed 
+    .method public hidebysig specialname rtspecialname instance void .ctor(bool) cil managed
     {
         ldarg.0
         call    instance void [System.Runtime]System.Object::.ctor()
@@ -117,7 +129,7 @@
     }
 
     .method public hidebysig instance void 'special.LdsfldInitonlyInCtor..ctor_Invalid_InitOnly'(char) cil managed { ret }
-    .method public hidebysig specialname rtspecialname instance void .ctor(char) cil managed 
+    .method public hidebysig specialname rtspecialname instance void .ctor(char) cil managed
     {
         ldarg.0
         call    instance void [System.Runtime]System.Object::.ctor()
@@ -127,7 +139,7 @@
     }
 
     .method public hidebysig instance void 'special.LdsfldStslfdInitonlyCctor..cctor_Valid'() cil managed { ret }
-    .method public hidebysig specialname rtspecialname instance void .cctor() cil managed 
+    .method public hidebysig specialname rtspecialname instance void .cctor() cil managed
     {
         ldsflda int32 FieldTestsType::StaticInitonlyField
         pop
@@ -145,8 +157,8 @@
     .field public initonly int32 InstanceInitonlyField
     .field public static initonly int32 StaticInitonlyField
 
-    .method public hidebysig instance void 'special.LdfldStlfdInitonlyCctor..cctor_Invalid_InitOnly.InitOnly'() cil managed { ret }
-    .method public hidebysig specialname rtspecialname instance void .cctor() cil managed 
+    .method public hidebysig instance void 'special.LdfldStlfdInitonlyCctor..cctor_Invalid_InitOnly'() cil managed { ret }
+    .method public hidebysig specialname rtspecialname instance void .cctor() cil managed
     {
         ldsfld class OtherType OtherType::Instance
         ldflda int32 OtherType::InstanceInitonlyField

diff --git a/src/tests/ilverify/ILTests/FunctionPointerTests.il b/src/tests/ilverify/ILTests/FunctionPointerTests.il
@@ -56,7 +56,7 @@
         ret
     }
 
-    .method public hidebysig instance void Call.SendFunctionPointerWithInaccessibleReturnType_Invalid_MethodAccess_MethodAccess() cil managed
+    .method public hidebysig instance void Call.SendFunctionPointerWithInaccessibleReturnType_Invalid_MethodAccess.MethodAccess() cil managed
     {
         newobj instance void C::.ctor()
         ldftn class D/D_Private D::StaticMethodReturningPrivateTypeInstance(int32)

diff --git a/src/tests/ilverify/TestDataLoader.cs b/src/tests/ilverify/TestDataLoader.cs
@@ -18,7 +18,7 @@
 namespace ILVerification.Tests
 {
     /// <summary>
-    /// Parses the methods in the test assemblies. 
+    /// Parses the methods in the test assemblies.
     /// It loads all assemblies from the test folder defined in <code>TestDataLoader.TestAssemblyPath</code>
     /// This class feeds the xunit Theories
     /// </summary>
@@ -106,8 +106,8 @@ private static TheoryData<TestCase> GetTestTypeFromDll(Func<string[], TypeDefini
         /// <summary>
         /// Returns all methods that contain valid IL code based on the following naming convention:
         /// [FriendlyName]_Valid
-        /// The method must contain 1 '_'. The part before the '_' is a friendly name describing what the method does. 
-        /// The word after the '_' has to be 'Valid' (Case sensitive) 
+        /// The method must contain 1 '_'. The part before the '_' is a friendly name describing what the method does.
+        /// The word after the '_' has to be 'Valid' (Case sensitive)
         /// E.g.: 'SimpleAdd_Valid'
         /// </summary>
         public static TheoryData<TestCase> GetMethodsWithValidIL()
@@ -129,9 +129,9 @@ public static TheoryData<TestCase> GetMethodsWithValidIL()
         /// The method name must contain 2 '_' characters.
         /// 1. part: a friendly name
         /// 2. part: must be the word 'Invalid' (Case sensitive)
-        /// 3. part: the expected VerifierErrors as string separated by '.'.      
+        /// 3. part: the expected VerifierErrors as string separated by '.'.
         /// E.g.: SimpleAdd_Invalid_ExpectedNumericType
-        /// </summary>      
+        /// </summary>
         public static TheoryData<TestCase> GetMethodsWithInvalidIL()
         {
             var methodSelector = new Func<string[], MethodDefinitionHandle, TestCase>((mparams, methodHandle) =>
@@ -176,20 +176,37 @@ private static TheoryData<TestCase> GetTestMethodsFromDll(Func<string[], MethodD
                     var method = (EcmaMethod)testModule.GetMethod(methodHandle);
                     var methodName = method.Name;
 
-                    if (!String.IsNullOrEmpty(methodName) && methodName.Contains("_"))
+                    if (!methodName.Contains('_', StringComparison.Ordinal))
+                        continue;
+
+                    var index = methodName.LastIndexOf("_Valid", StringComparison.Ordinal);
+                    if (index < 0)
+                        index = methodName.LastIndexOf("_Invalid", StringComparison.Ordinal);
+                    if (index < 0)
+                        continue;
+
+                    var substring = methodName.Substring(index + 1);
+                    var split = substring.Split('_');
+                    string[] mparams = new string[split.Length + 1];
+                    split.CopyTo(mparams, 1);
+                    mparams[0] = methodName.Substring(0, index);
+                    // examples of methodName to mparams transformation:
+                    //   * `get_Property` -> [ 'get_Property' ]
+                    //   * `CheckSomething_Valid` -> [ 'CheckSomething', 'Valid' ]
+                    //   * 'WrongMethod_Invalid_BranchOutOfTry' -> [ 'WrongMethod', 'Invalid', 'BranchOutOfTry' ]
+                    //   * 'MoreWrongMethod_Invalid_TypeAccess.InitOnly' -> [ 'MoreWrongMethod', 'Invalid', 'TypeAccess', 'InitOnly' ]
+                    //   * 'special.set_MyProperty.set_MyProperty_Invalid_InitOnly' -> [ 'special.set_MyProperty.set_MyProperty', 'Invalid', 'InitOnly' ]
+
+                    var specialMethodHandle = HandleSpecialTests(mparams, method);
+                    var newItem = methodSelector(mparams, specialMethodHandle);
+
+                    if (newItem != null)
                     {
-                        var mparams = methodName.Split('_');
-                        var specialMethodHandle = HandleSpecialTests(mparams, method);
-                        var newItem = methodSelector(mparams, specialMethodHandle);
-
-                        if (newItem != null)
-                        {
-                            newItem.TestName = mparams[0];
-                            newItem.MethodName = methodName;
-                            newItem.ModuleName = testDllName;
+                        newItem.TestName = mparams[0];
+                        newItem.MethodName = methodName;
+                        newItem.ModuleName = testDllName;
 
-                            retVal.Add(newItem);
-                        }
+                        retVal.Add(newItem);
                     }
                 }
             }