From ca113e4903f8690246bf68ac9a57f8854bb71ae1 Mon Sep 17 00:00:00 2001 From: ElektroKill Date: Tue, 28 Feb 2023 20:37:54 +0100 Subject: [PATCH 1/9] Add `Parameter.CreateParameterDefinition` helper method --- src/AsmResolver.DotNet/Collections/Parameter.cs | 5 +++++ src/AsmResolver.DotNet/Collections/ParameterCollection.cs | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/src/AsmResolver.DotNet/Collections/Parameter.cs b/src/AsmResolver.DotNet/Collections/Parameter.cs index feca2527a..8c4d339cf 100644 --- a/src/AsmResolver.DotNet/Collections/Parameter.cs +++ b/src/AsmResolver.DotNet/Collections/Parameter.cs @@ -76,6 +76,11 @@ public TypeSignature ParameterType /// public string Name => Definition?.Name ?? GetDummyArgumentName(MethodSignatureIndex); + /// + /// Creates a for this parameter and adds it to the method definition if it does not exist. + /// + public void CreateParameterDefinition() => _parentCollection?.CreateParameterDefinition(this); + [SuppressMessage("ReSharper", "InconsistentlySynchronizedField")] private static string GetDummyArgumentName(int index) { diff --git a/src/AsmResolver.DotNet/Collections/ParameterCollection.cs b/src/AsmResolver.DotNet/Collections/ParameterCollection.cs index a2e8b4c5d..1a8d1f662 100644 --- a/src/AsmResolver.DotNet/Collections/ParameterCollection.cs +++ b/src/AsmResolver.DotNet/Collections/ParameterCollection.cs @@ -142,6 +142,12 @@ private void UpdateParameterTypes() return _owner.ParameterDefinitions.FirstOrDefault(p => p.Sequence == sequence); } + internal void CreateParameterDefinition(Parameter parameter) { + if (parameter == ThisParameter || parameter.Definition is not null) + return; + _owner.ParameterDefinitions.Add(new ParameterDefinition(parameter.Sequence, Utf8String.Empty, 0)); + } + internal void PushParameterUpdateToSignature(Parameter parameter) { if (_owner.Signature is null) From e27b6e8c2d70fb189fb409a81b676925856763bb Mon Sep 17 00:00:00 2001 From: ElektroKill Date: Tue, 28 Feb 2023 20:38:40 +0100 Subject: [PATCH 2/9] Don't use unbound generic type for virtual hidden this parameter --- .../Collections/ParameterCollection.cs | 19 ++++++++++++++++--- .../Types/GenericParameterSignature.cs | 2 +- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/AsmResolver.DotNet/Collections/ParameterCollection.cs b/src/AsmResolver.DotNet/Collections/ParameterCollection.cs index 1a8d1f662..e04305dbd 100644 --- a/src/AsmResolver.DotNet/Collections/ParameterCollection.cs +++ b/src/AsmResolver.DotNet/Collections/ParameterCollection.cs @@ -127,11 +127,24 @@ private void UpdateParameterTypes() private TypeSignature? GetThisParameterType() { - if (_owner.DeclaringType is null) + var declaringType = _owner.DeclaringType; + if (declaringType is null) return null; - var result = _owner.DeclaringType.ToTypeSignature(); - if (_owner.DeclaringType.IsValueType) + TypeSignature result; + if (declaringType.GenericParameters.Count > 0) + { + var genArgs = new TypeSignature[declaringType.GenericParameters.Count]; + for (int i = 0; i < genArgs.Length; i++) + genArgs[i] = new GenericParameterSignature(_owner.Module, GenericParameterType.Type, i); + result = declaringType.MakeGenericInstanceType(genArgs); + } + else + { + result = declaringType.ToTypeSignature(); + } + + if (declaringType.IsValueType) result = result.MakeByReferenceType(); return result; diff --git a/src/AsmResolver.DotNet/Signatures/Types/GenericParameterSignature.cs b/src/AsmResolver.DotNet/Signatures/Types/GenericParameterSignature.cs index f29083c29..0c4b8f333 100644 --- a/src/AsmResolver.DotNet/Signatures/Types/GenericParameterSignature.cs +++ b/src/AsmResolver.DotNet/Signatures/Types/GenericParameterSignature.cs @@ -26,7 +26,7 @@ public GenericParameterSignature(GenericParameterType parameterType, int index) /// The module in which this generic parameter signature resides. /// Indicates the parameter signature is declared by a type or a method. /// The index of the referenced parameter. - public GenericParameterSignature(ModuleDefinition module, GenericParameterType parameterType, int index) + public GenericParameterSignature(ModuleDefinition? module, GenericParameterType parameterType, int index) { Scope = module; ParameterType = parameterType; From 7d43fb22fbc90de3808a0fa6f5816db906c6891b Mon Sep 17 00:00:00 2001 From: ElektroKill Date: Tue, 28 Feb 2023 21:02:22 +0100 Subject: [PATCH 3/9] Add unit test to cover hidden this parameter changes --- .../Collections/ParameterCollectionTest.cs | 19 +++++++++++++++++++ .../GenericInstanceMethods.cs | 9 +++++++++ 2 files changed, 28 insertions(+) create mode 100644 test/TestBinaries/DotNet/AsmResolver.DotNet.TestCases.Methods/GenericInstanceMethods.cs diff --git a/test/AsmResolver.DotNet.Tests/Collections/ParameterCollectionTest.cs b/test/AsmResolver.DotNet.Tests/Collections/ParameterCollectionTest.cs index 847c046a1..87bb6615e 100644 --- a/test/AsmResolver.DotNet.Tests/Collections/ParameterCollectionTest.cs +++ b/test/AsmResolver.DotNet.Tests/Collections/ParameterCollectionTest.cs @@ -24,6 +24,13 @@ private static MethodDefinition ObtainInstanceTestMethod(string name) return type.Methods.First(m => m.Name == name); } + private static MethodDefinition ObtainGenericInstanceTestMethod(string name) + { + var module = ModuleDefinition.FromFile(typeof(GenericInstanceMethods<,>).Assembly.Location); + var type = module.TopLevelTypes.First(t => t.Name == "GenericInstanceMethods`2"); + return type.Methods.First(m => m.Name == name); + } + [Fact] public void ReadEmptyParametersFromStaticMethod() { @@ -111,6 +118,18 @@ public void ReadMultipleParametersFromInstanceMethod() Assert.Equal(nameof(InstanceMethods), method.Parameters.ThisParameter.ParameterType.Name); } + [Fact] + public void ReadEmptyParametersFromGenericInstanceMethod() + { + var method = ObtainGenericInstanceTestMethod(nameof(GenericInstanceMethods.InstanceParameterlessMethod)); + Assert.Empty(method.Parameters); + Assert.NotNull(method.Parameters.ThisParameter); + var genericInstanceType = method.Parameters.ThisParameter.ParameterType as GenericInstanceTypeSignature; + Assert.NotNull(genericInstanceType); + Assert.True(genericInstanceType.TypeArguments.Count == 2); + Assert.Equal("GenericInstanceMethods`2", genericInstanceType.GenericType.Name); + } + [Fact] public void ReadReturnTypeFromStaticParameterlessMethod() { diff --git a/test/TestBinaries/DotNet/AsmResolver.DotNet.TestCases.Methods/GenericInstanceMethods.cs b/test/TestBinaries/DotNet/AsmResolver.DotNet.TestCases.Methods/GenericInstanceMethods.cs new file mode 100644 index 000000000..d48a2bb4f --- /dev/null +++ b/test/TestBinaries/DotNet/AsmResolver.DotNet.TestCases.Methods/GenericInstanceMethods.cs @@ -0,0 +1,9 @@ +namespace AsmResolver.DotNet.TestCases.Methods +{ + public class GenericInstanceMethods + { + public void InstanceParameterlessMethod() + { + } + } +} From c3a8193ab95fe389396e23b219274b03b6b5b1f2 Mon Sep 17 00:00:00 2001 From: ElektroKill Date: Tue, 28 Feb 2023 21:03:58 +0100 Subject: [PATCH 4/9] Fix brace style --- src/AsmResolver.DotNet/Collections/ParameterCollection.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/AsmResolver.DotNet/Collections/ParameterCollection.cs b/src/AsmResolver.DotNet/Collections/ParameterCollection.cs index e04305dbd..70abb5e7d 100644 --- a/src/AsmResolver.DotNet/Collections/ParameterCollection.cs +++ b/src/AsmResolver.DotNet/Collections/ParameterCollection.cs @@ -155,7 +155,8 @@ private void UpdateParameterTypes() return _owner.ParameterDefinitions.FirstOrDefault(p => p.Sequence == sequence); } - internal void CreateParameterDefinition(Parameter parameter) { + internal void CreateParameterDefinition(Parameter parameter) + { if (parameter == ThisParameter || parameter.Definition is not null) return; _owner.ParameterDefinitions.Add(new ParameterDefinition(parameter.Sequence, Utf8String.Empty, 0)); From dae99095fd2df0831fae87f95722243d25a436a7 Mon Sep 17 00:00:00 2001 From: ElektroKill Date: Tue, 28 Feb 2023 21:06:32 +0100 Subject: [PATCH 5/9] Fix parameter count calculation for method signatures with ExplicitThis --- src/AsmResolver.DotNet/Signatures/MethodSignatureBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AsmResolver.DotNet/Signatures/MethodSignatureBase.cs b/src/AsmResolver.DotNet/Signatures/MethodSignatureBase.cs index ecb5f4c81..08c7b1703 100644 --- a/src/AsmResolver.DotNet/Signatures/MethodSignatureBase.cs +++ b/src/AsmResolver.DotNet/Signatures/MethodSignatureBase.cs @@ -175,7 +175,7 @@ protected void WriteParametersAndReturnType(BlobSerializationContext context) public int GetTotalParameterCount() { int count = ParameterTypes.Count + SentinelParameterTypes.Count; - if (HasThis || ExplicitThis) + if (HasThis && !ExplicitThis) count++; return count; } From 87ce374a4deed7c160ead93cdbf9d4f1bb8a0d68 Mon Sep 17 00:00:00 2001 From: ElektroKill Date: Thu, 2 Mar 2023 18:57:48 +0100 Subject: [PATCH 6/9] Improve unit test for instantiated generic virtual this parameter --- .../Collections/ParameterCollectionTest.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/AsmResolver.DotNet.Tests/Collections/ParameterCollectionTest.cs b/test/AsmResolver.DotNet.Tests/Collections/ParameterCollectionTest.cs index 87bb6615e..1f8e9c4b8 100644 --- a/test/AsmResolver.DotNet.Tests/Collections/ParameterCollectionTest.cs +++ b/test/AsmResolver.DotNet.Tests/Collections/ParameterCollectionTest.cs @@ -124,10 +124,15 @@ public void ReadEmptyParametersFromGenericInstanceMethod() var method = ObtainGenericInstanceTestMethod(nameof(GenericInstanceMethods.InstanceParameterlessMethod)); Assert.Empty(method.Parameters); Assert.NotNull(method.Parameters.ThisParameter); - var genericInstanceType = method.Parameters.ThisParameter.ParameterType as GenericInstanceTypeSignature; - Assert.NotNull(genericInstanceType); - Assert.True(genericInstanceType.TypeArguments.Count == 2); + var genericInstanceType = Assert.IsAssignableFrom(method.Parameters.ThisParameter.ParameterType); Assert.Equal("GenericInstanceMethods`2", genericInstanceType.GenericType.Name); + Assert.Equal(2, genericInstanceType.TypeArguments.Count); + Assert.All(genericInstanceType.TypeArguments, (typeArgument, i) => + { + var genericParameterSignature = Assert.IsAssignableFrom(typeArgument); + Assert.Equal(GenericParameterType.Type, genericParameterSignature.ParameterType); + Assert.Equal(i, genericParameterSignature.Index); + }); } [Fact] From 2b0c0aadbd743886cf747cbfd30abe77f0beb39e Mon Sep 17 00:00:00 2001 From: ElektroKill Date: Thu, 2 Mar 2023 19:00:19 +0100 Subject: [PATCH 7/9] Replace `CreateParameterDefinition` with `GetOrCreateDefinition` --- src/AsmResolver.DotNet/Collections/Parameter.cs | 6 ++++-- .../Collections/ParameterCollection.cs | 12 ++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/AsmResolver.DotNet/Collections/Parameter.cs b/src/AsmResolver.DotNet/Collections/Parameter.cs index 8c4d339cf..434e370ff 100644 --- a/src/AsmResolver.DotNet/Collections/Parameter.cs +++ b/src/AsmResolver.DotNet/Collections/Parameter.cs @@ -77,9 +77,11 @@ public TypeSignature ParameterType public string Name => Definition?.Name ?? GetDummyArgumentName(MethodSignatureIndex); /// - /// Creates a for this parameter and adds it to the method definition if it does not exist. + /// Creates a or returns the existing corresponding to this parameter. + /// If a is created it is automatically added to the method definition. /// - public void CreateParameterDefinition() => _parentCollection?.CreateParameterDefinition(this); + /// The created or existing or null if this parameter is virtual. + public ParameterDefinition? GetOrCreateDefinition() => _parentCollection?.GetOrCreateParameterDefinition(this); [SuppressMessage("ReSharper", "InconsistentlySynchronizedField")] private static string GetDummyArgumentName(int index) diff --git a/src/AsmResolver.DotNet/Collections/ParameterCollection.cs b/src/AsmResolver.DotNet/Collections/ParameterCollection.cs index 70abb5e7d..1d28bc3db 100644 --- a/src/AsmResolver.DotNet/Collections/ParameterCollection.cs +++ b/src/AsmResolver.DotNet/Collections/ParameterCollection.cs @@ -155,11 +155,15 @@ private void UpdateParameterTypes() return _owner.ParameterDefinitions.FirstOrDefault(p => p.Sequence == sequence); } - internal void CreateParameterDefinition(Parameter parameter) + internal ParameterDefinition? GetOrCreateParameterDefinition(Parameter parameter) { - if (parameter == ThisParameter || parameter.Definition is not null) - return; - _owner.ParameterDefinitions.Add(new ParameterDefinition(parameter.Sequence, Utf8String.Empty, 0)); + if (parameter == ThisParameter) + return null; + if (parameter.Definition is not null) + return parameter.Definition; + var parameterDefinition = new ParameterDefinition(parameter.Sequence, Utf8String.Empty, 0); + _owner.ParameterDefinitions.Add(parameterDefinition); + return parameterDefinition; } internal void PushParameterUpdateToSignature(Parameter parameter) From a17db5f1c650ad42f2ab3ccd0a0361f684e6ea2f Mon Sep 17 00:00:00 2001 From: ElektroKill Date: Thu, 2 Mar 2023 19:17:28 +0100 Subject: [PATCH 8/9] Make `GetOrCreateDefinition` throw an exception instead of returning null --- src/AsmResolver.DotNet/Collections/Parameter.cs | 8 ++++++-- src/AsmResolver.DotNet/Collections/ParameterCollection.cs | 5 +++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/AsmResolver.DotNet/Collections/Parameter.cs b/src/AsmResolver.DotNet/Collections/Parameter.cs index 434e370ff..ce23969a0 100644 --- a/src/AsmResolver.DotNet/Collections/Parameter.cs +++ b/src/AsmResolver.DotNet/Collections/Parameter.cs @@ -80,8 +80,12 @@ public TypeSignature ParameterType /// Creates a or returns the existing corresponding to this parameter. /// If a is created it is automatically added to the method definition. /// - /// The created or existing or null if this parameter is virtual. - public ParameterDefinition? GetOrCreateDefinition() => _parentCollection?.GetOrCreateParameterDefinition(this); + public ParameterDefinition GetOrCreateDefinition() + { + if (_parentCollection is null) + throw new InvalidOperationException("Cannot create a parameter definition for a parameter that has been removed from its parent collection."); + return _parentCollection.GetOrCreateParameterDefinition(this); + } [SuppressMessage("ReSharper", "InconsistentlySynchronizedField")] private static string GetDummyArgumentName(int index) diff --git a/src/AsmResolver.DotNet/Collections/ParameterCollection.cs b/src/AsmResolver.DotNet/Collections/ParameterCollection.cs index 1d28bc3db..1692a8db3 100644 --- a/src/AsmResolver.DotNet/Collections/ParameterCollection.cs +++ b/src/AsmResolver.DotNet/Collections/ParameterCollection.cs @@ -155,12 +155,13 @@ private void UpdateParameterTypes() return _owner.ParameterDefinitions.FirstOrDefault(p => p.Sequence == sequence); } - internal ParameterDefinition? GetOrCreateParameterDefinition(Parameter parameter) + internal ParameterDefinition GetOrCreateParameterDefinition(Parameter parameter) { if (parameter == ThisParameter) - return null; + throw new InvalidOperationException("Cannot retrieve a parameter definition for the virtual this parameter."); if (parameter.Definition is not null) return parameter.Definition; + var parameterDefinition = new ParameterDefinition(parameter.Sequence, Utf8String.Empty, 0); _owner.ParameterDefinitions.Add(parameterDefinition); return parameterDefinition; From 9cbf3aabb74c653f1e72b6463a7c8376e6969af1 Mon Sep 17 00:00:00 2001 From: ElektroKill Date: Thu, 2 Mar 2023 19:50:09 +0100 Subject: [PATCH 9/9] Add unit tests for `GetOrCreateDefinition` --- .../Collections/ParameterCollectionTest.cs | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/AsmResolver.DotNet.Tests/Collections/ParameterCollectionTest.cs b/test/AsmResolver.DotNet.Tests/Collections/ParameterCollectionTest.cs index 1f8e9c4b8..8dda148a2 100644 --- a/test/AsmResolver.DotNet.Tests/Collections/ParameterCollectionTest.cs +++ b/test/AsmResolver.DotNet.Tests/Collections/ParameterCollectionTest.cs @@ -4,6 +4,7 @@ using AsmResolver.DotNet.Signatures.Types; using AsmResolver.DotNet.TestCases.Methods; using AsmResolver.PE.DotNet.Metadata.Strings; +using AsmResolver.PE.DotNet.Metadata.Tables.Rows; using Xunit; namespace AsmResolver.DotNet.Tests.Collections @@ -214,5 +215,48 @@ public void UnnamedParameterShouldResultInDummyName() param.Name = null; Assert.All(method.Parameters, p => Assert.Equal(p.Name, $"A_{p.MethodSignatureIndex}")); } + + [Fact] + public void GetOrCreateDefinitionShouldCreateNewDefinition() + { + var dummyModule = new ModuleDefinition("TestModule"); + var corLibTypesFactory = dummyModule.CorLibTypeFactory; + var method = new MethodDefinition("TestMethodNoParameterDefinitions", + MethodAttributes.Public | MethodAttributes.Static, + MethodSignature.CreateStatic(corLibTypesFactory.Void, corLibTypesFactory.Int32)); + + var param = Assert.Single(method.Parameters); + + Assert.Null(param.Definition); + var definition = param.GetOrCreateDefinition(); + + Assert.Equal(param.Sequence, definition.Sequence); + Assert.Equal(Utf8String.Empty, definition.Name); + Assert.Equal((ParameterAttributes)0, definition.Attributes); + Assert.Contains(definition, method.ParameterDefinitions); + Assert.Same(definition, param.Definition); + } + + [Fact] + public void GetOrCreateDefinitionShouldReturnExistingDefinition() + { + var method = ObtainStaticTestMethod(nameof(MultipleMethods.SingleParameterMethod)); + + var param = Assert.Single(method.Parameters); + + var existingDefinition = param.Definition; + Assert.NotNull(existingDefinition); + var definition = param.GetOrCreateDefinition(); + Assert.Same(existingDefinition, definition); + } + + [Fact] + public void GetOrCreateDefinitionThrowsOnVirtualThisParameter() + { + var method = ObtainInstanceTestMethod(nameof(InstanceMethods.InstanceParameterlessMethod)); + + Assert.NotNull(method.Parameters.ThisParameter); + Assert.Throws(() => method.Parameters.ThisParameter.GetOrCreateDefinition()); + } } }