From f21da2fc5615a310e9d5ea6ba36b04fccd3380b9 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Mon, 23 Dec 2024 09:25:40 -0600 Subject: [PATCH 1/7] Add test for saving and reading managed resources --- .../tests/AssemblyBuilderTests.cs | 128 +++++++++--------- .../AssemblySaveResourceTests.cs | 95 +++++++++++++ .../tests/System.Reflection.Emit.Tests.csproj | 1 + 3 files changed, 160 insertions(+), 64 deletions(-) create mode 100644 src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs diff --git a/src/libraries/System.Reflection.Emit/tests/AssemblyBuilderTests.cs b/src/libraries/System.Reflection.Emit/tests/AssemblyBuilderTests.cs index 7a6d835fa63b6a..c2b4cad04c313c 100644 --- a/src/libraries/System.Reflection.Emit/tests/AssemblyBuilderTests.cs +++ b/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(() => d ()); - } + Assert.Throws(() => 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(() => d ()); - } + Assert.Throws(() => 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(() => d ()); - } + Assert.Throws(() => 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() diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs new file mode 100644 index 00000000000000..519a7182adac78 --- /dev/null +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs @@ -0,0 +1,95 @@ +// 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 Microsoft.DotNet.RemoteExecutor; +using Xunit; + +namespace System.Reflection.Emit.Tests +{ + [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] + public class AssemblySaveResourceTests + { + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(new byte[] { 1 }, "01")] + [InlineData(new byte[] { 1, 2 }, "01-02")] // Verify blob padding by adding a byte. + public void ManagedResources(byte[] byteValue, string byteValueExpected) + { + PersistedAssemblyBuilder ab = new PersistedAssemblyBuilder(new AssemblyName("MyAssemblyWithResource"), typeof(object).Assembly); + ab.DefineDynamicModule("MyModule"); + MetadataBuilder metadata = ab.GenerateMetadata(out BlobBuilder ilStream, out _); + + using MemoryStream memoryStream = new MemoryStream(); + ResourceWriter myResourceWriter = new ResourceWriter(memoryStream); + myResourceWriter.AddResource("StringResource", "Value"); + myResourceWriter.AddResource("ByteResource", byteValue); + myResourceWriter.Close(); + + byte[] data = memoryStream.ToArray(); + BlobBuilder resourceBlob = new BlobBuilder(); + resourceBlob.WriteInt32(data.Length); + resourceBlob.WriteBytes(data); + + 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, + managedResources: resourceBlob); + + BlobBuilder blob = new BlobBuilder(); + peBuilder.Serialize(blob); + + // Create a temporary assembly. + string tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + ".dll"); + using (FileStream fileStream = new FileStream(tempFilePath, FileMode.Create, FileAccess.Write)) + { + blob.WriteContentTo(fileStream); + } + + // In order to verify the resources work with ResourceManager, we need to load the assembly. + using (RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke(static (tempFilePath, byteValue, byteValueExpected) => + { + Assembly readAssembly = Assembly.LoadFile(tempFilePath); + + // 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()); + + void Verify(IDictionaryEnumerator resources) + { + Assert.True(resources.MoveNext()); + DictionaryEntry resource = (DictionaryEntry)resources.Current; + Assert.Equal("ByteResource", resource.Key); + Assert.Equal(byteValueExpected, byteValue); + + Assert.True(resources.MoveNext()); + resource = (DictionaryEntry)resources.Current; + Assert.Equal("StringResource", resource.Key); + Assert.Equal("Value", resource.Value); + + Assert.False(resources.MoveNext()); + } + }, tempFilePath, BitConverter.ToString(byteValue), byteValueExpected)) { } + + File.Delete(tempFilePath); + } + } +} diff --git a/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj b/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj index 56d7b3869a1665..f92aaea6308756 100644 --- a/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj +++ b/src/libraries/System.Reflection.Emit/tests/System.Reflection.Emit.Tests.csproj @@ -71,6 +71,7 @@ + From d199ed7801b9255bf5d4864c5fdf45b1b6ddbdd3 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Mon, 23 Dec 2024 12:20:41 -0600 Subject: [PATCH 2/7] Use TempFile helper for tests; align Resource section --- .../AssemblySaveResourceTests.cs | 7 +------ .../Metadata/Ecma335/MetadataSizes.cs | 2 +- .../PortableExecutable/ManagedTextSection.cs | 21 +++++++++++-------- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs index 519a7182adac78..58008ad27f441d 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs @@ -53,10 +53,7 @@ public void ManagedResources(byte[] byteValue, string byteValueExpected) // Create a temporary assembly. string tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + ".dll"); - using (FileStream fileStream = new FileStream(tempFilePath, FileMode.Create, FileAccess.Write)) - { - blob.WriteContentTo(fileStream); - } + using TempFile file = new TempFile(tempFilePath, blob.ToArray()); // In order to verify the resources work with ResourceManager, we need to load the assembly. using (RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke(static (tempFilePath, byteValue, byteValueExpected) => @@ -88,8 +85,6 @@ void Verify(IDictionaryEnumerator resources) Assert.False(resources.MoveNext()); } }, tempFilePath, BitConverter.ToString(byteValue), byteValueExpected)) { } - - File.Delete(tempFilePath); } } } diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs index 526cbcdcee9a84..84180091f1dc23 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs @@ -12,7 +12,7 @@ namespace System.Reflection.Metadata.Ecma335 /// public sealed class MetadataSizes { - private const int StreamAlignment = 4; + internal const int StreamAlignment = 4; // Call the length of the string (including the terminator) m (we require m <= 255); internal const int MaxMetadataVersionByteCount = 0xff - 1; diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs index bd30dc7e491024..ed471efdd8dee2 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs @@ -4,6 +4,7 @@ using System.Diagnostics; using System.Reflection.Internal; using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; namespace System.Reflection.PortableExecutable { @@ -40,8 +41,8 @@ internal sealed class ManagedTextSection public int MetadataSize { get; } /// - /// The size of managed resource data stream. - /// Aligned to . + /// The size of managed resource data stream (unaligned). + /// When written, will be aligned to . /// public int ResourceDataSize { get; } @@ -147,14 +148,16 @@ public int CalculateOffsetToMappedFieldDataStream() internal int ComputeOffsetToDebugDirectory() { - Debug.Assert(MetadataSize % 4 == 0); - Debug.Assert(ResourceDataSize % 4 == 0); + Debug.Assert(MetadataSize % MetadataSizes.StreamAlignment == 0); - return + int offset = ComputeOffsetToMetadata() + MetadataSize + - ResourceDataSize + + BitArithmetic.Align(ResourceDataSize, ManagedResourcesDataAlignment) + StrongNameSignatureSize; + + Debug.Assert(offset % MetadataSizes.StreamAlignment == 0); + return offset; } private int ComputeOffsetToImportTable() @@ -254,7 +257,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; @@ -278,6 +280,7 @@ public void Serialize( if (resourceBuilderOpt != null) { builder.LinkSuffix(resourceBuilderOpt); + builder.Align(ManagedTextSection.ManagedResourcesDataAlignment); } // strong name signature: @@ -387,7 +390,7 @@ private void WriteCorHeader(BlobBuilder builder, int textSectionRva, int entryPo int metadataRva = textSectionRva + ComputeOffsetToMetadata(); int resourcesRva = metadataRva + MetadataSize; - int signatureRva = resourcesRva + ResourceDataSize; + int signatureRva = resourcesRva + BitArithmetic.Align(ResourceDataSize, ManagedResourcesDataAlignment); int start = builder.Count; @@ -410,7 +413,7 @@ private void WriteCorHeader(BlobBuilder builder, int textSectionRva, int entryPo // ResourcesDirectory: builder.WriteUInt32((uint)(ResourceDataSize == 0 ? 0 : resourcesRva)); // 28 - builder.WriteUInt32((uint)ResourceDataSize); + builder.WriteUInt32((uint)BitArithmetic.Align(ResourceDataSize, ManagedResourcesDataAlignment)); // StrongNameSignatureDirectory: builder.WriteUInt32((uint)(StrongNameSignatureSize == 0 ? 0 : signatureRva)); // 36 From c011569c092f33615ab39138f0602f58dd787214 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Mon, 23 Dec 2024 15:02:31 -0600 Subject: [PATCH 3/7] Align just the resource buffer --- .../System/Reflection/PortableExecutable/ManagedTextSection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs index ed471efdd8dee2..d8a0a69d055cb3 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs @@ -280,7 +280,7 @@ public void Serialize( if (resourceBuilderOpt != null) { builder.LinkSuffix(resourceBuilderOpt); - builder.Align(ManagedTextSection.ManagedResourcesDataAlignment); + builder.WriteBytes(0, BitArithmetic.Align(resourceBuilderOpt.Count, ManagedTextSection.ManagedResourcesDataAlignment) - resourceBuilderOpt.Count); } // strong name signature: From f857e01d8eae25d27d487806b11811bacde11b61 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Tue, 24 Dec 2024 10:06:58 -0600 Subject: [PATCH 4/7] Verify ManagedPEBuilder.Serialize() doesn't modify incoming resource blob for padding --- .../AssemblySaveResourceTests.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs index 58008ad27f441d..332620604cee0a 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs @@ -18,7 +18,7 @@ public class AssemblySaveResourceTests { [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [InlineData(new byte[] { 1 }, "01")] - [InlineData(new byte[] { 1, 2 }, "01-02")] // Verify blob padding by adding a byte. + [InlineData(new byte[] { 1, 2 }, "01-02")] // Verify blob alignment padding by adding a byte. public void ManagedResources(byte[] byteValue, string byteValueExpected) { PersistedAssemblyBuilder ab = new PersistedAssemblyBuilder(new AssemblyName("MyAssemblyWithResource"), typeof(object).Assembly); @@ -35,6 +35,7 @@ public void ManagedResources(byte[] byteValue, string byteValueExpected) BlobBuilder resourceBlob = new BlobBuilder(); resourceBlob.WriteInt32(data.Length); resourceBlob.WriteBytes(data); + int resourceBlobSize = resourceBlob.Count; metadata.AddManifestResource( ManifestResourceAttributes.Public, @@ -51,12 +52,17 @@ public void ManagedResources(byte[] byteValue, string byteValueExpected) BlobBuilder blob = new BlobBuilder(); peBuilder.Serialize(blob); + // Ensure the the resource blob wasn't modified by Serialize() due to alignment padding. + // Serialize() pads after the blob to align at ManagedTextSection.ManagedResourcesDataAlignment bytes. + Assert.Equal(resourceBlobSize, resourceBlob.Count); + // Create a temporary assembly. string tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + ".dll"); using TempFile file = new TempFile(tempFilePath, blob.ToArray()); - // In order to verify the resources work with ResourceManager, we need to load the assembly. - using (RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke(static (tempFilePath, byteValue, byteValueExpected) => + // To verify the resources work with runtime APIs, load the assembly into the process instead of + // the normal testing approach of using MetadataLoadContext. + using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke(static (tempFilePath, byteValue, byteValueExpected) => { Assembly readAssembly = Assembly.LoadFile(tempFilePath); @@ -84,7 +90,7 @@ void Verify(IDictionaryEnumerator resources) Assert.False(resources.MoveNext()); } - }, tempFilePath, BitConverter.ToString(byteValue), byteValueExpected)) { } + }, tempFilePath, BitConverter.ToString(byteValue), byteValueExpected); } } } From cb2efb35e64b568941fbdf9d54c08a5399230b90 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Wed, 15 Jan 2025 14:26:59 -0600 Subject: [PATCH 5/7] Remove Assert; test using LoadContext instead of RemoteExecutor --- .../AssemblySaveResourceTests.cs | 49 ++++++++++--------- .../PortableExecutable/ManagedTextSection.cs | 1 - 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs index 332620604cee0a..c9e2012686356b 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs @@ -8,7 +8,6 @@ using System.Reflection.Metadata.Ecma335; using System.Reflection.PortableExecutable; using System.Resources; -using Microsoft.DotNet.RemoteExecutor; using Xunit; namespace System.Reflection.Emit.Tests @@ -16,10 +15,10 @@ namespace System.Reflection.Emit.Tests [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] public class AssemblySaveResourceTests { - [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [InlineData(new byte[] { 1 }, "01")] - [InlineData(new byte[] { 1, 2 }, "01-02")] // Verify blob alignment padding by adding a byte. - public void ManagedResources(byte[] byteValue, string byteValueExpected) + [Theory] + [InlineData(new byte[] { 1 })] + [InlineData(new byte[] { 1, 2 })] // Verify blob alignment padding by adding a byte. + public void ManagedResources(byte[] byteValue) { PersistedAssemblyBuilder ab = new PersistedAssemblyBuilder(new AssemblyName("MyAssemblyWithResource"), typeof(object).Assembly); ab.DefineDynamicModule("MyModule"); @@ -56,15 +55,12 @@ public void ManagedResources(byte[] byteValue, string byteValueExpected) // Serialize() pads after the blob to align at ManagedTextSection.ManagedResourcesDataAlignment bytes. Assert.Equal(resourceBlobSize, resourceBlob.Count); - // Create a temporary assembly. - string tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + ".dll"); - using TempFile file = new TempFile(tempFilePath, blob.ToArray()); - // To verify the resources work with runtime APIs, load the assembly into the process instead of // the normal testing approach of using MetadataLoadContext. - using RemoteInvokeHandle remoteHandle = RemoteExecutor.Invoke(static (tempFilePath, byteValue, byteValueExpected) => + TestAssemblyLoadContext testAssemblyLoadContext = new(); + try { - Assembly readAssembly = Assembly.LoadFile(tempFilePath); + Assembly readAssembly = testAssemblyLoadContext.LoadFromStream(new MemoryStream(blob.ToArray())); // Use ResourceReader to read the resources. using Stream readStream = readAssembly.GetManifestResourceStream("MyResource.resources")!; @@ -75,22 +71,27 @@ public void ManagedResources(byte[] byteValue, string byteValueExpected) 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(byteValueExpected, byteValue); + void Verify(IDictionaryEnumerator resources) + { + Assert.True(resources.MoveNext()); + DictionaryEntry resource = (DictionaryEntry)resources.Current; + Assert.Equal("ByteResource", resource.Key); + Assert.Equal(byteValue, resource.Value); - Assert.True(resources.MoveNext()); - resource = (DictionaryEntry)resources.Current; - Assert.Equal("StringResource", resource.Key); - Assert.Equal("Value", resource.Value); + Assert.True(resources.MoveNext()); + resource = (DictionaryEntry)resources.Current; + Assert.Equal("StringResource", resource.Key); + Assert.Equal("Value", resource.Value); - Assert.False(resources.MoveNext()); - } - }, tempFilePath, BitConverter.ToString(byteValue), byteValueExpected); + Assert.False(resources.MoveNext()); + } } } } + diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs index d8a0a69d055cb3..5a78d368039b90 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs @@ -156,7 +156,6 @@ internal int ComputeOffsetToDebugDirectory() BitArithmetic.Align(ResourceDataSize, ManagedResourcesDataAlignment) + StrongNameSignatureSize; - Debug.Assert(offset % MetadataSizes.StreamAlignment == 0); return offset; } From e8cbf2707b31c28eabcd89d166561058eeddd093 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Fri, 14 Feb 2025 14:47:01 -0600 Subject: [PATCH 6/7] Remove Asserts and padding --- .../AssemblySaveResourceTests.cs | 18 ++++++++++++------ .../Metadata/Ecma335/MetadataSizes.cs | 2 +- .../PortableExecutable/ManagedTextSection.cs | 17 +++++------------ 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs index c9e2012686356b..c4a2d997bcee97 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveResourceTests.cs @@ -18,16 +18,21 @@ public class AssemblySaveResourceTests [Theory] [InlineData(new byte[] { 1 })] [InlineData(new byte[] { 1, 2 })] // Verify blob alignment padding by adding a byte. - public void ManagedResources(byte[] byteValue) + 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 _); + 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", byteValue); + myResourceWriter.AddResource("ByteResource", byteValues); myResourceWriter.Close(); byte[] data = memoryStream.ToArray(); @@ -46,14 +51,15 @@ public void ManagedResources(byte[] byteValue) header: PEHeaderBuilder.CreateLibraryHeader(), metadataRootBuilder: new MetadataRootBuilder(metadata), ilStream: ilStream, + mappedFieldData: fieldData, managedResources: resourceBlob); BlobBuilder blob = new BlobBuilder(); peBuilder.Serialize(blob); - // Ensure the the resource blob wasn't modified by Serialize() due to alignment padding. - // Serialize() pads after the blob to align at ManagedTextSection.ManagedResourcesDataAlignment bytes. + // 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. @@ -82,7 +88,7 @@ void Verify(IDictionaryEnumerator resources) Assert.True(resources.MoveNext()); DictionaryEntry resource = (DictionaryEntry)resources.Current; Assert.Equal("ByteResource", resource.Key); - Assert.Equal(byteValue, resource.Value); + Assert.Equal(byteValues, resource.Value); Assert.True(resources.MoveNext()); resource = (DictionaryEntry)resources.Current; diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs index 84180091f1dc23..526cbcdcee9a84 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataSizes.cs @@ -12,7 +12,7 @@ namespace System.Reflection.Metadata.Ecma335 /// public sealed class MetadataSizes { - internal const int StreamAlignment = 4; + private const int StreamAlignment = 4; // Call the length of the string (including the terminator) m (we require m <= 255); internal const int MaxMetadataVersionByteCount = 0xff - 1; diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs index 5a78d368039b90..f2241e3df8214c 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs @@ -41,8 +41,7 @@ internal sealed class ManagedTextSection public int MetadataSize { get; } /// - /// The size of managed resource data stream (unaligned). - /// When written, will be aligned to . + /// The size of managed resource data stream. /// public int ResourceDataSize { get; } @@ -148,15 +147,11 @@ public int CalculateOffsetToMappedFieldDataStream() internal int ComputeOffsetToDebugDirectory() { - Debug.Assert(MetadataSize % MetadataSizes.StreamAlignment == 0); - - int offset = + return ComputeOffsetToMetadata() + MetadataSize + - BitArithmetic.Align(ResourceDataSize, ManagedResourcesDataAlignment) + + ResourceDataSize + StrongNameSignatureSize; - - return offset; } private int ComputeOffsetToImportTable() @@ -189,7 +184,6 @@ private int ComputeOffsetToMetadata() public int ComputeSizeOfTextSection() { - Debug.Assert(MappedFieldDataSize % MappedFieldDataAlignment == 0); return CalculateOffsetToMappedFieldDataStream() + MappedFieldDataSize; } @@ -279,7 +273,6 @@ public void Serialize( if (resourceBuilderOpt != null) { builder.LinkSuffix(resourceBuilderOpt); - builder.WriteBytes(0, BitArithmetic.Align(resourceBuilderOpt.Count, ManagedTextSection.ManagedResourcesDataAlignment) - resourceBuilderOpt.Count); } // strong name signature: @@ -389,7 +382,7 @@ private void WriteCorHeader(BlobBuilder builder, int textSectionRva, int entryPo int metadataRva = textSectionRva + ComputeOffsetToMetadata(); int resourcesRva = metadataRva + MetadataSize; - int signatureRva = resourcesRva + BitArithmetic.Align(ResourceDataSize, ManagedResourcesDataAlignment); + int signatureRva = resourcesRva + ResourceDataSize; int start = builder.Count; @@ -412,7 +405,7 @@ private void WriteCorHeader(BlobBuilder builder, int textSectionRva, int entryPo // ResourcesDirectory: builder.WriteUInt32((uint)(ResourceDataSize == 0 ? 0 : resourcesRva)); // 28 - builder.WriteUInt32((uint)BitArithmetic.Align(ResourceDataSize, ManagedResourcesDataAlignment)); + builder.WriteUInt32((uint)ResourceDataSize); // StrongNameSignatureDirectory: builder.WriteUInt32((uint)(StrongNameSignatureSize == 0 ? 0 : signatureRva)); // 36 From 306bfb8c341da34aff8fd04bbdecc5080b796245 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Fri, 14 Feb 2025 14:55:01 -0600 Subject: [PATCH 7/7] Remove unnecessary using statement --- .../System/Reflection/PortableExecutable/ManagedTextSection.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs index f2241e3df8214c..7ed8d1381c2737 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs @@ -4,7 +4,6 @@ using System.Diagnostics; using System.Reflection.Internal; using System.Reflection.Metadata; -using System.Reflection.Metadata.Ecma335; namespace System.Reflection.PortableExecutable {