Skip to content

Commit 78a3f96

Browse files
Resolve generic descriptor method signatures in ILC (dotnet#128123)
NativeAOT could not resolve XML descriptor method signatures that reference generic parameters, e.g. `System.Void Method(T)`, while ILLink accepted the same descriptor. - **Descriptor matching** - Formats method signatures with method/type generic context. - Resolves signature variables back to their generic parameter names for XML comparison. - **ILTrim coverage and parity** - Re-enables existing ILTrim coverage for descriptor-rooted methods by removing expected-failure entries that now pass. - Removes the test-specific `--disable-opt unreachablebodies test` workaround. - Adds an ILTrim `IsWorthConvertingToThrow` approximation so empty method bodies composed of an optional `nop` followed by `ret` are preserved as-is, matching ILLink behavior closely enough for these tests. - Uses that heuristic when deciding whether to rewrite unreachable method bodies. - **ILCompiler test coverage** - Updates the shared LinkXml test expectation so NativeAOT/ILCompiler now expects the generic descriptor-rooted method to be kept. Example descriptor now handled consistently: ```xml <type fullname="MyGenericClass`1"> <method signature="System.Void GenericMethod(T)" /> </type> ``` --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
1 parent 7210d22 commit 78a3f96

4 files changed

Lines changed: 54 additions & 19 deletions

File tree

src/coreclr/tools/Common/Compiler/ProcessLinkerXmlBase.cs

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,8 @@ protected static string GetAttribute(XPathNavigator nav, string attribute)
530530
public static string GetMethodSignature(MethodDesc meth, bool includeGenericParameters)
531531
{
532532
StringBuilder sb = new StringBuilder();
533-
CecilTypeNameFormatter.Instance.AppendName(sb, meth.Signature.ReturnType);
533+
var formatter = new CecilTypeNameFormatter(meth);
534+
formatter.AppendName(sb, meth.Signature.ReturnType);
534535
sb.Append(' ');
535536
sb.Append(meth.GetName());
536537
if (includeGenericParameters && meth.HasInstantiation)
@@ -545,7 +546,7 @@ public static string GetMethodSignature(MethodDesc meth, bool includeGenericPara
545546
if (i > 0)
546547
sb.Append(',');
547548

548-
CecilTypeNameFormatter.Instance.AppendName(sb, meth.Signature[i]);
549+
formatter.AppendName(sb, meth.Signature[i]);
549550
}
550551

551552
sb.Append(')');
@@ -572,7 +573,14 @@ protected void LogWarning(XPathNavigator position, DiagnosticId id, params strin
572573

573574
private sealed class CecilTypeNameFormatter : TypeNameFormatter
574575
{
575-
public static readonly CecilTypeNameFormatter Instance = new CecilTypeNameFormatter();
576+
public static readonly CecilTypeNameFormatter Instance = new CecilTypeNameFormatter(null);
577+
578+
private readonly MethodDesc? _method;
579+
580+
public CecilTypeNameFormatter(MethodDesc? method)
581+
{
582+
_method = method;
583+
}
576584

577585
public override void AppendName(StringBuilder sb, ArrayType type)
578586
{
@@ -620,9 +628,21 @@ public override void AppendName(StringBuilder sb, GenericParameterDesc type)
620628
}
621629
public override void AppendName(StringBuilder sb, SignatureMethodVariable type)
622630
{
631+
if (_method is not null &&
632+
type.Index < _method.Instantiation.Length &&
633+
_method.Instantiation[type.Index] is GenericParameterDesc genericParameter)
634+
{
635+
sb.Append(genericParameter.Name);
636+
}
623637
}
624638
public override void AppendName(StringBuilder sb, SignatureTypeVariable type)
625639
{
640+
if (_method is not null &&
641+
type.Index < _method.OwningType.Instantiation.Length &&
642+
_method.OwningType.Instantiation[type.Index] is GenericParameterDesc genericParameter)
643+
{
644+
sb.Append(genericParameter.Name);
645+
}
626646
}
627647
protected override void AppendNameForInstantiatedType(StringBuilder sb, DefType type)
628648
{

src/coreclr/tools/ILTrim.Core/DependencyAnalysis/TokenBased/MethodDefinitionNode.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,12 @@
77
using System.Reflection.Metadata;
88
using System.Reflection.Metadata.Ecma335;
99

10+
using Internal.IL;
1011
using Internal.TypeSystem;
1112
using Internal.TypeSystem.Ecma;
1213

14+
using Mono.Linker;
15+
1316
namespace ILCompiler.DependencyAnalysis
1417
{
1518
/// <summary>
@@ -172,6 +175,8 @@ protected override EntityHandle WriteInternal(ModuleWritingContext writeContext)
172175
EcmaType ecmaType = (EcmaType)_module.GetObject(methodDef.GetDeclaringType());
173176
MethodBodyNode bodyNode = writeContext.Factory.MethodBody(_module, Handle);
174177
int bodyOffset = bodyNode.Marked
178+
|| !writeContext.Factory.Settings.Optimizations.IsEnabled(CodeOptimizations.UnreachableBodies, _module.Assembly.GetName().Name)
179+
|| !IsWorthConvertingToThrow(methodDef)
175180
? bodyNode.Write(writeContext)
176181
: writeContext.WriteUnreachableMethodBody(Handle, _module);
177182

@@ -201,6 +206,31 @@ protected override EntityHandle WriteInternal(ModuleWritingContext writeContext)
201206
return outputHandle;
202207
}
203208

209+
private bool IsWorthConvertingToThrow(MethodDefinition methodDef)
210+
{
211+
// Some bodies are cheaper size-wise to preserve as-is than to convert to a throw.
212+
const int EmptyBodyLength = 0; // No IL body.
213+
const int RetOnlyBodyLength = 1; // ret
214+
const int NopRetBodyLength = 2; // nop; ret
215+
216+
int rva = methodDef.RelativeVirtualAddress;
217+
if (rva == 0)
218+
return false;
219+
220+
BlobReader ilReader = _module.PEReader.GetMethodBody(rva).GetILReader();
221+
int ilLength = ilReader.Length;
222+
if (ilLength == EmptyBodyLength)
223+
return false;
224+
225+
if (ilLength == RetOnlyBodyLength)
226+
return ilReader.ReadByte() != (byte)ILOpcode.ret;
227+
228+
if (ilLength == NopRetBodyLength)
229+
return ilReader.ReadByte() != (byte)ILOpcode.nop || ilReader.ReadByte() != (byte)ILOpcode.ret;
230+
231+
return true;
232+
}
233+
204234
public override string ToString()
205235
{
206236
// TODO: would be nice to have a common formatter we can call into that also includes owning type

src/coreclr/tools/ILTrim.Tests/ILTrimExpectedFailures.txt

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ DataFlow.GenericParameterDataFlowMarking
119119
DataFlow.GenericParameterWarningLocation
120120
DataFlow.InterfaceImplementedThroughBaseValidation
121121
DataFlow.IReflectDataflow
122-
DataFlow.LocalDataFlowKeptMembers
123122
DataFlow.MemberTypes
124123
DataFlow.MemberTypesAllOnCopyAssembly
125124
DataFlow.MethodParametersDataFlow
@@ -174,9 +173,7 @@ Generics.VariantCasting
174173
Inheritance.AbstractClasses.NoKeptCtor.OverrideRemoval.CanDisableOverrideRemoval
175174
Inheritance.AbstractClasses.NoKeptCtor.OverrideRemoval.OverrideOfAbstractIsKept
176175
Inheritance.AbstractClasses.NoKeptCtor.OverrideRemoval.OverrideOfAbstractIsKeptNonEmpty
177-
Inheritance.AbstractClasses.NoKeptCtor.OverrideRemoval.OverrideOfVirtualCanBeRemoved
178176
Inheritance.AbstractClasses.NoKeptCtor.OverrideRemoval.OverrideOfVirtualCanBeRemoved2
179-
Inheritance.AbstractClasses.NoKeptCtor.OverrideRemoval.OverrideOfVirtualCanBeRemoved3
180177
Inheritance.AbstractClasses.NoKeptCtor.OverrideRemoval.OverrideThatAlsoFulfilsInterface
181178
Inheritance.AbstractClasses.NoKeptCtor.OverrideRemoval.PreservesOverriddenMethodOverrideOfUsedVirtualStillRemoved
182179
Inheritance.AbstractClasses.NoKeptCtor.OverrideRemoval.PreservesOverriddenMethodOverrideOfUsedVirtualStillRemoved2
@@ -229,7 +226,6 @@ Inheritance.Interfaces.OnReferenceType.NoKeptCtor.LocalDowncastDoesNotCuaseOther
229226
Inheritance.Interfaces.OnReferenceType.NoKeptCtor.PreserveDependencyPreservesInterfaceMethod
230227
Inheritance.Interfaces.OnReferenceType.NoKeptCtor.UnusedTypeHasExplicitInterfaceMethodPreservedViaXml
231228
Inheritance.Interfaces.OnReferenceType.NoKeptCtor.UnusedTypeHasExplicitInterfacePropertyPreservedViaXml
232-
Inheritance.Interfaces.OnReferenceType.NoKeptCtor.UnusedTypeHasInterfaceMethodPreservedViaXml
233229
Inheritance.Interfaces.OnReferenceType.NoKeptCtor.UnusedTypeWithPreserveMethodsAndInterfaceTypeMarked
234230
Inheritance.Interfaces.OnReferenceType.NoKeptCtorButInterfaceNeeded.ArrayWithIndexAssignedToReturnValue
235231
Inheritance.Interfaces.OnReferenceType.NoKeptCtorButInterfaceNeeded.FieldDowncastedToInterface
@@ -324,10 +320,7 @@ LinkXml.EmbeddedLinkXmlPreservesAdditionalAssemblyWithOverriddenMethod
324320
LinkXml.LinkXmlErrorCases
325321
LinkXml.UnusedFieldPreservedByLinkXmlIsKept
326322
LinkXml.UnusedInterfaceTypeOnTypeWithPreserveAllIsKept
327-
LinkXml.UnusedMethodPreservedByLinkXmlIsKept
328323
LinkXml.UnusedNonRequiredTypeIsRemoved
329-
LinkXml.UnusedTypeWithPreserveFieldsHasMethodsRemoved
330-
LinkXml.UnusedTypeWithPreserveNothingAndPreserveMembers
331324
LinkXml.UsedNonRequiredExportedTypeIsKept
332325
LinkXml.UsedNonRequiredExportedTypeIsKeptWhenRooted
333326
LinkXml.UsedNonRequiredTypeIsKeptWithSingleMethod
@@ -370,17 +363,13 @@ Reflection.AssemblyImportedViaReflectionWithDerivedType
370363
Reflection.CoreLibMessages
371364
Reflection.EventHanderTypeGetInvokeMethod
372365
Reflection.EventsUsedViaReflection
373-
Reflection.ExpressionCallString
374366
Reflection.IsAssignableFrom
375367
Reflection.MethodsUsedViaReflection
376368
Reflection.MethodUsedViaReflection
377369
Reflection.ObjectGetType
378370
Reflection.ObjectGetTypeLibraryMode
379371
Reflection.ParametersUsedViaReflection
380-
Reflection.PropertiesUsedViaReflection
381-
Reflection.PropertyUsedViaReflection
382372
Reflection.RunClassConstructor
383-
Reflection.RuntimeReflectionExtensionsCalls
384373
Reflection.TypeHierarchyLibraryModeSuppressions
385374
Reflection.TypeHierarchyReflectionWarnings
386375
Reflection.TypeMap
@@ -392,7 +381,6 @@ RequiresCapability.ReflectionAccessFromCompilerGeneratedCode
392381
RequiresCapability.RequiresAccessedThrough
393382
RequiresCapability.RequiresAttributeMismatch
394383
RequiresCapability.RequiresCapabilityFromCopiedAssembly
395-
RequiresCapability.RequiresCapabilityReflectionAnalysisEnabled
396384
RequiresCapability.RequiresExcludeStatics
397385
RequiresCapability.RequiresInCompilerGeneratedCode
398386
RequiresCapability.RequiresInCompilerGeneratedCodeRelease
@@ -506,7 +494,6 @@ UnreachableBody.ExplicitInstructionCheck
506494
UnreachableBody.InterfaceMethod
507495
UnreachableBody.LinkedOtherIncludedLibrary
508496
UnreachableBody.LinkedOtherIncludedLibraryNoInstanceCtor
509-
UnreachableBody.NotWorthConvertingEmpty
510497
UnreachableBody.NotWorthConvertingReturnDouble
511498
UnreachableBody.NotWorthConvertingReturnFalse
512499
UnreachableBody.NotWorthConvertingReturnFloat

src/tools/illink/test/Mono.Linker.Tests.Cases/LinkXml/UnusedMethodPreservedByLinkXmlIsKept.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,7 @@ private void NotPreservedMethod()
3838
{
3939
}
4040

41-
// NativeAOT fails to translate Cecil generic methods
42-
// https://github.com/dotnet/runtime/issues/80462
43-
[Kept(By = Tool.Trimmer)]
41+
[Kept]
4442
private void PreservedMethod5<T>(T arg)
4543
{
4644
}

0 commit comments

Comments
 (0)