From 02b968315037a5a8a9d291ba4d065e70118f1661 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 5 Oct 2023 22:07:23 -0700 Subject: [PATCH 01/14] Add initial MaxStack calculation and tests --- .../System/Reflection/Emit/ILGeneratorImpl.cs | 126 ++++++++++++------ .../AssemblySaveILGeneratorTests.cs | 78 ++++++++++- 2 files changed, 160 insertions(+), 44 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 7b61112ec8bf7a..d9cd6138e6abf5 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 @@ -1,10 +1,8 @@ // 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 @@ -17,6 +15,7 @@ internal sealed class ILGeneratorImpl : ILGenerator private readonly InstructionEncoder _il; private bool _hasDynamicStackAllocation; private int _maxStackSize; + private int _currentStack; internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) { @@ -42,40 +41,88 @@ internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) public override LocalBuilder DeclareLocal(Type localType, bool pinned) => throw new NotImplementedException(); public override Label DefineLabel() => throw new NotImplementedException(); - public override void Emit(OpCode opcode) + private static int GetStackChangeFor(StackBehaviour stackBehaviour) => + stackBehaviour switch + { + StackBehaviour.Pop0 or + StackBehaviour.Push0 => 0, + StackBehaviour.Pop1 or + StackBehaviour.Popi or + StackBehaviour.Popref or + StackBehaviour.Varpop => -1, + StackBehaviour.Pop1_pop1 or + StackBehaviour.Popi_pop1 or + StackBehaviour.Popi_popi or + StackBehaviour.Popi_popi8 or + StackBehaviour.Popi_popr4 or + StackBehaviour.Popi_popr8 or + StackBehaviour.Popref_pop1 or + StackBehaviour.Popref_popi => -2, + StackBehaviour.Popi_popi_popi or + StackBehaviour.Popref_popi_pop1 or + StackBehaviour.Popref_popi_popi or + StackBehaviour.Popref_popi_popi8 or + StackBehaviour.Popref_popi_popr4 or + StackBehaviour.Popref_popi_popr8 or + StackBehaviour.Popref_popi_popref => -3, + StackBehaviour.Push1 or + StackBehaviour.Pushi or + StackBehaviour.Pushi8 or + StackBehaviour.Pushr4 or + StackBehaviour.Pushr8 or + StackBehaviour.Pushref or + StackBehaviour.Varpush => 1, + StackBehaviour.Push1_push1 => 2, + _ => throw new InvalidOperationException() + }; + + private void UpdateStackSize(OpCode opCode) + { + _currentStack += GetStackChangeFor(opCode.StackBehaviourPush) + GetStackChangeFor(opCode.StackBehaviourPop); + + if (_currentStack > _maxStackSize) + { + _maxStackSize = _currentStack; + } + } + + public void EmitOpcode(OpCode opcode) { if (opcode == OpCodes.Localloc) { _hasDynamicStackAllocation = true; } + _il.OpCode((ILOpCode)opcode.Value); + UpdateStackSize(opcode); + } - // 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++; + public override void Emit(OpCode opcode) + { + EmitOpcode(opcode); } public override void Emit(OpCode opcode, byte arg) { - _il.OpCode((ILOpCode)opcode.Value); + EmitOpcode(opcode); _builder.WriteByte(arg); } public override void Emit(OpCode opcode, double arg) { - _il.OpCode((ILOpCode)opcode.Value); + EmitOpcode(opcode); _builder.WriteDouble(arg); } public override void Emit(OpCode opcode, float arg) { - _il.OpCode((ILOpCode)opcode.Value); + EmitOpcode(opcode); _builder.WriteSingle(arg); } public override void Emit(OpCode opcode, short arg) { - _il.OpCode((ILOpCode)opcode.Value); + EmitOpcode(opcode); _builder.WriteInt16(arg); } @@ -86,26 +133,25 @@ public override void Emit(OpCode opcode, int arg) { if (arg >= -1 && arg <= 8) { - _il.OpCode(arg switch + EmitOpcode(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, + -1 => OpCodes.Ldc_I4_M1, + 0 => OpCodes.Ldc_I4_0, + 1 => OpCodes.Ldc_I4_1, + 2 => OpCodes.Ldc_I4_2, + 3 => OpCodes.Ldc_I4_3, + 4 => OpCodes.Ldc_I4_4, + 5 => OpCodes.Ldc_I4_5, + 6 => OpCodes.Ldc_I4_6, + 7 => OpCodes.Ldc_I4_7, + _ => OpCodes.Ldc_I4_8 }); return; } if (arg >= -128 && arg <= 127) { - _il.OpCode(ILOpCode.Ldc_i4_s); - _builder.WriteSByte((sbyte)arg) ; + Emit(OpCodes.Ldc_I4_S, (sbyte)arg); return; } } @@ -113,27 +159,25 @@ public override void Emit(OpCode opcode, int arg) { if ((uint)arg <= 3) { - _il.OpCode(arg switch + EmitOpcode(arg switch { - 0 => ILOpCode.Ldarg_0, - 1 => ILOpCode.Ldarg_1, - 2 => ILOpCode.Ldarg_2, - _ => ILOpCode.Ldarg_3, + 0 => OpCodes.Ldarg_0, + 1 => OpCodes.Ldarg_1, + 2 => OpCodes.Ldarg_2, + _ => OpCodes.Ldarg_3, }); return; } if ((uint)arg <= byte.MaxValue) { - _il.OpCode(ILOpCode.Ldarg_s); - _builder.WriteByte((byte)arg); + Emit(OpCodes.Ldarg_S, (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); + Emit(OpCodes.Ldarg, (short)arg); return; } } @@ -141,15 +185,13 @@ public override void Emit(OpCode opcode, int arg) { if ((uint)arg <= byte.MaxValue) { - _il.OpCode(ILOpCode.Ldarga_s); - _builder.WriteByte((byte)arg); + Emit(OpCodes.Ldarga_S, (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); + Emit(OpCodes.Ldarga, (short)arg); return; } } @@ -157,27 +199,25 @@ public override void Emit(OpCode opcode, int arg) { if ((uint)arg <= byte.MaxValue) { - _il.OpCode(ILOpCode.Starg_s); - _builder.WriteByte((byte)arg); + Emit(OpCodes.Starg_S, (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); + Emit(OpCodes.Starg, (short)arg); return; } } // For everything else, put the opcode followed by the arg onto the stream of instructions. - _il.OpCode((ILOpCode)opcode.Value); + EmitOpcode(opcode); _builder.WriteInt32(arg); } public override void Emit(OpCode opcode, long arg) { - _il.OpCode((ILOpCode)opcode.Value); + EmitOpcode(opcode); _il.CodeBuilder.WriteInt64(arg); } @@ -187,7 +227,7 @@ public override void Emit(OpCode opcode, string str) // represented by str. ModuleBuilder modBuilder = (ModuleBuilder)_methodBuilder.Module; int tempVal = modBuilder.GetStringMetadataToken(str); - _il.OpCode((ILOpCode)opcode.Value); + EmitOpcode(opcode); _il.Token(tempVal); } diff --git a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs index 2f4dde63b79fb8..e9d1560991ab9c 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs @@ -168,5 +168,81 @@ public void MultipleTypesWithMultipleMethods() Assert.Equal(OpCodes.Ldc_I8.Value, longBody[0]); } } + + [Fact] + public void ILOffset_Test() + { + AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderTypeBuilderAndSaveMethod(out TypeBuilder type, out MethodInfo saveMethod); + MethodBuilder method = type.DefineMethod("Method1", MethodAttributes.Public | MethodAttributes.Static, typeof(Type), new Type[0]); + ILGenerator ilGenerator = method.GetILGenerator(); + + Assert.Equal(0, ilGenerator.ILOffset); + ilGenerator.Emit(OpCodes.Ret); + Assert.Equal(1, ilGenerator.ILOffset); + } + + [Fact] + public void ILMaxStack_Test() + { + using (TempFile file = TempFile.Create()) + { + AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderTypeBuilderAndSaveMethod(out TypeBuilder type, out MethodInfo saveMethod); + MethodBuilder method1 = type.DefineMethod("Method1", MethodAttributes.Public, typeof(long), new Type[] { typeof(int), typeof(long), typeof(short), typeof(byte) }); + ILGenerator il1 = method1.GetILGenerator(); + + // public int Method1(int x, int y, short z, byte r) => + // x + (z + 2 * (r + (8 * y + 3 * (y - (5 + x)))) + il1.Emit(OpCodes.Ldarg_1); // push 1 MaxStack 1 + il1.Emit(OpCodes.Ldarg_3); // push 1 MaxStack 2 + il1.Emit(OpCodes.Ldc_I4_2); // push 1 MaxStack 3 + il1.Emit(OpCodes.Ldarg_S, 4); // push 1 MaxStack 4 + il1.Emit(OpCodes.Ldc_I4_8); // push 1 MaxStack 5 + il1.Emit(OpCodes.Ldarg_2); // push 1 MaxStack 6 + il1.Emit(OpCodes.Mul); // pop 2 push 1 MaxStack 5 + il1.Emit(OpCodes.Ldc_I4_3); // push 1 MaxStack 6 + il1.Emit(OpCodes.Ldarg_2); // push 1 MaxStack 7 + il1.Emit(OpCodes.Ldc_I4_5); // push 1 MaxStack 8 + il1.Emit(OpCodes.Ldarg_1); // push 1 MaxStack 9 + il1.Emit(OpCodes.Add); // pop 2 push 1 stack size 8 + il1.Emit(OpCodes.Sub); // pop 2 push 1 stack size 7 + il1.Emit(OpCodes.Mul); // pop 2 push 1 stack size 6 + il1.Emit(OpCodes.Add); // pop 2 push 1 stack size 5 + il1.Emit(OpCodes.Add); // pop 2 push 1 stack size 4 + il1.Emit(OpCodes.Mul); // pop 2 push 1 stack size 3 + il1.Emit(OpCodes.Add); // pop 2 push 1 stack size 2 + il1.Emit(OpCodes.Add); // pop 2 push 1 stack size 1 + il1.Emit(OpCodes.Ret); // pop 1 stack size 0 + + MethodBuilder method2 = type.DefineMethod("Method2", MethodAttributes.Public, typeof(int), new Type[] { typeof(int), typeof(byte) }); + ILGenerator il2 = method2.GetILGenerator(); + + // int Method2(int x, int y) => x + (y + 18); + il2.Emit(OpCodes.Ldarg_1); // push 1 MaxStack 1 + il2.Emit(OpCodes.Ldarg_2); // push 1 MaxStack 2 + il2.Emit(OpCodes.Ldc_I4_S, 8); // push 1 MaxStack 3 + il2.Emit(OpCodes.Add); // pop 2 push 1 stack size 2 + il2.Emit(OpCodes.Add); // pop 2 push 1 stack size 1 + il2.Emit(OpCodes.Ret); // pop 1 stack size 0 + + saveMethod.Invoke(ab, new object[] { file.Path }); + + MethodInfo getMaxStackSizeMethod = LoadILGenerator_GetMaxStackSizeMethod(); + Assert.Equal(9, getMaxStackSizeMethod.Invoke(il1, new object[0])); + Assert.Equal(3, getMaxStackSizeMethod.Invoke(il2, new object[0])); + + Assembly assemblyFromDisk = AssemblySaveTools.LoadAssemblyFromPath(file.Path); + Type typeFromDisk = assemblyFromDisk.Modules.First().GetType("MyType"); + MethodBody body1 = typeFromDisk.GetMethod("Method1").GetMethodBody(); + MethodBody body2 = typeFromDisk.GetMethod("Method2").GetMethodBody(); + Assert.Equal(9, body1.MaxStackSize); + Assert.Equal(8, body2.MaxStackSize); // apparently doesn't write lower than 8 + } + } + + private MethodInfo LoadILGenerator_GetMaxStackSizeMethod() + { + Type ilgType = Type.GetType("System.Reflection.Emit.ILGeneratorImpl, System.Reflection.Emit", throwOnError: true)!; + return ilgType.GetMethod("GetMaxStackSize", BindingFlags.NonPublic | BindingFlags.Instance, Type.EmptyTypes); + } } -} +} From 49c69adba3180b4969d64f24631ad58d09ba1c35 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Mon, 9 Oct 2023 11:23:23 -0700 Subject: [PATCH 02/14] Apply suggestions from code review Co-authored-by: Aaron Robinson --- .../System/Reflection/Emit/ILGeneratorImpl.cs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 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 d9cd6138e6abf5..b33c37f914c285 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 @@ -46,10 +46,10 @@ private static int GetStackChangeFor(StackBehaviour stackBehaviour) => { StackBehaviour.Pop0 or StackBehaviour.Push0 => 0, - StackBehaviour.Pop1 or - StackBehaviour.Popi or - StackBehaviour.Popref or - StackBehaviour.Varpop => -1, + StackBehaviour.Pop1 + or StackBehaviour.Popi + or StackBehaviour.Popref + or StackBehaviour.Varpop => -1, StackBehaviour.Pop1_pop1 or StackBehaviour.Popi_pop1 or StackBehaviour.Popi_popi or @@ -80,10 +80,7 @@ private void UpdateStackSize(OpCode opCode) { _currentStack += GetStackChangeFor(opCode.StackBehaviourPush) + GetStackChangeFor(opCode.StackBehaviourPop); - if (_currentStack > _maxStackSize) - { - _maxStackSize = _currentStack; - } + _maxStackSize = Math.Max(_maxStackSize, _currentStack); } public void EmitOpcode(OpCode opcode) @@ -97,10 +94,7 @@ public void EmitOpcode(OpCode opcode) UpdateStackSize(opcode); } - public override void Emit(OpCode opcode) - { - EmitOpcode(opcode); - } + public override void Emit(OpCode opcode) => EmitOpcode(opcode); public override void Emit(OpCode opcode, byte arg) { From 06be81da2da780eb96bc7999653bfed01ecf8225 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Mon, 9 Oct 2023 13:43:48 -0700 Subject: [PATCH 03/14] Update src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ILGeneratorImpl.cs Co-authored-by: Aaron Robinson --- .../System/Reflection/Emit/ILGeneratorImpl.cs | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 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 b33c37f914c285..d1d4b8490e879e 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 @@ -44,34 +44,34 @@ internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) private static int GetStackChangeFor(StackBehaviour stackBehaviour) => stackBehaviour switch { - StackBehaviour.Pop0 or - StackBehaviour.Push0 => 0, + StackBehaviour.Pop0 + or StackBehaviour.Push0 => 0, StackBehaviour.Pop1 or StackBehaviour.Popi or StackBehaviour.Popref or StackBehaviour.Varpop => -1, - StackBehaviour.Pop1_pop1 or - StackBehaviour.Popi_pop1 or - StackBehaviour.Popi_popi or - StackBehaviour.Popi_popi8 or - StackBehaviour.Popi_popr4 or - StackBehaviour.Popi_popr8 or - StackBehaviour.Popref_pop1 or - StackBehaviour.Popref_popi => -2, - StackBehaviour.Popi_popi_popi or - StackBehaviour.Popref_popi_pop1 or - StackBehaviour.Popref_popi_popi or - StackBehaviour.Popref_popi_popi8 or - StackBehaviour.Popref_popi_popr4 or - StackBehaviour.Popref_popi_popr8 or - StackBehaviour.Popref_popi_popref => -3, - StackBehaviour.Push1 or - StackBehaviour.Pushi or - StackBehaviour.Pushi8 or - StackBehaviour.Pushr4 or - StackBehaviour.Pushr8 or - StackBehaviour.Pushref or - StackBehaviour.Varpush => 1, + StackBehaviour.Pop1_pop1 + or StackBehaviour.Popi_pop1 + or StackBehaviour.Popi_popi + or StackBehaviour.Popi_popi8 + or StackBehaviour.Popi_popr4 + or StackBehaviour.Popi_popr8 + or StackBehaviour.Popref_pop1 + or StackBehaviour.Popref_popi => -2, + StackBehaviour.Popi_popi_popi + or StackBehaviour.Popref_popi_pop1 + or StackBehaviour.Popref_popi_popi + or StackBehaviour.Popref_popi_popi8 + or StackBehaviour.Popref_popi_popr4 + or StackBehaviour.Popref_popi_popr8 + or StackBehaviour.Popref_popi_popref => -3, + StackBehaviour.Push1 + or StackBehaviour.Pushi + or StackBehaviour.Pushi8 + or StackBehaviour.Pushr4 + or StackBehaviour.Pushr8 + or StackBehaviour.Pushref + or StackBehaviour.Varpush => 1, StackBehaviour.Push1_push1 => 2, _ => throw new InvalidOperationException() }; From 90cdba858b3bbfedc7ba768a2c9c7e2334bad987 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Mon, 9 Oct 2023 13:44:53 -0700 Subject: [PATCH 04/14] Remove space --- .../PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs index e9d1560991ab9c..97ec018b0e7f4d 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs @@ -245,4 +245,4 @@ private MethodInfo LoadILGenerator_GetMaxStackSizeMethod() return ilgType.GetMethod("GetMaxStackSize", BindingFlags.NonPublic | BindingFlags.Instance, Type.EmptyTypes); } } -} +} From f94e244871c6e651eb98a1215508178320002d9e Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Tue, 10 Oct 2023 09:31:46 -0700 Subject: [PATCH 05/14] Add new line between each switch case groups --- .../src/System/Reflection/Emit/ILGeneratorImpl.cs | 6 ++++++ 1 file changed, 6 insertions(+) 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 d1d4b8490e879e..0c79df25f23fff 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 @@ -46,10 +46,12 @@ private static int GetStackChangeFor(StackBehaviour stackBehaviour) => { StackBehaviour.Pop0 or StackBehaviour.Push0 => 0, + StackBehaviour.Pop1 or StackBehaviour.Popi or StackBehaviour.Popref or StackBehaviour.Varpop => -1, + StackBehaviour.Pop1_pop1 or StackBehaviour.Popi_pop1 or StackBehaviour.Popi_popi @@ -58,6 +60,7 @@ or StackBehaviour.Popi_popr4 or StackBehaviour.Popi_popr8 or StackBehaviour.Popref_pop1 or StackBehaviour.Popref_popi => -2, + StackBehaviour.Popi_popi_popi or StackBehaviour.Popref_popi_pop1 or StackBehaviour.Popref_popi_popi @@ -65,6 +68,7 @@ or StackBehaviour.Popref_popi_popi8 or StackBehaviour.Popref_popi_popr4 or StackBehaviour.Popref_popi_popr8 or StackBehaviour.Popref_popi_popref => -3, + StackBehaviour.Push1 or StackBehaviour.Pushi or StackBehaviour.Pushi8 @@ -72,7 +76,9 @@ or StackBehaviour.Pushr4 or StackBehaviour.Pushr8 or StackBehaviour.Pushref or StackBehaviour.Varpush => 1, + StackBehaviour.Push1_push1 => 2, + _ => throw new InvalidOperationException() }; From ee3be835d7adc41e2b9c16b9527b7097f5909df0 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Tue, 10 Oct 2023 11:46:37 -0700 Subject: [PATCH 06/14] Remove spaces --- .../src/System/Reflection/Emit/ILGeneratorImpl.cs | 12 ++++++------ 1 file changed, 6 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 0c79df25f23fff..c829841ca5693d 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 @@ -46,12 +46,12 @@ private static int GetStackChangeFor(StackBehaviour stackBehaviour) => { StackBehaviour.Pop0 or StackBehaviour.Push0 => 0, - + StackBehaviour.Pop1 or StackBehaviour.Popi or StackBehaviour.Popref or StackBehaviour.Varpop => -1, - + StackBehaviour.Pop1_pop1 or StackBehaviour.Popi_pop1 or StackBehaviour.Popi_popi @@ -60,7 +60,7 @@ or StackBehaviour.Popi_popr4 or StackBehaviour.Popi_popr8 or StackBehaviour.Popref_pop1 or StackBehaviour.Popref_popi => -2, - + StackBehaviour.Popi_popi_popi or StackBehaviour.Popref_popi_pop1 or StackBehaviour.Popref_popi_popi @@ -68,7 +68,7 @@ or StackBehaviour.Popref_popi_popi8 or StackBehaviour.Popref_popi_popr4 or StackBehaviour.Popref_popi_popr8 or StackBehaviour.Popref_popi_popref => -3, - + StackBehaviour.Push1 or StackBehaviour.Pushi or StackBehaviour.Pushi8 @@ -76,9 +76,9 @@ or StackBehaviour.Pushr4 or StackBehaviour.Pushr8 or StackBehaviour.Pushref or StackBehaviour.Varpush => 1, - + StackBehaviour.Push1_push1 => 2, - + _ => throw new InvalidOperationException() }; From ae7f502dc38fab5d4373911852d871bacf7025a2 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 12 Oct 2023 15:05:59 -0700 Subject: [PATCH 07/14] Make OpCode.StackChange() public and use the method --- .../Reflection/Emit/RuntimeILGenerator.cs | 2 +- .../src/System/Reflection/Emit/Opcode.cs | 2 +- .../System/Reflection/Emit/ILGeneratorImpl.cs | 44 +------------------ .../ref/System.Reflection.Primitives.cs | 1 + 4 files changed, 4 insertions(+), 45 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs index 6cf211e713e383..2e58c7447e9ca2 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs @@ -128,7 +128,7 @@ internal void InternalEmit(OpCode opcode) m_ILStream[m_length++] = (byte)opcodeValue; } - UpdateStackSize(opcode, opcode.StackChange()); + UpdateStackSize(opcode, opcode.StackDifference()); } [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/Opcode.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/Opcode.cs index e9fdc1ec38c9ad..72342fc27e61a9 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/Opcode.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/Opcode.cs @@ -45,7 +45,7 @@ internal OpCode(OpCodeValues value, int flags) internal bool EndsUncondJmpBlk() => (m_flags & EndsUncondJmpBlkFlag) != 0; - internal int StackChange() => + public int StackDifference() => m_flags >> StackChangeShift; public OperandType OperandType => (OperandType)(m_flags & OperandTypeMask); 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 c829841ca5693d..bbb41113d27334 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 @@ -41,51 +41,9 @@ internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) public override LocalBuilder DeclareLocal(Type localType, bool pinned) => throw new NotImplementedException(); public override Label DefineLabel() => throw new NotImplementedException(); - private static int GetStackChangeFor(StackBehaviour stackBehaviour) => - stackBehaviour switch - { - StackBehaviour.Pop0 - or StackBehaviour.Push0 => 0, - - StackBehaviour.Pop1 - or StackBehaviour.Popi - or StackBehaviour.Popref - or StackBehaviour.Varpop => -1, - - StackBehaviour.Pop1_pop1 - or StackBehaviour.Popi_pop1 - or StackBehaviour.Popi_popi - or StackBehaviour.Popi_popi8 - or StackBehaviour.Popi_popr4 - or StackBehaviour.Popi_popr8 - or StackBehaviour.Popref_pop1 - or StackBehaviour.Popref_popi => -2, - - StackBehaviour.Popi_popi_popi - or StackBehaviour.Popref_popi_pop1 - or StackBehaviour.Popref_popi_popi - or StackBehaviour.Popref_popi_popi8 - or StackBehaviour.Popref_popi_popr4 - or StackBehaviour.Popref_popi_popr8 - or StackBehaviour.Popref_popi_popref => -3, - - StackBehaviour.Push1 - or StackBehaviour.Pushi - or StackBehaviour.Pushi8 - or StackBehaviour.Pushr4 - or StackBehaviour.Pushr8 - or StackBehaviour.Pushref - or StackBehaviour.Varpush => 1, - - StackBehaviour.Push1_push1 => 2, - - _ => throw new InvalidOperationException() - }; - private void UpdateStackSize(OpCode opCode) { - _currentStack += GetStackChangeFor(opCode.StackBehaviourPush) + GetStackChangeFor(opCode.StackBehaviourPop); - + _currentStack += opCode.StackDifference(); _maxStackSize = Math.Max(_maxStackSize, _currentStack); } diff --git a/src/libraries/System.Reflection.Primitives/ref/System.Reflection.Primitives.cs b/src/libraries/System.Reflection.Primitives/ref/System.Reflection.Primitives.cs index 2bd85772fef7bf..d8c0a9de6d6743 100644 --- a/src/libraries/System.Reflection.Primitives/ref/System.Reflection.Primitives.cs +++ b/src/libraries/System.Reflection.Primitives/ref/System.Reflection.Primitives.cs @@ -33,6 +33,7 @@ public enum FlowControl public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; } public bool Equals(System.Reflection.Emit.OpCode obj) { throw null; } public override int GetHashCode() { throw null; } + public int StackDifference() { throw null; } public static bool operator ==(System.Reflection.Emit.OpCode a, System.Reflection.Emit.OpCode b) { throw null; } public static bool operator !=(System.Reflection.Emit.OpCode a, System.Reflection.Emit.OpCode b) { throw null; } public override string? ToString() { throw null; } From 22108d23b78adbbf3c96058944ef416839526994 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Fri, 20 Oct 2023 17:12:49 -0700 Subject: [PATCH 08/14] Abstract LocalBuilder, emit LocalBuilder in ILGenerator --- .../System.Private.CoreLib.csproj | 2 +- .../Reflection/Emit/DynamicILGenerator.cs | 4 +- .../Reflection/Emit/RuntimeILGenerator.cs | 6 +- ...LocalBuilder.cs => RuntimeLocalBuilder.cs} | 18 +- .../src/System.Private.CoreLib.csproj | 1 - .../System/Reflection/Emit/LocalBuilder.cs | 37 ---- .../System.Private.CoreLib.Shared.projitems | 5 +- .../System/Reflection/Emit/LocalBuilder.cs | 22 +++ .../System.Reflection.Emit.ILGeneration.cs | 5 +- .../tests/LocalBuilder/LocalBuilderMethod.cs | 45 +++++ ....Reflection.Emit.ILGeneration.Tests.csproj | 1 + .../src/Resources/Strings.resx | 3 + .../src/System.Reflection.Emit.csproj | 1 + .../System/Reflection/Emit/ILGeneratorImpl.cs | 45 ++++- .../Reflection/Emit/LocalBuilderImpl.cs | 32 ++++ .../Reflection/Emit/ModuleBuilderImpl.cs | 10 +- .../System/Reflection/Emit/SignatureHelper.cs | 15 ++ .../AssemblySaveILGeneratorTests.cs | 173 ++++++++++++++++++ .../System.Private.CoreLib.csproj | 2 +- .../Emit/RuntimeILGenerator.Mono.cs | 13 +- ...er.Mono.cs => RuntimeLocalBuilder.Mono.cs} | 9 +- 21 files changed, 371 insertions(+), 78 deletions(-) rename src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/{LocalBuilder.cs => RuntimeLocalBuilder.cs} (65%) delete mode 100644 src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs create mode 100644 src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs create mode 100644 src/libraries/System.Reflection.Emit.ILGeneration/tests/LocalBuilder/LocalBuilderMethod.cs create mode 100644 src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/LocalBuilderImpl.cs rename src/mono/System.Private.CoreLib/src/System/Reflection/Emit/{LocalBuilder.Mono.cs => RuntimeLocalBuilder.Mono.cs} (87%) diff --git a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj index 20073739b2bbd0..377157b7091670 100644 --- a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -162,7 +162,6 @@ - @@ -170,6 +169,7 @@ + diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicILGenerator.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicILGenerator.cs index 81372c76ce2ec8..0c920d1ea12bdc 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicILGenerator.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicILGenerator.cs @@ -36,14 +36,14 @@ public override LocalBuilder DeclareLocal(Type localType, bool pinned) { ArgumentNullException.ThrowIfNull(localType); - LocalBuilder localBuilder; + RuntimeLocalBuilder localBuilder; RuntimeType? rtType = localType as RuntimeType; if (rtType == null) throw new ArgumentException(SR.Argument_MustBeRuntimeType); - localBuilder = new LocalBuilder(m_localCount, localType, m_methodBuilder, pinned); + localBuilder = new RuntimeLocalBuilder(m_localCount, localType, m_methodBuilder, pinned); // add the localType to local signature m_localSignature.AddArgument(localType, pinned); m_localCount++; diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs index 2e58c7447e9ca2..9d720e6bc14144 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs @@ -819,8 +819,8 @@ public override void Emit(OpCode opcode, LocalBuilder local) ArgumentNullException.ThrowIfNull(local); // Puts the opcode onto the IL stream followed by the information for local variable local. - int tempVal = local.GetLocalIndex(); - if (local.GetMethodBuilder() != m_methodBuilder) + int tempVal = local.LocalIndex; + if (local.Method != m_methodBuilder) { throw new ArgumentException(SR.Argument_UnmatchedMethodForLocal, nameof(local)); } @@ -1185,7 +1185,7 @@ public override LocalBuilder DeclareLocal(Type localType, bool pinned) // add the localType to local signature m_localSignature.AddArgument(localType, pinned); - return new LocalBuilder(m_localCount++, localType, methodBuilder, pinned); + return new RuntimeLocalBuilder(m_localCount++, localType, methodBuilder, pinned); } public override void UsingNamespace(string usingNamespace) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeLocalBuilder.cs similarity index 65% rename from src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs rename to src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeLocalBuilder.cs index 9dd4d6d5c2cfd9..3a78bac31e44cb 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeLocalBuilder.cs @@ -3,7 +3,7 @@ namespace System.Reflection.Emit { - public sealed class LocalBuilder : LocalVariableInfo + internal sealed class RuntimeLocalBuilder : LocalBuilder { #region Private Data Members private readonly int m_localIndex; @@ -13,9 +13,9 @@ public sealed class LocalBuilder : LocalVariableInfo #endregion #region Constructor - internal LocalBuilder(int localIndex, Type localType, MethodInfo methodBuilder) + internal RuntimeLocalBuilder(int localIndex, Type localType, MethodInfo methodBuilder) : this(localIndex, localType, methodBuilder, false) { } - internal LocalBuilder(int localIndex, Type localType, MethodInfo methodBuilder, bool isPinned) + internal RuntimeLocalBuilder(int localIndex, Type localType, MethodInfo methodBuilder, bool isPinned) { m_isPinned = isPinned; m_localIndex = localIndex; @@ -24,21 +24,11 @@ internal LocalBuilder(int localIndex, Type localType, MethodInfo methodBuilder, } #endregion - #region Internal Members - internal int GetLocalIndex() - { - return m_localIndex; - } - internal MethodInfo GetMethodBuilder() - { - return m_methodBuilder; - } - #endregion - #region LocalVariableInfo Override public override bool IsPinned => m_isPinned; public override Type LocalType => m_localType; public override int LocalIndex => m_localIndex; + public override MethodInfo Method => m_methodBuilder; #endregion } } diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj b/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj index 8f45dd1a4d5b92..a0b87bacc1bc73 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj @@ -144,7 +144,6 @@ - diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs deleted file mode 100644 index 2d1afc04d5b730..00000000000000 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs +++ /dev/null @@ -1,37 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.Reflection.Emit -{ - public sealed class LocalBuilder : LocalVariableInfo - { - internal LocalBuilder() - { - // Prevent generating a default constructor - } - - public override bool IsPinned - { - get - { - return default; - } - } - - public override int LocalIndex - { - get - { - return default; - } - } - - public override Type LocalType - { - get - { - return default; - } - } - } -} diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 2b774a2f4fd550..670238c1534d73 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -416,7 +416,6 @@ - @@ -672,7 +671,9 @@ + + @@ -2719,4 +2720,4 @@ - + \ No newline at end of file diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs new file mode 100644 index 00000000000000..2bb2565fb9f7ed --- /dev/null +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs @@ -0,0 +1,22 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Reflection.Emit +{ + public abstract class LocalBuilder : LocalVariableInfo + { + /// + /// Initializes a new instance of the class. + /// + /// + /// This constructor is invoked by derived classes. + /// + protected LocalBuilder() { } + + /// + /// Returns the method where the local variable declared. + /// + /// Can be used for validating the containing method when referencing the local. + public abstract MethodInfo Method { get; } + } +} diff --git a/src/libraries/System.Reflection.Emit.ILGeneration/ref/System.Reflection.Emit.ILGeneration.cs b/src/libraries/System.Reflection.Emit.ILGeneration/ref/System.Reflection.Emit.ILGeneration.cs index 93f73136d55ace..746b9924bcd689 100644 --- a/src/libraries/System.Reflection.Emit.ILGeneration/ref/System.Reflection.Emit.ILGeneration.cs +++ b/src/libraries/System.Reflection.Emit.ILGeneration/ref/System.Reflection.Emit.ILGeneration.cs @@ -65,12 +65,13 @@ public virtual void ThrowException([System.Diagnostics.CodeAnalysis.DynamicallyA public static bool operator ==(System.Reflection.Emit.Label a, System.Reflection.Emit.Label b) { throw null; } public static bool operator !=(System.Reflection.Emit.Label a, System.Reflection.Emit.Label b) { throw null; } } - public sealed partial class LocalBuilder : System.Reflection.LocalVariableInfo + public abstract class LocalBuilder : System.Reflection.LocalVariableInfo { - internal LocalBuilder() { } + protected LocalBuilder() { } public override bool IsPinned { get { throw null; } } public override int LocalIndex { get { throw null; } } public override System.Type LocalType { get { throw null; } } + public abstract MethodInfo Method { get; } } public abstract partial class ParameterBuilder { diff --git a/src/libraries/System.Reflection.Emit.ILGeneration/tests/LocalBuilder/LocalBuilderMethod.cs b/src/libraries/System.Reflection.Emit.ILGeneration/tests/LocalBuilder/LocalBuilderMethod.cs new file mode 100644 index 00000000000000..4e4b28d3ead74c --- /dev/null +++ b/src/libraries/System.Reflection.Emit.ILGeneration/tests/LocalBuilder/LocalBuilderMethod.cs @@ -0,0 +1,45 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Xunit; + +namespace System.Reflection.Emit.Tests +{ + // Mono doesn't support LocalBuilder.Method + [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.IsNotMonoRuntime))] + public class LocalBuilderMethod + { + [Fact] + public void LocalBuilder_MethodReturnsCorrectMethod() + { + TypeBuilder type = Helpers.DynamicType(TypeAttributes.NotPublic); + MethodBuilder method1 = type.DefineMethod("Method1", MethodAttributes.Public); + MethodBuilder method2 = type.DefineMethod("Method2", MethodAttributes.Public); + ILGenerator ilGenerator1 = method1.GetILGenerator(); + ILGenerator ilGenerator2 = method2.GetILGenerator(); + LocalBuilder local1 = ilGenerator1.DeclareLocal(typeof(int)); + LocalBuilder local2 = ilGenerator2.DeclareLocal(typeof(string), true); + + Assert.Equal(method1, local1.Method); + Assert.Equal(method2, local2.Method); + } + + [Fact] + public void LocalBuilder_UseLocalsThatBelongsToTheMethod() + { + TypeBuilder type = Helpers.DynamicType(TypeAttributes.NotPublic); + MethodBuilder method1 = type.DefineMethod("Method1", MethodAttributes.Public); + MethodBuilder method2 = type.DefineMethod("Method2", MethodAttributes.Public); + ILGenerator ilGenerator1 = method1.GetILGenerator(); + ILGenerator ilGenerator2 = method2.GetILGenerator(); + LocalBuilder local1 = ilGenerator1.DeclareLocal(typeof(short)); + LocalBuilder local2 = ilGenerator2.DeclareLocal(typeof(long), true); + + ilGenerator1.Emit(OpCodes.Ldloc, local1); + ilGenerator2.Emit(OpCodes.Ldloc, local2); + + Assert.Throws(() => ilGenerator1.Emit(OpCodes.Ldloc, local2)); + Assert.Throws(() => ilGenerator2.Emit(OpCodes.Ldloc, local1)); + } + } +} diff --git a/src/libraries/System.Reflection.Emit.ILGeneration/tests/System.Reflection.Emit.ILGeneration.Tests.csproj b/src/libraries/System.Reflection.Emit.ILGeneration/tests/System.Reflection.Emit.ILGeneration.Tests.csproj index fbd22531f9188b..214600d06c5808 100644 --- a/src/libraries/System.Reflection.Emit.ILGeneration/tests/System.Reflection.Emit.ILGeneration.Tests.csproj +++ b/src/libraries/System.Reflection.Emit.ILGeneration/tests/System.Reflection.Emit.ILGeneration.Tests.csproj @@ -22,6 +22,7 @@ + diff --git a/src/libraries/System.Reflection.Emit/src/Resources/Strings.resx b/src/libraries/System.Reflection.Emit/src/Resources/Strings.resx index f33ce87899cb59..690b09b0ab69ef 100644 --- a/src/libraries/System.Reflection.Emit/src/Resources/Strings.resx +++ b/src/libraries/System.Reflection.Emit/src/Resources/Strings.resx @@ -186,4 +186,7 @@ Method body should not exist. + + Local passed in does not belong to this ILGenerator. + \ 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 3c6273439966ac..03b448b163bc71 100644 --- a/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj +++ b/src/libraries/System.Reflection.Emit/src/System.Reflection.Emit.csproj @@ -12,6 +12,7 @@ + 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 bbb41113d27334..ad88e5a7c46be9 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 @@ -1,6 +1,7 @@ // 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.Generic; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; using System.Runtime.InteropServices; @@ -16,6 +17,8 @@ internal sealed class ILGeneratorImpl : ILGenerator private bool _hasDynamicStackAllocation; private int _maxStackSize; private int _currentStack; + private List? _locals; + private int _localCount; internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) { @@ -29,6 +32,7 @@ internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) internal InstructionEncoder Instructions => _il; internal bool HasDynamicStackAllocation => _hasDynamicStackAllocation; + internal List? Locals => _locals; public override int ILOffset => _il.Offset; @@ -38,7 +42,20 @@ internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) 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 LocalBuilder DeclareLocal(Type localType, bool pinned) + { + if (_methodBuilder is not MethodBuilderImpl methodBuilder) + throw new NotSupportedException(); + + ArgumentNullException.ThrowIfNull(localType); + + LocalBuilder local = new LocalBuilderImpl(_localCount++, localType, methodBuilder, pinned); + _locals ??= new(); + _locals.Add(local); + + return local; + } + public override Label DefineLabel() => throw new NotImplementedException(); private void UpdateStackSize(OpCode opCode) @@ -192,7 +209,31 @@ public override void Emit(OpCode opcode, string str) 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, LocalBuilder local) + { + ArgumentNullException.ThrowIfNull(local); + + if (local.Method != _methodBuilder) + { + throw new ArgumentException(SR.Argument_UnmatchedMethodForLocal, nameof(local)); + } + + int tempVal = local.LocalIndex; + if (opcode.Name!.StartsWith("ldloca")) + { + _il.LoadLocalAddress(tempVal); + } + else if (opcode.Name.StartsWith("ldloc")) + { + _il.LoadLocal(tempVal); + } + else if (opcode.Name.StartsWith("stloc")) + { + _il.StoreLocal(tempVal); + } + + UpdateStackSize(opcode); + } 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(); diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/LocalBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/LocalBuilderImpl.cs new file mode 100644 index 00000000000000..0c95af7a2c0fe0 --- /dev/null +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/LocalBuilderImpl.cs @@ -0,0 +1,32 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Reflection.Emit +{ + internal sealed class LocalBuilderImpl : LocalBuilder + { + #region Private Data Members + private readonly int _localIndex; + private readonly Type _localType; + private readonly MethodInfo _method; + private readonly bool _isPinned; + #endregion + + #region Constructor + internal LocalBuilderImpl(int index, Type type, MethodInfo method, bool isPinned) + { + _isPinned = isPinned; + _localIndex = index; + _localType = type; + _method = method; + } + #endregion + + #region LocalVariableInfo Override + public override bool IsPinned => _isPinned; + public override Type LocalType => _localType; + public override int LocalIndex => _localIndex; + public override MethodInfo Method => _method; + #endregion + } +} 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 c30398046ab60e..7c7dd430e502bd 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 @@ -188,7 +188,9 @@ private void WriteMethods(TypeBuilderImpl typeBuilder, List - methodBodyEncoder.AddMethodBody( + private static int AddMethodBody(MethodBuilderImpl method, ILGeneratorImpl il, StandaloneSignatureHandle signature, MethodBodyStreamEncoder bodyEncoder) => + bodyEncoder.AddMethodBody( instructionEncoder: il.Instructions, maxStack: il.GetMaxStackSize(), - localVariablesSignature: default, // TODO + localVariablesSignature: signature, attributes: method.InitLocals ? MethodBodyAttributes.InitLocals : MethodBodyAttributes.None, hasDynamicStackAllocation: il.HasDynamicStackAllocation); diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs index 5e00821235d997..8a165f306fbe9c 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs @@ -1,6 +1,7 @@ // 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.Generic; using System.Collections.Immutable; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; @@ -10,6 +11,20 @@ namespace System.Reflection.Emit // TODO: Only support simple signatures. More complex signatures (generics, array, byref, pointers etc) will be added. internal static class MetadataSignatureHelper { + internal static BlobBuilder LocalSignatureEncoder(List locals, ModuleBuilderImpl module) + { + BlobBuilder localSignature = new(); + LocalVariablesEncoder encoder = new BlobEncoder(localSignature).LocalVariableSignature(locals.Count); + + foreach(LocalBuilder local in locals) + { + WriteSignatureForType(encoder.AddVariable().Type(local.LocalType.IsByRef, local.IsPinned), + local.LocalType.IsByRef ? local.LocalType.GetElementType()! : local.LocalType, module); + } + + return localSignature; + } + internal static BlobBuilder FieldSignatureEncoder(Type fieldType, ModuleBuilderImpl module) { BlobBuilder fieldSignature = new(); diff --git a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs index 97ec018b0e7f4d..fce867e1bde9b3 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs @@ -244,5 +244,178 @@ private MethodInfo LoadILGenerator_GetMaxStackSizeMethod() Type ilgType = Type.GetType("System.Reflection.Emit.ILGeneratorImpl, System.Reflection.Emit", throwOnError: true)!; return ilgType.GetMethod("GetMaxStackSize", BindingFlags.NonPublic | BindingFlags.Instance, Type.EmptyTypes); } + + [Fact] + public void LocalBuilderMultipleLocalsUsage() + { + using (TempFile file = TempFile.Create()) + { + AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderTypeBuilderAndSaveMethod(out TypeBuilder type, out MethodInfo saveMethod); + MethodBuilder methodBuilder = type.DefineMethod("Method1", MethodAttributes.Public | MethodAttributes.Static, typeof(int), new[] { typeof(int), typeof(string) }); + ILGenerator il = methodBuilder.GetILGenerator(); + LocalBuilder intLocal = il.DeclareLocal(typeof(int)); + LocalBuilder stringLocal = il.DeclareLocal(typeof(string)); + LocalBuilder shortLocal = il.DeclareLocal(typeof(short)); + LocalBuilder int2Local = il.DeclareLocal(typeof(int)); + il.Emit(OpCodes.Ldarg, 1); + il.Emit(OpCodes.Stloc_1); + il.Emit(OpCodes.Ldstr, "string value"); + il.Emit(OpCodes.Stloc, stringLocal); + il.Emit(OpCodes.Ldloc, stringLocal); + il.Emit(OpCodes.Starg, 1); + il.Emit(OpCodes.Ldarg_0); + il.Emit(OpCodes.Stloc_0); + il.Emit(OpCodes.Ldc_I4_S, 120); + il.Emit(OpCodes.Stloc, 2); + il.Emit(OpCodes.Ldloc, shortLocal); + il.Emit(OpCodes.Ldloc, 0); + il.Emit(OpCodes.Add); + il.Emit(OpCodes.Stloc, intLocal); + il.Emit(OpCodes.Ldloca, intLocal); + il.Emit(OpCodes.Ldind_I); + il.Emit(OpCodes.Stloc, int2Local); + il.Emit(OpCodes.Ldloc_3); + il.Emit(OpCodes.Ret); + + MethodInfo getMaxStackSizeMethod = LoadILGenerator_GetMaxStackSizeMethod(); + Assert.Equal(2, getMaxStackSizeMethod.Invoke(il, new object[0])); + saveMethod.Invoke(ab, new object[] { file.Path }); + + Assembly assemblyFromDisk = AssemblySaveTools.LoadAssemblyFromPath(file.Path); + Type typeFromDisk = assemblyFromDisk.Modules.First().GetType("MyType"); + MethodBody body = typeFromDisk.GetMethod("Method1").GetMethodBody(); + Assert.Equal(4, body.LocalVariables.Count); + byte[]? bodyBytes = body.GetILAsByteArray(); + Assert.Equal((byte)OpCodes.Ldarg_1.Value, bodyBytes[0]); + Assert.Equal((byte)OpCodes.Stloc_1.Value, bodyBytes[1]); + Assert.Equal((byte)OpCodes.Ldstr.Value, bodyBytes[2]); + Assert.Equal((byte)OpCodes.Stloc_1.Value, bodyBytes[7]); + Assert.Equal((byte)OpCodes.Ldloc_1.Value, bodyBytes[8]); + Assert.Equal((byte)OpCodes.Starg_S.Value, bodyBytes[9]); + Assert.Equal((byte)OpCodes.Ldarg_0.Value, bodyBytes[11]); + Assert.Equal((byte)OpCodes.Stloc_0.Value, bodyBytes[12]); + Assert.Equal((byte)OpCodes.Ldc_I4_S.Value, bodyBytes[13]); + Assert.Equal(120, BitConverter.ToInt32(bodyBytes.AsSpan().Slice(14, 4))); + Assert.Equal(0xFE, bodyBytes[18]); // Stloc instruction occupies 2 bytes 0xfe0e + Assert.Equal(0x0E, bodyBytes[19]); + Assert.Equal(2, BitConverter.ToInt32(bodyBytes.AsSpan().Slice(20, 4))); // index 2 of 'il.Emit(OpCodes.Stloc, 2);' instruction + Assert.Equal((byte)OpCodes.Ldloc_2.Value, bodyBytes[24]); + Assert.Equal(0xFE, bodyBytes[25]); // Ldloc = 0xfe0c + Assert.Equal(0x0C, bodyBytes[26]); + Assert.Equal(0, BitConverter.ToInt32(bodyBytes.AsSpan().Slice(27, 4))); // index 0 of 'il.Emit(OpCodes.Ldloc, 0);' instruction + Assert.Equal((byte)OpCodes.Add.Value, bodyBytes[31]); + Assert.Equal((byte)OpCodes.Stloc_0.Value, bodyBytes[32]); + Assert.Equal((byte)OpCodes.Ldloca_S.Value, bodyBytes[33]); + Assert.Equal(0, bodyBytes[34]); // intLocal index is 0 for 'il.Emit(OpCodes.Ldloca, intLocal);' instruction + Assert.Equal((byte)OpCodes.Ldind_I.Value, bodyBytes[35]); + Assert.Equal((byte)OpCodes.Stloc_3.Value, bodyBytes[36]); + Assert.Equal((byte)OpCodes.Ldloc_3.Value, bodyBytes[37]); + Assert.Equal(OpCodes.Ret.Value, bodyBytes[38]); + } + } + + [Fact] + public void LocalBuilderMultipleTypesWithMultipleMethodsWithLocals() + { + using (TempFile file = TempFile.Create()) + { + AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderTypeBuilderAndSaveMethod(out TypeBuilder type, out MethodInfo saveMethod); + MethodBuilder methodBuilder = type.DefineMethod("Method1", MethodAttributes.Public | MethodAttributes.Static, typeof(string), new[] { typeof(int), typeof(string) }); + ILGenerator il = methodBuilder.GetILGenerator(); + LocalBuilder intLocal = il.DeclareLocal(typeof(int)); + LocalBuilder stringLocal = il.DeclareLocal(typeof(string)); + LocalBuilder shortLocal = il.DeclareLocal(typeof(short)); + il.Emit(OpCodes.Ldstr, "string value"); + il.Emit(OpCodes.Stloc, stringLocal); + il.Emit(OpCodes.Ldloc, stringLocal); + il.Emit(OpCodes.Starg, 1); + il.Emit(OpCodes.Ldarg_0); + il.Emit(OpCodes.Stloc_0); + il.Emit(OpCodes.Ldc_I4_S, 120); + il.Emit(OpCodes.Stloc, 2); + il.Emit(OpCodes.Ldloc, shortLocal); + il.Emit(OpCodes.Ldloc, 0); + il.Emit(OpCodes.Add); + il.Emit(OpCodes.Stloc, intLocal); + il.Emit(OpCodes.Ldloc, stringLocal); + il.Emit(OpCodes.Ret); + MethodBuilder multiplyMethod = type.DefineMethod("MultiplyMethod", MethodAttributes.Public, typeof(int), new[] { typeof(int) }); + ILGenerator multiplyMethodIL = multiplyMethod.GetILGenerator(); + LocalBuilder iLocal = multiplyMethodIL.DeclareLocal(typeof(int)); + LocalBuilder shLocal = multiplyMethodIL.DeclareLocal(typeof(short)); + multiplyMethodIL.Emit(OpCodes.Ldarg, 1); + multiplyMethodIL.Emit(OpCodes.Stloc, iLocal); + multiplyMethodIL.Emit(OpCodes.Ldloc, iLocal); + multiplyMethodIL.Emit(OpCodes.Ldc_I4_S, 11); + multiplyMethodIL.Emit(OpCodes.Stloc, shLocal); + multiplyMethodIL.Emit(OpCodes.Ldloc, shLocal); + multiplyMethodIL.Emit(OpCodes.Mul); + multiplyMethodIL.Emit(OpCodes.Stloc, iLocal); + multiplyMethodIL.Emit(OpCodes.Ldloc, iLocal); + multiplyMethodIL.Emit(OpCodes.Ret); + + TypeBuilder anotherType = ab.GetDynamicModule("MyModule").DefineType("AnotherType", TypeAttributes.NotPublic, type); + MethodBuilder stringMethod = anotherType.DefineMethod("StringMethod", MethodAttributes.FamORAssem, typeof(string), Type.EmptyTypes); + ILGenerator stringMethodIL = stringMethod.GetILGenerator(); + LocalBuilder strLocal = stringMethodIL.DeclareLocal(typeof(string)); + stringMethodIL.Emit(OpCodes.Ldstr, "Hello world!"); + stringMethodIL.Emit(OpCodes.Stloc, strLocal); + stringMethodIL.Emit(OpCodes.Ldloc, strLocal); + stringMethodIL.Emit(OpCodes.Ret); + MethodBuilder typeMethod = anotherType.DefineMethod("TypeMethod", MethodAttributes.Family, type, new[] { anotherType, type }); + ILGenerator typeMethodIL = typeMethod.GetILGenerator(); + typeMethodIL.Emit(OpCodes.Ldarg, 1); + LocalBuilder typeLocal = typeMethodIL.DeclareLocal(type); + LocalBuilder anotherTypeLocal = typeMethodIL.DeclareLocal(anotherType); + typeMethodIL.Emit(OpCodes.Stloc, anotherTypeLocal); + typeMethodIL.Emit(OpCodes.Ldloc, anotherTypeLocal); + typeMethodIL.Emit(OpCodes.Stloc, typeLocal); + typeMethodIL.Emit(OpCodes.Ldloc, typeLocal); + typeMethodIL.Emit(OpCodes.Ret); + MethodBuilder longMethod = anotherType.DefineMethod("LongMethod", MethodAttributes.Static, typeof(long), Type.EmptyTypes); + ILGenerator longMethodIL = longMethod.GetILGenerator(); + longMethodIL.Emit(OpCodes.Ldc_I8, 1234567L); + LocalBuilder longLocal = longMethodIL.DeclareLocal(typeof(long)); + LocalBuilder shiftLocal = longMethodIL.DeclareLocal(typeof(int)); + longMethodIL.Emit(OpCodes.Stloc, longLocal); + longMethodIL.Emit(OpCodes.Ldc_I4_5); + longMethodIL.Emit(OpCodes.Stloc, shiftLocal); + longMethodIL.Emit(OpCodes.Ldloc, longLocal); + longMethodIL.Emit(OpCodes.Ldloc, shiftLocal); + longMethodIL.Emit(OpCodes.Shl); + longMethodIL.Emit(OpCodes.Stloc, longLocal); + longMethodIL.Emit(OpCodes.Ldloc, longLocal); + 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"); + Assert.Equal(2, typeFromDisk.GetMethod("MultiplyMethod").GetMethodBody().LocalVariables.Count); + Assert.Equal(3, typeFromDisk.GetMethod("Method1").GetMethodBody().LocalVariables.Count); + Type anotherTypeFromDisk = moduleFromFile.GetType("AnotherType"); + Assert.Equal(1, anotherTypeFromDisk.GetMethod("StringMethod", BindingFlags.NonPublic | BindingFlags.Instance).GetMethodBody().LocalVariables.Count); + Assert.Equal(2, anotherTypeFromDisk.GetMethod("TypeMethod", BindingFlags.NonPublic | BindingFlags.Instance).GetMethodBody().LocalVariables.Count); + Assert.Equal(2, anotherTypeFromDisk.GetMethod("LongMethod", BindingFlags.NonPublic | BindingFlags.Static).GetMethodBody().LocalVariables.Count); + } + } + + [Fact] + public void LocalBuilderExceptions() + { + using (TempFile file = TempFile.Create()) + { + AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderTypeBuilderAndSaveMethod(out TypeBuilder type, out MethodInfo saveMethod); + ILGenerator il = type.DefineMethod("Method1", MethodAttributes.Public).GetILGenerator(); + ILGenerator anotherIL = type.DefineMethod("AnotherMethod", MethodAttributes.Public).GetILGenerator(); + LocalBuilder stringLocal = il.DeclareLocal(typeof(string)); + LocalBuilder nullBuilder = null; + + Assert.Throws(() => il.DeclareLocal(null!)); + Assert.Throws(() => il.Emit(OpCodes.Ldloc, nullBuilder)); + Assert.Throws(() => anotherIL.Emit(OpCodes.Ldloc, stringLocal)); + } + } } } diff --git a/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj index 229ae79f7f8cc1..98644b9bc2154d 100644 --- a/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -225,7 +225,6 @@ - @@ -235,6 +234,7 @@ + diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.Mono.cs index 755c552a67b85a..fe3c23e60a084e 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.Mono.cs @@ -207,7 +207,7 @@ public LabelData(int addr, int maxStack) private int code_len; private int max_stack; private int cur_stack; - private LocalBuilder[]? locals; + private RuntimeLocalBuilder[]? locals; private ILExceptionInfo[]? ex_handlers; private int num_token_fixups; private object? token_fixups; @@ -441,19 +441,19 @@ public override LocalBuilder DeclareLocal(Type localType, bool pinned) ArgumentNullException.ThrowIfNull(localType); if (localType.IsUserType) throw new NotSupportedException(SR.PlatformNotSupported_UserDefinedSubclassesOfType); - LocalBuilder res = new LocalBuilder(localType, this); + RuntimeLocalBuilder res = new RuntimeLocalBuilder(localType, this); res.is_pinned = pinned; if (locals != null) { - LocalBuilder[] new_l = new LocalBuilder[locals.Length + 1]; + RuntimeLocalBuilder[] new_l = new RuntimeLocalBuilder[locals.Length + 1]; Array.Copy(locals, new_l, locals.Length); new_l[locals.Length] = res; locals = new_l; } else { - locals = new LocalBuilder[1]; + locals = new RuntimeLocalBuilder[1]; locals[0] = res; } res.position = (ushort)(locals.Length - 1); @@ -619,10 +619,11 @@ public override void Emit(OpCode opcode, Label[] labels) public override void Emit(OpCode opcode, LocalBuilder local) { ArgumentNullException.ThrowIfNull(local); - if (local.ilgen != this) + RuntimeLocalBuilder localBuilder = (RuntimeLocalBuilder)local; + if (localBuilder.ilgen != this) throw new ArgumentException(SR.Argument_UnmatchedMethodForLocal, nameof(local)); - uint pos = local.position; + uint pos = localBuilder.position; if ((opcode == OpCodes.Ldloca_S || opcode == OpCodes.Ldloc_S || opcode == OpCodes.Stloc_S) && pos > 255) throw new InvalidOperationException(SR.InvalidOperation_BadInstructionOrIndexOutOfBound); diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeLocalBuilder.Mono.cs similarity index 87% rename from src/mono/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.Mono.cs rename to src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeLocalBuilder.Mono.cs index 017e072f7bb725..6ac19a061ed76d 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeLocalBuilder.Mono.cs @@ -25,7 +25,7 @@ // // -// System.Reflection.Emit/LocalBuilder.cs +// System.Reflection.Emit/RuntimeLocalBuilder.cs // // Authors: // Paolo Molaro (lupus@ximian.com) @@ -41,7 +41,7 @@ namespace System.Reflection.Emit { [StructLayout(LayoutKind.Sequential)] - public sealed partial class LocalBuilder : LocalVariableInfo + internal sealed partial class RuntimeLocalBuilder : LocalBuilder { #region Sync with MonoReflectionLocalBuilder in object-internals.h internal Type type; @@ -55,7 +55,7 @@ public sealed partial class LocalBuilder : LocalVariableInfo private int endOffset; [DynamicDependency(nameof(name))] // Automatically keeps all previous fields too due to StructLayout - internal LocalBuilder(Type t, ILGenerator ilgen) + internal RuntimeLocalBuilder(Type t, ILGenerator ilgen) { this.type = t; this.ilgen = ilgen; @@ -99,5 +99,8 @@ internal int EndOffset { get { return endOffset; } } + + // Mono uses ILGenerator instance instead of MethodBuilder, could not get reference to the MethodBuilder the local belongs to. + public override MethodInfo Method => throw new NotImplementedException(); } } From af5809f5b50e8ef3accf4095b06f80b00dfb2d21 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Wed, 25 Oct 2023 15:55:23 -0700 Subject: [PATCH 09/14] Remove left overs from merge, update tests --- .../AssemblySaveILGeneratorTests.cs | 17 +++++++++++++++-- .../ref/System.Reflection.Primitives.cs | 1 - 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs index fce867e1bde9b3..81a1a28067c2d9 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs @@ -3,6 +3,7 @@ using System.IO; using System.Linq; +using System.Runtime.CompilerServices; using Xunit; namespace System.Reflection.Emit.Tests @@ -255,8 +256,8 @@ public void LocalBuilderMultipleLocalsUsage() ILGenerator il = methodBuilder.GetILGenerator(); LocalBuilder intLocal = il.DeclareLocal(typeof(int)); LocalBuilder stringLocal = il.DeclareLocal(typeof(string)); - LocalBuilder shortLocal = il.DeclareLocal(typeof(short)); - LocalBuilder int2Local = il.DeclareLocal(typeof(int)); + LocalBuilder shortLocal = il.DeclareLocal(typeof(short), pinned: true); ; + LocalBuilder int2Local = il.DeclareLocal(typeof(int), pinned: false); il.Emit(OpCodes.Ldarg, 1); il.Emit(OpCodes.Stloc_1); il.Emit(OpCodes.Ldstr, "string value"); @@ -285,6 +286,18 @@ public void LocalBuilderMultipleLocalsUsage() Type typeFromDisk = assemblyFromDisk.Modules.First().GetType("MyType"); MethodBody body = typeFromDisk.GetMethod("Method1").GetMethodBody(); Assert.Equal(4, body.LocalVariables.Count); + Assert.Equal(intLocal.LocalIndex, body.LocalVariables[0].LocalIndex); + Assert.Equal(intLocal.LocalType.FullName, body.LocalVariables[0].LocalType.FullName); + Assert.Equal(intLocal.IsPinned, body.LocalVariables[0].IsPinned); + Assert.Equal(stringLocal.LocalIndex, body.LocalVariables[1].LocalIndex); + Assert.Equal(stringLocal.LocalType.FullName, body.LocalVariables[1].LocalType.FullName); + Assert.Equal(stringLocal.IsPinned, body.LocalVariables[1].IsPinned); + Assert.Equal(shortLocal.LocalIndex, body.LocalVariables[2].LocalIndex); + Assert.Equal(shortLocal.LocalType.FullName, body.LocalVariables[2].LocalType.FullName); + Assert.Equal(shortLocal.IsPinned, body.LocalVariables[2].IsPinned); + Assert.Equal(int2Local.LocalIndex, body.LocalVariables[3].LocalIndex); + Assert.Equal(int2Local.LocalType.FullName, body.LocalVariables[3].LocalType.FullName); + Assert.Equal(int2Local.IsPinned, body.LocalVariables[3].IsPinned); byte[]? bodyBytes = body.GetILAsByteArray(); Assert.Equal((byte)OpCodes.Ldarg_1.Value, bodyBytes[0]); Assert.Equal((byte)OpCodes.Stloc_1.Value, bodyBytes[1]); diff --git a/src/libraries/System.Reflection.Primitives/ref/System.Reflection.Primitives.cs b/src/libraries/System.Reflection.Primitives/ref/System.Reflection.Primitives.cs index 7e3db4fef8305d..c5d2beb82b6ba8 100644 --- a/src/libraries/System.Reflection.Primitives/ref/System.Reflection.Primitives.cs +++ b/src/libraries/System.Reflection.Primitives/ref/System.Reflection.Primitives.cs @@ -34,7 +34,6 @@ public enum FlowControl public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; } public bool Equals(System.Reflection.Emit.OpCode obj) { throw null; } public override int GetHashCode() { throw null; } - public int StackDifference() { throw null; } public static bool operator ==(System.Reflection.Emit.OpCode a, System.Reflection.Emit.OpCode b) { throw null; } public static bool operator !=(System.Reflection.Emit.OpCode a, System.Reflection.Emit.OpCode b) { throw null; } public override string? ToString() { throw null; } From 2cd5a8484b852e4491bce2b182dc3b3bce1c95f8 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Wed, 25 Oct 2023 17:19:06 -0700 Subject: [PATCH 10/14] Apply suggestions from code review Co-authored-by: Aaron Robinson --- .../src/System/Reflection/Emit/LocalBuilder.cs | 2 +- .../tests/LocalBuilder/LocalBuilderMethod.cs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs index 2bb2565fb9f7ed..a112a2bb0f0ce0 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs @@ -14,7 +14,7 @@ public abstract class LocalBuilder : LocalVariableInfo protected LocalBuilder() { } /// - /// Returns the method where the local variable declared. + /// Returns the method where the local variable is declared. /// /// Can be used for validating the containing method when referencing the local. public abstract MethodInfo Method { get; } diff --git a/src/libraries/System.Reflection.Emit.ILGeneration/tests/LocalBuilder/LocalBuilderMethod.cs b/src/libraries/System.Reflection.Emit.ILGeneration/tests/LocalBuilder/LocalBuilderMethod.cs index 4e4b28d3ead74c..14a4e3f2697120 100644 --- a/src/libraries/System.Reflection.Emit.ILGeneration/tests/LocalBuilder/LocalBuilderMethod.cs +++ b/src/libraries/System.Reflection.Emit.ILGeneration/tests/LocalBuilder/LocalBuilderMethod.cs @@ -5,7 +5,6 @@ namespace System.Reflection.Emit.Tests { - // Mono doesn't support LocalBuilder.Method [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.IsNotMonoRuntime))] public class LocalBuilderMethod { @@ -25,7 +24,7 @@ public void LocalBuilder_MethodReturnsCorrectMethod() } [Fact] - public void LocalBuilder_UseLocalsThatBelongsToTheMethod() + public void LocalBuilder_UseLocalsThatBelongToTheMethod() { TypeBuilder type = Helpers.DynamicType(TypeAttributes.NotPublic); MethodBuilder method1 = type.DefineMethod("Method1", MethodAttributes.Public); From ce9f966d2ff10eb05b4f691abfc8e235dd6ffb18 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 26 Oct 2023 11:29:14 -0700 Subject: [PATCH 11/14] Remove public LocalBuilder.Method, and apply other feedback --- .../Reflection/Emit/RuntimeILGenerator.cs | 2 +- .../Reflection/Emit/RuntimeLocalBuilder.cs | 5 ++- .../System/Reflection/Emit/LocalBuilder.cs | 6 --- .../System.Reflection.Emit.ILGeneration.cs | 1 - .../tests/LocalBuilder/LocalBuilderMethod.cs | 44 ------------------- ....Reflection.Emit.ILGeneration.Tests.csproj | 1 - .../System/Reflection/Emit/ILGeneratorImpl.cs | 22 +++++----- .../Reflection/Emit/LocalBuilderImpl.cs | 5 ++- .../Reflection/Emit/ModuleBuilderImpl.cs | 2 +- .../AssemblySaveILGeneratorTests.cs | 1 - .../Emit/RuntimeLocalBuilder.Mono.cs | 3 -- 11 files changed, 22 insertions(+), 70 deletions(-) delete mode 100644 src/libraries/System.Reflection.Emit.ILGeneration/tests/LocalBuilder/LocalBuilderMethod.cs diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs index 998da1a3a13d34..75894ef0b1c6e7 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs @@ -820,7 +820,7 @@ public override void Emit(OpCode opcode, LocalBuilder local) // Puts the opcode onto the IL stream followed by the information for local variable local. int tempVal = local.LocalIndex; - if (local.Method != m_methodBuilder) + if (((RuntimeLocalBuilder)local).GetMethodBuilder() != m_methodBuilder) { throw new ArgumentException(SR.Argument_UnmatchedMethodForLocal, nameof(local)); } diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeLocalBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeLocalBuilder.cs index 3a78bac31e44cb..e85c08daabefbe 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeLocalBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeLocalBuilder.cs @@ -24,11 +24,14 @@ internal RuntimeLocalBuilder(int localIndex, Type localType, MethodInfo methodBu } #endregion + #region Internal Members + internal MethodInfo GetMethodBuilder() => m_methodBuilder; + #endregion + #region LocalVariableInfo Override public override bool IsPinned => m_isPinned; public override Type LocalType => m_localType; public override int LocalIndex => m_localIndex; - public override MethodInfo Method => m_methodBuilder; #endregion } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs index a112a2bb0f0ce0..7c38c287bf22b5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/LocalBuilder.cs @@ -12,11 +12,5 @@ public abstract class LocalBuilder : LocalVariableInfo /// This constructor is invoked by derived classes. /// protected LocalBuilder() { } - - /// - /// Returns the method where the local variable is declared. - /// - /// Can be used for validating the containing method when referencing the local. - public abstract MethodInfo Method { get; } } } diff --git a/src/libraries/System.Reflection.Emit.ILGeneration/ref/System.Reflection.Emit.ILGeneration.cs b/src/libraries/System.Reflection.Emit.ILGeneration/ref/System.Reflection.Emit.ILGeneration.cs index 746b9924bcd689..8b0d002ea40782 100644 --- a/src/libraries/System.Reflection.Emit.ILGeneration/ref/System.Reflection.Emit.ILGeneration.cs +++ b/src/libraries/System.Reflection.Emit.ILGeneration/ref/System.Reflection.Emit.ILGeneration.cs @@ -71,7 +71,6 @@ protected LocalBuilder() { } public override bool IsPinned { get { throw null; } } public override int LocalIndex { get { throw null; } } public override System.Type LocalType { get { throw null; } } - public abstract MethodInfo Method { get; } } public abstract partial class ParameterBuilder { diff --git a/src/libraries/System.Reflection.Emit.ILGeneration/tests/LocalBuilder/LocalBuilderMethod.cs b/src/libraries/System.Reflection.Emit.ILGeneration/tests/LocalBuilder/LocalBuilderMethod.cs deleted file mode 100644 index 14a4e3f2697120..00000000000000 --- a/src/libraries/System.Reflection.Emit.ILGeneration/tests/LocalBuilder/LocalBuilderMethod.cs +++ /dev/null @@ -1,44 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Xunit; - -namespace System.Reflection.Emit.Tests -{ - [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.IsNotMonoRuntime))] - public class LocalBuilderMethod - { - [Fact] - public void LocalBuilder_MethodReturnsCorrectMethod() - { - TypeBuilder type = Helpers.DynamicType(TypeAttributes.NotPublic); - MethodBuilder method1 = type.DefineMethod("Method1", MethodAttributes.Public); - MethodBuilder method2 = type.DefineMethod("Method2", MethodAttributes.Public); - ILGenerator ilGenerator1 = method1.GetILGenerator(); - ILGenerator ilGenerator2 = method2.GetILGenerator(); - LocalBuilder local1 = ilGenerator1.DeclareLocal(typeof(int)); - LocalBuilder local2 = ilGenerator2.DeclareLocal(typeof(string), true); - - Assert.Equal(method1, local1.Method); - Assert.Equal(method2, local2.Method); - } - - [Fact] - public void LocalBuilder_UseLocalsThatBelongToTheMethod() - { - TypeBuilder type = Helpers.DynamicType(TypeAttributes.NotPublic); - MethodBuilder method1 = type.DefineMethod("Method1", MethodAttributes.Public); - MethodBuilder method2 = type.DefineMethod("Method2", MethodAttributes.Public); - ILGenerator ilGenerator1 = method1.GetILGenerator(); - ILGenerator ilGenerator2 = method2.GetILGenerator(); - LocalBuilder local1 = ilGenerator1.DeclareLocal(typeof(short)); - LocalBuilder local2 = ilGenerator2.DeclareLocal(typeof(long), true); - - ilGenerator1.Emit(OpCodes.Ldloc, local1); - ilGenerator2.Emit(OpCodes.Ldloc, local2); - - Assert.Throws(() => ilGenerator1.Emit(OpCodes.Ldloc, local2)); - Assert.Throws(() => ilGenerator2.Emit(OpCodes.Ldloc, local1)); - } - } -} diff --git a/src/libraries/System.Reflection.Emit.ILGeneration/tests/System.Reflection.Emit.ILGeneration.Tests.csproj b/src/libraries/System.Reflection.Emit.ILGeneration/tests/System.Reflection.Emit.ILGeneration.Tests.csproj index 214600d06c5808..fbd22531f9188b 100644 --- a/src/libraries/System.Reflection.Emit.ILGeneration/tests/System.Reflection.Emit.ILGeneration.Tests.csproj +++ b/src/libraries/System.Reflection.Emit.ILGeneration/tests/System.Reflection.Emit.ILGeneration.Tests.csproj @@ -22,7 +22,6 @@ - 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 308618dc797596..8cee30a362e98e 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 @@ -17,8 +17,7 @@ internal sealed class ILGeneratorImpl : ILGenerator private bool _hasDynamicStackAllocation; private int _maxStackSize; private int _currentStack; - private List? _locals; - private int _localCount; + private List _locals = new(); internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) { @@ -29,10 +28,9 @@ internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) } internal int GetMaxStackSize() => _maxStackSize; - internal InstructionEncoder Instructions => _il; internal bool HasDynamicStackAllocation => _hasDynamicStackAllocation; - internal List? Locals => _locals; + internal List Locals => _locals; public override int ILOffset => _il.Offset; @@ -42,6 +40,7 @@ internal ILGeneratorImpl(MethodBuilder methodBuilder, int size) 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) { if (_methodBuilder is not MethodBuilderImpl methodBuilder) @@ -49,8 +48,7 @@ public override LocalBuilder DeclareLocal(Type localType, bool pinned) ArgumentNullException.ThrowIfNull(localType); - LocalBuilder local = new LocalBuilderImpl(_localCount++, localType, methodBuilder, pinned); - _locals ??= new(); + LocalBuilder local = new LocalBuilderImpl(_locals.Count, localType, methodBuilder, pinned); _locals.Add(local); return local; @@ -209,31 +207,35 @@ public override void Emit(OpCode opcode, string str) 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) { ArgumentNullException.ThrowIfNull(local); - if (local.Method != _methodBuilder) + if (((LocalBuilderImpl)local).GetMethodBuilder() != _methodBuilder) { throw new ArgumentException(SR.Argument_UnmatchedMethodForLocal, nameof(local)); } int tempVal = local.LocalIndex; - if (opcode.Name!.StartsWith("ldloca")) + string name = opcode.Name!; + + if (name.StartsWith("ldloca")) { _il.LoadLocalAddress(tempVal); } - else if (opcode.Name.StartsWith("ldloc")) + else if (name.StartsWith("ldloc")) { _il.LoadLocal(tempVal); } - else if (opcode.Name.StartsWith("stloc")) + else if (name.StartsWith("stloc")) { _il.StoreLocal(tempVal); } UpdateStackSize(opcode); } + 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(); diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/LocalBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/LocalBuilderImpl.cs index 0c95af7a2c0fe0..5c7d2796b868f5 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/LocalBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/LocalBuilderImpl.cs @@ -22,11 +22,14 @@ internal LocalBuilderImpl(int index, Type type, MethodInfo method, bool isPinned } #endregion + #region Internal members + internal MethodInfo GetMethodBuilder() => _method; + #endregion + #region LocalVariableInfo Override public override bool IsPinned => _isPinned; public override Type LocalType => _localType; public override int LocalIndex => _localIndex; - public override MethodInfo Method => _method; #endregion } } 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 7c7dd430e502bd..0dde640eae2694 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 @@ -188,7 +188,7 @@ private void WriteMethods(TypeBuilderImpl typeBuilder, List throw new NotImplementedException(); } } From 8209fa1c1a0cf9fe01c6de842935710e65b21698 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 26 Oct 2023 14:02:52 -0700 Subject: [PATCH 12/14] Small fixes --- .../Reflection/Emit/LocalBuilderImpl.cs | 2 +- .../AssemblySaveILGeneratorTests.cs | 22 ++++++++----------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/LocalBuilderImpl.cs b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/LocalBuilderImpl.cs index 5c7d2796b868f5..fa9bffbec4345c 100644 --- a/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/LocalBuilderImpl.cs +++ b/src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/LocalBuilderImpl.cs @@ -22,7 +22,7 @@ internal LocalBuilderImpl(int index, Type type, MethodInfo method, bool isPinned } #endregion - #region Internal members + #region Internal Members internal MethodInfo GetMethodBuilder() => _method; #endregion diff --git a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs index d4379062c6fc1d..5c5141f544c14c 100644 --- a/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs +++ b/src/libraries/System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveILGeneratorTests.cs @@ -3,7 +3,6 @@ using System.IO; using System.Linq; -using System.Text; using Xunit; namespace System.Reflection.Emit.Tests @@ -533,18 +532,15 @@ public void LocalBuilderMultipleTypesWithMultipleMethodsWithLocals() [Fact] public void LocalBuilderExceptions() { - using (TempFile file = TempFile.Create()) - { - AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderTypeBuilderAndSaveMethod(out TypeBuilder type, out MethodInfo saveMethod); - ILGenerator il = type.DefineMethod("Method1", MethodAttributes.Public).GetILGenerator(); - ILGenerator anotherIL = type.DefineMethod("AnotherMethod", MethodAttributes.Public).GetILGenerator(); - LocalBuilder stringLocal = il.DeclareLocal(typeof(string)); - LocalBuilder nullBuilder = null; - - Assert.Throws(() => il.DeclareLocal(null!)); - Assert.Throws(() => il.Emit(OpCodes.Ldloc, nullBuilder)); - Assert.Throws(() => anotherIL.Emit(OpCodes.Ldloc, stringLocal)); - } + AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderTypeBuilderAndSaveMethod(out TypeBuilder type, out MethodInfo saveMethod); + ILGenerator il = type.DefineMethod("Method1", MethodAttributes.Public).GetILGenerator(); + ILGenerator anotherIL = type.DefineMethod("AnotherMethod", MethodAttributes.Public).GetILGenerator(); + LocalBuilder stringLocal = il.DeclareLocal(typeof(string)); + LocalBuilder nullBuilder = null; + + Assert.Throws(() => il.DeclareLocal(null!)); + Assert.Throws(() => il.Emit(OpCodes.Ldloc, nullBuilder)); + Assert.Throws(() => anotherIL.Emit(OpCodes.Ldloc, stringLocal)); } } } From 47b436bd6e2c1d7a5215082153cb018f6a4d68b4 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 26 Oct 2023 21:13:51 -0700 Subject: [PATCH 13/14] Avoid invalid cast exception. Co-authored-by: Jan Kotas --- .../src/System/Reflection/Emit/RuntimeILGenerator.cs | 2 +- .../src/System/Reflection/Emit/ILGeneratorImpl.cs | 2 +- .../src/System/Reflection/Emit/RuntimeILGenerator.Mono.cs | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs index dd897f8424a49c..34220de0bb2fcd 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs @@ -820,7 +820,7 @@ public override void Emit(OpCode opcode, LocalBuilder local) // Puts the opcode onto the IL stream followed by the information for local variable local. int tempVal = local.LocalIndex; - if (((RuntimeLocalBuilder)local).GetMethodBuilder() != m_methodBuilder) + if (local is RuntimeLocalBuilder localBuilder && localBuilder.GetMethodBuilder() != m_methodBuilder) { throw new ArgumentException(SR.Argument_UnmatchedMethodForLocal, nameof(local)); } 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 b24803ca0fd21c..6c91173bd2215e 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 @@ -245,7 +245,7 @@ public override void Emit(OpCode opcode, LocalBuilder local) { ArgumentNullException.ThrowIfNull(local); - if (((LocalBuilderImpl)local).GetMethodBuilder() != _methodBuilder) + if (local is LocalBuilderImpl localBuilder && localBuilder.GetMethodBuilder() != _methodBuilder) { throw new ArgumentException(SR.Argument_UnmatchedMethodForLocal, nameof(local)); } diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.Mono.cs index fe3c23e60a084e..ae834947bc078b 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.Mono.cs @@ -619,8 +619,7 @@ public override void Emit(OpCode opcode, Label[] labels) public override void Emit(OpCode opcode, LocalBuilder local) { ArgumentNullException.ThrowIfNull(local); - RuntimeLocalBuilder localBuilder = (RuntimeLocalBuilder)local; - if (localBuilder.ilgen != this) + if (local is not RuntimeLocalBuilder localBuilder || localBuilder.ilgen != this) throw new ArgumentException(SR.Argument_UnmatchedMethodForLocal, nameof(local)); uint pos = localBuilder.position; From 09a7ed3613b8c3583617cd436d4dc2977a5377f1 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 26 Oct 2023 21:20:58 -0700 Subject: [PATCH 14/14] Apply suggestions from code review --- .../src/System/Reflection/Emit/RuntimeILGenerator.cs | 2 +- .../src/System/Reflection/Emit/ILGeneratorImpl.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs index 34220de0bb2fcd..b8992dbe5d29b3 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeILGenerator.cs @@ -820,7 +820,7 @@ public override void Emit(OpCode opcode, LocalBuilder local) // Puts the opcode onto the IL stream followed by the information for local variable local. int tempVal = local.LocalIndex; - if (local is RuntimeLocalBuilder localBuilder && localBuilder.GetMethodBuilder() != m_methodBuilder) + if (local is not RuntimeLocalBuilder localBuilder || localBuilder.GetMethodBuilder() != m_methodBuilder) { throw new ArgumentException(SR.Argument_UnmatchedMethodForLocal, nameof(local)); } 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 6c91173bd2215e..e7331233adb3dc 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 @@ -245,7 +245,7 @@ public override void Emit(OpCode opcode, LocalBuilder local) { ArgumentNullException.ThrowIfNull(local); - if (local is LocalBuilderImpl localBuilder && localBuilder.GetMethodBuilder() != _methodBuilder) + if (local is not LocalBuilderImpl localBuilder || localBuilder.GetMethodBuilder() != _methodBuilder) { throw new ArgumentException(SR.Argument_UnmatchedMethodForLocal, nameof(local)); }