Skip to content

Commit 2432268

Browse files
MichalStrehovskyMichalStrehovsky
authored
Fix constrained call corner cases (dotnet#111178)
1 parent dc341ae commit 2432268

File tree

15 files changed

+246
-109
lines changed

15 files changed

+246
-109
lines changed

‎src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,11 @@ public static unsafe IntPtr ResolveStaticDispatchOnType(RuntimeTypeHandle instan
547547
return result;
548548
}
549549

550+
public static unsafe IntPtr ResolveDispatchOnType(RuntimeTypeHandle instanceType, RuntimeTypeHandle interfaceType, int slot)
551+
{
552+
return RuntimeImports.RhResolveDispatchOnType(instanceType.ToMethodTable(), interfaceType.ToMethodTable(), checked((ushort)slot));
553+
}
554+
550555
public static bool IsUnmanagedPointerType(RuntimeTypeHandle typeHandle)
551556
{
552557
return typeHandle.ToMethodTable()->IsPointer;

‎src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/GenericDictionaryCell.cs

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,41 @@ internal override IntPtr Create(TypeBuilder builder)
140140
}
141141
}
142142

143+
/// <summary>
144+
/// Used for non-generic instance constrained Methods
145+
/// </summary>
146+
private class NonGenericInstanceConstrainedMethodCell : GenericDictionaryCell
147+
{
148+
internal TypeDesc ConstraintType;
149+
internal TypeDesc ConstrainedMethodType;
150+
internal int ConstrainedMethodSlot;
151+
152+
internal override void Prepare(TypeBuilder builder)
153+
{
154+
if (ConstraintType.IsCanonicalSubtype(CanonicalFormKind.Any) || ConstrainedMethodType.IsCanonicalSubtype(CanonicalFormKind.Any))
155+
Environment.FailFast("Unable to compute call information for a canonical type/method.");
156+
157+
builder.RegisterForPreparation(ConstraintType);
158+
builder.RegisterForPreparation(ConstrainedMethodType);
159+
}
160+
161+
internal override IntPtr Create(TypeBuilder builder)
162+
{
163+
IntPtr result = RuntimeAugments.ResolveDispatchOnType(
164+
builder.GetRuntimeTypeHandle(ConstraintType),
165+
builder.GetRuntimeTypeHandle(ConstrainedMethodType),
166+
ConstrainedMethodSlot);
167+
168+
Debug.Assert(result != IntPtr.Zero);
169+
170+
return result;
171+
}
172+
}
173+
143174
/// <summary>
144175
/// Used for generic static constrained Methods
145176
/// </summary>
146-
private class GenericStaticConstrainedMethodCell : GenericDictionaryCell
177+
private class GenericConstrainedMethodCell : GenericDictionaryCell
147178
{
148179
internal DefType ConstraintType;
149180
internal InstantiatedMethod ConstrainedMethod;
@@ -512,29 +543,45 @@ internal static GenericDictionaryCell ParseAndCreateCell(NativeLayoutInfoLoadCon
512543
break;
513544

514545
case FixupSignatureKind.NonGenericStaticConstrainedMethod:
515-
{
546+
case FixupSignatureKind.NonGenericInstanceConstrainedMethod:
547+
{
516548
var constraintType = nativeLayoutInfoLoadContext.GetType(ref parser);
517549
var constrainedMethodType = nativeLayoutInfoLoadContext.GetType(ref parser);
518550
var constrainedMethodSlot = parser.GetUnsigned();
519-
TypeLoaderLogger.WriteLine("NonGenericStaticConstrainedMethod: " + constraintType.ToString() + " Method " + constrainedMethodType.ToString() + ", slot #" + constrainedMethodSlot.LowLevelToString());
520551

521-
cell = new NonGenericStaticConstrainedMethodCell()
552+
string kindString = kind == FixupSignatureKind.NonGenericStaticConstrainedMethod ? "NonGenericStaticConstrainedMethod: " : "NonGenericInstanceConstrainedMethod: ";
553+
554+
TypeLoaderLogger.WriteLine(kindString + constraintType.ToString() + " Method " + constrainedMethodType.ToString() + ", slot #" + constrainedMethodSlot.LowLevelToString());
555+
556+
if (kind == FixupSignatureKind.NonGenericStaticConstrainedMethod)
522557
{
523-
ConstraintType = constraintType,
524-
ConstrainedMethodType = constrainedMethodType,
525-
ConstrainedMethodSlot = (int)constrainedMethodSlot
526-
};
558+
cell = new NonGenericStaticConstrainedMethodCell()
559+
{
560+
ConstraintType = constraintType,
561+
ConstrainedMethodType = constrainedMethodType,
562+
ConstrainedMethodSlot = (int)constrainedMethodSlot
563+
};
564+
}
565+
else
566+
{
567+
cell = new NonGenericInstanceConstrainedMethodCell()
568+
{
569+
ConstraintType = constraintType,
570+
ConstrainedMethodType = constrainedMethodType,
571+
ConstrainedMethodSlot = (int)constrainedMethodSlot
572+
};
573+
}
527574
}
528575
break;
529576

530-
case FixupSignatureKind.GenericStaticConstrainedMethod:
531-
{
577+
case FixupSignatureKind.GenericConstrainedMethod:
578+
{
532579
TypeDesc constraintType = nativeLayoutInfoLoadContext.GetType(ref parser);
533580
MethodDesc constrainedMethod = nativeLayoutInfoLoadContext.GetMethod(ref parser);
534581

535-
TypeLoaderLogger.WriteLine("GenericStaticConstrainedMethod: " + constraintType.ToString() + " Method " + constrainedMethod.ToString());
582+
TypeLoaderLogger.WriteLine("GenericConstrainedMethod: " + constraintType.ToString() + " Method " + constrainedMethod.ToString());
536583

537-
cell = new GenericStaticConstrainedMethodCell()
584+
cell = new GenericConstrainedMethodCell()
538585
{
539586
ConstraintType = (DefType)constraintType,
540587
ConstrainedMethod = (InstantiatedMethod)constrainedMethod,

‎src/coreclr/tools/Common/Compiler/TypeExtensions.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,9 @@ public static MethodDesc TryResolveConstraintMethodApprox(this TypeDesc constrai
469469
potentialInterfaceMethod.GetTypicalMethodDefinition(), (InstantiatedType)potentialInterfaceType);
470470
}
471471

472-
method = canonType.ResolveInterfaceMethodToVirtualMethodOnType(potentialInterfaceMethod);
472+
method = canonType.ResolveInterfaceMethodToVirtualMethodOnType(potentialInterfaceMethod)
473+
// Do not lose track of `method` if we were able to resolve it previously
474+
?? method;
473475

474476
// See code:#TryResolveConstraintMethodApprox_DoNotReturnParentMethod
475477
if (method != null && !method.OwningType.IsValueType)

‎src/coreclr/tools/Common/Internal/NativeFormat/NativeFormat.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,9 @@ enum FixupSignatureKind : uint
9898
// unused = 0x17,
9999
// unused = 0x18,
100100
// unused = 0x19,
101-
// unused = 0x20,
101+
NonGenericInstanceConstrainedMethod = 0x20,
102102
NonGenericStaticConstrainedMethod = 0x21,
103-
GenericStaticConstrainedMethod = 0x22,
103+
GenericConstrainedMethod = 0x22,
104104

105105
NotYetSupported = 0xee,
106106
}

‎src/coreclr/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -936,8 +936,6 @@ public override DefaultInterfaceMethodResolution ResolveVariantInterfaceMethodTo
936936

937937
public static DefaultInterfaceMethodResolution ResolveVariantInterfaceMethodToDefaultImplementationOnType(MethodDesc interfaceMethod, MetadataType currentType, out MethodDesc impl)
938938
{
939-
Debug.Assert(interfaceMethod.Signature.IsStatic);
940-
941939
MetadataType interfaceType = (MetadataType)interfaceMethod.OwningType;
942940
bool foundInterface = IsInterfaceImplementedOnType(currentType, interfaceType);
943941

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -955,30 +955,38 @@ public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultCo
955955
TypeDesc instantiatedConstraintType = _constraintType.GetNonRuntimeDeterminedTypeFromRuntimeDeterminedSubtypeViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation);
956956
MethodDesc implMethod;
957957

958+
MethodDesc instantiatedConstrainedMethodDefinition = instantiatedConstrainedMethod.GetMethodDefinition();
959+
958960
if (instantiatedConstrainedMethod.OwningType.IsInterface)
959961
{
960962
if (instantiatedConstrainedMethod.Signature.IsStatic)
961963
{
962-
implMethod = instantiatedConstraintType.GetClosestDefType().ResolveVariantInterfaceMethodToStaticVirtualMethodOnType(instantiatedConstrainedMethod);
963-
if (implMethod == null)
964-
{
965-
DefaultInterfaceMethodResolution resolution =
966-
instantiatedConstraintType.GetClosestDefType().ResolveVariantInterfaceMethodToDefaultImplementationOnType(instantiatedConstrainedMethod, out implMethod);
967-
if (resolution != DefaultInterfaceMethodResolution.DefaultImplementation)
968-
{
969-
// TODO: diamond/reabstraction
970-
ThrowHelper.ThrowInvalidProgramException();
971-
}
972-
}
964+
implMethod = instantiatedConstraintType.GetClosestDefType().ResolveVariantInterfaceMethodToStaticVirtualMethodOnType(instantiatedConstrainedMethodDefinition);
973965
}
974966
else
975967
{
976-
throw new NotImplementedException();
968+
implMethod = instantiatedConstraintType.GetClosestDefType().ResolveVariantInterfaceMethodToVirtualMethodOnType(instantiatedConstrainedMethodDefinition);
969+
}
970+
971+
if (implMethod == null)
972+
{
973+
DefaultInterfaceMethodResolution resolution =
974+
instantiatedConstraintType.GetClosestDefType().ResolveVariantInterfaceMethodToDefaultImplementationOnType(instantiatedConstrainedMethodDefinition, out implMethod);
975+
if (resolution != DefaultInterfaceMethodResolution.DefaultImplementation)
976+
{
977+
// TODO: diamond/reabstraction: https://github.com/dotnet/runtime/issues/72589
978+
ThrowHelper.ThrowInvalidProgramException();
979+
}
977980
}
978981
}
979982
else
980983
{
981-
implMethod = instantiatedConstraintType.GetClosestDefType().FindVirtualFunctionTargetMethodOnObjectType(instantiatedConstrainedMethod);
984+
implMethod = instantiatedConstraintType.GetClosestDefType().FindVirtualFunctionTargetMethodOnObjectType(instantiatedConstrainedMethodDefinition);
985+
}
986+
987+
if (instantiatedConstrainedMethod != instantiatedConstrainedMethodDefinition)
988+
{
989+
implMethod = implMethod.MakeInstantiatedMethod(instantiatedConstrainedMethod.Instantiation);
982990
}
983991

984992
// AOT use of this generic lookup is restricted to finding methods on valuetypes (runtime usage of this slot in universal generics is more flexible)
@@ -987,21 +995,10 @@ public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultCo
987995
factory.MetadataManager.NoteOverridingMethod(_constrainedMethod, implMethod);
988996

989997
// TODO-SIZE: this is address taken only in the delegate target case
990-
if (implMethod.Signature.IsStatic)
991-
{
992-
if (implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific).IsSharedByGenericInstantiations)
993-
return factory.ExactCallableAddressTakenAddress(implMethod);
994-
else
995-
return factory.AddressTakenMethodEntrypoint(implMethod);
996-
}
997-
else if (implMethod.HasInstantiation)
998-
{
998+
if (implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific).IsSharedByGenericInstantiations)
999999
return factory.ExactCallableAddressTakenAddress(implMethod);
1000-
}
10011000
else
1002-
{
1003-
return factory.AddressTakenMethodEntrypoint(implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific));
1004-
}
1001+
return factory.AddressTakenMethodEntrypoint(implMethod);
10051002
}
10061003

10071004
public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb)

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1414,7 +1414,6 @@ public NativeLayoutConstrainedMethodDictionarySlotNode(MethodDesc constrainedMet
14141414
_directCall = directCall;
14151415
Debug.Assert(_constrainedMethod.OwningType.IsInterface);
14161416
Debug.Assert(!_constrainedMethod.HasInstantiation || !directCall);
1417-
Debug.Assert(_constrainedMethod.Signature.IsStatic);
14181417
}
14191418

14201419
protected sealed override string GetName(NodeFactory factory) =>
@@ -1428,10 +1427,12 @@ protected sealed override FixupSignatureKind SignatureKind
14281427
{
14291428
get
14301429
{
1431-
if (_constrainedMethod.HasInstantiation)
1432-
return FixupSignatureKind.GenericStaticConstrainedMethod;
1433-
else
1434-
return FixupSignatureKind.NonGenericStaticConstrainedMethod;
1430+
return (_constrainedMethod.HasInstantiation, _constrainedMethod.Signature.IsStatic) switch
1431+
{
1432+
(true, _) => FixupSignatureKind.GenericConstrainedMethod,
1433+
(false, true) => FixupSignatureKind.NonGenericStaticConstrainedMethod,
1434+
(false, false) => FixupSignatureKind.NonGenericInstanceConstrainedMethod,
1435+
};
14351436
}
14361437
}
14371438

@@ -1477,13 +1478,13 @@ protected sealed override Vertex WriteSignatureVertex(NativeWriter writer, NodeF
14771478
Vertex constraintType = factory.NativeLayout.TypeSignatureVertex(_constraintType).WriteVertex(factory);
14781479
if (_constrainedMethod.HasInstantiation)
14791480
{
1480-
Debug.Assert(SignatureKind is FixupSignatureKind.GenericStaticConstrainedMethod);
1481+
Debug.Assert(SignatureKind is FixupSignatureKind.GenericConstrainedMethod);
14811482
Vertex constrainedMethodVertex = factory.NativeLayout.MethodEntry(_constrainedMethod).WriteVertex(factory);
14821483
return writer.GetTuple(constraintType, constrainedMethodVertex);
14831484
}
14841485
else
14851486
{
1486-
Debug.Assert(SignatureKind is FixupSignatureKind.NonGenericStaticConstrainedMethod);
1487+
Debug.Assert(SignatureKind is FixupSignatureKind.NonGenericStaticConstrainedMethod or FixupSignatureKind.NonGenericInstanceConstrainedMethod);
14871488
Vertex methodType = factory.NativeLayout.TypeSignatureVertex(_constrainedMethod.OwningType).WriteVertex(factory);
14881489
var canonConstrainedMethod = _constrainedMethod.GetCanonMethodTarget(CanonicalFormKind.Specific);
14891490
int interfaceSlot = VirtualMethodSlotHelper.GetVirtualMethodSlot(factory, canonConstrainedMethod, canonConstrainedMethod.OwningType);

‎src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -550,42 +550,39 @@ private void ImportCall(ILOpcode opcode, int token)
550550

551551
bool allowInstParam = opcode != ILOpcode.ldvirtftn && opcode != ILOpcode.ldftn;
552552

553-
if (directCall && resolvedConstraint && exactContextNeedsRuntimeLookup)
553+
if (directCall && resolvedConstraint && (exactContextNeedsRuntimeLookup || forceUseRuntimeLookup))
554554
{
555555
// We want to do a direct call to a shared method on a valuetype. We need to provide
556556
// a generic context, but the JitInterface doesn't provide a way for us to do it from here.
557557
// So we do the next best thing and ask RyuJIT to look up a fat pointer.
558-
//
559-
// We have the canonical version of the method - find the runtime determined version.
560-
// This is simplified because we know the method is on a valuetype.
561-
Debug.Assert(targetMethod.OwningType.IsValueType);
562558

563559
if (forceUseRuntimeLookup)
564560
{
565-
// The below logic would incorrectly resolve the lookup into the first match we found,
566-
// but there was a compile-time ambiguity due to shared code. The correct fix should
567-
// use the ConstrainedMethodUseLookupResult dictionary entry so that the exact
568-
// dispatch can be computed with the help of the generic dictionary.
569-
// We fail the compilation here to avoid bad codegen. This is not actually an invalid program.
570-
// https://github.com/dotnet/runtimelab/issues/1431
571-
ThrowHelper.ThrowInvalidProgramException();
561+
var constrainedCallInfo = new ConstrainedCallInfo(_constrained, runtimeDeterminedMethod);
562+
_dependencies.Add(GetGenericLookupHelper(ReadyToRunHelperId.ConstrainedDirectCall, constrainedCallInfo), reason);
572563
}
573-
574-
MethodDesc targetOfLookup;
575-
if (_constrained.IsRuntimeDeterminedType)
576-
targetOfLookup = _compilation.TypeSystemContext.GetMethodForRuntimeDeterminedType(targetMethod.GetTypicalMethodDefinition(), (RuntimeDeterminedType)_constrained);
577-
else if (_constrained.HasInstantiation)
578-
targetOfLookup = _compilation.TypeSystemContext.GetMethodForInstantiatedType(targetMethod.GetTypicalMethodDefinition(), (InstantiatedType)_constrained);
579564
else
580-
targetOfLookup = targetMethod.GetMethodDefinition();
581-
if (targetOfLookup.HasInstantiation)
582565
{
583-
targetOfLookup = targetOfLookup.MakeInstantiatedMethod(runtimeDeterminedMethod.Instantiation);
584-
}
585-
Debug.Assert(targetOfLookup.GetCanonMethodTarget(CanonicalFormKind.Specific) == targetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific));
586-
_dependencies.Add(GetGenericLookupHelper(ReadyToRunHelperId.MethodEntry, targetOfLookup), reason);
566+
// We have the canonical version of the method - find the runtime determined version.
567+
// This is simplified because we know the method is on a valuetype.
568+
Debug.Assert(targetMethod.OwningType.IsValueType);
569+
570+
MethodDesc targetOfLookup;
571+
if (_constrained.IsRuntimeDeterminedType)
572+
targetOfLookup = _compilation.TypeSystemContext.GetMethodForRuntimeDeterminedType(targetMethod.GetTypicalMethodDefinition(), (RuntimeDeterminedType)_constrained);
573+
else if (_constrained.HasInstantiation)
574+
targetOfLookup = _compilation.TypeSystemContext.GetMethodForInstantiatedType(targetMethod.GetTypicalMethodDefinition(), (InstantiatedType)_constrained);
575+
else
576+
targetOfLookup = targetMethod.GetMethodDefinition();
577+
if (targetOfLookup.HasInstantiation)
578+
{
579+
targetOfLookup = targetOfLookup.MakeInstantiatedMethod(runtimeDeterminedMethod.Instantiation);
580+
}
581+
Debug.Assert(targetOfLookup.GetCanonMethodTarget(CanonicalFormKind.Specific) == targetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific));
582+
_dependencies.Add(GetGenericLookupHelper(ReadyToRunHelperId.MethodEntry, targetOfLookup), reason);
587583

588-
targetForDelegate = targetOfLookup;
584+
targetForDelegate = targetOfLookup;
585+
}
589586
}
590587
else if (directCall && !allowInstParam && targetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific).RequiresInstArg())
591588
{
@@ -700,6 +697,7 @@ private void ImportCall(ILOpcode opcode, int token)
700697
{
701698
Debug.Assert(targetMethod.OwningType.IsInterface && targetMethod.IsVirtual && _constrained != null);
702699

700+
// TODO: https://github.com/dotnet/runtime/issues/72589
703701
ThrowHelper.ThrowBadImageFormatException();
704702
}
705703
else if (method.Signature.IsStatic)

0 commit comments

Comments
 (0)