Fix alignment padding and add test for saving managed resources (#110915)

* Add test for saving and reading managed resources

* Use TempFile helper for tests; align Resource section

* Verify ManagedPEBuilder.Serialize() doesn't modify incoming resource blob for padding

* Remove Assert; test using LoadContext instead of RemoteExecutor

* Remove Asserts and padding

* Remove unnecessary using statement

Author: steveharter

src/libraries/System.Reflection.Emit/tests/AssemblyBuilderTests.cs

@@ -411,100 +411,100 @@ private static void VerifyAssemblyBuilder(AssemblyBuilder assembly, AssemblyName
             Assert.Empty(assembly.GetTypes());
         }
 
-	private static void SamplePrivateMethod ()
-	{
-	}
+        private static void SamplePrivateMethod()
+        {
+        }
 
-	internal static void SampleInternalMethod ()
-	{
-	}
+        internal static void SampleInternalMethod()
+        {
+        }
 
-	[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsReflectionEmitSupported))]
-	void Invoke_Private_CrossAssembly_ThrowsMethodAccessException()
-	{
-	    TypeBuilder tb = Helpers.DynamicType(TypeAttributes.Public);
-	    var mb = tb.DefineMethod ("MyMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(void), new Type[] {  });
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsReflectionEmitSupported))]
+        void Invoke_Private_CrossAssembly_ThrowsMethodAccessException()
+        {
+            TypeBuilder tb = Helpers.DynamicType(TypeAttributes.Public);
+            var mb = tb.DefineMethod("MyMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(void), new Type[] { });
 
-	    var ilg = mb.GetILGenerator ();
+            var ilg = mb.GetILGenerator();
 
-	    var callee = typeof (AssemblyTests).GetMethod ("SamplePrivateMethod", BindingFlags.Static | BindingFlags.NonPublic);
+            var callee = typeof(AssemblyTests).GetMethod("SamplePrivateMethod", BindingFlags.Static | BindingFlags.NonPublic);
 
-	    ilg.Emit (OpCodes.Call, callee);
-	    ilg.Emit (OpCodes.Ret);
+            ilg.Emit(OpCodes.Call, callee);
+            ilg.Emit(OpCodes.Ret);
 
-	    var ty = tb.CreateType ();
+            var ty = tb.CreateType();
 
-	    var mi = ty.GetMethod ("MyMethod", BindingFlags.Static | BindingFlags.Public);
+            var mi = ty.GetMethod("MyMethod", BindingFlags.Static | BindingFlags.Public);
 
-	    var d = (Action) mi.CreateDelegate (typeof(Action));
+            var d = (Action)mi.CreateDelegate(typeof(Action));
 
-	    Assert.Throws<MethodAccessException>(() => d ());
-	}
+            Assert.Throws<MethodAccessException>(() => d());
+        }
 
-	[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsReflectionEmitSupported))]
-	void Invoke_Internal_CrossAssembly_ThrowsMethodAccessException()
-	{
-	    TypeBuilder tb = Helpers.DynamicType(TypeAttributes.Public);
-	    var mb = tb.DefineMethod ("MyMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(void), new Type[] {  });
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsReflectionEmitSupported))]
+        void Invoke_Internal_CrossAssembly_ThrowsMethodAccessException()
+        {
+            TypeBuilder tb = Helpers.DynamicType(TypeAttributes.Public);
+            var mb = tb.DefineMethod("MyMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(void), new Type[] { });
 
-	    var ilg = mb.GetILGenerator ();
+            var ilg = mb.GetILGenerator();
 
-	    var callee = typeof (AssemblyTests).GetMethod ("SampleInternalMethod", BindingFlags.Static | BindingFlags.NonPublic);
+            var callee = typeof(AssemblyTests).GetMethod("SampleInternalMethod", BindingFlags.Static | BindingFlags.NonPublic);
 
-	    ilg.Emit (OpCodes.Call, callee);
-	    ilg.Emit (OpCodes.Ret);
+            ilg.Emit(OpCodes.Call, callee);
+            ilg.Emit(OpCodes.Ret);
 
-	    var ty = tb.CreateType ();
+            var ty = tb.CreateType();
 
-	    var mi = ty.GetMethod ("MyMethod", BindingFlags.Static | BindingFlags.Public);
+            var mi = ty.GetMethod("MyMethod", BindingFlags.Static | BindingFlags.Public);
 
-	    var d = (Action) mi.CreateDelegate (typeof(Action));
+            var d = (Action)mi.CreateDelegate(typeof(Action));
 
-	    Assert.Throws<MethodAccessException>(() => d ());
-	}
+            Assert.Throws<MethodAccessException>(() => d());
+        }
 
-	[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsReflectionEmitSupported))]
-	void Invoke_Private_SameAssembly_ThrowsMethodAccessException()
-	{
-	    ModuleBuilder modb = Helpers.DynamicModule();
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsReflectionEmitSupported))]
+        void Invoke_Private_SameAssembly_ThrowsMethodAccessException()
+        {
+            ModuleBuilder modb = Helpers.DynamicModule();
 
-	    string calleeName = "PrivateMethod";
+            string calleeName = "PrivateMethod";
 
-	    TypeBuilder tbCalled = modb.DefineType ("CalledClass", TypeAttributes.Public);
-	    var mbCalled = tbCalled.DefineMethod (calleeName, MethodAttributes.Private | MethodAttributes.Static);
-	    mbCalled.GetILGenerator().Emit (OpCodes.Ret);
+            TypeBuilder tbCalled = modb.DefineType("CalledClass", TypeAttributes.Public);
+            var mbCalled = tbCalled.DefineMethod(calleeName, MethodAttributes.Private | MethodAttributes.Static);
+            mbCalled.GetILGenerator().Emit(OpCodes.Ret);
 
-	    var tyCalled = tbCalled.CreateType();
-	    var callee = tyCalled.GetMethod (calleeName, BindingFlags.NonPublic | BindingFlags.Static);
+            var tyCalled = tbCalled.CreateType();
+            var callee = tyCalled.GetMethod(calleeName, BindingFlags.NonPublic | BindingFlags.Static);
 
-	    TypeBuilder tb = modb.DefineType("CallerClass", TypeAttributes.Public);
-	    var mb = tb.DefineMethod ("MyMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(void), new Type[] {  });
+            TypeBuilder tb = modb.DefineType("CallerClass", TypeAttributes.Public);
+            var mb = tb.DefineMethod("MyMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(void), new Type[] { });
 
-	    var ilg = mb.GetILGenerator ();
+            var ilg = mb.GetILGenerator();
 
-	    ilg.Emit (OpCodes.Call, callee);
-	    ilg.Emit (OpCodes.Ret);
+            ilg.Emit(OpCodes.Call, callee);
+            ilg.Emit(OpCodes.Ret);
 
-	    var ty = tb.CreateType ();
+            var ty = tb.CreateType();
 
-	    var mi = ty.GetMethod ("MyMethod", BindingFlags.Static | BindingFlags.Public);
+            var mi = ty.GetMethod("MyMethod", BindingFlags.Static | BindingFlags.Public);
 
-	    var d = (Action) mi.CreateDelegate (typeof(Action));
+            var d = (Action)mi.CreateDelegate(typeof(Action));
 
-	    Assert.Throws<MethodAccessException>(() => d ());
-	}
+            Assert.Throws<MethodAccessException>(() => d());
+        }
 
-    [Fact]
-    public void DefineDynamicAssembly_AssemblyBuilderLocationIsEmpty_InternalAssemblyBuilderLocationIsEmpty()
-    {
-        AssemblyBuilder assembly = Helpers.DynamicAssembly(nameof(DefineDynamicAssembly_AssemblyBuilderLocationIsEmpty_InternalAssemblyBuilderLocationIsEmpty));
-        Assembly internalAssemblyBuilder  = AppDomain.CurrentDomain.GetAssemblies()
-                .FirstOrDefault(a => a.FullName == assembly.FullName);
+        [Fact]
+        public void DefineDynamicAssembly_AssemblyBuilderLocationIsEmpty_InternalAssemblyBuilderLocationIsEmpty()
+        {
+            AssemblyBuilder assembly = Helpers.DynamicAssembly(nameof(DefineDynamicAssembly_AssemblyBuilderLocationIsEmpty_InternalAssemblyBuilderLocationIsEmpty));
+            Assembly internalAssemblyBuilder = AppDomain.CurrentDomain.GetAssemblies()
+                    .FirstOrDefault(a => a.FullName == assembly.FullName);
 
-        Assert.Empty(assembly.Location);
-        Assert.NotNull(internalAssemblyBuilder);
-        Assert.Empty(internalAssemblyBuilder.Location);
-    }
+            Assert.Empty(assembly.Location);
+            Assert.NotNull(internalAssemblyBuilder);
+            Assert.Empty(internalAssemblyBuilder.Location);
+        }
 
         [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         public static void ThrowsWhenDynamicCodeNotSupported()

src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs

@@ -0,0 +1,103 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections;
+using System.Globalization;
+using System.IO;
+using System.Reflection.Metadata;
+using System.Reflection.Metadata.Ecma335;
+using System.Reflection.PortableExecutable;
+using System.Resources;
+using Xunit;
+
+namespace System.Reflection.Emit.Tests
+{
+    [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
+    public class AssemblySaveResourceTests
+    {
+        [Theory]
+        [InlineData(new byte[] { 1 })]
+        [InlineData(new byte[] { 1, 2 })] // Verify blob alignment padding by adding a byte.
+        public void ManagedResourcesAndFieldData(byte[] byteValues)
+        {
+            PersistedAssemblyBuilder ab = new PersistedAssemblyBuilder(new AssemblyName("MyAssemblyWithResource"), typeof(object).Assembly);
+            ab.DefineDynamicModule("MyModule");
+            MetadataBuilder metadata = ab.GenerateMetadata(out BlobBuilder ilStream, out BlobBuilder fieldData);
+
+            // We shouldn't have any field data.
+            Assert.Equal(0, fieldData.Count);
+            fieldData = new ();
+            fieldData.WriteBytes(byteValues);
+
+            using MemoryStream memoryStream = new MemoryStream();
+            ResourceWriter myResourceWriter = new ResourceWriter(memoryStream);
+            myResourceWriter.AddResource("StringResource", "Value");
+            myResourceWriter.AddResource("ByteResource", byteValues);
+            myResourceWriter.Close();
+
+            byte[] data = memoryStream.ToArray();
+            BlobBuilder resourceBlob = new BlobBuilder();
+            resourceBlob.WriteInt32(data.Length);
+            resourceBlob.WriteBytes(data);
+            int resourceBlobSize = resourceBlob.Count;
+
+            metadata.AddManifestResource(
+                ManifestResourceAttributes.Public,
+                metadata.GetOrAddString("MyResource.resources"),
+                implementation: default,
+                offset: 0);
+
+            ManagedPEBuilder peBuilder = new ManagedPEBuilder(
+                            header: PEHeaderBuilder.CreateLibraryHeader(),
+                            metadataRootBuilder: new MetadataRootBuilder(metadata),
+                            ilStream: ilStream,
+                            mappedFieldData: fieldData,
+                            managedResources: resourceBlob);
+
+            BlobBuilder blob = new BlobBuilder();
+            peBuilder.Serialize(blob);
+
+            // Ensure the the blobs passed to Serialize() weren't modified due to alignment padding
+            Assert.Equal(resourceBlobSize, resourceBlob.Count);
+            Assert.Equal(byteValues.Length, fieldData.Count);
+
+            // To verify the resources work with runtime APIs, load the assembly into the process instead of
+            // the normal testing approach of using MetadataLoadContext.
+            TestAssemblyLoadContext testAssemblyLoadContext = new();
+            try
+            {
+                Assembly readAssembly = testAssemblyLoadContext.LoadFromStream(new MemoryStream(blob.ToArray()));
+
+                // Use ResourceReader to read the resources.
+                using Stream readStream = readAssembly.GetManifestResourceStream("MyResource.resources")!;
+                using ResourceReader reader = new(readStream);
+                Verify(reader.GetEnumerator());
+
+                // Use ResourceManager to read the resources.
+                ResourceManager rm = new ResourceManager("MyResource", readAssembly);
+                ResourceSet resourceSet = rm.GetResourceSet(CultureInfo.InvariantCulture, createIfNotExists: true, tryParents: false);
+                Verify(resourceSet.GetEnumerator());
+            }
+            finally
+            {
+                testAssemblyLoadContext.Unload();
+            }
+
+            void Verify(IDictionaryEnumerator resources)
+            {
+                Assert.True(resources.MoveNext());
+                DictionaryEntry resource = (DictionaryEntry)resources.Current;
+                Assert.Equal("ByteResource", resource.Key);
+                Assert.Equal(byteValues, resource.Value);
+
+                Assert.True(resources.MoveNext());
+                resource = (DictionaryEntry)resources.Current;
+                Assert.Equal("StringResource", resource.Key);
+                Assert.Equal("Value", resource.Value);
+
+                Assert.False(resources.MoveNext());
+            }
+        }
+    }
+}
+

src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj

@@ -71,6 +71,7 @@
     <Compile Include="PersistedAssemblyBuilder\AssemblySaveTypeBuilderAPIsTests.cs" />
     <Compile Include="PersistedAssemblyBuilder\AssemblySaveModuleBuilderTests.cs" />
     <Compile Include="PersistedAssemblyBuilder\AssemblySavePropertyBuilderTests.cs" />
+    <Compile Include="PersistedAssemblyBuilder\AssemblySaveResourceTests.cs" />
     <Compile Include="PersistedAssemblyBuilder\AssemblySaveTools.cs" />
     <Compile Include="PersistedAssemblyBuilder\AssemblySaveTypeBuilderTests.cs" />
     <Compile Include="PortablePdb\ILGeneratorScopesAndSequencePointsTests.cs" />

src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs

@@ -41,7 +41,6 @@ internal sealed class ManagedTextSection
 
         /// <summary>
         /// The size of managed resource data stream.
-        /// Aligned to <see cref="ManagedResourcesDataAlignment"/>.
         /// </summary>
         public int ResourceDataSize { get; }
 
@@ -147,9 +146,6 @@ public int CalculateOffsetToMappedFieldDataStream()
 
         internal int ComputeOffsetToDebugDirectory()
         {
-            Debug.Assert(MetadataSize % 4 == 0);
-            Debug.Assert(ResourceDataSize % 4 == 0);
-
             return
                 ComputeOffsetToMetadata() +
                 MetadataSize +
@@ -187,7 +183,6 @@ private int ComputeOffsetToMetadata()
 
         public int ComputeSizeOfTextSection()
         {
-            Debug.Assert(MappedFieldDataSize % MappedFieldDataAlignment == 0);
             return CalculateOffsetToMappedFieldDataStream() + MappedFieldDataSize;
         }
 
@@ -254,7 +249,6 @@ public void Serialize(
             Debug.Assert(ilBuilder.Count == ILStreamSize);
             Debug.Assert((mappedFieldDataBuilderOpt?.Count ?? 0) == MappedFieldDataSize);
             Debug.Assert((resourceBuilderOpt?.Count ?? 0) == ResourceDataSize);
-            Debug.Assert((resourceBuilderOpt?.Count ?? 0) % 4 == 0);
 
             // TODO: avoid recalculation
             int importTableRva = GetImportTableDirectoryEntry(relativeVirtualAddess).RelativeVirtualAddress;