fix(824): publish ILGlobals opcode tables from an explicit static constructor

UtilitiesCS.Test/NewtonsoftHelpers/SDILReader/ILGlobals_Tests.cs

@@ -1,3 +1,4 @@
+using System.Reflection;
 using System.Reflection.Emit;
 using FluentAssertions;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -8,46 +9,182 @@ namespace UtilitiesCS.Test.NewtonsoftHelpers.SDILReader
     [TestClass]
     public class ILGlobals_Tests
     {
+        /// <summary>
+        /// The first read of the static field is what triggers publication, so this test performs
+        /// no Act: reading the field in the Assert is the property under test.
+        /// </summary>
         [TestMethod]
-        public void LoadOpCodes_Initializes_SingleByteOpCodes()
+        public void SingleByteOpCodes_IsPublishedWithFullLength()
         {
-            // Arrange & Act
-            ILGlobals.LoadOpCodes();
-
             // Assert
             ILGlobals.singleByteOpCodes.Should().NotBeNull();
             ILGlobals.singleByteOpCodes.Length.Should().Be(0x100);
         }
 
+        /// <summary>
+        /// The multi-byte counterpart of the single-byte publication assertion above. It likewise
+        /// performs no Act, because reading the field is what triggers publication.
+        /// </summary>
         [TestMethod]
-        public void LoadOpCodes_Initializes_MultiByteOpCodes()
+        public void MultiByteOpCodes_IsPublishedWithFullLength()
         {
-            // Arrange & Act
-            ILGlobals.LoadOpCodes();
-
             // Assert
             ILGlobals.multiByteOpCodes.Should().NotBeNull();
             ILGlobals.multiByteOpCodes.Length.Should().Be(0x100);
         }
 
+        /// <summary>
+        /// Primary regression gate for issue #824. The opcode tables must be published exactly
+        /// once, so a call to <see cref="ILGlobals.LoadOpCodes"/> must force type initialization
+        /// rather than allocate and refill a replacement table. Reference identity across the call
+        /// is what distinguishes the two behaviours.
+        /// </summary>
         [TestMethod]
-        public void LoadOpCodes_PopulatesKnownSingleByteOpCodes()
+        public void LoadOpCodes_DoesNotRepublishPublishedTables()
         {
-            // Arrange & Act
+            // Arrange - capture the currently published table references.
+            var singleBefore = ILGlobals.singleByteOpCodes;
+            var multiBefore = ILGlobals.multiByteOpCodes;
+
+            // Act
             ILGlobals.LoadOpCodes();
 
-            // Assert - Nop is 0x00, a well-known single-byte opcode
-            ILGlobals.singleByteOpCodes[0x00].Should().Be(OpCodes.Nop);
+            // Assert
+            ILGlobals
+                .singleByteOpCodes.Should()
+                .BeSameAs(
+                    singleBefore,
+                    "LoadOpCodes() must force type initialization rather than republish the "
+                        + "single-byte table, because a concurrent reader would otherwise observe "
+                        + "a freshly allocated array before the fill loop has populated it"
+                );
+            ILGlobals
+                .multiByteOpCodes.Should()
+                .BeSameAs(
+                    multiBefore,
+                    "LoadOpCodes() must force type initialization rather than republish the "
+                        + "multi-byte table, because a concurrent reader would otherwise observe "
+                        + "a freshly allocated array before the fill loop has populated it"
+                );
         }
 
+        /// <summary>
+        /// Structural anti-regression gate for issue #824. A future edit that removes the
+        /// <c>readonly</c> modifier from the single-byte table would reopen the publication window
+        /// with no other test signal, so the field's init-only encoding is asserted directly.
+        /// </summary>
         [TestMethod]
-        public void LoadOpCodes_PopulatesKnownOpCode_Ret()
+        public void SingleByteOpCodes_FieldIsInitOnly()
         {
             // Arrange & Act
-            ILGlobals.LoadOpCodes();
+            var field = typeof(ILGlobals).GetField(
+                nameof(ILGlobals.singleByteOpCodes),
+                BindingFlags.Public | BindingFlags.Static
+            );
+
+            // Assert
+            field.Should().NotBeNull("the public static single-byte opcode table must exist");
+            field!
+                .IsInitOnly.Should()
+                .BeTrue(
+                    "a readonly field can only be assigned by the static constructor, so any "
+                        + "future reassignment outside it is a compile-time error rather than a "
+                        + "runtime race"
+                );
+        }
+
+        /// <summary>
+        /// Structural anti-regression gate for issue #824, the multi-byte counterpart of the
+        /// single-byte init-only assertion above.
+        /// </summary>
+        [TestMethod]
+        public void MultiByteOpCodes_FieldIsInitOnly()
+        {
+            // Arrange & Act
+            var field = typeof(ILGlobals).GetField(
+                nameof(ILGlobals.multiByteOpCodes),
+                BindingFlags.Public | BindingFlags.Static
+            );
+
+            // Assert
+            field.Should().NotBeNull("the public static multi-byte opcode table must exist");
+            field!
+                .IsInitOnly.Should()
+                .BeTrue(
+                    "a readonly field can only be assigned by the static constructor, so any "
+                        + "future reassignment outside it is a compile-time error rather than a "
+                        + "runtime race"
+                );
+        }
+
+        /// <summary>
+        /// Supporting test, not a gate for issue #824. It passes on the unfixed tree as well,
+        /// because a single-threaded test observes a fully populated table either way. Its value is
+        /// that it replaces two weak spot checks and would catch a fix that publishes the tables
+        /// safely but fills them wrongly.
+        /// </summary>
+        [TestMethod]
+        public void OpCodeTables_ContainEveryOpCodeDeclaredOnOpCodes()
+        {
+            // Arrange
+            FieldInfo[] fields = typeof(OpCodes).GetFields(
+                BindingFlags.Public | BindingFlags.Static
+            );
+            int assertedCount = 0;
+
+            // Act & Assert
+            foreach (FieldInfo field in fields)
+            {
+                if (field.FieldType != typeof(OpCode))
+                {
+                    continue;
+                }
+
+                OpCode declared = (OpCode)field.GetValue(null)!;
+                ushort value = (ushort)declared.Value;
+                if (value < 0x100)
+                {
+                    ILGlobals
+                        .singleByteOpCodes[value]
+                        .Should()
+                        .Be(
+                            declared,
+                            "the single-byte table must hold {0} at index 0x{1:X2}",
+                            field.Name,
+                            value
+                        );
+                }
+                else
+                {
+                    (value & 0xff00)
+                        .Should()
+                        .Be(
+                            0xfe00,
+                            "every opcode at or above 0x100 must carry the 0xFE prefix, and {0} "
+                                + "does not",
+                            field.Name
+                        );
+                    ILGlobals
+                        .multiByteOpCodes[value & 0xff]
+                        .Should()
+                        .Be(
+                            declared,
+                            "the multi-byte table must hold {0} at index 0x{1:X2}",
+                            field.Name,
+                            value & 0xff
+                        );
+                }
+
+                assertedCount++;
+            }
 
-            // Assert - Ret opcode is 0x2A
-            ILGlobals.singleByteOpCodes[0x2A].Should().Be(OpCodes.Ret);
+            assertedCount
+                .Should()
+                .BeGreaterThan(
+                    0,
+                    "the reflection enumeration must find opcode fields, otherwise this test "
+                        + "would assert nothing at all"
+                );
         }
 
         [TestMethod]

UtilitiesCS/NewtonsoftHelpers/SDIL Reader/ILGlobals.cs

@@ -3,6 +3,7 @@
 using System.Collections.Generic;
 using System.Reflection;
 using System.Reflection.Emit;
+using System.Runtime.CompilerServices;
 using System.Text;
 
 namespace SDILReader
@@ -111,17 +112,34 @@ public static class ILGlobals
     {
         public static Dictionary<int, object> Cache = new Dictionary<int, object>();
 
-        // Invariant: these are populated by LoadOpCodes() before any read of the tables;
-        // annotated null! (rather than nullable) to preserve the non-null contract that
-        // consumers (e.g. MethodBodyReader) already rely on. Behavior unchanged.
-        public static OpCode[] multiByteOpCodes = null!;
-        public static OpCode[] singleByteOpCodes = null!;
+        /// <summary>
+        /// Multi-byte (0xFE-prefixed) opcode table, indexed by the low byte of the opcode value.
+        /// Published once by the static constructor of <see cref="ILGlobals"/>, fully populated,
+        /// and never reassigned thereafter. <c>readonly</c> prevents reassignment of the array
+        /// reference; it does not prevent element mutation, so callers must treat the contents as
+        /// read-only.
+        /// </summary>
+        public static readonly OpCode[] multiByteOpCodes;
+
+        /// <summary>
+        /// Single-byte opcode table, indexed by the opcode value. Published once by the static
+        /// constructor of <see cref="ILGlobals"/>, fully populated, and never reassigned
+        /// thereafter. <c>readonly</c> prevents reassignment of the array reference; it does not
+        /// prevent element mutation, so callers must treat the contents as read-only.
+        /// </summary>
+        public static readonly OpCode[] singleByteOpCodes;
         public static Module[]? modules = null;
 
-        public static void LoadOpCodes()
+        /// <summary>
+        /// Builds both opcode tables in locals and publishes each one exactly once, after the
+        /// reflection loop has run to completion. Assigning the fields only at the end is what
+        /// removes the window in which a reader could observe a table that had been allocated but
+        /// not yet filled.
+        /// </summary>
+        static ILGlobals()
         {
-            singleByteOpCodes = new OpCode[0x100];
-            multiByteOpCodes = new OpCode[0x100];
+            OpCode[] singleTable = new OpCode[0x100];
+            OpCode[] multiTable = new OpCode[0x100];
             FieldInfo[] infoArray1 = typeof(OpCodes).GetFields();
             for (int num1 = 0; num1 < infoArray1.Length; num1++)
             {
@@ -134,18 +152,31 @@ public static void LoadOpCodes()
                     ushort num2 = (ushort)code1.Value;
                     if (num2 < 0x100)
                     {
-                        singleByteOpCodes[(int)num2] = code1;
+                        singleTable[(int)num2] = code1;
                     }
                     else
                     {
                         if ((num2 & 0xff00) != 0xfe00)
                         {
                             throw new Exception("Invalid OpCode.");
                         }
-                        multiByteOpCodes[num2 & 0xff] = code1;
+                        multiTable[num2 & 0xff] = code1;
                     }
                 }
             }
+            singleByteOpCodes = singleTable;
+            multiByteOpCodes = multiTable;
+        }
+
+        /// <summary>
+        /// Forces the opcode tables to be published, if they have not been already. Retained for
+        /// the five existing call sites; the tables themselves are built by the static constructor,
+        /// so this method neither allocates nor reassigns anything and is safe to call repeatedly
+        /// from any thread.
+        /// </summary>
+        public static void LoadOpCodes()
+        {
+            RuntimeHelpers.RunClassConstructor(typeof(ILGlobals).TypeHandle);
         }
 
         /// <summary>