Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/8.0-staging] Fix polluted CompareState when comparing element types in a signature #98249

Merged
merged 1 commit into from
Feb 12, 2024
Merged

[release/8.0-staging] Fix polluted CompareState when comparing element types in a signature #98249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/coreclr/vm/siginfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4204,8 +4204,6 @@ MetaSig::CompareTypeDefsUnderSubstitutions(
SigPointer inst1 = pSubst1->GetInst();
SigPointer inst2 = pSubst2->GetInst();

TokenPairList visited { pVisited };
CompareState state{ &visited };
for (DWORD i = 0; i < pTypeDef1->GetNumGenericArgs(); i++)
{
PCCOR_SIGNATURE startInst1 = inst1.GetPtr();
Expand All @@ -4214,6 +4212,8 @@ MetaSig::CompareTypeDefsUnderSubstitutions(
PCCOR_SIGNATURE startInst2 = inst2.GetPtr();
IfFailThrow(inst2.SkipExactlyOne());
PCCOR_SIGNATURE endInst2ptr = inst2.GetPtr();
TokenPairList visited{ pVisited };
CompareState state{ &visited };
if (!CompareElementType(
startInst1,
startInst2,
Expand Down Expand Up @@ -4382,8 +4382,6 @@ MetaSig::CompareMethodSigs(
IfFailThrow(CorSigUncompressData_EndPtr(pSig1, pEndSig1, &ArgCount1));
IfFailThrow(CorSigUncompressData_EndPtr(pSig2, pEndSig2, &ArgCount2));

TokenPairList visited{ pVisited };

if (ArgCount1 != ArgCount2)
{
if ((callConv & IMAGE_CEE_CS_CALLCONV_MASK) != IMAGE_CEE_CS_CALLCONV_VARARG)
Expand All @@ -4405,7 +4403,6 @@ MetaSig::CompareMethodSigs(
// to correctly handle overloads, where there are a number of varargs methods
// to pick from, like m1(int,...) and m2(int,int,...), etc.

CompareState state{ &visited };
// <= because we want to include a check of the return value!
for (i = 0; i <= ArgCount1; i++)
{
Expand Down Expand Up @@ -4437,6 +4434,8 @@ MetaSig::CompareMethodSigs(
else
{
// We are in bounds on both sides. Compare the element.
TokenPairList visited{ pVisited };
CompareState state{ &visited };
if (!CompareElementType(
pSig1,
pSig2,
Expand All @@ -4461,7 +4460,6 @@ MetaSig::CompareMethodSigs(
}

// do return type as well
CompareState state{ &visited };
for (i = 0; i <= ArgCount1; i++)
{
if (i == 0 && skipReturnTypeSig)
Expand All @@ -4476,6 +4474,8 @@ MetaSig::CompareMethodSigs(
}
else
{
TokenPairList visited{ pVisited };
CompareState state{ &visited };
if (!CompareElementType(
pSig1,
pSig2,
Expand Down
6 changes: 6 additions & 0 deletions src/tests/baseservices/typeequivalence/contracts/Types.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ public interface ISparseType
int MultiplyBy20(int a);
}

[Guid("BD752276-52DF-4CD1-8C62-49D202F15C8D")]
public struct TestValueType
{
public int Field;
}

//
// Below types are used in type punning tests and shouldn't be used anywhere else.
//
Expand Down
12 changes: 12 additions & 0 deletions src/tests/baseservices/typeequivalence/impl/Impls.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,15 @@ public static int GetField_3(OnlyLoadOnce_3 s)
return s.Field;
}
}

public static class MethodCall
{
// Include a generic type in the method signature before the type using type equivalence to ensure that
// processing of the generic type does not affect subsequent type processing during signature comparison.
public static System.Collections.Generic.List<int> InterfaceAfterGeneric(IEmptyType t) => null;
public static System.Collections.Generic.List<int> ValueTypeAfterGeneric(TestValueType t) => null;

// Generic type after the type using type equivalence should also not affect processing.
public static void InterfaceBeforeGeneric(IEmptyType t, System.Collections.Generic.List<int> l) { }
public static void ValueTypeBeforeGeneric(TestValueType t, System.Collections.Generic.List<int> l) { }
}
87 changes: 44 additions & 43 deletions src/tests/baseservices/typeequivalence/simple/Simple.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public struct EquivalentValueType
public int A;
}

[PlatformSpecific(TestPlatforms.Windows)]
public class Simple
{
private class EmptyType2 : IEmptyType
Expand All @@ -30,7 +31,8 @@ public static object Create()
}
}

private static void InterfaceTypesFromDifferentAssembliesAreEquivalent()
[Fact]
public static void InterfaceTypesFromDifferentAssembliesAreEquivalent()
{
Console.WriteLine($"{nameof(InterfaceTypesFromDifferentAssembliesAreEquivalent)}");
var inAsm = EmptyType.Create();
Expand All @@ -44,9 +46,10 @@ void AreNotSameObject(IEmptyType a, IEmptyType b)
}
}

private static void ValidateTypeInstanceEquality()
[Fact]
public static void TypeInstanceEquality()
{
Console.WriteLine($"{nameof(ValidateTypeInstanceEquality)}");
Console.WriteLine($"{nameof(TypeInstanceEquality)}");
var inAsm = EmptyType.Create();
var otherAsm = EmptyType2.Create();

Expand Down Expand Up @@ -110,7 +113,8 @@ public override string ScaleString(string s)
}
}

private static void InterfaceTypesMethodOperations()
[Fact]
public static void InterfaceTypesMethodOperations()
{
Console.WriteLine($"{nameof(InterfaceTypesMethodOperations)}");

Expand Down Expand Up @@ -141,7 +145,8 @@ private static void InterfaceTypesMethodOperations()
}
}

private static void CallSparseInterface()
[Fact]
public static void CallSparseInterface()
{
Console.WriteLine($"{nameof(CallSparseInterface)}");

Expand All @@ -156,9 +161,10 @@ private static void CallSparseInterface()
Assert.Equal(input * 18, sparseType.MultiplyBy18(input));
}

private static void TestArrayEquivalence()
[Fact]
public static void ArrayEquivalence()
{
Console.WriteLine($"{nameof(TestArrayEquivalence)}");
Console.WriteLine($"{nameof(ArrayEquivalence)}");
var inAsm = EmptyType.Create();
var otherAsm = EmptyType2.Create();

Expand All @@ -173,9 +179,10 @@ private static void TestArrayEquivalence()
Assert.False(inAsmInterfaceType.MakeArrayType(1).IsEquivalentTo(otherAsmInterfaceType.MakeArrayType(2)));
}

private static void TestByRefEquivalence()
[Fact]
public static void ByRefEquivalence()
{
Console.WriteLine($"{nameof(TestByRefEquivalence)}");
Console.WriteLine($"{nameof(ByRefEquivalence)}");
var inAsm = EmptyType.Create();
var otherAsm = EmptyType2.Create();

Expand All @@ -197,9 +204,10 @@ public void Method(V input)
}
}

private static void TestGenericClassNonEquivalence()
[Fact]
public static void GenericClassNonEquivalence()
{
Console.WriteLine($"{nameof(TestGenericClassNonEquivalence)}");
Console.WriteLine($"{nameof(GenericClassNonEquivalence)}");
var inAsm = EmptyType.Create();
var otherAsm = EmptyType2.Create();

Expand All @@ -209,9 +217,10 @@ private static void TestGenericClassNonEquivalence()
Assert.False(typeof(Generic<>).MakeGenericType(inAsmInterfaceType).IsEquivalentTo(typeof(Generic<>).MakeGenericType(otherAsmInterfaceType)));
}

private static void TestGenericInterfaceEquivalence()
[Fact]
public static void GenericInterfaceEquivalence()
{
Console.WriteLine($"{nameof(TestGenericInterfaceEquivalence)}");
Console.WriteLine($"{nameof(GenericInterfaceEquivalence)}");
var inAsm = EmptyType.Create();
var otherAsm = EmptyType2.Create();

Expand All @@ -221,9 +230,10 @@ private static void TestGenericInterfaceEquivalence()
Assert.True(typeof(IGeneric<>).MakeGenericType(inAsmInterfaceType).IsEquivalentTo(typeof(IGeneric<>).MakeGenericType(otherAsmInterfaceType)));
}

private static unsafe void TestTypeEquivalenceWithTypePunning()
[Fact]
public static unsafe void TypeEquivalenceWithTypePunning()
{
Console.WriteLine($"{nameof(TestTypeEquivalenceWithTypePunning)}");
Console.WriteLine($"{nameof(TypeEquivalenceWithTypePunning)}");

{
Console.WriteLine($"-- GetFunctionPointer()");
Expand Down Expand Up @@ -260,10 +270,11 @@ private static unsafe void TestTypeEquivalenceWithTypePunning()
}
}

[Fact]
[MethodImpl (MethodImplOptions.NoInlining)]
private static void TestLoadingValueTypesWithMethod()
public static void LoadValueTypesWithMethod()
{
Console.WriteLine($"{nameof(TestLoadingValueTypesWithMethod)}");
Console.WriteLine($"{nameof(LoadValueTypesWithMethod)}");
Console.WriteLine($"-- {typeof(ValueTypeWithStaticMethod).Name}");
Assert.Throws<TypeLoadException>(() => LoadInvalidType());
}
Expand All @@ -274,7 +285,8 @@ private static void LoadInvalidType()
Console.WriteLine($"-- {typeof(ValueTypeWithInstanceMethod).Name}");
}

private static void TestCastsOptimizations()
[Fact]
public static void CastsOptimizations()
{
string otherTypeName = $"{typeof(EquivalentValueType).FullName},{typeof(EmptyType).Assembly.GetName().Name}";
Type otherEquivalentValueType = Type.GetType(otherTypeName);
Expand All @@ -285,32 +297,21 @@ private static void TestCastsOptimizations()
EquivalentValueType inst = (EquivalentValueType)otherEquivalentValueTypeInstance;
}

public static int Main()
[Fact]
public static void MethodCallSignature()
{
if (!OperatingSystem.IsWindows())
{
return 100;
}
try
{
InterfaceTypesFromDifferentAssembliesAreEquivalent();
ValidateTypeInstanceEquality();
InterfaceTypesMethodOperations();
CallSparseInterface();
TestByRefEquivalence();
TestArrayEquivalence();
TestGenericClassNonEquivalence();
TestGenericInterfaceEquivalence();
TestTypeEquivalenceWithTypePunning();
TestLoadingValueTypesWithMethod();
TestCastsOptimizations();
}
catch (Exception e)
{
Console.WriteLine($"Test Failure: {e}");
return 101;
}
Console.WriteLine($"{nameof(MethodCallSignature)}");

Console.WriteLine($"-- {nameof(MethodCall.InterfaceAfterGeneric)}");
MethodCall.InterfaceAfterGeneric((IEmptyType)EmptyType2.Create());

Console.WriteLine($"-- {nameof(MethodCall.ValueTypeAfterGeneric)}");
MethodCall.ValueTypeAfterGeneric(new TestValueType());

Console.WriteLine($"-- {nameof(MethodCall.InterfaceBeforeGeneric)}");
MethodCall.InterfaceBeforeGeneric((IEmptyType)EmptyType2.Create(), null);

return 100;
Console.WriteLine($"-- {nameof(MethodCall.ValueTypeBeforeGeneric)}");
MethodCall.ValueTypeBeforeGeneric(new TestValueType(), null);
}
}
Loading