Skip to content

Commit

Permalink
Track pending marked members (dotnet/linker#1768)
Browse files Browse the repository at this point in the history
Track pending marked members

This enables tracking of pending marked items which need to be fully marked by MarkStep. Contributes to dotnet/linker#1735 - this change is in preparation for running custom steps during MarkStep, where they could change the Annotations state in ways that interact with other marking logic. The idea is that custom steps can call Annotations methods without triggering a lot of other processing, because the full logic is deferred. This way custom steps don't need to be re-entrant.

This change causes Annotations.Mark to place members into a set of "pending" items which will get fully marked ("processed") later. "pending" items are already considered marked.

AddPreservedMethod conceptually adds a conditional dependency from source -> destination - so if source gets marked, destination should get marked. This change will immediately call Annotations.Mark on the destination if the source is already marked, and otherwise track the condition to be applied if the source gets marked later.

SetPreserve can change the TypePreserve of a type after it has been marked. This change will track any changes to TypePreserve and apply them later even for types which have already been marked. This works a little differently from AddPreservedMethod, to avoid traversing type members in Annotations.

Note that once we allow custom steps to run during during MarkStep, Process will call ProcessMarkedPending - but this isn't required in this change. The intention here is just to add adding extra tracking (that will actually be used by dotnet/linker#1666), while mostly preserving existing behavior.

Commit migrated from dotnet/linker@24ba73a
  • Loading branch information
sbomer committed Jan 22, 2021
1 parent 1e2d7a2 commit 4453795
Show file tree
Hide file tree
Showing 10 changed files with 331 additions and 102 deletions.
159 changes: 85 additions & 74 deletions src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -193,26 +193,15 @@ void Initialize ()
{
foreach (AssemblyDefinition assembly in _context.GetAssemblies ())
InitializeAssembly (assembly);

ProcessMarkedPending ();
}

protected virtual void InitializeAssembly (AssemblyDefinition assembly)
{
var action = _context.Annotations.GetAction (assembly);
switch (action) {
case AssemblyAction.Copy:
case AssemblyAction.Save:
Tracer.AddDirectDependency (assembly, new DependencyInfo (DependencyKind.AssemblyAction, action), marked: false);
MarkEntireAssembly (assembly);
break;
case AssemblyAction.Link:
case AssemblyAction.AddBypassNGen:
case AssemblyAction.AddBypassNGenUsed:
MarkAssembly (assembly);

foreach (TypeDefinition type in assembly.MainModule.Types)
InitializeType (type);
break;
}
if (action == AssemblyAction.Copy || action == AssemblyAction.Save)
MarkAssembly (assembly, new DependencyInfo (DependencyKind.AssemblyAction, action));
}

void Complete ()
Expand All @@ -235,27 +224,6 @@ static bool TypeIsDynamicInterfaceCastableImplementation (TypeDefinition type)
return false;
}

void InitializeType (TypeDefinition type)
{
if (type.HasNestedTypes) {
foreach (var nested in type.NestedTypes)
InitializeType (nested);
}

if (!Annotations.IsMarked (type))
return;

// We may get here for a type marked by an earlier step, or by a type
// marked indirectly as the result of some other InitializeType call.
// Just track this as already marked, and don't include a new source.
MarkType (type, DependencyInfo.AlreadyMarked, type);

if (type.HasFields)
InitializeFields (type);
if (type.HasMethods)
InitializeMethods (type.Methods);
}

protected bool IsFullyPreserved (TypeDefinition type)
{
if (Annotations.TryGetPreserve (type, out TypePreserve preserve) && preserve == TypePreserve.All)
Expand All @@ -273,20 +241,6 @@ protected bool IsFullyPreserved (TypeDefinition type)
return false;
}

void InitializeFields (TypeDefinition type)
{
foreach (FieldDefinition field in type.Fields)
if (Annotations.IsMarked (field))
MarkField (field, DependencyInfo.AlreadyMarked);
}

void InitializeMethods (Collection<MethodDefinition> methods)
{
foreach (MethodDefinition method in methods)
if (Annotations.IsMarked (method))
EnqueueMethod (method, DependencyInfo.AlreadyMarked);
}

internal void MarkEntireType (TypeDefinition type, bool includeBaseTypes, in DependencyInfo reason, IMemberDefinition sourceLocationMember)
{
MarkEntireTypeInternal (type, includeBaseTypes, reason, sourceLocationMember, new HashSet<TypeDefinition> ());
Expand Down Expand Up @@ -375,7 +329,8 @@ void Process ()
continue;
if (!Annotations.IsMarked (type))
continue;
_context.MarkingHelpers.MarkExportedType (exported, assembly.MainModule, new DependencyInfo (DependencyKind.ExportedType, type));
var di = new DependencyInfo (DependencyKind.ExportedType, type);
_context.MarkingHelpers.MarkExportedType (exported, assembly.MainModule, di);
}
}
}
Expand All @@ -400,6 +355,48 @@ bool ProcessPrimaryQueue ()
return true;
}

bool ProcessMarkedPending ()
{
bool marked = false;
foreach (var pending in Annotations.GetMarkedPending ()) {
marked = true;

// Some pending items might be processed by the time we get to them.
if (Annotations.IsProcessed (pending))
continue;

switch (pending) {
case TypeDefinition type:
MarkType (type, DependencyInfo.AlreadyMarked, null);
break;
case MethodDefinition method:
MarkMethod (method, DependencyInfo.AlreadyMarked, null);
// Methods will not actually be processed until we drain the method queue.
break;
case FieldDefinition field:
MarkField (field, DependencyInfo.AlreadyMarked);
break;
case ModuleDefinition module:
MarkModule (module, DependencyInfo.AlreadyMarked);
break;
case ExportedType exportedType:
Annotations.SetProcessed (exportedType);
// No additional processing is done for exported types.
break;
default:
throw new NotImplementedException (pending.GetType ().ToString ());
}
}

foreach (var type in Annotations.GetPendingPreserve ()) {
marked = true;
Debug.Assert (Annotations.IsProcessed (type));
ApplyPreserveInfo (type);
}

return marked;
}

void ProcessPendingTypeChecks ()
{
for (int i = 0; i < _pending_isinst_instr.Count; ++i) {
Expand Down Expand Up @@ -1229,18 +1226,22 @@ void MarkCustomAttributeArgument (CustomAttributeArgument argument, ICustomAttri

protected bool CheckProcessed (IMetadataTokenProvider provider)
{
if (Annotations.IsProcessed (provider))
return true;

Annotations.Processed (provider);
return false;
return !Annotations.SetProcessed (provider);
}

protected void MarkAssembly (AssemblyDefinition assembly)

protected void MarkAssembly (AssemblyDefinition assembly, DependencyInfo reason)
{
Annotations.Mark (assembly, reason);
if (CheckProcessed (assembly))
return;

var action = _context.Annotations.GetAction (assembly);
if (action == AssemblyAction.Copy || action == AssemblyAction.Save) {
MarkEntireAssembly (assembly);
return;
}

ProcessModuleType (assembly);

LazyMarkCustomAttributes (assembly);
Expand All @@ -1253,6 +1254,8 @@ protected void MarkAssembly (AssemblyDefinition assembly)

void MarkEntireAssembly (AssemblyDefinition assembly)
{
Debug.Assert (Annotations.IsProcessed (assembly));

MarkCustomAttributes (assembly, new DependencyInfo (DependencyKind.AssemblyOrModuleAttribute, assembly), null);
MarkCustomAttributes (assembly.MainModule, new DependencyInfo (DependencyKind.AssemblyOrModuleAttribute, assembly.MainModule), null);

Expand Down Expand Up @@ -1391,6 +1394,12 @@ void MarkField (FieldDefinition field, in DependencyInfo reason)
throw new ArgumentOutOfRangeException ($"Internal error: unsupported field dependency {reason.Kind}");
#endif

if (reason.Kind == DependencyKind.AlreadyMarked) {
Debug.Assert (Annotations.IsMarked (field));
} else {
Annotations.Mark (field, reason);
}

if (CheckProcessed (field))
return;

Expand Down Expand Up @@ -1433,13 +1442,6 @@ void MarkField (FieldDefinition field, in DependencyInfo reason)
Annotations.SetPreservedStaticCtor (parent);
Annotations.SetSubstitutedInit (parent);
}

if (reason.Kind == DependencyKind.AlreadyMarked) {
Debug.Assert (Annotations.IsMarked (field));
return;
}

Annotations.Mark (field, reason);
}

protected virtual bool IgnoreScope (IMetadataScope scope)
Expand All @@ -1448,9 +1450,16 @@ protected virtual bool IgnoreScope (IMetadataScope scope)
return Annotations.GetAction (assembly) != AssemblyAction.Link;
}

void MarkScope (IMetadataScope scope, TypeDefinition type)
void MarkModule (ModuleDefinition module, DependencyInfo reason)
{
Annotations.Mark (scope, new DependencyInfo (DependencyKind.ScopeOfType, type));
if (reason.Kind == DependencyKind.AlreadyMarked) {
Debug.Assert (Annotations.IsMarked (module));
} else {
Annotations.Mark (module, reason);
}
if (CheckProcessed (module))
return;
MarkAssembly (module.Assembly, new DependencyInfo (DependencyKind.AssemblyOfModule, module));
}

protected virtual void MarkSerializable (TypeDefinition type)
Expand Down Expand Up @@ -1533,7 +1542,7 @@ protected internal virtual TypeDefinition MarkType (TypeReference reference, Dep
// will call MarkType on the attribute type itself).
// If for some reason we do keep the attribute type (could be because of previous reference which would cause IL2045
// or because of a copy assembly with a reference and so on) then we should not spam the warnings due to the type itself.
if (sourceLocationMember.DeclaringType != type)
if (sourceLocationMember != null && sourceLocationMember.DeclaringType != type)
_context.LogWarning (
$"Attribute '{type.GetDisplayName ()}' is being referenced in code but the linker was " +
$"instructed to remove all instances of this attribute. If the attribute instances are necessary make sure to " +
Expand All @@ -1546,7 +1555,7 @@ protected internal virtual TypeDefinition MarkType (TypeReference reference, Dep
if (CheckProcessed (type))
return null;

MarkScope (type.Scope, type);
MarkModule (type.Scope as ModuleDefinition, new DependencyInfo (DependencyKind.ScopeOfType, type));
MarkType (type.BaseType, new DependencyInfo (DependencyKind.BaseType, type), type);
MarkType (type.DeclaringType, new DependencyInfo (DependencyKind.DeclaringType, type), type);
MarkCustomAttributes (type, new DependencyInfo (DependencyKind.CustomAttribute, type), type);
Expand Down Expand Up @@ -1613,6 +1622,7 @@ protected internal virtual TypeDefinition MarkType (TypeReference reference, Dep
DoAdditionalTypeProcessing (type);

ApplyPreserveInfo (type);
ApplyPreserveMethods (type);

return type;
}
Expand Down Expand Up @@ -2278,9 +2288,10 @@ static IGenericParameterProvider GetGenericProviderFromInstance (IGenericInstanc

void ApplyPreserveInfo (TypeDefinition type)
{
ApplyPreserveMethods (type);

if (Annotations.TryGetPreserve (type, out TypePreserve preserve)) {
if (!Annotations.SetAppliedPreserve (type, preserve))
throw new InternalErrorException ($"Type {type} already has applied {preserve}.");

var di = new DependencyInfo (DependencyKind.TypePreserve, type);

switch (preserve) {
Expand Down Expand Up @@ -2350,6 +2361,7 @@ void ApplyPreserveMethods (TypeDefinition type)
if (list == null)
return;

Annotations.ClearPreservedMethods (type);
MarkMethodCollection (list, new DependencyInfo (DependencyKind.PreservedMethod, type), type);
}

Expand All @@ -2359,6 +2371,7 @@ void ApplyPreserveMethods (MethodDefinition method)
if (list == null)
return;

Annotations.ClearPreservedMethods (method);
MarkMethodCollection (list, new DependencyInfo (DependencyKind.PreservedMethod, method), method);
}

Expand Down Expand Up @@ -2479,7 +2492,6 @@ protected virtual MethodDefinition MarkMethod (MethodReference reference, Depend
AssemblyDefinition ResolveAssembly (IMetadataScope scope)
{
AssemblyDefinition assembly = _context.Resolve (scope);
MarkAssembly (assembly);
return assembly;
}

Expand Down Expand Up @@ -2760,8 +2772,7 @@ void ProcessInteropMethod (MethodDefinition method)
{
if (method.IsPInvokeImpl) {
var pii = method.PInvokeInfo;
Annotations.Mark (pii.Module, new DependencyInfo (DependencyKind.InteropMethodDependency, method));

Annotations.MarkProcessed (pii.Module, new DependencyInfo (DependencyKind.InteropMethodDependency, method));
if (!string.IsNullOrEmpty (_context.PInvokesListFile)) {
_context.PInvokes.Add (new PInvokeInfo {
AssemblyName = method.DeclaringType.Module.Name,
Expand Down Expand Up @@ -3127,7 +3138,7 @@ protected virtual void MarkInterfaceImplementation (InterfaceImplementation ifac
MarkCustomAttributes (iface, new DependencyInfo (DependencyKind.CustomAttribute, iface), type);
// Blame the interface type on the interfaceimpl itself.
MarkType (iface.InterfaceType, new DependencyInfo (DependencyKind.InterfaceImplementationInterfaceType, iface), type);
Annotations.Mark (iface, new DependencyInfo (DependencyKind.InterfaceImplementationOnType, type));
Annotations.MarkProcessed (iface, new DependencyInfo (DependencyKind.InterfaceImplementationOnType, type));
}

//
Expand Down
10 changes: 4 additions & 6 deletions src/tools/illink/src/linker/Linker.Steps/ResolveFromXmlStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -167,12 +167,14 @@ protected override void ProcessMethod (TypeDefinition type, MethodDefinition met
if (Annotations.IsMarked (method))
Context.LogWarning ($"Duplicate preserve of '{method.GetDisplayName ()}'", 2025, _xmlDocumentLocation);

Annotations.Mark (method, new DependencyInfo (DependencyKind.XmlDescriptor, _xmlDocumentLocation));
Annotations.MarkIndirectlyCalledMethod (method);
Annotations.SetAction (method, MethodAction.Parse);

if (!(bool) customData)
if (!(bool) customData) {
Annotations.AddPreservedMethod (type, method);
} else {
Annotations.Mark (method, new DependencyInfo (DependencyKind.XmlDescriptor, _xmlDocumentLocation));
}
}

void ProcessMethodIfNotNull (TypeDefinition type, MethodDefinition method, object customData)
Expand Down Expand Up @@ -222,8 +224,6 @@ protected override void ProcessEvent (TypeDefinition type, EventDefinition @even
if (Annotations.IsMarked (@event))
Context.LogWarning ($"Duplicate preserve of '{@event.FullName}'", 2025, _xmlDocumentLocation);

Annotations.Mark (@event, new DependencyInfo (DependencyKind.XmlDescriptor, _xmlDocumentLocation));

ProcessMethod (type, @event.AddMethod, null, customData);
ProcessMethod (type, @event.RemoveMethod, null, customData);
ProcessMethodIfNotNull (type, @event.InvokeMethod, customData);
Expand All @@ -236,8 +236,6 @@ protected override void ProcessProperty (TypeDefinition type, PropertyDefinition
if (Annotations.IsMarked (property))
Context.LogWarning ($"Duplicate preserve of '{property.FullName}'", 2025, _xmlDocumentLocation);

Annotations.Mark (property, new DependencyInfo (DependencyKind.XmlDescriptor, _xmlDocumentLocation));

ProcessPropertyAccessors (type, property, accessors, customData);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ protected override void Process ()
AssemblyAction action = Context.Annotations.GetAction (assembly);
switch (action) {
case AssemblyAction.CopyUsed:
Annotations.Mark (assembly.MainModule, di);
goto case AssemblyAction.Copy;
case AssemblyAction.Copy:
Annotations.Mark (assembly.MainModule, di);
// Mark Step will take care of marking whole assembly
return;
case AssemblyAction.Link:
Expand Down
Loading

0 comments on commit 4453795

Please sign in to comment.