When invoking class constructor ensure class is initialized (#40293)
authorDavid Wrighton <davidwr@microsoft.com>
Wed, 5 Aug 2020 16:10:15 +0000 (09:10 -0700)
committerGitHub <noreply@github.com>
Wed, 5 Aug 2020 16:10:15 +0000 (09:10 -0700)
When invoking the class constructor method via reflection invoke, ensure that the class constructor is run via the standard run class constructor pathway instead of running it explicitly. Do the same for module constructors.

src/coreclr/src/System.Private.CoreLib/src/System/Reflection/INVOCATION_FLAGS.cs
src/coreclr/src/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs
src/libraries/System.Reflection/tests/ConstructorInfoTests.cs

index e7d9b97..b6a5551 100644 (file)
@@ -14,7 +14,8 @@ namespace System.Reflection
         INVOCATION_FLAGS_INITIALIZED = 0x00000001,
         // it's used for both method and field to signify that no access is allowed
         INVOCATION_FLAGS_NO_INVOKE = 0x00000002,
-        /* unused 0x00000004 */
+        // Set for static ctors, to ensure that the static ctor is run as a static ctor before it is explicitly executed via reflection
+        INVOCATION_FLAGS_RUN_CLASS_CONSTRUCTOR = 0x00000004,
         // Set for static ctors and ctors on abstract types, which
         // can be invoked only if the "this" object is provided (even if it's null).
         INVOCATION_FLAGS_NO_CTOR_INVOKE = 0x00000008,
index 9d0086c..e06251e 100644 (file)
@@ -5,6 +5,7 @@ using System.Collections.Generic;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
+using System.Runtime.CompilerServices;
 using System.Text;
 using RuntimeTypeCache = System.RuntimeType.RuntimeTypeCache;
 
@@ -47,7 +48,12 @@ namespace System.Reflection
                         // We don't need other flags if this method cannot be invoked
                         invocationFlags |= INVOCATION_FLAGS.INVOCATION_FLAGS_NO_INVOKE;
                     }
-                    else if (IsStatic || declaringType != null && declaringType.IsAbstract)
+                    else if (IsStatic)
+                    {
+                        invocationFlags |= INVOCATION_FLAGS.INVOCATION_FLAGS_RUN_CLASS_CONSTRUCTOR |
+                                           INVOCATION_FLAGS.INVOCATION_FLAGS_NO_CTOR_INVOKE;
+                    }
+                    else if (declaringType != null && declaringType.IsAbstract)
                     {
                         invocationFlags |= INVOCATION_FLAGS.INVOCATION_FLAGS_NO_CTOR_INVOKE;
                     }
@@ -280,6 +286,21 @@ namespace System.Reflection
             // check basic method consistency. This call will throw if there are problems in the target/method relationship
             CheckConsistency(obj);
 
+            if ((invocationFlags & INVOCATION_FLAGS.INVOCATION_FLAGS_RUN_CLASS_CONSTRUCTOR) != 0)
+            {
+                // Run the class constructor through the class constructor mechanism instead of the Invoke path.
+                // This avoids allowing mutation of readonly static fields, and initializes the type correctly.
+
+                var declaringType = DeclaringType;
+
+                if (declaringType != null)
+                    RuntimeHelpers.RunClassConstructor(declaringType.TypeHandle);
+                else
+                    RuntimeHelpers.RunModuleConstructor(Module.ModuleHandle);
+
+                return null;
+            }
+
             Signature sig = Signature;
 
             // get the signature
index 38a5e46..a104245 100644 (file)
@@ -69,6 +69,27 @@ namespace System.Reflection.Tests
         }
 
         [Fact]
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/40351", TestRuntimes.Mono)]
+        public void Invoke_StaticConstructorMultipleTimes()
+        {
+            ConstructorInfo[] constructors = GetConstructors(typeof(ClassWithStaticConstructorThatIsCalledMultipleTimesViaReflection));
+            Assert.Equal(1, constructors.Length);
+            // The first time the static cctor is called, it should run the cctor twice
+            // Once to initialize run the cctor as a cctor
+            // The second to run it as a method which is invoked.
+            Assert.Equal(0, ClassWithStaticConstructorThatIsCalledMultipleTimesViaReflection.VisibleStatics.s_cctorCallCount);
+            object obj = constructors[0].Invoke(null, new object[] { });
+            Assert.Null(obj);
+            Assert.Equal(1, ClassWithStaticConstructorThatIsCalledMultipleTimesViaReflection.VisibleStatics.s_cctorCallCount);
+
+            // Subsequent invocations of the static cctor should not run the cctor at all, as it has already executed
+            // and running multiple times opens up the possibility of modifying read only static data
+            obj = constructors[0].Invoke(null, new object[] { });
+            Assert.Null(obj);
+            Assert.Equal(1, ClassWithStaticConstructorThatIsCalledMultipleTimesViaReflection.VisibleStatics.s_cctorCallCount);
+        }
+
+        [Fact]
         [ActiveIssue("https://github.com/mono/mono/issues/15024", TestRuntimes.Mono)]
         public void Invoke_StaticConstructor_ThrowsMemberAccessException()
         {
@@ -236,6 +257,20 @@ namespace System.Reflection.Tests
         static ClassWithStaticConstructor() { }
     }
 
+    // Use this class only from the Invoke_StaticConstructorMultipleTimes method
+    public static class ClassWithStaticConstructorThatIsCalledMultipleTimesViaReflection
+    {
+        public static class VisibleStatics
+        {
+            public static int s_cctorCallCount;
+        }
+
+        static ClassWithStaticConstructorThatIsCalledMultipleTimesViaReflection()
+        {
+            VisibleStatics.s_cctorCallCount++;
+        }
+    }
+
     public struct StructWith1Constructor
     {
         public int x;