From 90359dd61d02babfb53bc2b412a5dc3fa06ac0d5 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 13 Jan 2021 16:50:15 +0000 Subject: [PATCH 01/12] Track pending marked members --- src/linker/Linker.Steps/MarkStep.cs | 39 ++++++++++++++------------- src/linker/Linker/Annotations.cs | 42 ++++++++++++++++++++++++----- 2 files changed, 57 insertions(+), 24 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 17ffb5aee072..fec1f218f7ee 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -241,7 +241,7 @@ void InitializeType (TypeDefinition type) InitializeType (nested); } - if (!Annotations.IsMarked (type)) + if (!Annotations.IsMarkedPending (type)) return; // We may get here for a type marked by an earlier step, or by a type @@ -275,14 +275,14 @@ protected bool IsFullyPreserved (TypeDefinition type) void InitializeFields (TypeDefinition type) { foreach (FieldDefinition field in type.Fields) - if (Annotations.IsMarked (field)) + if (Annotations.IsMarkedPending (field)) MarkField (field, DependencyInfo.AlreadyMarked); } void InitializeMethods (Collection methods) { foreach (MethodDefinition method in methods) - if (Annotations.IsMarked (method)) + if (Annotations.IsMarkedPending (method)) EnqueueMethod (method, DependencyInfo.AlreadyMarked); } @@ -1228,15 +1228,16 @@ 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) { + // This conceptually marks an assembly (marking the module type and) + // attributes) even though it does not mark the AssemblyDefinition itself + // in Annotations. Other steps rely on the ModuleDefinition being marked instead. + + Annotations.Mark (assembly, reason); if (CheckProcessed (assembly)) return; @@ -1390,6 +1391,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.IsMarkedPending (field)); + } else { + Annotations.Mark (field, reason); + } + if (CheckProcessed (field)) return; @@ -1432,13 +1439,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) @@ -1450,6 +1450,9 @@ protected virtual bool IgnoreScope (IMetadataScope scope) void MarkScope (IMetadataScope scope, TypeDefinition type) { Annotations.Mark (scope, new DependencyInfo (DependencyKind.ScopeOfType, type)); + var di = new DependencyInfo (DependencyKind.ScopeOfType, type); + Annotations.Mark (scope, di); + MarkAssembly (scope.Assembly, di); } protected virtual void MarkSerializable (TypeDefinition type) @@ -1515,7 +1518,7 @@ protected internal virtual TypeDefinition MarkType (TypeReference reference, Dep // Track a mark reason for each call to MarkType. switch (reason.Kind) { case DependencyKind.AlreadyMarked: - Debug.Assert (Annotations.IsMarked (type)); + Debug.Assert (Annotations.IsMarkedPending (type)); break; default: Annotations.Mark (type, reason); @@ -2509,7 +2512,7 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo // only once per method. switch (reason.Kind) { case DependencyKind.AlreadyMarked: - Debug.Assert (Annotations.IsMarked (method)); + Debug.Assert (Annotations.IsMarkedPending (method)); break; default: Annotations.Mark (method, reason); diff --git a/src/linker/Linker/Annotations.cs b/src/linker/Linker/Annotations.cs index ebb241f1f1d9..40123620b0fb 100644 --- a/src/linker/Linker/Annotations.cs +++ b/src/linker/Linker/Annotations.cs @@ -49,7 +49,9 @@ public partial class AnnotationStore protected readonly Dictionary field_values = new Dictionary (); protected readonly HashSet field_init = new HashSet (); protected readonly HashSet fieldType_init = new HashSet (); - protected readonly HashSet marked = new HashSet (); + + // TODO: make this a simple hashset, don't count. + protected readonly Dictionary marked_pending = new Dictionary (); protected readonly HashSet processed = new HashSet (); protected readonly Dictionary preserved_types = new Dictionary (); protected readonly Dictionary preserved_type_members = new (); @@ -169,13 +171,22 @@ public bool HasSubstitutedInit (TypeDefinition type) [Obsolete ("Mark token providers with a reason instead.")] public void Mark (IMetadataTokenProvider provider) { - marked.Add (provider); + AddMarkPending (provider); + } + + void AddMarkPending (IMetadataTokenProvider provider) + { + if (!marked_pending.TryGetValue (provider, out int count)) { + marked_pending.Add (provider, 1); + } else { + marked_pending[provider] = count + 1; + } } public void Mark (IMetadataTokenProvider provider, in DependencyInfo reason) { Debug.Assert (!(reason.Kind == DependencyKind.AlreadyMarked)); - marked.Add (provider); + AddMarkPending (provider); Tracer.AddDirectDependency (provider, reason, marked: true); } @@ -192,9 +203,14 @@ public void Mark (CustomAttribute attribute, in DependencyInfo reason) Tracer.AddDirectDependency (attribute, reason, marked: true); } + public bool IsMarkedPending (IMetadataTokenProvider provider) + { + return marked_pending.ContainsKey (provider); + } + public bool IsMarked (IMetadataTokenProvider provider) { - return marked.Contains (provider); + return processed.Contains (provider) || marked_pending.ContainsKey (provider); } public bool IsMarked (CustomAttribute attribute) @@ -241,9 +257,23 @@ public bool IsRelevantToVariantCasting (TypeDefinition type) return types_relevant_to_variant_casting.Contains (type); } - public void Processed (IMetadataTokenProvider provider) + void RemoveMarkedPending (IMetadataTokenProvider provider) + { + if (!marked_pending.TryGetValue (provider, out int count)) { + throw new InvalidOperationException (); + } + if (count == 1) { + marked_pending.Remove (provider); + } else { + marked_pending[provider] = count - 1; + } + } + + public bool SetProcessed (IMetadataTokenProvider provider) { - processed.Add (provider); + Debug.Assert (marked_pending.ContainsKey (provider)); + RemoveMarkedPending (provider); + return processed.Add (provider); } public bool IsProcessed (IMetadataTokenProvider provider) From 813c595d5eabeea541ebbd0ec028b113d1699758 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 14 Jan 2021 19:45:00 +0000 Subject: [PATCH 02/12] Track pending preserved types and methods - Call Annotations.Mark for preserved methods up-front, if the key was already preserved. This way we only track preserved methods for things that aren't marked already. - Track pending preserve info that needs to be applied - Clean up logic for pending marked members The Mark* methods in MarkStep "fully" mark members (base types, parameters, etc.), whereas the methods in Annotations only perform a "shallow" mark (they track the fact that this member is to be kept, but they don't process implications of this fact - for example, they may mark methods without marking the containing type). MarkStep fulfills "shallow" marks by keeping pieces of IL that are required. Each Mark* method in MarkStep now serves two purposes: - Recursive marking of required IL (for example base types) It calls Mark* on everything that is required, with a "reason" so that this gets traced. The base cases call Annotations.Mark to trace the reason and keep the required IL. After this is done, the original member passed to Mark* is considered "processed". We set the processed bit early enough to ensure that the recursive logic is only performed once. - Fulfilling "shallow" marks Similar to the above, but in this case, it does not need to trace the dependency again (Annotations.Mark has already been called). We need to ensure that it transitions the mark state from "pending" -> "processed" so that anything else which does a shallow mark of the member doesn't introduce new "pending" state. --- src/linker/Linker.Steps/MarkStep.cs | 82 ++++++++++-- src/linker/Linker.Steps/ResolveFromXmlStep.cs | 9 +- src/linker/Linker/Annotations.cs | 124 +++++++++++++----- src/linker/Linker/DependencyInfo.cs | 1 + src/linker/Linker/MarkingHelpers.cs | 4 +- .../CustomStepCanPreserveMethodsAfterMark.cs | 31 +++++ .../CustomStepCanPreserveMethodsBeforeMark.cs | 31 +++++ .../Dependencies/PreserveMethodsSubStep.cs | 38 ++++++ 8 files changed, 265 insertions(+), 55 deletions(-) create mode 100644 test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsAfterMark.cs create mode 100644 test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsBeforeMark.cs create mode 100644 test/Mono.Linker.Tests.Cases/Extensibility/Dependencies/PreserveMethodsSubStep.cs diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index fec1f218f7ee..e9760941a7b8 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -352,7 +352,7 @@ private void MarkEntireTypeInternal (TypeDefinition type, bool includeBaseTypes, void Process () { - while (ProcessPrimaryQueue () || ProcessLazyAttributes () || ProcessLateMarkedAttributes ()) { + while (ProcessPrimaryQueue () || ProcessMarkedPending() || ProcessLazyAttributes () || ProcessLateMarkedAttributes ()) { // deal with [TypeForwardedTo] pseudo-attributes foreach (AssemblyDefinition assembly in _context.GetAssemblies ()) { @@ -374,7 +374,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); } } } @@ -399,6 +400,47 @@ bool ProcessPrimaryQueue () return true; } + bool ProcessMarkedPending () + { + var allPending = Annotations.MarkedPending (); + bool marked = false; + foreach (var pending in Annotations.MarkedPending ()) { + 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, type); + Debug.Assert (!Annotations.IsMarkedPending (type)); + break; + case MethodDefinition method: + EnqueueMethod (method, DependencyInfo.AlreadyMarked); + break; + case FieldDefinition field: + MarkField (field, DependencyInfo.AlreadyMarked); + Debug.Assert (!Annotations.IsMarkedPending (field)); + break; + case ModuleDefinition module: + MarkModule (module, DependencyInfo.AlreadyMarked); + Debug.Assert (!Annotations.IsMarkedPending (module)); + break; + default: + throw new NotImplementedException (pending.GetType ().ToString ()); + } + } + + foreach (var type in Annotations.PendingPreserve ()) { + marked = true; + Debug.Assert (Annotations.IsProcessed (type)); + ApplyPreserveInfo (type); + } + + return marked; + } + void ProcessPendingTypeChecks () { for (int i = 0; i < _pending_isinst_instr.Count; ++i) { @@ -1231,6 +1273,7 @@ protected bool CheckProcessed (IMetadataTokenProvider provider) return !Annotations.SetProcessed (provider); } + protected void MarkAssembly (AssemblyDefinition assembly, DependencyInfo reason) { // This conceptually marks an assembly (marking the module type and) @@ -1392,7 +1435,7 @@ void MarkField (FieldDefinition field, in DependencyInfo reason) #endif if (reason.Kind == DependencyKind.AlreadyMarked) { - Debug.Assert (Annotations.IsMarkedPending (field)); + Debug.Assert (Annotations.IsMarked (field)); } else { Annotations.Mark (field, reason); } @@ -1447,12 +1490,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)); - var di = new DependencyInfo (DependencyKind.ScopeOfType, type); - Annotations.Mark (scope, di); - MarkAssembly (scope.Assembly, di); + 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) @@ -1518,7 +1565,7 @@ protected internal virtual TypeDefinition MarkType (TypeReference reference, Dep // Track a mark reason for each call to MarkType. switch (reason.Kind) { case DependencyKind.AlreadyMarked: - Debug.Assert (Annotations.IsMarkedPending (type)); + Debug.Assert (Annotations.IsMarked (type)); break; default: Annotations.Mark (type, reason); @@ -1548,7 +1595,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); @@ -1612,6 +1659,9 @@ protected internal virtual TypeDefinition MarkType (TypeReference reference, Dep MarkMethodsIf (type.Methods, HasOnSerializeOrDeserializeAttribute, new DependencyInfo (DependencyKind.SerializationMethodForType, type), type); } + ApplyPreserveInfo (type); + ApplyPreserveMethods (type); + DoAdditionalTypeProcessing (type); ApplyPreserveInfo (type); @@ -2280,9 +2330,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 InvalidOperationException (); + var di = new DependencyInfo (DependencyKind.TypePreserve, type); switch (preserve) { @@ -2352,6 +2403,7 @@ void ApplyPreserveMethods (TypeDefinition type) if (list == null) return; + Annotations.ClearPreservedMethods (type); MarkMethodCollection (list, new DependencyInfo (DependencyKind.PreservedMethod, type), type); } @@ -2361,6 +2413,7 @@ void ApplyPreserveMethods (MethodDefinition method) if (list == null) return; + Annotations.ClearPreservedMethods (method); MarkMethodCollection (list, new DependencyInfo (DependencyKind.PreservedMethod, method), method); } @@ -2512,7 +2565,7 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo // only once per method. switch (reason.Kind) { case DependencyKind.AlreadyMarked: - Debug.Assert (Annotations.IsMarkedPending (method)); + Debug.Assert (Annotations.IsMarked (method)); break; default: Annotations.Mark (method, reason); @@ -2761,7 +2814,7 @@ void ProcessInteropMethod (MethodDefinition method) if (method.IsPInvokeImpl) { var pii = method.PInvokeInfo; Annotations.Mark (pii.Module, new DependencyInfo (DependencyKind.InteropMethodDependency, method)); - + Annotations.SetProcessed (pii.Module); if (!string.IsNullOrEmpty (_context.PInvokesListFile)) { _context.PInvokes.Add (new PInvokeInfo { AssemblyName = method.DeclaringType.Module.Name, @@ -3128,6 +3181,7 @@ protected virtual void MarkInterfaceImplementation (InterfaceImplementation ifac // 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.SetProcessed (iface); } // diff --git a/src/linker/Linker.Steps/ResolveFromXmlStep.cs b/src/linker/Linker.Steps/ResolveFromXmlStep.cs index 4d66a35341db..76b0ad714c72 100644 --- a/src/linker/Linker.Steps/ResolveFromXmlStep.cs +++ b/src/linker/Linker.Steps/ResolveFromXmlStep.cs @@ -171,8 +171,11 @@ protected override void ProcessMethod (TypeDefinition type, MethodDefinition met 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) @@ -222,8 +225,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); @@ -236,8 +237,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); } diff --git a/src/linker/Linker/Annotations.cs b/src/linker/Linker/Annotations.cs index 40123620b0fb..9bb9b93811c3 100644 --- a/src/linker/Linker/Annotations.cs +++ b/src/linker/Linker/Annotations.cs @@ -50,10 +50,10 @@ public partial class AnnotationStore protected readonly HashSet field_init = new HashSet (); protected readonly HashSet fieldType_init = new HashSet (); - // TODO: make this a simple hashset, don't count. - protected readonly Dictionary marked_pending = new Dictionary (); + protected readonly HashSet marked_pending = new HashSet (); protected readonly HashSet processed = new HashSet (); - protected readonly Dictionary preserved_types = new Dictionary (); + protected readonly Dictionary preserved_types = new Dictionary (); + protected readonly HashSet pending_preserve = new HashSet (); protected readonly Dictionary preserved_type_members = new (); protected readonly Dictionary preserved_exportedtype_members = new (); protected readonly Dictionary> preserved_methods = new Dictionary> (); @@ -171,22 +171,15 @@ public bool HasSubstitutedInit (TypeDefinition type) [Obsolete ("Mark token providers with a reason instead.")] public void Mark (IMetadataTokenProvider provider) { - AddMarkPending (provider); - } - - void AddMarkPending (IMetadataTokenProvider provider) - { - if (!marked_pending.TryGetValue (provider, out int count)) { - marked_pending.Add (provider, 1); - } else { - marked_pending[provider] = count + 1; - } + if (!processed.Contains (provider)) + marked_pending.Add (provider); } public void Mark (IMetadataTokenProvider provider, in DependencyInfo reason) { Debug.Assert (!(reason.Kind == DependencyKind.AlreadyMarked)); - AddMarkPending (provider); + if (!processed.Contains (provider)) + marked_pending.Add (provider); Tracer.AddDirectDependency (provider, reason, marked: true); } @@ -203,14 +196,20 @@ public void Mark (CustomAttribute attribute, in DependencyInfo reason) Tracer.AddDirectDependency (attribute, reason, marked: true); } + public IEnumerable MarkedPending() + { + foreach (var pending in marked_pending.ToArray ()) + yield return pending; + } + public bool IsMarkedPending (IMetadataTokenProvider provider) { - return marked_pending.ContainsKey (provider); + return marked_pending.Contains (provider); } public bool IsMarked (IMetadataTokenProvider provider) { - return processed.Contains (provider) || marked_pending.ContainsKey (provider); + return processed.Contains (provider) || marked_pending.Contains (provider); } public bool IsMarked (CustomAttribute attribute) @@ -257,36 +256,75 @@ public bool IsRelevantToVariantCasting (TypeDefinition type) return types_relevant_to_variant_casting.Contains (type); } - void RemoveMarkedPending (IMetadataTokenProvider provider) + public bool SetProcessed (IMetadataTokenProvider provider) { - if (!marked_pending.TryGetValue (provider, out int count)) { - throw new InvalidOperationException (); - } - if (count == 1) { - marked_pending.Remove (provider); - } else { - marked_pending[provider] = count - 1; + if (processed.Add (provider)) { + Debug.Assert (marked_pending.Remove (provider)); + return true; } + + return false; } - public bool SetProcessed (IMetadataTokenProvider provider) + public bool IsProcessed (IMetadataTokenProvider provider) { - Debug.Assert (marked_pending.ContainsKey (provider)); - RemoveMarkedPending (provider); - return processed.Add (provider); + return processed.Contains (provider); } - public bool IsProcessed (IMetadataTokenProvider provider) + public IEnumerable PendingPreserve () { - return processed.Contains (provider); + foreach (var type in pending_preserve.ToArray ()) + yield return type; + } + + public bool SetAppliedPreserve (TypeDefinition type, TypePreserve preserve) + { + if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) { + throw new InvalidOperationException (); + } + + if (preserve != existing.preserve) + throw new InvalidOperationException (); + + if (existing.applied) { + Debug.Assert (!pending_preserve.Contains (type)); + return false; + } + + preserved_types[type] = (existing.preserve, true); + pending_preserve.Remove (type); + return true; + } + + public bool HasAppliedPreserve (TypeDefinition type, TypePreserve preserve) + { + if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) { + throw new InvalidOperationException (); + } + + if (preserve != existing.preserve) + throw new InvalidOperationException (); + + return existing.applied; } public void SetPreserve (TypeDefinition type, TypePreserve preserve) { - if (preserved_types.TryGetValue (type, out TypePreserve existing)) - preserved_types[type] = ChoosePreserveActionWhichPreservesTheMost (existing, preserve); - else - preserved_types.Add (type, preserve); + Debug.Assert (preserve != TypePreserve.Nothing); + if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) { + preserved_types.Add (type, (preserve, false)); + return; + } + + Debug.Assert (existing.preserve != TypePreserve.Nothing); + var newPreserve = ChoosePreserveActionWhichPreservesTheMost (existing.preserve, preserve); + if (newPreserve != existing.preserve) { + if (existing.applied) { + var addedPending = pending_preserve.Add (type); + Debug.Assert (addedPending); + } + preserved_types[type] = (newPreserve, false); + } } public static TypePreserve ChoosePreserveActionWhichPreservesTheMost (TypePreserve leftPreserveAction, TypePreserve rightPreserveAction) @@ -312,7 +350,7 @@ public static TypePreserve ChoosePreserveActionWhichPreservesTheMost (TypePreser public bool TryGetPreserve (TypeDefinition type, out TypePreserve preserve) { - return preserved_types.TryGetValue (type, out preserve); + return preserved_types.TryGetValue (type, preserve); } public void SetMembersPreserve (TypeDefinition type, TypePreserveMembers preserve) @@ -411,6 +449,11 @@ public List GetPreservedMethods (TypeDefinition type) return GetPreservedMethods (type as IMemberDefinition); } + public bool ClearPreservedMethods (TypeDefinition type) + { + return preserved_methods.Remove (type); + } + public void AddPreservedMethod (TypeDefinition type, MethodDefinition method) { AddPreservedMethod (type as IMemberDefinition, method); @@ -421,6 +464,11 @@ public List GetPreservedMethods (MethodDefinition method) return GetPreservedMethods (method as IMemberDefinition); } + public bool ClearPreservedMethods (MethodDefinition key) + { + return preserved_methods.Remove (key); + } + public void AddPreservedMethod (MethodDefinition key, MethodDefinition method) { AddPreservedMethod (key as IMemberDefinition, method); @@ -436,6 +484,12 @@ List GetPreservedMethods (IMemberDefinition definition) void AddPreservedMethod (IMemberDefinition definition, MethodDefinition method) { + if (IsMarked (definition)) { + Mark (method, new DependencyInfo (DependencyKind.PreservedMethod, definition)); + Debug.Assert (GetPreservedMethods (definition) == null); + return; + } + var methods = GetPreservedMethods (definition); if (methods == null) { methods = new List (); diff --git a/src/linker/Linker/DependencyInfo.cs b/src/linker/Linker/DependencyInfo.cs index 758a424e1022..c5254038d12b 100644 --- a/src/linker/Linker/DependencyInfo.cs +++ b/src/linker/Linker/DependencyInfo.cs @@ -51,6 +51,7 @@ public enum DependencyKind // Modules and assemblies ScopeOfType = 28, // type -> module/assembly + AssemblyOfModule = 80, // module -> assembly TypeInAssembly = 29, // assembly -> type ModuleOfExportedType = 30, // exported type -> module ExportedType = 31, // type -> exported type diff --git a/src/linker/Linker/MarkingHelpers.cs b/src/linker/Linker/MarkingHelpers.cs index 0fd829722ed8..794cf466c802 100644 --- a/src/linker/Linker/MarkingHelpers.cs +++ b/src/linker/Linker/MarkingHelpers.cs @@ -1,4 +1,4 @@ -using System; +using System; using Mono.Cecil; namespace Mono.Linker @@ -15,6 +15,8 @@ public MarkingHelpers (LinkContext context) public void MarkExportedType (ExportedType type, ModuleDefinition module, in DependencyInfo reason) { _context.Annotations.Mark (type, reason); + if (!_context.Annotations.SetProcessed (type)) + return; if (_context.KeepTypeForwarderOnlyAssemblies) _context.Annotations.Mark (module, new DependencyInfo (DependencyKind.ModuleOfExportedType, type)); } diff --git a/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsAfterMark.cs b/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsAfterMark.cs new file mode 100644 index 000000000000..4cb681e00a2b --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsAfterMark.cs @@ -0,0 +1,31 @@ +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Metadata; + +namespace Mono.Linker.Tests.Cases.Extensibility +{ +#if !NETCOREAPP + [IgnoreTestCase ("Specific to the illink build")] +#endif + [SetupCompileBefore ("customstep.dll", new[] { "Dependencies/PreserveMethodsSubStep.cs" }, new[] { "illink.dll", "Mono.Cecil.dll", "netstandard.dll" })] + [SetupLinkerArgument ("--custom-step", "+MarkStep:PreserveMethodsSubStep,customstep.dll")] + public class CustomStepCanPreserveMethodsAfterMark + { + public static void Main () + { + UsedType.UsedMethod (); + } + + [Kept] + static class UsedType { + [Kept] + public static void UsedMethod () { } + + [Kept] + public static void PreservedForType () { } + + [Kept] + public static void PreservedForMethod_UsedMethod () { } + } + + } +} \ No newline at end of file diff --git a/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsBeforeMark.cs b/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsBeforeMark.cs new file mode 100644 index 000000000000..fee7250af330 --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsBeforeMark.cs @@ -0,0 +1,31 @@ +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Metadata; + +namespace Mono.Linker.Tests.Cases.Extensibility +{ +#if !NETCOREAPP + [IgnoreTestCase ("Specific to the illink build")] +#endif + [SetupCompileBefore ("customstep.dll", new[] { "Dependencies/PreserveMethodsSubStep.cs" }, new[] { "illink.dll", "Mono.Cecil.dll", "netstandard.dll" })] + [SetupLinkerArgument ("--custom-step", "-MarkStep:PreserveMethodsSubStep,customstep.dll")] + public class CustomStepCanPreserveMethodsBeforeMark + { + public static void Main () + { + UsedType.UsedMethod (); + } + + [Kept] + static class UsedType { + [Kept] + public static void UsedMethod () { } + + [Kept] + public static void PreservedForType () { } + + [Kept] + public static void PreservedForMethod_UsedMethod () { } + } + + } +} \ No newline at end of file diff --git a/test/Mono.Linker.Tests.Cases/Extensibility/Dependencies/PreserveMethodsSubStep.cs b/test/Mono.Linker.Tests.Cases/Extensibility/Dependencies/PreserveMethodsSubStep.cs new file mode 100644 index 000000000000..895771fc98d4 --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/Extensibility/Dependencies/PreserveMethodsSubStep.cs @@ -0,0 +1,38 @@ +using System; +using Mono.Cecil; +using Mono.Linker.Steps; + +class PreserveMethodsSubStep : BaseStep +{ + + protected override void Process () { + foreach (var assembly in Context.GetAssemblies ()) { + foreach (var type in assembly.MainModule.Types) + ProcessType (type); + } + } + + void ProcessType (TypeDefinition type) + { + if (type.HasNestedTypes) { + foreach (var nested in type.NestedTypes) + ProcessType (nested); + } + + + foreach (var method in type.Methods) { + if (method.Name == "PreservedForType") + Annotations.AddPreservedMethod (type, method); + + ProcessMethod (method); + } + } + + public void ProcessMethod (MethodDefinition method) + { + foreach (var m in method.DeclaringType.Methods) { + if (m.Name == $"PreservedForMethod_{method.Name}") + Annotations.AddPreservedMethod (method, m); + } + } +} From 0a3be76d2bc1f5a0d6bfdfe968cadb6451a9985c Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 19 Jan 2021 18:24:11 +0000 Subject: [PATCH 03/12] Cleanup after rebase Call MarkMethod instead of EnqueueMethod so that early-marked methods get an action assigned. This used to happen in ResolveFromAssemblyStep. Also fix up typepreserve to correctly track accessibility bits, and add a testcase for copyused marking behavior. Fix formatting --- src/linker/Linker.Steps/MarkStep.cs | 7 +++--- .../Linker.Steps/RootAssemblyInputStep.cs | 3 +-- src/linker/Linker/Annotations.cs | 25 +++++++++++-------- src/linker/Linker/DependencyInfo.cs | 2 +- .../CustomStepCanPreserveMethodsAfterMark.cs | 3 ++- .../CustomStepCanPreserveMethodsBeforeMark.cs | 3 ++- .../Dependencies/PreserveMethodsSubStep.cs | 3 ++- .../CopyUsedAssemblyWithMainEntryRoot.cs | 4 +++ .../CopyUsedAssemblyWithMainEntryRoot_Lib.cs | 6 +++++ 9 files changed, 35 insertions(+), 21 deletions(-) create mode 100644 test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyUsedAssemblyWithMainEntryRoot_Lib.cs diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index e9760941a7b8..3c7a0c0d40da 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -283,7 +283,7 @@ void InitializeMethods (Collection methods) { foreach (MethodDefinition method in methods) if (Annotations.IsMarkedPending (method)) - EnqueueMethod (method, DependencyInfo.AlreadyMarked); + MarkMethod (method, DependencyInfo.AlreadyMarked, null); } internal void MarkEntireType (TypeDefinition type, bool includeBaseTypes, in DependencyInfo reason, IMemberDefinition sourceLocationMember) @@ -352,7 +352,7 @@ private void MarkEntireTypeInternal (TypeDefinition type, bool includeBaseTypes, void Process () { - while (ProcessPrimaryQueue () || ProcessMarkedPending() || ProcessLazyAttributes () || ProcessLateMarkedAttributes ()) { + while (ProcessPrimaryQueue () || ProcessMarkedPending () || ProcessLazyAttributes () || ProcessLateMarkedAttributes ()) { // deal with [TypeForwardedTo] pseudo-attributes foreach (AssemblyDefinition assembly in _context.GetAssemblies ()) { @@ -402,7 +402,6 @@ bool ProcessPrimaryQueue () bool ProcessMarkedPending () { - var allPending = Annotations.MarkedPending (); bool marked = false; foreach (var pending in Annotations.MarkedPending ()) { marked = true; @@ -417,7 +416,7 @@ bool ProcessMarkedPending () Debug.Assert (!Annotations.IsMarkedPending (type)); break; case MethodDefinition method: - EnqueueMethod (method, DependencyInfo.AlreadyMarked); + MarkMethod (method, DependencyInfo.AlreadyMarked, null); break; case FieldDefinition field: MarkField (field, DependencyInfo.AlreadyMarked); diff --git a/src/linker/Linker.Steps/RootAssemblyInputStep.cs b/src/linker/Linker.Steps/RootAssemblyInputStep.cs index 30577931fed5..7279cf01a760 100644 --- a/src/linker/Linker.Steps/RootAssemblyInputStep.cs +++ b/src/linker/Linker.Steps/RootAssemblyInputStep.cs @@ -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: diff --git a/src/linker/Linker/Annotations.cs b/src/linker/Linker/Annotations.cs index 9bb9b93811c3..be8e7bc09bcf 100644 --- a/src/linker/Linker/Annotations.cs +++ b/src/linker/Linker/Annotations.cs @@ -196,7 +196,7 @@ public void Mark (CustomAttribute attribute, in DependencyInfo reason) Tracer.AddDirectDependency (attribute, reason, marked: true); } - public IEnumerable MarkedPending() + public IEnumerable MarkedPending () { foreach (var pending in marked_pending.ToArray ()) yield return pending; @@ -262,7 +262,7 @@ public bool SetProcessed (IMetadataTokenProvider provider) Debug.Assert (marked_pending.Remove (provider)); return true; } - + return false; } @@ -282,7 +282,6 @@ public bool SetAppliedPreserve (TypeDefinition type, TypePreserve preserve) if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) { throw new InvalidOperationException (); } - if (preserve != existing.preserve) throw new InvalidOperationException (); @@ -301,7 +300,6 @@ public bool HasAppliedPreserve (TypeDefinition type, TypePreserve preserve) if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) { throw new InvalidOperationException (); } - if (preserve != existing.preserve) throw new InvalidOperationException (); @@ -315,7 +313,6 @@ public void SetPreserve (TypeDefinition type, TypePreserve preserve) preserved_types.Add (type, (preserve, false)); return; } - Debug.Assert (existing.preserve != TypePreserve.Nothing); var newPreserve = ChoosePreserveActionWhichPreservesTheMost (existing.preserve, preserve); if (newPreserve != existing.preserve) { @@ -350,7 +347,13 @@ public static TypePreserve ChoosePreserveActionWhichPreservesTheMost (TypePreser public bool TryGetPreserve (TypeDefinition type, out TypePreserve preserve) { - return preserved_types.TryGetValue (type, preserve); + if (preserved_types.TryGetValue (type, out (TypePreserve preserve, bool _applied) existing)) { + preserve = existing.preserve; + return true; + } + + preserve = default (TypePreserve); + return false; } public void SetMembersPreserve (TypeDefinition type, TypePreserveMembers preserve) @@ -375,11 +378,6 @@ static TypePreserveMembers CombineMembers (TypePreserveMembers left, TypePreserv return TypePreserveMembers.AllVisibleOrInternal; } - public bool TryGetPreservedMembers (TypeDefinition type, out TypePreserveMembers preserve) - { - return preserved_type_members.TryGetValue (type, out preserve); - } - public void SetMembersPreserve (ExportedType type, TypePreserveMembers preserve) { if (preserved_exportedtype_members.TryGetValue (type, out TypePreserveMembers existing)) @@ -388,6 +386,11 @@ public void SetMembersPreserve (ExportedType type, TypePreserveMembers preserve) preserved_exportedtype_members.Add (type, preserve); } + public bool TryGetPreservedMembers (TypeDefinition type, out TypePreserveMembers preserve) + { + return preserved_type_members.TryGetValue (type, out preserve); + } + public bool TryGetPreservedMembers (ExportedType type, out TypePreserveMembers preserve) { return preserved_exportedtype_members.TryGetValue (type, out preserve); diff --git a/src/linker/Linker/DependencyInfo.cs b/src/linker/Linker/DependencyInfo.cs index c5254038d12b..cfb0e51b755d 100644 --- a/src/linker/Linker/DependencyInfo.cs +++ b/src/linker/Linker/DependencyInfo.cs @@ -51,7 +51,7 @@ public enum DependencyKind // Modules and assemblies ScopeOfType = 28, // type -> module/assembly - AssemblyOfModule = 80, // module -> assembly + AssemblyOfModule = 81, // module -> assembly TypeInAssembly = 29, // assembly -> type ModuleOfExportedType = 30, // exported type -> module ExportedType = 31, // type -> exported type diff --git a/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsAfterMark.cs b/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsAfterMark.cs index 4cb681e00a2b..b6dd62202053 100644 --- a/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsAfterMark.cs +++ b/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsAfterMark.cs @@ -16,7 +16,8 @@ public static void Main () } [Kept] - static class UsedType { + static class UsedType + { [Kept] public static void UsedMethod () { } diff --git a/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsBeforeMark.cs b/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsBeforeMark.cs index fee7250af330..7ded140fc7b4 100644 --- a/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsBeforeMark.cs +++ b/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsBeforeMark.cs @@ -16,7 +16,8 @@ public static void Main () } [Kept] - static class UsedType { + static class UsedType + { [Kept] public static void UsedMethod () { } diff --git a/test/Mono.Linker.Tests.Cases/Extensibility/Dependencies/PreserveMethodsSubStep.cs b/test/Mono.Linker.Tests.Cases/Extensibility/Dependencies/PreserveMethodsSubStep.cs index 895771fc98d4..4cd24f2742a3 100644 --- a/test/Mono.Linker.Tests.Cases/Extensibility/Dependencies/PreserveMethodsSubStep.cs +++ b/test/Mono.Linker.Tests.Cases/Extensibility/Dependencies/PreserveMethodsSubStep.cs @@ -5,7 +5,8 @@ class PreserveMethodsSubStep : BaseStep { - protected override void Process () { + protected override void Process () + { foreach (var assembly in Context.GetAssemblies ()) { foreach (var type in assembly.MainModule.Types) ProcessType (type); diff --git a/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithMainEntryRoot.cs b/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithMainEntryRoot.cs index e9f020d16209..2c9a71daf7b5 100644 --- a/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithMainEntryRoot.cs +++ b/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithMainEntryRoot.cs @@ -1,5 +1,6 @@ using Mono.Linker.Tests.Cases.Expectations.Assertions; using Mono.Linker.Tests.Cases.Expectations.Metadata; +using Mono.Linker.Tests.Cases.Libraries.Dependencies; namespace Mono.Linker.Tests.Cases.Libraries { @@ -9,6 +10,8 @@ namespace Mono.Linker.Tests.Cases.Libraries [Kept] [KeptMember (".ctor()")] [SetupLinkerAction ("copyused", "test")] + [SetupCompileBefore ("lib.dll", new[] { "Dependencies/CopyUsedAssemblyWithMainEntryRoot_Lib.cs" })] + [KeptTypeInAssembly ("lib.dll", typeof (CopyUsedAssemblyWithMainEntryRoot_Lib))] public class CopyUsedAssemblyWithMainEntryRoot { [Kept] @@ -24,6 +27,7 @@ public void UnusedPublicMethod () [Kept] private void UnusedPrivateMethod () { + new CopyUsedAssemblyWithMainEntryRoot_Lib (); } } } \ No newline at end of file diff --git a/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyUsedAssemblyWithMainEntryRoot_Lib.cs b/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyUsedAssemblyWithMainEntryRoot_Lib.cs new file mode 100644 index 000000000000..3ce2fb1a1b6a --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyUsedAssemblyWithMainEntryRoot_Lib.cs @@ -0,0 +1,6 @@ +namespace Mono.Linker.Tests.Cases.Libraries.Dependencies +{ + public class CopyUsedAssemblyWithMainEntryRoot_Lib + { + } +} \ No newline at end of file From deb8fd8998002641744451a5c37bbdc67357b4ea Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 20 Jan 2021 00:32:47 +0000 Subject: [PATCH 04/12] Replace InitializeAssembly logic --- src/linker/Linker.Steps/MarkStep.cs | 69 ++++------------------------- 1 file changed, 9 insertions(+), 60 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 3c7a0c0d40da..86313d91c85a 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -190,28 +190,7 @@ public virtual void Process (LinkContext context) void Initialize () { - foreach (AssemblyDefinition assembly in _context.GetAssemblies ()) - InitializeAssembly (assembly); - } - - 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; - } + ProcessMarkedPending (); } void Complete () @@ -234,27 +213,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.IsMarkedPending (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) @@ -272,20 +230,6 @@ protected bool IsFullyPreserved (TypeDefinition type) return false; } - void InitializeFields (TypeDefinition type) - { - foreach (FieldDefinition field in type.Fields) - if (Annotations.IsMarkedPending (field)) - MarkField (field, DependencyInfo.AlreadyMarked); - } - - void InitializeMethods (Collection methods) - { - foreach (MethodDefinition method in methods) - if (Annotations.IsMarkedPending (method)) - MarkMethod (method, DependencyInfo.AlreadyMarked, null); - } - internal void MarkEntireType (TypeDefinition type, bool includeBaseTypes, in DependencyInfo reason, IMemberDefinition sourceLocationMember) { MarkEntireTypeInternal (type, includeBaseTypes, reason, sourceLocationMember, new HashSet ()); @@ -404,6 +348,10 @@ bool ProcessMarkedPending () { bool marked = false; foreach (var pending in Annotations.MarkedPending ()) { + var assemblyAction = Annotations.GetAction (GetAssemblyFromMetadataTokenProvider (pending)); + if (assemblyAction == AssemblyAction.Skip) + continue; + marked = true; // Some pending items might be processed by the time we get to them. @@ -417,6 +365,7 @@ bool ProcessMarkedPending () 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); @@ -717,7 +666,7 @@ void MarkMarshalSpec (IMarshalInfoProvider spec, in DependencyInfo reason, IMemb void MarkCustomAttributes (ICustomAttributeProvider provider, in DependencyInfo reason, IMemberDefinition sourceLocationMember) { if (provider.HasCustomAttributes) { - bool providerInLinkedAssembly = Annotations.GetAction (GetAssemblyFromCustomAttributeProvider (provider)) == AssemblyAction.Link; + bool providerInLinkedAssembly = Annotations.GetAction (GetAssemblyFromMetadataTokenProvider (provider)) == AssemblyAction.Link; bool markOnUse = _context.KeepUsedAttributeTypesOnly && providerInLinkedAssembly; foreach (CustomAttribute ca in provider.CustomAttributes) { @@ -859,7 +808,7 @@ void MarkMembers (TypeDefinition typeDefinition, IEnumerable } } - protected static AssemblyDefinition GetAssemblyFromCustomAttributeProvider (ICustomAttributeProvider provider) + protected static AssemblyDefinition GetAssemblyFromMetadataTokenProvider (IMetadataTokenProvider provider) { return provider switch { @@ -1337,7 +1286,7 @@ bool ProcessLazyAttributes () continue; } - if (_context.Annotations.HasLinkerAttribute (resolved.DeclaringType) && Annotations.GetAction (GetAssemblyFromCustomAttributeProvider (assemblyLevelAttribute.Provider)) == AssemblyAction.Link) + if (_context.Annotations.HasLinkerAttribute (resolved.DeclaringType) && Annotations.GetAction (GetAssemblyFromMetadataTokenProvider (assemblyLevelAttribute.Provider)) == AssemblyAction.Link) continue; if (!ShouldMarkTopLevelCustomAttribute (assemblyLevelAttribute, resolved)) { From da975541de0ee57bc4c34c5ce48b60f4903159df Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 20 Jan 2021 01:44:56 +0000 Subject: [PATCH 05/12] Keep assembly initialization based on action And remove currently failing testcase. --- src/linker/Linker.Steps/MarkStep.cs | 32 +++++++++++++------ src/linker/Linker.Steps/ResolveFromXmlStep.cs | 1 - .../CopyUsedAssemblyWithMainEntryRoot.cs | 4 --- .../CopyUsedAssemblyWithMainEntryRoot_Lib.cs | 6 ---- .../LinkAttributes/LinkerAttributeRemoval.cs | 1 - 5 files changed, 22 insertions(+), 22 deletions(-) delete mode 100644 test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyUsedAssemblyWithMainEntryRoot_Lib.cs diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 86313d91c85a..24852ca0fba0 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -190,9 +190,19 @@ public virtual void Process (LinkContext context) void Initialize () { + foreach (AssemblyDefinition assembly in _context.GetAssemblies ()) + InitializeAssembly (assembly); + ProcessMarkedPending (); } + protected virtual void InitializeAssembly (AssemblyDefinition assembly) + { + var action = _context.Annotations.GetAction (assembly); + if (action == AssemblyAction.Copy || action == AssemblyAction.Save) + MarkAssembly (assembly, new DependencyInfo (DependencyKind.AssemblyAction, action)); + } + void Complete () { foreach (var body in _unreachableBodies) { @@ -348,7 +358,8 @@ bool ProcessMarkedPending () { bool marked = false; foreach (var pending in Annotations.MarkedPending ()) { - var assemblyAction = Annotations.GetAction (GetAssemblyFromMetadataTokenProvider (pending)); + var assembly = GetAssemblyFromMetadataTokenProvider (pending); + var assemblyAction = Annotations.GetAction (assembly); if (assemblyAction == AssemblyAction.Skip) continue; @@ -1224,14 +1235,16 @@ protected bool CheckProcessed (IMetadataTokenProvider provider) protected void MarkAssembly (AssemblyDefinition assembly, DependencyInfo reason) { - // This conceptually marks an assembly (marking the module type and) - // attributes) even though it does not mark the AssemblyDefinition itself - // in Annotations. Other steps rely on the ModuleDefinition being marked instead. - 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); @@ -1244,6 +1257,8 @@ protected void MarkAssembly (AssemblyDefinition assembly, DependencyInfo reason) 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); @@ -1530,7 +1545,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 != type && 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 " + @@ -1607,12 +1622,10 @@ protected internal virtual TypeDefinition MarkType (TypeReference reference, Dep MarkMethodsIf (type.Methods, HasOnSerializeOrDeserializeAttribute, new DependencyInfo (DependencyKind.SerializationMethodForType, type), type); } - ApplyPreserveInfo (type); - ApplyPreserveMethods (type); - DoAdditionalTypeProcessing (type); ApplyPreserveInfo (type); + ApplyPreserveMethods (type); return type; } @@ -2482,7 +2495,6 @@ protected virtual MethodDefinition MarkMethod (MethodReference reference, Depend AssemblyDefinition ResolveAssembly (IMetadataScope scope) { AssemblyDefinition assembly = _context.Resolve (scope); - MarkAssembly (assembly); return assembly; } diff --git a/src/linker/Linker.Steps/ResolveFromXmlStep.cs b/src/linker/Linker.Steps/ResolveFromXmlStep.cs index 76b0ad714c72..176feb183194 100644 --- a/src/linker/Linker.Steps/ResolveFromXmlStep.cs +++ b/src/linker/Linker.Steps/ResolveFromXmlStep.cs @@ -167,7 +167,6 @@ 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); diff --git a/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithMainEntryRoot.cs b/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithMainEntryRoot.cs index 2c9a71daf7b5..e9f020d16209 100644 --- a/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithMainEntryRoot.cs +++ b/test/Mono.Linker.Tests.Cases/Libraries/CopyUsedAssemblyWithMainEntryRoot.cs @@ -1,6 +1,5 @@ using Mono.Linker.Tests.Cases.Expectations.Assertions; using Mono.Linker.Tests.Cases.Expectations.Metadata; -using Mono.Linker.Tests.Cases.Libraries.Dependencies; namespace Mono.Linker.Tests.Cases.Libraries { @@ -10,8 +9,6 @@ namespace Mono.Linker.Tests.Cases.Libraries [Kept] [KeptMember (".ctor()")] [SetupLinkerAction ("copyused", "test")] - [SetupCompileBefore ("lib.dll", new[] { "Dependencies/CopyUsedAssemblyWithMainEntryRoot_Lib.cs" })] - [KeptTypeInAssembly ("lib.dll", typeof (CopyUsedAssemblyWithMainEntryRoot_Lib))] public class CopyUsedAssemblyWithMainEntryRoot { [Kept] @@ -27,7 +24,6 @@ public void UnusedPublicMethod () [Kept] private void UnusedPrivateMethod () { - new CopyUsedAssemblyWithMainEntryRoot_Lib (); } } } \ No newline at end of file diff --git a/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyUsedAssemblyWithMainEntryRoot_Lib.cs b/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyUsedAssemblyWithMainEntryRoot_Lib.cs deleted file mode 100644 index 3ce2fb1a1b6a..000000000000 --- a/test/Mono.Linker.Tests.Cases/Libraries/Dependencies/CopyUsedAssemblyWithMainEntryRoot_Lib.cs +++ /dev/null @@ -1,6 +0,0 @@ -namespace Mono.Linker.Tests.Cases.Libraries.Dependencies -{ - public class CopyUsedAssemblyWithMainEntryRoot_Lib - { - } -} \ No newline at end of file diff --git a/test/Mono.Linker.Tests.Cases/LinkAttributes/LinkerAttributeRemoval.cs b/test/Mono.Linker.Tests.Cases/LinkAttributes/LinkerAttributeRemoval.cs index 6cafb467ad08..45fb3bdfc4a1 100644 --- a/test/Mono.Linker.Tests.Cases/LinkAttributes/LinkerAttributeRemoval.cs +++ b/test/Mono.Linker.Tests.Cases/LinkAttributes/LinkerAttributeRemoval.cs @@ -5,7 +5,6 @@ using System; using System.ComponentModel; using System.Diagnostics.CodeAnalysis; -using System.Reflection; using Mono.Linker.Tests.Cases.Expectations.Assertions; using Mono.Linker.Tests.Cases.Expectations.Metadata; using Mono.Linker.Tests.Cases.LinkAttributes.Dependencies; From c19599de90c9d0db3b42ca6e155c762e286c667a Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 20 Jan 2021 02:14:36 +0000 Subject: [PATCH 06/12] Don't process pending items in Process yet --- src/linker/Linker.Steps/MarkStep.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 24852ca0fba0..9afdd4a5a19d 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -306,7 +306,7 @@ private void MarkEntireTypeInternal (TypeDefinition type, bool includeBaseTypes, void Process () { - while (ProcessPrimaryQueue () || ProcessMarkedPending () || ProcessLazyAttributes () || ProcessLateMarkedAttributes ()) { + while (ProcessPrimaryQueue () || ProcessLazyAttributes () || ProcessLateMarkedAttributes ()) { // deal with [TypeForwardedTo] pseudo-attributes foreach (AssemblyDefinition assembly in _context.GetAssemblies ()) { From cca717c4467b5ed3acf2331eec719028858b0a1c Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 20 Jan 2021 17:04:28 +0000 Subject: [PATCH 07/12] Remove Skip case and asserts --- src/linker/Linker.Steps/MarkStep.cs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 9afdd4a5a19d..42baebff5081 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -358,11 +358,6 @@ bool ProcessMarkedPending () { bool marked = false; foreach (var pending in Annotations.MarkedPending ()) { - var assembly = GetAssemblyFromMetadataTokenProvider (pending); - var assemblyAction = Annotations.GetAction (assembly); - if (assemblyAction == AssemblyAction.Skip) - continue; - marked = true; // Some pending items might be processed by the time we get to them. @@ -372,7 +367,6 @@ bool ProcessMarkedPending () switch (pending) { case TypeDefinition type: MarkType (type, DependencyInfo.AlreadyMarked, type); - Debug.Assert (!Annotations.IsMarkedPending (type)); break; case MethodDefinition method: MarkMethod (method, DependencyInfo.AlreadyMarked, null); @@ -380,11 +374,9 @@ bool ProcessMarkedPending () break; case FieldDefinition field: MarkField (field, DependencyInfo.AlreadyMarked); - Debug.Assert (!Annotations.IsMarkedPending (field)); break; case ModuleDefinition module: MarkModule (module, DependencyInfo.AlreadyMarked); - Debug.Assert (!Annotations.IsMarkedPending (module)); break; default: throw new NotImplementedException (pending.GetType ().ToString ()); @@ -677,7 +669,7 @@ void MarkMarshalSpec (IMarshalInfoProvider spec, in DependencyInfo reason, IMemb void MarkCustomAttributes (ICustomAttributeProvider provider, in DependencyInfo reason, IMemberDefinition sourceLocationMember) { if (provider.HasCustomAttributes) { - bool providerInLinkedAssembly = Annotations.GetAction (GetAssemblyFromMetadataTokenProvider (provider)) == AssemblyAction.Link; + bool providerInLinkedAssembly = Annotations.GetAction (GetAssemblyFromCustomAttributeProvider (provider)) == AssemblyAction.Link; bool markOnUse = _context.KeepUsedAttributeTypesOnly && providerInLinkedAssembly; foreach (CustomAttribute ca in provider.CustomAttributes) { @@ -819,7 +811,7 @@ void MarkMembers (TypeDefinition typeDefinition, IEnumerable } } - protected static AssemblyDefinition GetAssemblyFromMetadataTokenProvider (IMetadataTokenProvider provider) + protected static AssemblyDefinition GetAssemblyFromCustomAttributeProvider (ICustomAttributeProvider provider) { return provider switch { @@ -1301,7 +1293,7 @@ bool ProcessLazyAttributes () continue; } - if (_context.Annotations.HasLinkerAttribute (resolved.DeclaringType) && Annotations.GetAction (GetAssemblyFromMetadataTokenProvider (assemblyLevelAttribute.Provider)) == AssemblyAction.Link) + if (_context.Annotations.HasLinkerAttribute (resolved.DeclaringType) && Annotations.GetAction (GetAssemblyFromCustomAttributeProvider (assemblyLevelAttribute.Provider)) == AssemblyAction.Link) continue; if (!ShouldMarkTopLevelCustomAttribute (assemblyLevelAttribute, resolved)) { From bd8595319e9d6ff00aaf2375ea44b883603d0fe2 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 20 Jan 2021 17:46:03 +0000 Subject: [PATCH 08/12] Support early marking of exports --- src/linker/Linker.Steps/MarkStep.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 42baebff5081..23c73ea7f2ca 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -378,6 +378,10 @@ bool ProcessMarkedPending () 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 ()); } From 9f3753319c6677b8d48efa38b8a8d737b13a530b Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 20 Jan 2021 19:19:27 +0000 Subject: [PATCH 09/12] PR feedback - Return arrays instead of IEnumerable for pending Annotations - Rename methods to Get* - Add comment for marked_pending - Don't make state changes inside a Debug assert - Change sourceLocationMember to null when marking pending types - Add exception messages --- src/linker/Linker.Steps/MarkStep.cs | 10 ++++----- src/linker/Linker/Annotations.cs | 32 +++++++++++++++-------------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 23c73ea7f2ca..b5437318a435 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -357,7 +357,7 @@ bool ProcessPrimaryQueue () bool ProcessMarkedPending () { bool marked = false; - foreach (var pending in Annotations.MarkedPending ()) { + foreach (var pending in Annotations.GetMarkedPending ()) { marked = true; // Some pending items might be processed by the time we get to them. @@ -366,7 +366,7 @@ bool ProcessMarkedPending () switch (pending) { case TypeDefinition type: - MarkType (type, DependencyInfo.AlreadyMarked, type); + MarkType (type, DependencyInfo.AlreadyMarked, null); break; case MethodDefinition method: MarkMethod (method, DependencyInfo.AlreadyMarked, null); @@ -387,7 +387,7 @@ bool ProcessMarkedPending () } } - foreach (var type in Annotations.PendingPreserve ()) { + foreach (var type in Annotations.GetPendingPreserve ()) { marked = true; Debug.Assert (Annotations.IsProcessed (type)); ApplyPreserveInfo (type); @@ -1541,7 +1541,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 != type && 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 " + @@ -2289,7 +2289,7 @@ void ApplyPreserveInfo (TypeDefinition type) { if (Annotations.TryGetPreserve (type, out TypePreserve preserve)) { if (!Annotations.SetAppliedPreserve (type, preserve)) - throw new InvalidOperationException (); + throw new InvalidOperationException ($"Type {type} already has applied {preserve}."); var di = new DependencyInfo (DependencyKind.TypePreserve, type); diff --git a/src/linker/Linker/Annotations.cs b/src/linker/Linker/Annotations.cs index be8e7bc09bcf..350c17765824 100644 --- a/src/linker/Linker/Annotations.cs +++ b/src/linker/Linker/Annotations.cs @@ -50,6 +50,9 @@ public partial class AnnotationStore protected readonly HashSet field_init = new HashSet (); protected readonly HashSet fieldType_init = new HashSet (); + // Annotations.Mark will add unmarked items to marked_pending, to be fully marked later ("processed") by MarkStep. + // Items go through state changes from "unmarked" -> "pending" -> "processed". "pending" items are only tracked + // once, and once "processed", an item never becomes "pending" again. protected readonly HashSet marked_pending = new HashSet (); protected readonly HashSet processed = new HashSet (); protected readonly Dictionary preserved_types = new Dictionary (); @@ -196,10 +199,9 @@ public void Mark (CustomAttribute attribute, in DependencyInfo reason) Tracer.AddDirectDependency (attribute, reason, marked: true); } - public IEnumerable MarkedPending () + public IMetadataTokenProvider[] GetMarkedPending () { - foreach (var pending in marked_pending.ToArray ()) - yield return pending; + return marked_pending.ToArray (); } public bool IsMarkedPending (IMetadataTokenProvider provider) @@ -259,7 +261,8 @@ public bool IsRelevantToVariantCasting (TypeDefinition type) public bool SetProcessed (IMetadataTokenProvider provider) { if (processed.Add (provider)) { - Debug.Assert (marked_pending.Remove (provider)); + if (!marked_pending.Remove (provider)) + throw new InvalidOperationException ($"{provider} must be marked before it can be processed."); return true; } @@ -271,19 +274,18 @@ public bool IsProcessed (IMetadataTokenProvider provider) return processed.Contains (provider); } - public IEnumerable PendingPreserve () + public TypeDefinition[] GetPendingPreserve () { - foreach (var type in pending_preserve.ToArray ()) - yield return type; + return pending_preserve.ToArray (); } public bool SetAppliedPreserve (TypeDefinition type, TypePreserve preserve) { - if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) { - throw new InvalidOperationException (); - } + if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) + throw new InvalidOperationException ($"Type {type} must have a TypePreserve before it can be applied."); + if (preserve != existing.preserve) - throw new InvalidOperationException (); + throw new InvalidOperationException ($"Type {type} does not have {preserve}. The TypePreserve may have changed before the call to {nameof (SetAppliedPreserve)}."); if (existing.applied) { Debug.Assert (!pending_preserve.Contains (type)); @@ -297,11 +299,11 @@ public bool SetAppliedPreserve (TypeDefinition type, TypePreserve preserve) public bool HasAppliedPreserve (TypeDefinition type, TypePreserve preserve) { - if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) { - throw new InvalidOperationException (); - } + if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) + throw new InvalidOperationException ($"Type {type} must have a TypePreserve before it can be applied."); + if (preserve != existing.preserve) - throw new InvalidOperationException (); + throw new InvalidOperationException ($"Type {type} does not have {preserve}. The TypePreserve may have changed before the call to {nameof (HasAppliedPreserve)}."); return existing.applied; } From 2b1fdbf8e78a43d1dece76465d61f86d89a278ba Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 20 Jan 2021 19:57:08 +0000 Subject: [PATCH 10/12] Add test for marked method in unmarked type We used to ignore such methods in Initialize, but the new logic for pending marked items will keep the method (and its type) as long as it was marked before MarkStep. --- .../CustomStepCanPreserveMethodsAfterMark.cs | 7 +++++++ .../CustomStepCanPreserveMethodsBeforeMark.cs | 9 +++++++++ .../Extensibility/Dependencies/PreserveMethodsSubStep.cs | 3 +++ 3 files changed, 19 insertions(+) diff --git a/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsAfterMark.cs b/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsAfterMark.cs index b6dd62202053..a9da1fbd0278 100644 --- a/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsAfterMark.cs +++ b/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsAfterMark.cs @@ -28,5 +28,12 @@ public static void PreservedForType () { } public static void PreservedForMethod_UsedMethod () { } } + // Annotations.Mark in a CustomStep before MarkStep will not necessarily keep a method, + // if it belongs to an unmarked type. + static class UnusedType + { + public static void MarkedMethod () { } + } + } } \ No newline at end of file diff --git a/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsBeforeMark.cs b/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsBeforeMark.cs index 7ded140fc7b4..17ca281af1e9 100644 --- a/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsBeforeMark.cs +++ b/test/Mono.Linker.Tests.Cases/Extensibility/CustomStepCanPreserveMethodsBeforeMark.cs @@ -28,5 +28,14 @@ public static void PreservedForType () { } public static void PreservedForMethod_UsedMethod () { } } + // Annotations.Mark in a CustomStep before MarkStep will keep a method, + // even if it belongs to an otherwise unmarked type. + [Kept] + static class UnusedType + { + [Kept] + public static void MarkedMethod () { } + } + } } \ No newline at end of file diff --git a/test/Mono.Linker.Tests.Cases/Extensibility/Dependencies/PreserveMethodsSubStep.cs b/test/Mono.Linker.Tests.Cases/Extensibility/Dependencies/PreserveMethodsSubStep.cs index 4cd24f2742a3..f67df3119724 100644 --- a/test/Mono.Linker.Tests.Cases/Extensibility/Dependencies/PreserveMethodsSubStep.cs +++ b/test/Mono.Linker.Tests.Cases/Extensibility/Dependencies/PreserveMethodsSubStep.cs @@ -31,6 +31,9 @@ void ProcessType (TypeDefinition type) public void ProcessMethod (MethodDefinition method) { + if (method.Name == "MarkedMethod") + Annotations.Mark (method); + foreach (var m in method.DeclaringType.Methods) { if (m.Name == $"PreservedForMethod_{method.Name}") Annotations.AddPreservedMethod (method, m); From d39f4d38ba87e1f0d73985fc0dfafa619fef9398 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 20 Jan 2021 20:01:03 +0000 Subject: [PATCH 11/12] InvalidOperationException -> InternalErrorException --- src/linker/Linker.Steps/MarkStep.cs | 2 +- src/linker/Linker/Annotations.cs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index b5437318a435..70eb25a175a6 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -2289,7 +2289,7 @@ void ApplyPreserveInfo (TypeDefinition type) { if (Annotations.TryGetPreserve (type, out TypePreserve preserve)) { if (!Annotations.SetAppliedPreserve (type, preserve)) - throw new InvalidOperationException ($"Type {type} already has applied {preserve}."); + throw new InternalErrorException ($"Type {type} already has applied {preserve}."); var di = new DependencyInfo (DependencyKind.TypePreserve, type); diff --git a/src/linker/Linker/Annotations.cs b/src/linker/Linker/Annotations.cs index 350c17765824..c531a6b2ecb9 100644 --- a/src/linker/Linker/Annotations.cs +++ b/src/linker/Linker/Annotations.cs @@ -262,7 +262,7 @@ public bool SetProcessed (IMetadataTokenProvider provider) { if (processed.Add (provider)) { if (!marked_pending.Remove (provider)) - throw new InvalidOperationException ($"{provider} must be marked before it can be processed."); + throw new InternalErrorException ($"{provider} must be marked before it can be processed."); return true; } @@ -282,10 +282,10 @@ public TypeDefinition[] GetPendingPreserve () public bool SetAppliedPreserve (TypeDefinition type, TypePreserve preserve) { if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) - throw new InvalidOperationException ($"Type {type} must have a TypePreserve before it can be applied."); + throw new InternalErrorException ($"Type {type} must have a TypePreserve before it can be applied."); if (preserve != existing.preserve) - throw new InvalidOperationException ($"Type {type} does not have {preserve}. The TypePreserve may have changed before the call to {nameof (SetAppliedPreserve)}."); + throw new InternalErrorException ($"Type {type} does not have {preserve}. The TypePreserve may have changed before the call to {nameof (SetAppliedPreserve)}."); if (existing.applied) { Debug.Assert (!pending_preserve.Contains (type)); @@ -300,10 +300,10 @@ public bool SetAppliedPreserve (TypeDefinition type, TypePreserve preserve) public bool HasAppliedPreserve (TypeDefinition type, TypePreserve preserve) { if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) - throw new InvalidOperationException ($"Type {type} must have a TypePreserve before it can be applied."); + throw new InternalErrorException ($"Type {type} must have a TypePreserve before it can be applied."); if (preserve != existing.preserve) - throw new InvalidOperationException ($"Type {type} does not have {preserve}. The TypePreserve may have changed before the call to {nameof (HasAppliedPreserve)}."); + throw new InternalErrorException ($"Type {type} does not have {preserve}. The TypePreserve may have changed before the call to {nameof (HasAppliedPreserve)}."); return existing.applied; } From 709163bd52b8682c7bbd20f41d5512938c952a12 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 21 Jan 2021 18:32:11 +0000 Subject: [PATCH 12/12] Add MarkProcessed helper To directly mark an item without adding it to the pending set first. --- src/linker/Linker.Steps/MarkStep.cs | 6 ++---- src/linker/Linker/Annotations.cs | 9 +++++++++ src/linker/Linker/MarkingHelpers.cs | 3 +-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/linker/Linker.Steps/MarkStep.cs b/src/linker/Linker.Steps/MarkStep.cs index 70eb25a175a6..117091db0ae3 100644 --- a/src/linker/Linker.Steps/MarkStep.cs +++ b/src/linker/Linker.Steps/MarkStep.cs @@ -2769,8 +2769,7 @@ void ProcessInteropMethod (MethodDefinition method) { if (method.IsPInvokeImpl) { var pii = method.PInvokeInfo; - Annotations.Mark (pii.Module, new DependencyInfo (DependencyKind.InteropMethodDependency, method)); - Annotations.SetProcessed (pii.Module); + Annotations.MarkProcessed (pii.Module, new DependencyInfo (DependencyKind.InteropMethodDependency, method)); if (!string.IsNullOrEmpty (_context.PInvokesListFile)) { _context.PInvokes.Add (new PInvokeInfo { AssemblyName = method.DeclaringType.Module.Name, @@ -3136,8 +3135,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.SetProcessed (iface); + Annotations.MarkProcessed (iface, new DependencyInfo (DependencyKind.InterfaceImplementationOnType, type)); } // diff --git a/src/linker/Linker/Annotations.cs b/src/linker/Linker/Annotations.cs index c531a6b2ecb9..fa87ebcdf1d7 100644 --- a/src/linker/Linker/Annotations.cs +++ b/src/linker/Linker/Annotations.cs @@ -274,6 +274,15 @@ public bool IsProcessed (IMetadataTokenProvider provider) return processed.Contains (provider); } + public bool MarkProcessed (IMetadataTokenProvider provider, in DependencyInfo reason) + { + Debug.Assert (!(reason.Kind == DependencyKind.AlreadyMarked)); + Tracer.AddDirectDependency (provider, reason, marked: true); + // The item may or may not be pending. + marked_pending.Remove (provider); + return processed.Add (provider); + } + public TypeDefinition[] GetPendingPreserve () { return pending_preserve.ToArray (); diff --git a/src/linker/Linker/MarkingHelpers.cs b/src/linker/Linker/MarkingHelpers.cs index 794cf466c802..aa5d95b258e8 100644 --- a/src/linker/Linker/MarkingHelpers.cs +++ b/src/linker/Linker/MarkingHelpers.cs @@ -14,8 +14,7 @@ public MarkingHelpers (LinkContext context) public void MarkExportedType (ExportedType type, ModuleDefinition module, in DependencyInfo reason) { - _context.Annotations.Mark (type, reason); - if (!_context.Annotations.SetProcessed (type)) + if (!_context.Annotations.MarkProcessed (type, reason)) return; if (_context.KeepTypeForwarderOnlyAssemblies) _context.Annotations.Mark (module, new DependencyInfo (DependencyKind.ModuleOfExportedType, type));