Skip to content

Commit 45c608b

Browse files
sbomersbomer
authored
Keep metadata for rooted members in ILLink (#102850)
ILLink wasn't keeping metadata (specifically parameter names) for rooted members, whether rooted as: - part of a root assembly - by a root descriptor XML. - by DebuggerDisplayAttribute/DebuggerTypeProxyAttribute This ensures all of those mark members visible to reflection so that method parameter names are kept.
1 parent 992e027 commit 45c608b

File tree

7 files changed

+221
-48
lines changed

7 files changed

+221
-48
lines changed

‎src/tools/illink/src/linker/Linker.Steps/MarkStep.cs‎

Lines changed: 55 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -343,19 +343,8 @@ internal void MarkEntireType (TypeDefinition type, in DependencyInfo reason, Mes
343343

344344
MarkGenericParameterProvider (type, origin);
345345

346-
if (type.HasFields) {
347-
foreach (FieldDefinition field in type.Fields) {
348-
MarkFieldVisibleToReflection (field, new DependencyInfo (DependencyKind.MemberOfType, type), origin);
349-
}
350-
}
351-
352-
if (type.HasMethods) {
353-
foreach (MethodDefinition method in type.Methods) {
354-
if (IsFullyPreservedAction (Annotations.GetAction (type.Module.Assembly)))
355-
Annotations.SetAction (method, MethodAction.ForceParse);
356-
MarkMethodVisibleToReflection (method, new DependencyInfo (DependencyKind.MemberOfType, type), origin);
357-
}
358-
}
346+
MarkFieldsVisibleToReflection (type, new DependencyInfo (DependencyKind.MemberOfType, type), origin);
347+
MarkMethodsVisibleToReflection (type, new DependencyInfo (DependencyKind.MemberOfType, type), origin);
359348

360349
if (type.HasProperties) {
361350
foreach (var property in type.Properties) {
@@ -1897,11 +1886,47 @@ internal void MarkMethodVisibleToReflection (MethodReference method, in Dependen
18971886
}
18981887
}
18991888

1889+
bool MarkMethodsVisibleToReflection (TypeDefinition type, in DependencyInfo reason, MessageOrigin origin)
1890+
{
1891+
if (!type.HasMethods)
1892+
return false;
1893+
1894+
foreach (MethodDefinition method in type.Methods) {
1895+
if (IsFullyPreservedAction (Annotations.GetAction (type.Module.Assembly)))
1896+
Annotations.SetAction (method, MethodAction.ForceParse);
1897+
MarkMethodVisibleToReflection (method, reason, origin);
1898+
}
1899+
return true;
1900+
}
1901+
19001902
internal void MarkFieldVisibleToReflection (FieldReference field, in DependencyInfo reason, in MessageOrigin origin)
19011903
{
19021904
MarkField (field, reason, origin);
19031905
}
19041906

1907+
bool MarkFieldsVisibleToReflection (TypeDefinition type, in DependencyInfo reason, MessageOrigin origin, bool markBackingFieldsOnlyIfPropertyMarked = false)
1908+
{
1909+
if (!type.HasFields)
1910+
return false;
1911+
1912+
foreach (FieldDefinition field in type.Fields) {
1913+
if (markBackingFieldsOnlyIfPropertyMarked && field.Name.EndsWith (">k__BackingField", StringComparison.Ordinal)) {
1914+
// We can't reliably construct the expected property name from the backing field name for all compilers
1915+
// because csc shortens the name of the backing field in some cases
1916+
// For example:
1917+
// Field Name = <IFoo<int>.Bar>k__BackingField
1918+
// Property Name = IFoo<System.Int32>.Bar
1919+
//
1920+
// instead we will search the properties and find the one that makes use of the current backing field
1921+
var propertyDefinition = SearchPropertiesForMatchingFieldDefinition (field);
1922+
if (propertyDefinition != null && !Annotations.IsMarked (propertyDefinition))
1923+
continue;
1924+
}
1925+
MarkFieldVisibleToReflection (field, reason, origin);
1926+
}
1927+
return true;
1928+
}
1929+
19051930
internal void MarkPropertyVisibleToReflection (PropertyDefinition property, in DependencyInfo reason, in MessageOrigin origin)
19061931
{
19071932
// Marking the property itself actually doesn't keep it (it only marks its attributes and records the dependency), we have to mark the methods on it
@@ -2257,33 +2282,28 @@ void MarkTypeWithDebuggerDisplayAttributeValue (TypeDefinition type, CustomAttri
22572282

22582283
MethodDefinition? method = GetMethodWithNoParameters (type, methodName);
22592284
if (method != null) {
2260-
MarkMethod (method, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
2285+
MarkMethodVisibleToReflection (method, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
22612286
continue;
22622287
}
22632288
} else {
22642289
FieldDefinition? field = GetField (type, realMatch);
22652290
if (field != null) {
2266-
MarkField (field, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
2291+
MarkFieldVisibleToReflection (field, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
22672292
continue;
22682293
}
22692294

22702295
PropertyDefinition? property = GetProperty (type, realMatch);
22712296
if (property != null) {
2272-
if (property.GetMethod != null) {
2273-
MarkMethod (property.GetMethod, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
2274-
}
2275-
if (property.SetMethod != null) {
2276-
MarkMethod (property.SetMethod, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
2277-
}
2297+
MarkPropertyVisibleToReflection (property, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
22782298
continue;
22792299
}
22802300
}
22812301

22822302
while (true) {
22832303
// Currently if we don't understand the DebuggerDisplayAttribute we mark everything on the type
22842304
// This can be improved: dotnet/linker/issues/1873
2285-
MarkMethods (type, new DependencyInfo (DependencyKind.KeptForSpecialAttribute, attribute), origin);
2286-
MarkFields (type, includeStatic: true, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
2305+
MarkMethodsVisibleToReflection (type, new DependencyInfo (DependencyKind.KeptForSpecialAttribute, attribute), origin);
2306+
MarkFieldsVisibleToReflection (type, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
22872307
if (Context.TryResolve (type.BaseType) is not TypeDefinition baseType)
22882308
break;
22892309
type = baseType;
@@ -2311,8 +2331,8 @@ void MarkTypeWithDebuggerTypeProxyAttribute (TypeDefinition type, CustomAttribut
23112331
MarkType (proxyTypeReference, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
23122332

23132333
if (Context.TryResolve (proxyTypeReference) is TypeDefinition proxyType) {
2314-
MarkMethods (proxyType, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
2315-
MarkFields (proxyType, includeStatic: true, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
2334+
MarkMethodsVisibleToReflection (proxyType, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
2335+
MarkFieldsVisibleToReflection (proxyType, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), origin);
23162336
}
23172337
}
23182338
}
@@ -2781,16 +2801,16 @@ void ApplyPreserveInfo (TypeDefinition type)
27812801

27822802
switch (preserve) {
27832803
case TypePreserve.All:
2784-
MarkFields (type, true, di, typeOrigin);
2785-
MarkMethods (type, di, typeOrigin);
2804+
MarkFieldsVisibleToReflection (type, di, typeOrigin);
2805+
MarkMethodsVisibleToReflection (type, in di, typeOrigin);
27862806
return;
27872807

27882808
case TypePreserve.Fields:
2789-
if (!MarkFields (type, true, di, typeOrigin, true))
2809+
if (!MarkFieldsVisibleToReflection (type, di, typeOrigin, markBackingFieldsOnlyIfPropertyMarked: true))
27902810
Context.LogWarning (type, DiagnosticId.TypeHasNoFieldsToPreserve, type.GetDisplayName ());
27912811
break;
27922812
case TypePreserve.Methods:
2793-
if (!MarkMethods (type, di, typeOrigin))
2813+
if (!MarkMethodsVisibleToReflection (type, in di, typeOrigin))
27942814
Context.LogWarning (type, DiagnosticId.TypeHasNoMethodsToPreserve, type.GetDisplayName ());
27952815
break;
27962816
}
@@ -2802,18 +2822,18 @@ void ApplyPreserveInfo (TypeDefinition type)
28022822
if (type.HasMethods) {
28032823
foreach (var m in type.Methods) {
28042824
if ((members & TypePreserveMembers.Visible) != 0 && IsMethodVisible (m)) {
2805-
MarkMethod (m, di, typeOrigin);
2825+
MarkMethodVisibleToReflection (m, di, typeOrigin);
28062826
continue;
28072827
}
28082828

28092829
if ((members & TypePreserveMembers.Internal) != 0 && IsMethodInternal (m)) {
2810-
MarkMethod (m, di, typeOrigin);
2830+
MarkMethodVisibleToReflection (m, di, typeOrigin);
28112831
continue;
28122832
}
28132833

28142834
if ((members & TypePreserveMembers.Library) != 0) {
28152835
if (IsSpecialSerializationConstructor (m) || HasOnSerializeOrDeserializeAttribute (m)) {
2816-
MarkMethod (m, di, typeOrigin);
2836+
MarkMethodVisibleToReflection (m, di, typeOrigin);
28172837
continue;
28182838
}
28192839
}
@@ -2823,12 +2843,12 @@ void ApplyPreserveInfo (TypeDefinition type)
28232843
if (type.HasFields) {
28242844
foreach (var f in type.Fields) {
28252845
if ((members & TypePreserveMembers.Visible) != 0 && IsFieldVisible (f)) {
2826-
MarkField (f, di, typeOrigin);
2846+
MarkFieldVisibleToReflection (f, di, typeOrigin);
28272847
continue;
28282848
}
28292849

28302850
if ((members & TypePreserveMembers.Internal) != 0 && IsFieldInternal (f)) {
2831-
MarkField (f, di, typeOrigin);
2851+
MarkFieldVisibleToReflection (f, di, typeOrigin);
28322852
continue;
28332853
}
28342854
}
@@ -2876,27 +2896,14 @@ void ApplyPreserveMethods (MethodDefinition method, MessageOrigin origin)
28762896
MarkMethodCollection (list, new DependencyInfo (DependencyKind.PreservedMethod, method), origin);
28772897
}
28782898

2879-
protected bool MarkFields (TypeDefinition type, bool includeStatic, in DependencyInfo reason, MessageOrigin origin, bool markBackingFieldsOnlyIfPropertyMarked = false)
2899+
protected bool MarkFields (TypeDefinition type, bool includeStatic, in DependencyInfo reason, MessageOrigin origin)
28802900
{
28812901
if (!type.HasFields)
28822902
return false;
28832903

28842904
foreach (FieldDefinition field in type.Fields) {
28852905
if (!includeStatic && field.IsStatic)
28862906
continue;
2887-
2888-
if (markBackingFieldsOnlyIfPropertyMarked && field.Name.EndsWith (">k__BackingField", StringComparison.Ordinal)) {
2889-
// We can't reliably construct the expected property name from the backing field name for all compilers
2890-
// because csc shortens the name of the backing field in some cases
2891-
// For example:
2892-
// Field Name = <IFoo<int>.Bar>k__BackingField
2893-
// Property Name = IFoo<System.Int32>.Bar
2894-
//
2895-
// instead we will search the properties and find the one that makes use of the current backing field
2896-
var propertyDefinition = SearchPropertiesForMatchingFieldDefinition (field);
2897-
if (propertyDefinition != null && !Annotations.IsMarked (propertyDefinition))
2898-
continue;
2899-
}
29002907
MarkField (field, reason, origin);
29012908
}
29022909

‎src/tools/illink/test/Mono.Linker.Tests.Cases/Metadata/DebuggerDisplayNamesAreKept.cs‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
using System.Diagnostics;
2+
using Mono.Linker.Tests.Cases.Expectations.Assertions;
3+
using Mono.Linker.Tests.Cases.Expectations.Metadata;
4+
5+
namespace Mono.Linker.Tests.Cases.Metadata
6+
{
7+
[VerifyMetadataNames]
8+
public class DebuggerDisplayNamesAreKept
9+
{
10+
public static void Main ()
11+
{
12+
var t = new TypeWithDebuggerDisplayAttribute ();
13+
var u = new TypeWithDebuggerTypeProxyAttribute ();
14+
}
15+
16+
[KeptMember (".ctor()")]
17+
[KeptAttributeAttribute (typeof (DebuggerDisplayAttribute))]
18+
[DebuggerDisplay ("{MemberNotFound}")]
19+
class TypeWithDebuggerDisplayAttribute
20+
{
21+
[Kept]
22+
public void MethodWithKeptParameterName (int arg)
23+
{
24+
}
25+
}
26+
27+
[KeptMember (".ctor()")]
28+
[KeptAttributeAttribute (typeof (DebuggerTypeProxyAttribute))]
29+
[DebuggerTypeProxy (typeof (DebuggerTypeProxy))]
30+
class TypeWithDebuggerTypeProxyAttribute
31+
{
32+
}
33+
34+
[KeptMember (".ctor()")]
35+
class DebuggerTypeProxy
36+
{
37+
[Kept]
38+
public void MethodWithKeptParameterName (int arg)
39+
{
40+
}
41+
}
42+
}
43+
}

‎src/tools/illink/test/Mono.Linker.Tests.Cases/Metadata/RootAllAssemblyNamesAreKept.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
using Mono.Linker.Tests.Cases.Expectations.Assertions;
2+
using Mono.Linker.Tests.Cases.Expectations.Metadata;
3+
4+
namespace Mono.Linker.Tests.Cases.Metadata
5+
{
6+
[VerifyMetadataNames]
7+
[SetupLinkerArgument ("-a", "test.exe", "all")]
8+
[KeptMember (".ctor()")]
9+
public class RootAllAssemblyNamesAreKept
10+
{
11+
public static void Main ()
12+
{
13+
}
14+
15+
[Kept]
16+
public void InstanceMethodWithKeptParameterName (int arg)
17+
{
18+
}
19+
20+
[Kept]
21+
public static void MethodWithKeptParameterName (string str)
22+
{
23+
}
24+
}
25+
}

‎src/tools/illink/test/Mono.Linker.Tests.Cases/Metadata/RootDescriptorNamesAreKept.cs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
using Mono.Linker.Tests.Cases.Expectations.Assertions;
2+
using Mono.Linker.Tests.Cases.Expectations.Metadata;
3+
4+
namespace Mono.Linker.Tests.Cases.Metadata
5+
{
6+
[VerifyMetadataNames]
7+
[SetupLinkerDescriptorFile ("RootDescriptorNamesAreKept.xml")]
8+
public class RootDescriptorNamesAreKept
9+
{
10+
public static void Main ()
11+
{
12+
}
13+
14+
[KeptMember (".ctor()")]
15+
class RootedType
16+
{
17+
[Kept]
18+
void InstanceMethodWithKeptParameterName (int arg)
19+
{
20+
}
21+
22+
[Kept]
23+
static void MethodWithKeptParameterName (string str)
24+
{
25+
}
26+
}
27+
28+
[KeptMember (".ctor()")]
29+
class TypeWithPreserveMethods
30+
{
31+
[Kept]
32+
void InstanceMethodWithKeptParameterName (int arg)
33+
{
34+
}
35+
36+
[Kept]
37+
static void MethodWithKeptParameterName (string str)
38+
{
39+
}
40+
}
41+
}
42+
}

‎src/tools/illink/test/Mono.Linker.Tests.Cases/Metadata/RootDescriptorNamesAreKept.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<linker>
2+
<assembly fullname="test">
3+
<type fullname="Mono.Linker.Tests.Cases.Metadata.RootDescriptorNamesAreKept/RootedType" />
4+
<type fullname="Mono.Linker.Tests.Cases.Metadata.RootDescriptorNamesAreKept/TypeWithPreserveMethods" preserve="methods" />
5+
</assembly>
6+
</linker>

‎src/tools/illink/test/Mono.Linker.Tests.Cases/Metadata/RootLibraryAssemblyNamesAreKept.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
using Mono.Linker.Tests.Cases.Expectations.Assertions;
2+
using Mono.Linker.Tests.Cases.Expectations.Metadata;
3+
4+
namespace Mono.Linker.Tests.Cases.Metadata
5+
{
6+
[VerifyMetadataNames]
7+
[SetupLinkerArgument ("-a", "test.exe", "library")]
8+
[KeptMember (".ctor()")]
9+
public class RootLibraryAssemblyNamesAreKept
10+
{
11+
public static void Main ()
12+
{
13+
}
14+
15+
[Kept]
16+
public void InstanceMethodWithKeptParameterName (int arg)
17+
{
18+
}
19+
20+
[Kept]
21+
public static void MethodWithKeptParameterName (string str)
22+
{
23+
}
24+
}
25+
}

‎src/tools/illink/test/Mono.Linker.Tests.Cases/Metadata/RootVisibleAssemblyNamesAreKept.cs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
using Mono.Linker.Tests.Cases.Expectations.Assertions;
2+
using Mono.Linker.Tests.Cases.Expectations.Metadata;
3+
4+
namespace Mono.Linker.Tests.Cases.Metadata
5+
{
6+
[VerifyMetadataNames]
7+
[SetupLinkerArgument ("-a", "test.exe", "visible")]
8+
[KeptMember (".ctor()")]
9+
public class RootVisibleAssemblyNamesAreKept
10+
{
11+
public static void Main ()
12+
{
13+
}
14+
15+
[Kept]
16+
public void InstanceMethodWithKeptParameterName (int arg)
17+
{
18+
}
19+
20+
[Kept]
21+
public static void MethodWithKeptParameterName (string str)
22+
{
23+
}
24+
}
25+
}

0 commit comments

Comments
 (0)