Skip to content

Commit 4c16f5f

Browse files
sbomerCopilotCopilot
authored
Mark descriptor-preserved members as reflection-visible (#128272)
And mark their declaring types as reflection-visible. When a member is marked via descriptor it should be considered visible to reflection. And when that is the case, its declaring type is accessible via reflection, so the requirements for reflection-visible types should be preserved too. Fixes #128120 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 9dd152c commit 4c16f5f

12 files changed

Lines changed: 300 additions & 19 deletions

File tree

‎src/coreclr/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.Shared.xml‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,6 @@
99
<!-- Accessed via native code. -->
1010
<type fullname="System.Runtime.InteropServices.CustomMarshalers.*" />
1111

12-
<!-- Works around https://github.com/dotnet/runtime/issues/128120 -->
13-
<type fullname="System.StubHelpers.DateMarshaler" />
14-
<type fullname="System.StubHelpers.StructureMarshaler`1" />
15-
<type fullname="System.StubHelpers.BlittableArrayMarshaler`1" />
16-
<type fullname="System.StubHelpers.AnsiCharArrayMarshaler`2" />
17-
<type fullname="System.StubHelpers.LayoutClassMarshaler`1" />
18-
<type fullname="System.StubHelpers.BoolMarshaler`1" />
19-
<type fullname="System.StubHelpers.LPWSTRMarshaler" />
20-
<type fullname="System.StubHelpers.LPSTRArrayElementMarshaler`2" />
21-
<type fullname="System.StubHelpers.BSTRArrayElementMarshaler" />
22-
2312
<!-- GetActualImplementationForArrayGenericIListOrIReadOnlyListMethod depends on slots of these interfaces not changing -->
2413
<type fullname="System.Collections.Generic.IEnumerable`1" />
2514
<type fullname="System.Collections.Generic.ICollection`1" />

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ Inheritance.Interfaces.OnReferenceType.InterfaceTypeInOtherUsedOnlyByCopiedAssem
192192
Inheritance.Interfaces.OnReferenceType.InterfaceUsedOnlyAsConstraintIsKept
193193
Inheritance.Interfaces.OnReferenceType.NoInstanceCtor.NoInstanceCtorAndAssemblyPreserveAll
194194
Inheritance.Interfaces.OnReferenceType.NoInstanceCtor.NoInstanceCtorAndTypePreserveAll
195+
Inheritance.Interfaces.OnReferenceType.NoInstanceCtor.NoInstanceCtorAndTypePreserveMethodsWithInterfacesMarked
195196
Inheritance.Interfaces.OnReferenceType.NoKeptCtor.DynamicDependencyPreservesInterfaceMethod
196197
Inheritance.Interfaces.OnReferenceType.NoKeptCtor.ExplicitInterfaceCanBeRemovedFromClassWithOnlyStaticMethodUsed
197198
Inheritance.Interfaces.OnReferenceType.NoKeptCtor.GenericWithConstraintDoesNotCauseOtherTypesToKeepInterface
@@ -240,12 +241,14 @@ Inheritance.Interfaces.RecursiveInterfaces.InterfaceImplementedRecursively
240241
Inheritance.Interfaces.RecursiveInterfaces.OverrideOfRecursiveInterfaceIsRemoved
241242
Inheritance.Interfaces.RecursiveInterfaces.RecursiveInterfaceKept
242243
Inheritance.Interfaces.StaticInterfaceMethods.BaseProvidesInterfaceMethod
244+
Inheritance.Interfaces.StaticInterfaceMethods.DescriptorPreservedTypeIsReflectionVisible
243245
Inheritance.Interfaces.StaticInterfaceMethods.InstanceMethodsWithOverridesSwept
244246
Inheritance.Interfaces.StaticInterfaceMethods.OverrideInCopyAssembly
245247
Inheritance.Interfaces.StaticInterfaceMethods.OverrideInSaveAssembly
246248
Inheritance.Interfaces.StaticInterfaceMethods.RemovedInterfaceImplementationRemovedOverride
247249
Inheritance.Interfaces.StaticInterfaceMethods.StaticAbstractInterfaceMethods
248250
Inheritance.Interfaces.StaticInterfaceMethods.StaticAbstractInterfaceMethodsLibrary
251+
Inheritance.Interfaces.StaticInterfaceMethods.StaticAbstractMethodsPreservedViaDescriptor
249252
Inheritance.Interfaces.StaticInterfaceMethods.StaticInterfaceMethodsInPreservedScope
250253
Inheritance.Interfaces.StaticInterfaceMethods.StaticVirtualInterfaceMethodsInPreservedScope
251254
Inheritance.Interfaces.StaticInterfaceMethods.StaticVirtualInterfaceMethodsInPreservedScopeLibrary
@@ -294,6 +297,7 @@ LinkXml.CanPreserveExportedTypesUsingRegex
294297
LinkXml.EmbeddedLinkXmlFromCopyAssemblyIsProcessed
295298
LinkXml.EmbeddedLinkXmlPreservesAdditionalAssemblyWithOverriddenMethod
296299
LinkXml.LinkXmlErrorCases
300+
LinkXml.TypeWithPreserveFieldsHasBackingFieldsOfPropertiesRemoved
297301
LinkXml.UnusedFieldPreservedByLinkXmlIsKept
298302
LinkXml.UnusedInterfaceTypeOnTypeWithPreserveAllIsKept
299303
LinkXml.UnusedNonRequiredTypeIsRemoved

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,14 @@ protected override void ProcessType(TypeDefinition type, XPathNavigator nav)
154154
if (!required)
155155
return;
156156

157+
// For types with preserve="all"/"methods"/"fields", the type becomes
158+
// reflection-visible through its members via ApplyPreserveInfo. For explicit
159+
// <method>/<field> children, ProcessMethod/ProcessField handle it. For
160+
// preserve="nothing" (no members preserved), we schedule the type itself
161+
// for reflection-visible treatment.
162+
if (preserve is TypePreserve.Nothing)
163+
_context.Annotations.MarkPendingReflectionVisibleType(type, GetMessageOriginForPosition(nav));
164+
157165
_context.Annotations.Mark(type, new DependencyInfo(DependencyKind.XmlDescriptor, _xmlDocumentLocation), GetMessageOriginForPosition(nav));
158166

159167
if (type.IsNested)
@@ -184,7 +192,10 @@ protected override void ProcessField(TypeDefinition type, FieldDefinition field,
184192
if (!_preservedMembers.Add(field))
185193
LogDuplicatePreserve(field.FullName, nav);
186194

187-
_context.Annotations.Mark(field, new DependencyInfo(DependencyKind.XmlDescriptor, _xmlDocumentLocation), GetMessageOriginForPosition(nav));
195+
var reason = new DependencyInfo(DependencyKind.XmlDescriptor, _xmlDocumentLocation);
196+
var origin = GetMessageOriginForPosition(nav);
197+
_context.Annotations.MarkPendingReflectionVisibleField(field, origin);
198+
_context.Annotations.Mark(field, reason, origin);
188199
}
189200

190201
protected override void ProcessMethod(TypeDefinition type, MethodDefinition method, XPathNavigator nav, object? customData)
@@ -201,7 +212,10 @@ protected override void ProcessMethod(TypeDefinition type, MethodDefinition meth
201212
}
202213
else
203214
{
204-
_context.Annotations.Mark(method, new DependencyInfo(DependencyKind.XmlDescriptor, _xmlDocumentLocation), GetMessageOriginForPosition(nav));
215+
var reason = new DependencyInfo(DependencyKind.XmlDescriptor, _xmlDocumentLocation);
216+
var origin = GetMessageOriginForPosition(nav);
217+
_context.Annotations.MarkPendingReflectionVisibleMethod(method, origin);
218+
_context.Annotations.Mark(method, reason, origin);
205219
}
206220
}
207221

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,24 @@ bool ProcessMarkedPending()
504504
ApplyPreserveInfo(type);
505505
}
506506

507+
foreach (var (method, origin) in Annotations.DrainPendingReflectionVisibleMethods())
508+
{
509+
marked = true;
510+
MarkMethodVisibleToReflection(method, DependencyInfo.AlreadyMarked, origin);
511+
}
512+
513+
foreach (var (field, origin) in Annotations.DrainPendingReflectionVisibleFields())
514+
{
515+
marked = true;
516+
MarkFieldVisibleToReflection(field, DependencyInfo.AlreadyMarked, origin);
517+
}
518+
519+
foreach (var (type, origin) in Annotations.DrainPendingReflectionVisibleTypes())
520+
{
521+
marked = true;
522+
MarkTypeVisibleToReflection(type, DependencyInfo.AlreadyMarked, origin);
523+
}
524+
507525
return marked;
508526
}
509527

@@ -2009,6 +2027,11 @@ internal void MarkMethodVisibleToReflection(MethodReference method, in Dependenc
20092027
Annotations.MarkReflectionUsed(methodDefinition);
20102028
Annotations.MarkIndirectlyCalledMethod(methodDefinition);
20112029

2030+
// A reflection-visible method's DeclaringType is also accessible
2031+
// (e.g., via MethodBase.DeclaringType). Mark it as reflection-visible.
2032+
if (!Annotations.IsReflectionUsed(methodDefinition.DeclaringType))
2033+
MarkTypeVisibleToReflection(methodDefinition.DeclaringType, new DependencyInfo(DependencyKind.DeclaringType, methodDefinition), origin);
2034+
20122035
// On a reflectable method, perform generic data flow for the return type and all the parameter types
20132036
// This is a compensation for the DI issue described in https://github.com/dotnet/runtime/issues/81358
20142037
var methodOrigin = new MessageOrigin(methodDefinition);
@@ -2040,6 +2063,13 @@ internal void MarkFieldVisibleToReflection(FieldReference field, in DependencyIn
20402063
MarkField(field, reason, origin);
20412064
if (Context.Resolve(field) is FieldDefinition fieldDefinition)
20422065
{
2066+
Annotations.MarkReflectionUsed(fieldDefinition);
2067+
2068+
// A reflection-visible field's DeclaringType is also accessible
2069+
// (e.g., via FieldInfo.DeclaringType). Mark it as reflection-visible.
2070+
if (!Annotations.IsReflectionUsed(fieldDefinition.DeclaringType))
2071+
MarkTypeVisibleToReflection(fieldDefinition.DeclaringType, new DependencyInfo(DependencyKind.DeclaringType, fieldDefinition), origin);
2072+
20432073
// On a reflectable field, perform generic data flow for the field's type
20442074
// This is a compensation for the DI issue described in https://github.com/dotnet/runtime/issues/81358
20452075
var fieldOrigin = new MessageOrigin(fieldDefinition);

‎src/tools/illink/src/linker/Linker/Annotations.cs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ public partial class AnnotationStore
7171
protected readonly HashSet<MethodDefinition> indirectly_called = new HashSet<MethodDefinition>();
7272
protected readonly HashSet<TypeDefinition> types_relevant_to_variant_casting = new HashSet<TypeDefinition>();
7373
readonly HashSet<IMemberDefinition> reflection_used = new();
74+
readonly List<(MethodDefinition Method, MessageOrigin Origin)> pending_reflection_visible_methods = new();
75+
readonly List<(FieldDefinition Field, MessageOrigin Origin)> pending_reflection_visible_fields = new();
76+
readonly List<(TypeDefinition Type, MessageOrigin Origin)> pending_reflection_visible_types = new();
7477
AssemblyDefinition? entry_assembly;
7578

7679
public AnnotationStore(LinkContext context)
@@ -243,6 +246,45 @@ public bool IsRelevantToVariantCasting(TypeDefinition type)
243246
return types_relevant_to_variant_casting.Contains(type);
244247
}
245248

249+
/// <summary>
250+
/// Schedules a method for reflection-visible marking by MarkStep.
251+
/// </summary>
252+
internal void MarkPendingReflectionVisibleMethod(MethodDefinition method, in MessageOrigin origin)
253+
=> pending_reflection_visible_methods.Add((method, origin));
254+
255+
/// <summary>
256+
/// Schedules a field for reflection-visible marking by MarkStep.
257+
/// </summary>
258+
internal void MarkPendingReflectionVisibleField(FieldDefinition field, in MessageOrigin origin)
259+
=> pending_reflection_visible_fields.Add((field, origin));
260+
261+
/// <summary>
262+
/// Schedules a type for reflection-visible marking by MarkStep.
263+
/// </summary>
264+
internal void MarkPendingReflectionVisibleType(TypeDefinition type, in MessageOrigin origin)
265+
=> pending_reflection_visible_types.Add((type, origin));
266+
267+
internal (MethodDefinition Method, MessageOrigin Origin)[] DrainPendingReflectionVisibleMethods()
268+
{
269+
var result = pending_reflection_visible_methods.ToArray();
270+
pending_reflection_visible_methods.Clear();
271+
return result;
272+
}
273+
274+
internal (FieldDefinition Field, MessageOrigin Origin)[] DrainPendingReflectionVisibleFields()
275+
{
276+
var result = pending_reflection_visible_fields.ToArray();
277+
pending_reflection_visible_fields.Clear();
278+
return result;
279+
}
280+
281+
internal (TypeDefinition Type, MessageOrigin Origin)[] DrainPendingReflectionVisibleTypes()
282+
{
283+
var result = pending_reflection_visible_types.ToArray();
284+
pending_reflection_visible_types.Clear();
285+
return result;
286+
}
287+
246288
public bool SetProcessed(IMetadataTokenProvider provider)
247289
{
248290
if (processed.Add(provider))

‎src/tools/illink/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/OnReferenceType/NoInstanceCtor/NoInstanceCtorAndTypePreserveMethodsWithInterfacesMarked.cs‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,18 @@ namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.OnReferenceType.NoInsta
77
[Define("IL_ASSEMBLY_COMPILED")]
88
[SetupCompileBefore("library.dll", new[] { "Dependencies/NoInstanceCtorAndAssemblyPreserveAll_Lib.il" })]
99

10-
// Interfaces will be removed because there is no instance ctor that is marked.
11-
[RemovedInterfaceOnTypeInAssembly("library",
10+
// Interfaces are kept because preserve="methods" marks methods as reflection-visible,
11+
// which makes the declaring type reflection-visible (accessible via MethodBase.DeclaringType).
12+
[KeptInterfaceOnTypeInAssembly("library",
1213
"Mono.Linker.Tests.Cases.Inheritance.Interfaces.OnReferenceType.NoInstanceCtor.Dependencies.NoInstanceCtorAndAssemblyPreserveAll_Lib/A",
1314
"library",
14-
"Mono.Linker.Tests.Cases.Inheritance.Interfaces.OnReferenceType.NoInstanceCtor.Dependencies.NoInstanceCtorAndAssemblyPreserveAll_Lib/IFoo")]
15-
[RemovedInterfaceOnTypeInAssemblyAttribute("library",
15+
"Mono.Linker.Tests.Cases.Inheritance.Interfaces.OnReferenceType.NoInstanceCtor.Dependencies.NoInstanceCtorAndAssemblyPreserveAll_Lib/IFoo",
16+
Tool = Tool.Trimmer)]
17+
[KeptInterfaceOnTypeInAssemblyAttribute("library",
1618
"Mono.Linker.Tests.Cases.Inheritance.Interfaces.OnReferenceType.NoInstanceCtor.Dependencies.NoInstanceCtorAndAssemblyPreserveAll_Lib/A",
1719
"library",
18-
"Mono.Linker.Tests.Cases.Inheritance.Interfaces.OnReferenceType.NoInstanceCtor.Dependencies.NoInstanceCtorAndAssemblyPreserveAll_Lib/IBar")]
20+
"Mono.Linker.Tests.Cases.Inheritance.Interfaces.OnReferenceType.NoInstanceCtor.Dependencies.NoInstanceCtorAndAssemblyPreserveAll_Lib/IBar",
21+
Tool = Tool.Trimmer)]
1922

2023
// Methods should be kept because of the preserve methods
2124
[KeptMemberInAssembly("library",

‎src/tools/illink/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/DescriptorPreservedTypeIsReflectionVisible.cs‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Copyright(c) .NET Foundation and contributors. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System;
5+
using System.Reflection;
6+
using System.Runtime.InteropServices;
7+
using Mono.Linker.Tests.Cases.Expectations.Assertions;
8+
using Mono.Linker.Tests.Cases.Expectations.Metadata;
9+
10+
namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.StaticInterfaceMethods
11+
{
12+
/// <summary>
13+
/// Tests that a type preserved via XML descriptor is fully treated as reflection-visible:
14+
/// - Its static abstract interface implementations are preserved (variant casting)
15+
/// - Its explicit-layout fields are preserved (MarkImplicitlyUsedFields)
16+
/// These are all consequences of the type being obtainable via
17+
/// MethodBase.GetCurrentMethod().DeclaringType on a descriptor-preserved method.
18+
/// </summary>
19+
[SetupLinkerDescriptorFile("DescriptorPreservedTypeIsReflectionVisible.xml")]
20+
[ExpectedNoWarnings]
21+
public class DescriptorPreservedTypeIsReflectionVisible
22+
{
23+
[Kept]
24+
public static void Main()
25+
{
26+
CallMethodOnConstrainedType<DirectlyUsed>();
27+
UseViaReflection();
28+
}
29+
30+
[Kept]
31+
interface IStaticAbstract
32+
{
33+
[Kept]
34+
static abstract void Method();
35+
36+
[Kept]
37+
public static void Call<T>() where T : IStaticAbstract => T.Method();
38+
}
39+
40+
[Kept]
41+
[KeptInterface(typeof(IStaticAbstract))]
42+
class DirectlyUsed : IStaticAbstract
43+
{
44+
[Kept]
45+
[KeptOverride(typeof(IStaticAbstract))]
46+
public static void Method() { }
47+
}
48+
49+
// Reference type with explicit layout, preserved only via descriptor.
50+
// If the type is properly marked as reflection-visible, its fields should be
51+
// kept due to MarkImplicitlyUsedFields(explicit layout requires all fields).
52+
[Kept]
53+
[KeptInterface(typeof(IStaticAbstract))]
54+
[StructLayout(LayoutKind.Explicit)]
55+
class ExplicitLayoutPreservedViaDescriptor : IStaticAbstract
56+
{
57+
[Kept]
58+
[FieldOffset(0)]
59+
public int FieldA;
60+
61+
[Kept]
62+
[FieldOffset(4)]
63+
public int FieldB;
64+
65+
[Kept]
66+
[KeptOverride(typeof(IStaticAbstract))]
67+
public static void Method() { }
68+
69+
[Kept]
70+
[ExpectedWarning("IL2026", nameof(MethodBase.GetCurrentMethod))]
71+
public static Type GetMyType() => MethodBase.GetCurrentMethod().DeclaringType;
72+
}
73+
74+
[Kept]
75+
[ExpectedWarning("IL3050", nameof(MethodInfo.MakeGenericMethod), Tool.Analyzer | Tool.NativeAot, "")]
76+
static void UseViaReflection()
77+
{
78+
typeof(IStaticAbstract).GetMethod(nameof(IStaticAbstract.Call))
79+
.MakeGenericMethod(ExplicitLayoutPreservedViaDescriptor.GetMyType())
80+
.Invoke(null, null);
81+
}
82+
83+
[Kept]
84+
static void CallMethodOnConstrainedType<T>() where T : IStaticAbstract
85+
{
86+
T.Method();
87+
}
88+
}
89+
}

‎src/tools/illink/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/DescriptorPreservedTypeIsReflectionVisible.xml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<linker>
2+
<assembly fullname="test, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null">
3+
<type fullname="Mono.Linker.Tests.Cases.Inheritance.Interfaces.StaticInterfaceMethods.DescriptorPreservedTypeIsReflectionVisible/ExplicitLayoutPreservedViaDescriptor">
4+
<method name="Method" />
5+
<method name="GetMyType" />
6+
</type>
7+
</assembly>
8+
</linker>

0 commit comments

Comments
 (0)