From 81eba59a1a5bfbb694de64b2d56bc18cca246c9b Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Fri, 29 Sep 2023 17:55:37 -0700 Subject: [PATCH 1/5] Minimal ILGenerator implementation --- .../src/Resources/Strings.resx | 3 + .../src/System.Reflection.Emit.csproj | 1 + .../Reflection/Emit/AssemblyBuilderImpl.cs | 6 +- .../System/Reflection/Emit/ILGeneratorImpl.cs | 203 ++++++++++++++++++ .../Reflection/Emit/MethodBuilderImpl.cs | 37 +++- .../Reflection/Emit/ModuleBuilderImpl.cs | 27 ++- .../AssemblySaveILGeneratorTests.cs | 66 ++++++ .../AssemblySaveTools.cs | 8 + .../tests/System.Reflection.Emit.Tests.csproj | 1 + 9 files changed, 339 insertions(+), 13 deletions(-) create mode 100644 src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs create mode 100644 src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs diff --git a/src/libraries/System.Reflection.Emit/src/Resources/Strings.resx b/src/libraries/System.Reflection.Emit/src/Resources/Strings.resx index 54123a4d079f2f..f33ce87899cb59 100644 --- a/src/libraries/System.Reflection.Emit/src/Resources/Strings.resx +++ b/src/libraries/System.Reflection.Emit/src/Resources/Strings.resx @@ -183,4 +183,7 @@ Underlying type information on enumeration is not specified. + + Method body should not exist. + \ No newline at end of file diff --git a/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj b/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj index 0743b389cc64fc..3c6273439966ac 100644 --- a/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj +++ b/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj @@ -11,6 +11,7 @@ + diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/AssemblyBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/AssemblyBuilderImpl.cs index 35c590a5efb206..5a74358e6f3188 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/AssemblyBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/AssemblyBuilderImpl.cs @@ -86,12 +86,12 @@ internal void Save(Stream stream) hashAlgorithm: (AssemblyHashAlgorithm)_assemblyName.HashAlgorithm #pragma warning restore SYSLIB0037 ); - _module.WriteCustomAttributes(_customAttributes, assemblyHandle); - // Add module's metadata - _module.AppendMetadata(); var ilBuilder = new BlobBuilder(); + MethodBodyStreamEncoder methodBodyEncoder = new MethodBodyStreamEncoder(ilBuilder); + _module.AppendMetadata(methodBodyEncoder); + WritePEImage(stream, ilBuilder); _previouslySaved = true; } diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs new file mode 100644 index 00000000000000..47fd249c189eae --- /dev/null +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs @@ -0,0 +1,203 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Buffers.Binary; +using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; + +namespace System.Reflection.Emit +{ + internal sealed class ILGeneratorImpl : ILGenerator + { + private const int DefaultSize = 16; + private readonly MethodBuilder _methodBuilder; + private readonly BlobBuilder _builder; + private readonly InstructionEncoder _il; + private bool _hasDynamicStackAllocation; + + internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) + { + _methodBuilder = methodBuilder; + _builder = new BlobBuilder(Math.Max(size, DefaultSize)); + _il = new InstructionEncoder(_builder, new ControlFlowBuilder()); + } + + internal InstructionEncoder Instructions => _il; + internal bool HasDynamicStackAllocation => _hasDynamicStackAllocation; + + public override int ILOffset => _il.Offset; + + public override void BeginCatchBlock(Type? exceptionType) => throw new NotImplementedException(); + public override void BeginExceptFilterBlock() => throw new NotImplementedException(); + public override Label BeginExceptionBlock() => throw new NotImplementedException(); + public override void BeginFaultBlock() => throw new NotImplementedException(); + public override void BeginFinallyBlock() => throw new NotImplementedException(); + public override void BeginScope() => throw new NotImplementedException(); + public override LocalBuilder DeclareLocal(Type localType, bool pinned) => throw new NotImplementedException(); + public override Label DefineLabel() => throw new NotImplementedException(); + + public override void Emit(OpCode opcode) + { + if (opcode == OpCodes.Localloc) + { + _hasDynamicStackAllocation = true; + } + _il.OpCode((ILOpCode)opcode.Value); + } + + public override void Emit(OpCode opcode, byte arg) + { + _il.OpCode((ILOpCode)opcode.Value); + _builder.WriteByte(arg); + } + + public override void Emit(OpCode opcode, double arg) + { + _il.OpCode((ILOpCode)opcode.Value); + _builder.WriteDouble(arg); + } + + public override void Emit(OpCode opcode, float arg) + { + _il.OpCode((ILOpCode)opcode.Value); + _builder.WriteSingle(arg); + } + + public override void Emit(OpCode opcode, short arg) + { + _il.OpCode((ILOpCode)opcode.Value); + _builder.WriteInt16(arg); + } + + public override void Emit(OpCode opcode, int arg) + { + // Special-case several opcodes that have shorter variants for common values. + if (opcode.Equals(OpCodes.Ldc_I4)) + { + if (arg >= -1 && arg <= 8) + { + _il.OpCode(arg switch + { + -1 => ILOpCode.Ldc_i4_m1, + 0 => ILOpCode.Ldc_i4_0, + 1 => ILOpCode.Ldc_i4_1, + 2 => ILOpCode.Ldc_i4_2, + 3 => ILOpCode.Ldc_i4_3, + 4 => ILOpCode.Ldc_i4_4, + 5 => ILOpCode.Ldc_i4_5, + 6 => ILOpCode.Ldc_i4_6, + 7 => ILOpCode.Ldc_i4_7, + _ => ILOpCode.Ldc_i4_8, + }); + return; + } + + if (arg >= -128 && arg <= 127) + { + _il.OpCode(ILOpCode.Ldc_i4_s); + _builder.WriteSByte((sbyte)arg) ; + return; + } + } + else if (opcode.Equals(OpCodes.Ldarg)) + { + if ((uint)arg <= 3) + { + _il.OpCode(arg switch + { + 0 => ILOpCode.Ldarg_0, + 1 => ILOpCode.Ldarg_1, + 2 => ILOpCode.Ldarg_2, + _ => ILOpCode.Ldarg_3, + }); + return; + } + + if ((uint)arg <= byte.MaxValue) + { + _il.OpCode(ILOpCode.Ldarg_s); + _builder.WriteByte((byte)arg); + return; + } + + if ((uint)arg <= ushort.MaxValue) // this will be true except on misuse of the opcode + { + _il.OpCode(ILOpCode.Ldarg); + _builder.WriteInt16((short)arg); + return; + } + } + else if (opcode.Equals(OpCodes.Ldarga)) + { + if ((uint)arg <= byte.MaxValue) + { + _il.OpCode(ILOpCode.Ldarga_s); + _builder.WriteByte((byte)arg); + return; + } + + if ((uint)arg <= ushort.MaxValue) // this will be true except on misuse of the opcode + { + _il.OpCode(ILOpCode.Ldarga); + _builder.WriteInt16((short)arg); + return; + } + } + else if (opcode.Equals(OpCodes.Starg)) + { + if ((uint)arg <= byte.MaxValue) + { + _il.OpCode(ILOpCode.Starg_s); + _builder.WriteByte((byte)arg); + return; + } + + if ((uint)arg <= ushort.MaxValue) // this will be true except on misuse of the opcode + { + _il.OpCode(ILOpCode.Starg); + _builder.WriteInt16((short)arg); + return; + } + } + + // For everything else, put the opcode followed by the arg onto the stream of instructions. + _il.OpCode((ILOpCode)opcode.Value); + _builder.WriteInt32(arg); + } + + public override void Emit(OpCode opcode, long arg) + { + _il.OpCode((ILOpCode)opcode.Value); + _il.CodeBuilder.WriteInt64(arg); + } + + public override void Emit(OpCode opcode, string str) + { + // Puts the opcode onto the IL stream followed by the metadata token + // represented by str. + + ModuleBuilder modBuilder = (ModuleBuilder)_methodBuilder.Module; + int tempVal = modBuilder.GetStringMetadataToken(str); + _il.OpCode((ILOpCode)opcode.Value); + _il.Token(tempVal); + } + + public override void Emit(OpCode opcode, ConstructorInfo con) => throw new NotImplementedException(); + public override void Emit(OpCode opcode, Label label) => throw new NotImplementedException(); + public override void Emit(OpCode opcode, Label[] labels) => throw new NotImplementedException(); + public override void Emit(OpCode opcode, LocalBuilder local) => throw new NotImplementedException(); + public override void Emit(OpCode opcode, SignatureHelper signature) => throw new NotImplementedException(); + public override void Emit(OpCode opcode, FieldInfo field) => throw new NotImplementedException(); + public override void Emit(OpCode opcode, MethodInfo meth) => throw new NotImplementedException(); + public override void Emit(OpCode opcode, Type cls) => throw new NotImplementedException(); + public override void EmitCall(OpCode opcode, MethodInfo methodInfo, Type[]? optionalParameterTypes) => throw new NotImplementedException(); + public override void EmitCalli(OpCode opcode, CallingConventions callingConvention, Type? returnType, Type[]? parameterTypes, Type[]? optionalParameterTypes) => throw new NotImplementedException(); + public override void EmitCalli(OpCode opcode, CallingConvention unmanagedCallConv, Type? returnType, Type[]? parameterTypes) => throw new NotImplementedException(); + public override void EndExceptionBlock() => throw new NotImplementedException(); + public override void EndScope() => throw new NotImplementedException(); + public override void MarkLabel(Label loc) => throw new NotImplementedException(); + public override void UsingNamespace(string usingNamespace) => throw new NotImplementedException(); + } +} diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs index dbf38bca76d7fc..dd1eebcaa875b6 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs @@ -6,6 +6,7 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; namespace System.Reflection.Emit { @@ -20,6 +21,8 @@ internal sealed class MethodBuilderImpl : MethodBuilder private MethodAttributes _attributes; private MethodImplAttributes _methodImplFlags; private GenericTypeParameterBuilderImpl[]? _typeParameters; + private ILGeneratorImpl? _ilGenerator; + private bool _initLocals; internal DllImportData? _dllImportData; internal List? _customAttributes; @@ -46,8 +49,11 @@ internal MethodBuilderImpl(string name, MethodAttributes attributes, CallingConv } _methodImplFlags = MethodImplAttributes.IL; + _initLocals = true; } + internal ILGeneratorImpl? ILGeneratorImpl => _ilGenerator; + internal BlobBuilder GetMethodSignatureBlob() => MetadataSignatureHelper.MethodSignatureEncoder(_module, _parameterTypes, ReturnType, GetSignatureConvention(_callingConventions), GetGenericArguments().Length, !IsStatic); @@ -68,7 +74,14 @@ internal static SignatureCallingConvention GetSignatureConvention(CallingConvent return convention; } - protected override bool InitLocalsCore { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + protected override bool InitLocalsCore + { + get { ThrowIfGeneric(); return _initLocals; } + set { ThrowIfGeneric(); _initLocals = value; } + } + + private void ThrowIfGeneric() { if (IsGenericMethod && !IsGenericMethodDefinition) throw new InvalidOperationException(); } + protected override GenericTypeParameterBuilder[] DefineGenericParametersCore(params string[] names) { if (_typeParameters != null) @@ -98,7 +111,23 @@ protected override ParameterBuilder DefineParameterCore(int position, ParameterA return parameter; } - protected override ILGenerator GetILGeneratorCore(int size) => throw new NotImplementedException(); + protected override ILGenerator GetILGeneratorCore(int size) + { + if (IsGenericMethod && !IsGenericMethodDefinition) + { + throw new InvalidOperationException(); + } + + if ((_methodImplFlags & MethodImplAttributes.CodeTypeMask) != MethodImplAttributes.IL || + (_methodImplFlags & MethodImplAttributes.Unmanaged) != 0 || + (_attributes & MethodAttributes.PinvokeImpl) != 0) + { + throw new InvalidOperationException(SR.InvalidOperation_ShouldNotHaveMethodBody); + } + + return _ilGenerator ??= new ILGeneratorImpl(this, size); + } + protected override void SetCustomAttributeCore(ConstructorInfo con, ReadOnlySpan binaryAttribute) { // Handle pseudo custom attributes @@ -199,8 +228,8 @@ public override object Invoke(object? obj, BindingFlags invokeAttr, Binder? bind public override bool IsDefined(Type attributeType, bool inherit) => throw new NotSupportedException(SR.NotSupported_DynamicModule); [RequiresDynamicCode("The native code for this instantiation might not be available at runtime.")] - [RequiresUnreferencedCodeAttribute("If some of the generic arguments are annotated (either with DynamicallyAccessedMembersAttribute, or generic constraints), trimming can't validate that the requirements of those annotations are met.")] - public override MethodInfo MakeGenericMethod(params System.Type[] typeArguments) + [RequiresUnreferencedCode("If some of the generic arguments are annotated (either with DynamicallyAccessedMembersAttribute, or generic constraints), trimming can't validate that the requirements of those annotations are met.")] + public override MethodInfo MakeGenericMethod(params Type[] typeArguments) => throw new NotImplementedException(); } } diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs index c3ca6012544f34..81a6ca7691424f 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs @@ -98,7 +98,7 @@ internal Type GetTypeFromCoreAssembly(CoreTypeId typeId) return null; } - internal void AppendMetadata() + internal void AppendMetadata(MethodBodyStreamEncoder methodBodyEncoder) { // Add module metadata ModuleDefinitionHandle moduleHandle = _metadataBuilder.AddModule( @@ -161,7 +161,7 @@ internal void AppendMetadata() } WriteCustomAttributes(typeBuilder._customAttributes, typeHandle); - WriteMethods(typeBuilder, genericParams); + WriteMethods(typeBuilder, genericParams, methodBodyEncoder); WriteFields(typeBuilder); } @@ -180,11 +180,18 @@ internal void AppendMetadata() } } - private void WriteMethods(TypeBuilderImpl typeBuilder, List genericParams) + private void WriteMethods(TypeBuilderImpl typeBuilder, List genericParams, MethodBodyStreamEncoder methodBodyEncoder) { foreach (MethodBuilderImpl method in typeBuilder._methodDefinitions) { - MethodDefinitionHandle methodHandle = AddMethodDefinition(method, method.GetMethodSignatureBlob(), _nextParameterRowId); + int offset = -1; + ILGeneratorImpl? il = method.ILGeneratorImpl; + if (!method.IsAbstract && il != null) + { + offset = AddMethodBody(method, il, methodBodyEncoder); + } + + MethodDefinitionHandle methodHandle = AddMethodDefinition(method, method.GetMethodSignatureBlob(), offset, _nextParameterRowId); WriteCustomAttributes(method._customAttributes, methodHandle); _nextMethodDefRowId++; @@ -230,6 +237,14 @@ private void WriteMethods(TypeBuilderImpl typeBuilder, List + methodBodyEncoder.AddMethodBody( + instructionEncoder: il.Instructions, + maxStack: 8, // TODO + localVariablesSignature: default, // TODO + attributes: method.InitLocals ? MethodBodyAttributes.InitLocals : MethodBodyAttributes.None, + hasDynamicStackAllocation: il.HasDynamicStackAllocation); + private void WriteFields(TypeBuilderImpl typeBuilder) { foreach (FieldBuilderImpl field in typeBuilder._fieldDefinitions) @@ -348,13 +363,13 @@ private TypeDefinitionHandle AddTypeDefinition(TypeBuilderImpl type, EntityHandl fieldList: MetadataTokens.FieldDefinitionHandle(fieldToken), methodList: MetadataTokens.MethodDefinitionHandle(methodToken)); - private MethodDefinitionHandle AddMethodDefinition(MethodBuilderImpl method, BlobBuilder methodSignature, int parameterToken) => + private MethodDefinitionHandle AddMethodDefinition(MethodBuilderImpl method, BlobBuilder methodSignature, int offset, int parameterToken) => _metadataBuilder.AddMethodDefinition( attributes: method.Attributes, implAttributes: method.GetMethodImplementationFlags(), name: _metadataBuilder.GetOrAddString(method.Name), signature: _metadataBuilder.GetOrAddBlob(methodSignature), - bodyOffset: -1, // No body supported yet + bodyOffset: offset, parameterList: MetadataTokens.ParameterHandle(parameterToken)); private TypeReferenceHandle AddTypeReference(Type type, AssemblyReferenceHandle parent) => diff --git a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs new file mode 100644 index 00000000000000..a88453f4fffe30 --- /dev/null +++ b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs @@ -0,0 +1,66 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using System.Linq; +using Xunit; + +namespace System.Reflection.Emit.Tests +{ + public class AssemblySaveILGeneratorTests + { + [Fact] + public void MethodWithEmptyBody() + { + using (TempFile file = TempFile.Create()) + { + AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderAndSaveMethod(out MethodInfo saveMethod); + TypeBuilder type = ab.DefineDynamicModule("MyModule").DefineType("MyType", TypeAttributes.Public); + + var methodBuilder = type.DefineMethod("EmptyMethod", MethodAttributes.Public, typeof(void), new[] { typeof(Version) }); + var il = methodBuilder.GetILGenerator(); + il.Emit(OpCodes.Ret); + saveMethod.Invoke(ab, new object[] { file.Path }); + + Assembly assemblyFromDisk = AssemblySaveTools.LoadAssemblyFromPath(file.Path); + Type typeFromDisk = assemblyFromDisk.Modules.First().GetType("MyType"); + MethodInfo method = typeFromDisk.GetMethod("EmptyMethod"); + MethodBody body = method.GetMethodBody(); + Assert.Empty(body.LocalVariables); + Assert.Empty(body.ExceptionHandlingClauses); + byte[]? bodyBytes = body.GetILAsByteArray(); + Assert.NotNull(bodyBytes); + Assert.Equal(OpCodes.Ret.Value, bodyBytes[0]); + + } + } + + [Theory] + [InlineData(20)] + [InlineData(-10)] + public void MethodReturning_Int(int size) + { + using (TempFile file = TempFile.Create()) + { + AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderAndSaveMethod(out MethodInfo saveMethod); + TypeBuilder type = ab.DefineDynamicModule("MyModule").DefineType("MyType", TypeAttributes.Public); + MethodBuilder method = type.DefineMethod("TestMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(int), Type.EmptyTypes); + + ILGenerator ilGenerator = method.GetILGenerator(size); + int expectedReturn = 5; + ilGenerator.Emit(OpCodes.Ldc_I4, expectedReturn); + ilGenerator.Emit(OpCodes.Ret); + saveMethod.Invoke(ab, new object[] { file.Path }); + + Assembly assemblyFromDisk = AssemblySaveTools.LoadAssemblyFromPath(file.Path); + Type typeFromDisk = assemblyFromDisk.Modules.First().GetType("MyType"); + MethodInfo methodFromFile = typeFromDisk.GetMethod("TestMethod"); + MethodBody body = methodFromFile.GetMethodBody(); + byte[]? bodyBytes = body.GetILAsByteArray(); + Assert.NotNull(bodyBytes); + Assert.Equal(OpCodes.Ldc_I4_5.Value, bodyBytes[0]); + Assert.Equal(OpCodes.Ret.Value, bodyBytes[1]); + } + } + } +} diff --git a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTools.cs b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTools.cs index 088e33235e3974..3350d798854d85 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTools.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTools.cs @@ -11,6 +11,11 @@ namespace System.Reflection.Emit.Tests { internal static class AssemblySaveTools { + private static readonly AssemblyName s_assemblyName = new AssemblyName("MyDynamicAssembly") + { + Version = new Version("1.2.3.4"), + }; + internal static void WriteAssemblyToDisk(AssemblyName assemblyName, Type[] types, string fileLocation) { AssemblyBuilder assemblyBuilder = PopulateAssemblyBuilderAndSaveMethod( @@ -57,6 +62,9 @@ internal static void WriteAssemblyToStream(AssemblyName assemblyName, Type[] typ saveMethod.Invoke(assemblyBuilder, new object[] { stream }); } + internal static AssemblyBuilder PopulateAssemblyBuilderAndSaveMethod(out MethodInfo saveMethod) => + PopulateAssemblyBuilderAndSaveMethod(s_assemblyName, null, typeof(string), out saveMethod); + internal static AssemblyBuilder PopulateAssemblyBuilderAndSaveMethod(AssemblyName assemblyName, List? assemblyAttributes, Type parameterType, out MethodInfo saveMethod) { 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 1395d38e10de55..74471dbc3164dd 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 @@ -64,6 +64,7 @@ + From 6ee723c01f1cf7a8780437d6e9ce48e74666f24d Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Mon, 2 Oct 2023 11:01:12 -0700 Subject: [PATCH 2/5] Add more test, disable testing on browser --- .../AssemblySaveILGeneratorTests.cs | 57 ++++++++++++++++--- .../AssemblySaveTools.cs | 9 ++- 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs index a88453f4fffe30..58d61fb4896b6f 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs @@ -7,6 +7,7 @@ namespace System.Reflection.Emit.Tests { + [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] public class AssemblySaveILGeneratorTests { [Fact] @@ -14,11 +15,9 @@ public void MethodWithEmptyBody() { using (TempFile file = TempFile.Create()) { - AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderAndSaveMethod(out MethodInfo saveMethod); - TypeBuilder type = ab.DefineDynamicModule("MyModule").DefineType("MyType", TypeAttributes.Public); - - var methodBuilder = type.DefineMethod("EmptyMethod", MethodAttributes.Public, typeof(void), new[] { typeof(Version) }); - var il = methodBuilder.GetILGenerator(); + AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderTypeBuilderAndSaveMethod(out TypeBuilder type, out MethodInfo saveMethod); + MethodBuilder methodBuilder = type.DefineMethod("EmptyMethod", MethodAttributes.Public, typeof(void), new[] { typeof(Version) }); + ILGenerator il = methodBuilder.GetILGenerator(); il.Emit(OpCodes.Ret); saveMethod.Invoke(ab, new object[] { file.Path }); @@ -42,8 +41,7 @@ public void MethodReturning_Int(int size) { using (TempFile file = TempFile.Create()) { - AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderAndSaveMethod(out MethodInfo saveMethod); - TypeBuilder type = ab.DefineDynamicModule("MyModule").DefineType("MyType", TypeAttributes.Public); + AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderTypeBuilderAndSaveMethod(out TypeBuilder type, out MethodInfo saveMethod); MethodBuilder method = type.DefineMethod("TestMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(int), Type.EmptyTypes); ILGenerator ilGenerator = method.GetILGenerator(size); @@ -62,5 +60,50 @@ public void MethodReturning_Int(int size) Assert.Equal(OpCodes.Ret.Value, bodyBytes[1]); } } + + [Theory] + [InlineData(20)] + [InlineData(11)] + public void TypeWithTwoMethod_ReferenceMethodArguments(int multiplier) + { + using (TempFile file = TempFile.Create()) + { + AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderTypeBuilderAndSaveMethod(out TypeBuilder type, out MethodInfo saveMethod); + MethodBuilder multiplyMethod = type.DefineMethod("MultiplyMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(int), new Type[] { typeof(int) }); + multiplyMethod.DefineParameter(1, ParameterAttributes.None, "myParam"); + MethodBuilder addMethod = type.DefineMethod("AddMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(int), new Type[] { typeof(int), typeof(int) }); + addMethod.DefineParameter(1, ParameterAttributes.None, "firsParam"); + addMethod.DefineParameter(2, ParameterAttributes.None, "secondParam"); + + ILGenerator multiplyMethodIL = multiplyMethod.GetILGenerator(); + multiplyMethodIL.Emit(OpCodes.Ldarg_0); + multiplyMethodIL.Emit(OpCodes.Ldc_I4, multiplier); + multiplyMethodIL.Emit(OpCodes.Mul); + multiplyMethodIL.Emit(OpCodes.Ret); + + ILGenerator addMethodIL = addMethod.GetILGenerator(); + addMethodIL.Emit(OpCodes.Ldarg_0); + addMethodIL.Emit(OpCodes.Ldarg_1); + addMethodIL.Emit(OpCodes.Add); + addMethodIL.Emit(OpCodes.Ret); + + saveMethod.Invoke(ab, new object[] { file.Path }); + + Assembly assemblyFromDisk = AssemblySaveTools.LoadAssemblyFromPath(file.Path); + Type typeFromDisk = assemblyFromDisk.Modules.First().GetType("MyType"); + byte[]? multiplyBody = typeFromDisk.GetMethod("MultiplyMethod").GetMethodBody().GetILAsByteArray(); + byte[]? addBody = typeFromDisk.GetMethod("AddMethod").GetMethodBody().GetILAsByteArray(); + + Assert.NotNull(multiplyBody); + Assert.Equal(OpCodes.Ldarg_0.Value, multiplyBody[0]); + Assert.Equal(OpCodes.Ldc_I4_S.Value, multiplyBody[1]); + Assert.Equal(multiplier, multiplyBody[2]); + Assert.Equal(OpCodes.Mul.Value, multiplyBody[3]); + Assert.NotNull(addBody); + Assert.Equal(OpCodes.Ldarg_0.Value, addBody[0]); + Assert.Equal(OpCodes.Ldarg_1.Value, addBody[1]); + Assert.Equal(OpCodes.Add.Value, addBody[2]); + } + } } } diff --git a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTools.cs b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTools.cs index 3350d798854d85..a35d96fa33c84f 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTools.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTools.cs @@ -62,8 +62,13 @@ internal static void WriteAssemblyToStream(AssemblyName assemblyName, Type[] typ saveMethod.Invoke(assemblyBuilder, new object[] { stream }); } - internal static AssemblyBuilder PopulateAssemblyBuilderAndSaveMethod(out MethodInfo saveMethod) => - PopulateAssemblyBuilderAndSaveMethod(s_assemblyName, null, typeof(string), out saveMethod); + internal static AssemblyBuilder PopulateAssemblyBuilderTypeBuilderAndSaveMethod(out TypeBuilder typeBuilder, out MethodInfo saveMethod) + { + AssemblyBuilder ab = PopulateAssemblyBuilderAndSaveMethod(s_assemblyName, null, typeof(string), out saveMethod); + typeBuilder = ab.DefineDynamicModule("MyModule").DefineType("MyType", TypeAttributes.Public); + return ab; + } + internal static AssemblyBuilder PopulateAssemblyBuilderAndSaveMethod(AssemblyName assemblyName, List? assemblyAttributes, Type parameterType, out MethodInfo saveMethod) From 432da885c63f77ecd156f314692070e3ca48e889 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Mon, 2 Oct 2023 17:02:31 -0700 Subject: [PATCH 3/5] Apply feedback, add more test scenarios --- .../System/Reflection/Emit/ILGeneratorImpl.cs | 2 +- .../Reflection/Emit/MethodBuilderImpl.cs | 5 ++ .../Reflection/Emit/ModuleBuilderImpl.cs | 6 +- .../AssemblySaveILGeneratorTests.cs | 65 ++++++++++++++++++- .../AssemblySaveTools.cs | 3 +- 5 files changed, 75 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs index 47fd249c189eae..ab55c23ab47deb 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs @@ -20,6 +20,7 @@ internal sealed class ILGeneratorImpl : ILGenerator internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) { _methodBuilder = methodBuilder; + // For compat, runtime implementation doesn't throw for negative or zero value. _builder = new BlobBuilder(Math.Max(size, DefaultSize)); _il = new InstructionEncoder(_builder, new ControlFlowBuilder()); } @@ -177,7 +178,6 @@ public override void Emit(OpCode opcode, string str) { // Puts the opcode onto the IL stream followed by the metadata token // represented by str. - ModuleBuilder modBuilder = (ModuleBuilder)_methodBuilder.Module; int tempVal = modBuilder.GetStringMetadataToken(str); _il.OpCode((ILOpCode)opcode.Value); diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs index dd1eebcaa875b6..0b0fa27f091e75 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs @@ -125,6 +125,11 @@ protected override ILGenerator GetILGeneratorCore(int size) throw new InvalidOperationException(SR.InvalidOperation_ShouldNotHaveMethodBody); } + if ((_attributes & MethodAttributes.Abstract) != 0) + { + throw new InvalidOperationException(SR.InvalidOperation_ShouldNotHaveMethodBody); + } + return _ilGenerator ??= new ILGeneratorImpl(this, size); } diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs index 81a6ca7691424f..6fc5e57fb4fd0d 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs @@ -186,7 +186,7 @@ private void WriteMethods(TypeBuilderImpl typeBuilder, List throw new NotImplementedException(); public override int GetMethodMetadataToken(ConstructorInfo constructor) => throw new NotImplementedException(); public override int GetMethodMetadataToken(MethodInfo method) => throw new NotImplementedException(); - public override int GetStringMetadataToken(string stringConstant) => throw new NotImplementedException(); + + public override int GetStringMetadataToken(string stringConstant) => MetadataTokens.GetToken(_metadataBuilder.GetOrAddUserString(stringConstant)); + public override int GetTypeMetadataToken(Type type) => throw new NotImplementedException(); protected override void CreateGlobalFunctionsCore() => throw new NotImplementedException(); diff --git a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs index 58d61fb4896b6f..2f4dde63b79fb8 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs @@ -36,7 +36,7 @@ public void MethodWithEmptyBody() [Theory] [InlineData(20)] - [InlineData(-10)] + [InlineData(-10)] // For compat, runtime implementation doesn't throw for negative value. public void MethodReturning_Int(int size) { using (TempFile file = TempFile.Create()) @@ -105,5 +105,68 @@ public void TypeWithTwoMethod_ReferenceMethodArguments(int multiplier) Assert.Equal(OpCodes.Add.Value, addBody[2]); } } + + [Fact] + public void MultipleTypesWithMultipleMethods() + { + using (TempFile file = TempFile.Create()) + { + AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderTypeBuilderAndSaveMethod(out TypeBuilder type, out MethodInfo saveMethod); + MethodBuilder multiplyMethod = type.DefineMethod("MultiplyMethod", MethodAttributes.Public, typeof(short), new Type[] { typeof(short) }); + MethodBuilder addMethod = type.DefineMethod("AddMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(double), new Type[] { typeof(double) }); + + ILGenerator multiplyMethodIL = multiplyMethod.GetILGenerator(); + multiplyMethodIL.Emit(OpCodes.Ldarg, 1); + multiplyMethodIL.Emit(OpCodes.Ldc_I4_S, (short)11); + multiplyMethodIL.Emit(OpCodes.Mul); + multiplyMethodIL.Emit(OpCodes.Ret); + ILGenerator addMethodIL = addMethod.GetILGenerator(); + addMethodIL.Emit(OpCodes.Ldarg_0); + addMethodIL.Emit(OpCodes.Ldc_R8, (double)123456.123); + addMethodIL.Emit(OpCodes.Add); + addMethodIL.Emit(OpCodes.Ret); + + TypeBuilder anotherType = ab.GetDynamicModule("MyModule").DefineType("AnotherType", TypeAttributes.NotPublic); + MethodBuilder stringMethod = anotherType.DefineMethod("StringMethod", MethodAttributes.FamORAssem, typeof(string), Type.EmptyTypes); + MethodBuilder floatMethod = anotherType.DefineMethod("FloatMethod", MethodAttributes.Family, typeof(float), Type.EmptyTypes); + MethodBuilder longMethod = anotherType.DefineMethod("LongMethod", MethodAttributes.Static, typeof(long), Type.EmptyTypes); + + ILGenerator stringMethodIL = stringMethod.GetILGenerator(); + stringMethodIL.Emit(OpCodes.Ldstr, "Hello world!"); + stringMethodIL.Emit(OpCodes.Ret); + ILGenerator floatMethodIL = floatMethod.GetILGenerator(); + floatMethodIL.Emit(OpCodes.Ldc_R4, (float)123456.123); + floatMethodIL.Emit(OpCodes.Ret); + ILGenerator longMethodIL = longMethod.GetILGenerator(); + longMethodIL.Emit(OpCodes.Ldc_I8, (long)1234567); + longMethodIL.Emit(OpCodes.Ret); + + saveMethod.Invoke(ab, new object[] { file.Path }); + + Assembly assemblyFromDisk = AssemblySaveTools.LoadAssemblyFromPath(file.Path); + Module moduleFromFile = assemblyFromDisk.Modules.First(); + Type typeFromDisk = moduleFromFile.GetType("MyType"); + Type anotherTypeFromDisk = moduleFromFile.GetType("AnotherType"); + byte[]? multiplyBody = typeFromDisk.GetMethod("MultiplyMethod").GetMethodBody().GetILAsByteArray(); + byte[]? addBody = typeFromDisk.GetMethod("AddMethod").GetMethodBody().GetILAsByteArray(); + byte[]? stringBody = anotherTypeFromDisk.GetMethod("StringMethod", BindingFlags.NonPublic | BindingFlags.Instance).GetMethodBody().GetILAsByteArray(); + byte[]? floatBody = anotherTypeFromDisk.GetMethod("FloatMethod", BindingFlags.NonPublic | BindingFlags.Instance).GetMethodBody().GetILAsByteArray(); + byte[]? longBody = anotherTypeFromDisk.GetMethod("LongMethod", BindingFlags.NonPublic | BindingFlags.Static).GetMethodBody().GetILAsByteArray(); + + Assert.NotNull(multiplyBody); + Assert.Equal(OpCodes.Ldarg_1.Value, multiplyBody[0]); + Assert.Equal(OpCodes.Ldc_I4_S.Value, multiplyBody[1]); + Assert.Equal(11, multiplyBody[2]); + Assert.NotNull(addBody); + Assert.Equal(OpCodes.Ldarg_0.Value, addBody[0]); + Assert.Equal(OpCodes.Ldc_R8.Value, addBody[1]); + Assert.NotNull(stringBody); + Assert.Equal(OpCodes.Ldstr.Value, stringBody[0]); + Assert.NotNull(floatBody); + Assert.Equal(OpCodes.Ldc_R4.Value, floatBody[0]); + Assert.NotNull(longBody); + Assert.Equal(OpCodes.Ldc_I8.Value, longBody[0]); + } + } } } diff --git a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTools.cs b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTools.cs index a35d96fa33c84f..e756c8c4cf7cd3 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTools.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTools.cs @@ -68,8 +68,7 @@ internal static AssemblyBuilder PopulateAssemblyBuilderTypeBuilderAndSaveMethod( typeBuilder = ab.DefineDynamicModule("MyModule").DefineType("MyType", TypeAttributes.Public); return ab; } - - + internal static AssemblyBuilder PopulateAssemblyBuilderAndSaveMethod(AssemblyName assemblyName, List? assemblyAttributes, Type parameterType, out MethodInfo saveMethod) { From a478204cd80dd2dd8a8d91665651dfc7f4ec5b00 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Tue, 3 Oct 2023 11:24:37 -0700 Subject: [PATCH 4/5] Temporarily count the maxstack depth for each OpCode --- .../src/System/Reflection/Emit/ILGeneratorImpl.cs | 7 +++++++ .../src/System/Reflection/Emit/ModuleBuilderImpl.cs | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs index ab55c23ab47deb..9f4b9673794a25 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs @@ -16,6 +16,7 @@ internal sealed class ILGeneratorImpl : ILGenerator private readonly BlobBuilder _builder; private readonly InstructionEncoder _il; private bool _hasDynamicStackAllocation; + private int _maxStackSize; internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) { @@ -25,6 +26,8 @@ internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) _il = new InstructionEncoder(_builder, new ControlFlowBuilder()); } + internal int GetMaxStackSize() => _maxStackSize; + internal InstructionEncoder Instructions => _il; internal bool HasDynamicStackAllocation => _hasDynamicStackAllocation; @@ -46,6 +49,10 @@ public override void Emit(OpCode opcode) _hasDynamicStackAllocation = true; } _il.OpCode((ILOpCode)opcode.Value); + + // TODO for now only count the Opcodes emitted, in order to correctly we might need to make below API public + // https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/Opcode.cs#L48 + _maxStackSize++; } public override void Emit(OpCode opcode, byte arg) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs index 6fc5e57fb4fd0d..c30398046ab60e 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs @@ -240,7 +240,7 @@ private void WriteMethods(TypeBuilderImpl typeBuilder, List methodBodyEncoder.AddMethodBody( instructionEncoder: il.Instructions, - maxStack: 8, // TODO + maxStack: il.GetMaxStackSize(), localVariablesSignature: default, // TODO attributes: method.InitLocals ? MethodBodyAttributes.InitLocals : MethodBodyAttributes.None, hasDynamicStackAllocation: il.HasDynamicStackAllocation); From 103fd3e286427c8e1fc3dcd46d6b0debc3afd0eb Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Tue, 3 Oct 2023 14:39:08 -0700 Subject: [PATCH 5/5] Update src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs --- .../src/System/Reflection/Emit/ILGeneratorImpl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs index 9f4b9673794a25..7b61112ec8bf7a 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs @@ -50,7 +50,7 @@ public override void Emit(OpCode opcode) } _il.OpCode((ILOpCode)opcode.Value); - // TODO for now only count the Opcodes emitted, in order to correctly we might need to make below API public + // TODO: for now only count the Opcodes emitted, in order to calculate it correctly we might need to make internal Opcode APIs public // https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/Opcode.cs#L48 _maxStackSize++; }