From b4ee1b57cd3eabd7f37c39a985364f9cf88e02eb Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Thu, 6 Jun 2024 12:34:49 -0700 Subject: [PATCH 1/2] Allow TypePreserve info to be applied multiple times --- .../src/linker/Linker.Steps/MarkStep.cs | 5 +-- .../illink/src/linker/Linker/Annotations.cs | 20 ++--------- .../CommandLine/CustomStepApplyPreserve.cs | 33 +++++++++++++++++++ .../Dependencies/CustomApplyPreserveStep.cs | 13 ++++++++ .../Mono.Linker.Tests.Cases.csproj | 1 + 5 files changed, 50 insertions(+), 22 deletions(-) create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/CustomStepApplyPreserve.cs create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/Dependencies/CustomApplyPreserveStep.cs diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 3d6a155fa17e99..a925aa62ac58a2 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -35,9 +35,7 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; -using System.Reflection.Metadata.Ecma335; using System.Reflection.Runtime.TypeParsing; -using System.Runtime.CompilerServices; using System.Text.RegularExpressions; using ILCompiler.DependencyAnalysisFramework; using ILLink.Shared; @@ -479,7 +477,6 @@ bool ProcessMarkedPending () foreach (var type in Annotations.GetPendingPreserve ()) { marked = true; - Debug.Assert (Annotations.IsProcessed (type)); ApplyPreserveInfo (type); } @@ -2795,7 +2792,7 @@ void ApplyPreserveInfo (TypeDefinition type) if (Annotations.TryGetPreserve (type, out TypePreserve preserve)) { if (!Annotations.SetAppliedPreserve (type, preserve)) - throw new InternalErrorException ($"Type {type} already has applied {preserve}."); + return; var di = new DependencyInfo (DependencyKind.TypePreserve, type); diff --git a/src/tools/illink/src/linker/Linker/Annotations.cs b/src/tools/illink/src/linker/Linker/Annotations.cs index 68c8c320749966..2ecedc7dae1ed9 100644 --- a/src/tools/illink/src/linker/Linker/Annotations.cs +++ b/src/tools/illink/src/linker/Linker/Annotations.cs @@ -34,7 +34,6 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; -using System.Reflection.Metadata.Ecma335; using ILLink.Shared.TrimAnalysis; using Mono.Cecil; using Mono.Cecil.Cil; @@ -303,28 +302,13 @@ public bool SetAppliedPreserve (TypeDefinition type, TypePreserve preserve) return true; } - public bool HasAppliedPreserve (TypeDefinition type, TypePreserve preserve) - { - if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) - throw new InternalErrorException ($"Type {type} must have a TypePreserve before it can be applied."); - - if (preserve != existing.preserve) - throw new InternalErrorException ($"Type {type} does not have {preserve}. The TypePreserve may have changed before the call to {nameof (HasAppliedPreserve)}."); - - return existing.applied; - } - public void SetPreserve (TypeDefinition type, TypePreserve preserve) { Debug.Assert (preserve != TypePreserve.Nothing); if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) { preserved_types.Add (type, (preserve, false)); - if (IsProcessed (type)) { - // Required to track preserve for marked types where the existing preserve - // was Nothing (since these aren't explicitly tracked.) - var addedPending = pending_preserve.Add (type); - Debug.Assert (addedPending); - } + var addedPending = pending_preserve.Add (type); + Debug.Assert (addedPending); return; } Debug.Assert (existing.preserve != TypePreserve.Nothing); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/CustomStepApplyPreserve.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/CustomStepApplyPreserve.cs new file mode 100644 index 00000000000000..b3f7f99780776f --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/CustomStepApplyPreserve.cs @@ -0,0 +1,33 @@ +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Metadata; + +namespace Mono.Linker.Tests.Cases.CommandLine +{ + [SetupCompileBefore ("CustomApplyPreserveStep.dll", new[] { "Dependencies/CustomApplyPreserveStep.cs" }, new[] { "illink.dll", "Mono.Cecil.dll" })] + [SetupLinkerArgument ("--custom-step", "-MarkStep:CustomStep.CustomApplyPreserveStep,CustomApplyPreserveStep.dll")] + [SetupLinkerArgument ("--verbose")] + public class CustomStepApplyPreserve + { + [Kept] + public class HasPreserveApplied + { + [Kept] + public int Field; + + [Kept] + public HasPreserveApplied () { } + + [Kept] + public void Method () + { + } + + public class Nested { } + } + + [Kept] + public static void Main () + { + } + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/Dependencies/CustomApplyPreserveStep.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/Dependencies/CustomApplyPreserveStep.cs new file mode 100644 index 00000000000000..1975f2c357179a --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/Dependencies/CustomApplyPreserveStep.cs @@ -0,0 +1,13 @@ +namespace CustomStep +{ + public class CustomApplyPreserveStep : Mono.Linker.Steps.IStep + { + public void Process(Mono.Linker.LinkContext context) + { + var myType = context.GetType("Mono.Linker.Tests.Cases.CommandLine.CustomStepApplyPreserve/HasPreserveApplied"); + context.Annotations.SetPreserve(myType, Mono.Linker.TypePreserve.Methods); + // Make sure we can set preserve multiple times + context.Annotations.SetPreserve(myType, Mono.Linker.TypePreserve.All); + } + } +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj b/src/tools/illink/test/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj index 2b025d9861a116..f54ae5639f9b75 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj @@ -22,6 +22,7 @@ + From 28fd125cda01770afd09596152f7d16df920e0b1 Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Thu, 6 Jun 2024 15:41:42 -0700 Subject: [PATCH 2/2] Only apply preserve if the type is also marked --- src/tools/illink/src/linker/Linker/Annotations.cs | 6 ++++-- .../CommandLine/CustomStepApplyPreserve.cs | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/tools/illink/src/linker/Linker/Annotations.cs b/src/tools/illink/src/linker/Linker/Annotations.cs index 2ecedc7dae1ed9..bfcfe56e7d71e7 100644 --- a/src/tools/illink/src/linker/Linker/Annotations.cs +++ b/src/tools/illink/src/linker/Linker/Annotations.cs @@ -307,8 +307,10 @@ public void SetPreserve (TypeDefinition type, TypePreserve preserve) Debug.Assert (preserve != TypePreserve.Nothing); if (!preserved_types.TryGetValue (type, out (TypePreserve preserve, bool applied) existing)) { preserved_types.Add (type, (preserve, false)); - var addedPending = pending_preserve.Add (type); - Debug.Assert (addedPending); + if (IsProcessed (type)) { + var addedPending = pending_preserve.Add (type); + Debug.Assert (addedPending); + } return; } Debug.Assert (existing.preserve != TypePreserve.Nothing); diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/CustomStepApplyPreserve.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/CustomStepApplyPreserve.cs index b3f7f99780776f..ae48755d7fedfd 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/CustomStepApplyPreserve.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/CustomStepApplyPreserve.cs @@ -1,9 +1,10 @@ -using Mono.Linker.Tests.Cases.Expectations.Assertions; +using System; +using Mono.Linker.Tests.Cases.Expectations.Assertions; using Mono.Linker.Tests.Cases.Expectations.Metadata; namespace Mono.Linker.Tests.Cases.CommandLine { - [SetupCompileBefore ("CustomApplyPreserveStep.dll", new[] { "Dependencies/CustomApplyPreserveStep.cs" }, new[] { "illink.dll", "Mono.Cecil.dll" })] + [SetupCompileBefore ("CustomApplyPreserveStep.dll", new[] { "Dependencies/CustomApplyPreserveStep.cs" }, new[] { "illink.dll", "Mono.Cecil.dll" }, addAsReference: false)] [SetupLinkerArgument ("--custom-step", "-MarkStep:CustomStep.CustomApplyPreserveStep,CustomApplyPreserveStep.dll")] [SetupLinkerArgument ("--verbose")] public class CustomStepApplyPreserve @@ -28,6 +29,7 @@ public class Nested { } [Kept] public static void Main () { + Console.WriteLine (typeof (HasPreserveApplied).FullName); } } }