From 1a12567227f5c616ba3a088f2badd11442851eed Mon Sep 17 00:00:00 2001 From: Robert Xiao Date: Mon, 13 Apr 2020 06:09:05 -0700 Subject: [PATCH] Fix method signature comparison. Now that we generate methods in instantiated generic types, we were getting test failures from methods that were being detected as new methods. In actuality, they weren't new, but they differed only in generic type parameters from some base type method, and GetSignatureString ignores generic parameters completely. This fix eliminates the hacky GetSignatureString and replaces it with more-or-less proper signature comparison. This even manages to fix an incorrect test case from Methods.cs (because GetSignatureString was incorrectly incorporating the return type - when the return type should not be examined for signature checking). --- .../Reflection/ConstructorInfo.cs | 3 --- .../Reflection/MethodBase.cs | 20 +++++++++++++++++-- .../Reflection/MethodInfo.cs | 3 --- Il2CppInspector.Common/Reflection/TypeInfo.cs | 3 +-- .../GameAssembly-Methods-x64.cs | 2 +- .../GameAssembly-Methods-x86.cs | 2 +- Il2CppTests/TestExpectedResults/Methods.cs | 2 +- 7 files changed, 22 insertions(+), 13 deletions(-) diff --git a/Il2CppInspector.Common/Reflection/ConstructorInfo.cs b/Il2CppInspector.Common/Reflection/ConstructorInfo.cs index ad3843e..43c22fc 100644 --- a/Il2CppInspector.Common/Reflection/ConstructorInfo.cs +++ b/Il2CppInspector.Common/Reflection/ConstructorInfo.cs @@ -28,8 +28,5 @@ namespace Il2CppInspector.Reflection public override string ToString() => DeclaringType.Name + GetFullTypeParametersString() + "(" + string.Join(", ", DeclaredParameters.Select(x => x.ParameterType.Name)) + ")"; - - public override string GetSignatureString() => Name + GetFullTypeParametersString() - + "(" + string.Join(",", DeclaredParameters.Select(x => x.GetSignatureString())) + ")"; } } \ No newline at end of file diff --git a/Il2CppInspector.Common/Reflection/MethodBase.cs b/Il2CppInspector.Common/Reflection/MethodBase.cs index 0d0fd8c..ec37bb6 100644 --- a/Il2CppInspector.Common/Reflection/MethodBase.cs +++ b/Il2CppInspector.Common/Reflection/MethodBase.cs @@ -265,7 +265,7 @@ namespace Il2CppInspector.Reflection modifiers.Append("extern "); // Method hiding - if ((DeclaringType.BaseType?.GetAllMethods().Any(m => m.GetSignatureString() == GetSignatureString() && m.IsHideBySig) ?? false) + if ((DeclaringType.BaseType?.GetAllMethods().Any(m => SignatureEquals(m) && m.IsHideBySig) ?? false) && (((Attributes & MethodAttributes.VtableLayoutMask) == MethodAttributes.ReuseSlot && !IsVirtual) || (Attributes & MethodAttributes.VtableLayoutMask) == MethodAttributes.NewSlot)) modifiers.Append("new "); @@ -289,7 +289,23 @@ namespace Il2CppInspector.Reflection public string GetFullTypeParametersString() => !GetGenericArguments().Any()? "" : "[" + string.Join(",", GetGenericArguments().Select(p => p.Name)) + "]"; - public abstract string GetSignatureString(); + public bool SignatureEquals(MethodBase other) { + if (this == other) + return true; + if (Name != other.Name) + return false; + if (DeclaredParameters.Count != other.DeclaredParameters.Count) + return false; + if (genericArguments.Length != other.genericArguments.Length) + return false; + if (DeclaredParameters.All(p => !p.ParameterType.ContainsGenericParameters)) + return Enumerable.SequenceEqual(DeclaredParameters.Select(p => p.ParameterType), other.DeclaredParameters.Select(p => p.ParameterType)); + // We have to do something more expensive: check to see if the signatures are the same if the method type parameters are equated. + // We substitute generic arguments into every parameter type but use a common set of method type parameters. + return Enumerable.SequenceEqual( + rootDefinition.DeclaredParameters.Select(p => p.ParameterType.SubstituteGenericArguments(DeclaringType.GetGenericArguments(), genericArguments)), + other.rootDefinition.DeclaredParameters.Select(p => p.ParameterType.SubstituteGenericArguments(other.DeclaringType.GetGenericArguments(), genericArguments))); + } // List of operator overload metadata names // https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/operator-overloads diff --git a/Il2CppInspector.Common/Reflection/MethodInfo.cs b/Il2CppInspector.Common/Reflection/MethodInfo.cs index ff87108..4af0ea5 100644 --- a/Il2CppInspector.Common/Reflection/MethodInfo.cs +++ b/Il2CppInspector.Common/Reflection/MethodInfo.cs @@ -42,8 +42,5 @@ namespace Il2CppInspector.Reflection public override string ToString() => ReturnType.Name + " " + Name + GetFullTypeParametersString() + "(" + string.Join(", ", DeclaredParameters.Select(x => x.ParameterType.IsByRef? x.ParameterType.Name.TrimEnd('&') + " ByRef" : x.ParameterType.Name)) + ")"; - - public override string GetSignatureString() => ReturnParameter.GetSignatureString() + " " + Name + GetFullTypeParametersString() - + "(" + string.Join(",", DeclaredParameters.Select(x => x.GetSignatureString())) + ")"; } } \ No newline at end of file diff --git a/Il2CppInspector.Common/Reflection/TypeInfo.cs b/Il2CppInspector.Common/Reflection/TypeInfo.cs index fe4d2e3..7e0f894 100644 --- a/Il2CppInspector.Common/Reflection/TypeInfo.cs +++ b/Il2CppInspector.Common/Reflection/TypeInfo.cs @@ -1094,9 +1094,8 @@ namespace Il2CppInspector.Reflection { if (DeclaringMethod != null && DeclaringMethod.IsVirtual && !DeclaringMethod.IsAbstract && !DeclaringMethod.IsFinal && (DeclaringMethod.Attributes & MethodAttributes.VtableLayoutMask) == MethodAttributes.ReuseSlot) { // Find nearest ancestor base method which has us as a generic type parameter - var sig = DeclaringMethod.GetSignatureString(); var method = DeclaringMethod.DeclaringType.BaseType.GetAllMethods() - .FirstOrDefault(m => m.IsHideBySig && m.IsVirtual && m.GetSignatureString() == sig && m.GetGenericArguments().Any(p => p.Name == Name)); + .FirstOrDefault(m => m.IsHideBySig && m.IsVirtual && DeclaringMethod.SignatureEquals(m) && m.GetGenericArguments().Any(p => p.Name == Name)); // Stop if we are inherited from a base method if (method != null) diff --git a/Il2CppTests/TestExpectedResults/GameAssembly-Methods-x64.cs b/Il2CppTests/TestExpectedResults/GameAssembly-Methods-x64.cs index 69da2fd..591dfd9 100644 --- a/Il2CppTests/TestExpectedResults/GameAssembly-Methods-x64.cs +++ b/Il2CppTests/TestExpectedResults/GameAssembly-Methods-x64.cs @@ -1073,6 +1073,6 @@ namespace Il2CppTests.TestSources public TestNewNonVirtualMethod() {} // 0x0000000180123B00-0x0000000180123B10 // Methods - public int ValueTypeReturnMethod() => default; // 0x00000001802586A0-0x00000001802586B0 + public new int ValueTypeReturnMethod() => default; // 0x00000001802586A0-0x00000001802586B0 } } diff --git a/Il2CppTests/TestExpectedResults/GameAssembly-Methods-x86.cs b/Il2CppTests/TestExpectedResults/GameAssembly-Methods-x86.cs index 533a923..c019ddc 100644 --- a/Il2CppTests/TestExpectedResults/GameAssembly-Methods-x86.cs +++ b/Il2CppTests/TestExpectedResults/GameAssembly-Methods-x86.cs @@ -1073,6 +1073,6 @@ namespace Il2CppTests.TestSources public TestNewNonVirtualMethod() {} // 0x100F4510-0x100F4520 // Methods - public int ValueTypeReturnMethod() => default; // 0x101FA200-0x101FA210 + public new int ValueTypeReturnMethod() => default; // 0x101FA200-0x101FA210 } } diff --git a/Il2CppTests/TestExpectedResults/Methods.cs b/Il2CppTests/TestExpectedResults/Methods.cs index a6ada38..5301498 100644 --- a/Il2CppTests/TestExpectedResults/Methods.cs +++ b/Il2CppTests/TestExpectedResults/Methods.cs @@ -1073,6 +1073,6 @@ namespace Il2CppTests.TestSources public TestNewNonVirtualMethod() {} // 0x00A4ACE0-0x00A4ACE8 // Methods - public int ValueTypeReturnMethod() => default; // 0x00A4ACD8-0x00A4ACE0 + public new int ValueTypeReturnMethod() => default; // 0x00A4ACD8-0x00A4ACE0 } }