From f69d4fd2b09503494503a572a032252e1defe5ee Mon Sep 17 00:00:00 2001 From: vitek-karas Date: Tue, 7 Sep 2021 04:14:10 -0700 Subject: [PATCH 1/5] Workaround NRE in SweepStep Basically a workaround for #2260. This simply goes back to using the `Resolve` method and avoid the cache for now. --- src/linker/Linker.Steps/SweepStep.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/linker/Linker.Steps/SweepStep.cs b/src/linker/Linker.Steps/SweepStep.cs index cd9b17f3e3ad..c9415cfdb014 100644 --- a/src/linker/Linker.Steps/SweepStep.cs +++ b/src/linker/Linker.Steps/SweepStep.cs @@ -925,7 +925,10 @@ void UpdateScopeOfTypeReference (TypeReference type) // // Resolve to type definition to remove any type forwarding imports // - TypeDefinition td = resolver.TryResolve (type); + // Workaround for https://github.com/mono/linker/issues/2260 +#pragma warning disable RS0030 // Do not used banned APIs + TypeDefinition td = type.Resolve (); +#pragma warning restore RS0030 // Do not used banned APIs if (td == null) { // // This can happen when not all assembly refences were provided and we From 014d3f8a165d89f8a353fc4c97aea3df55847fd1 Mon Sep 17 00:00:00 2001 From: vitek-karas Date: Tue, 7 Sep 2021 06:44:39 -0700 Subject: [PATCH 2/5] Fix build --- src/linker/Linker.Steps/SweepStep.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/linker/Linker.Steps/SweepStep.cs b/src/linker/Linker.Steps/SweepStep.cs index c9415cfdb014..9e8e1e3d2ed1 100644 --- a/src/linker/Linker.Steps/SweepStep.cs +++ b/src/linker/Linker.Steps/SweepStep.cs @@ -568,7 +568,9 @@ struct AssemblyReferencesCorrector { readonly AssemblyDefinition assembly; readonly DefaultMetadataImporter importer; +#pragma warning disable IDE0052 // Remove unread private members readonly ITryResolveMetadata resolver; +#pragma warning restore IDE0052 // Remove unread private members HashSet updated; bool changedAnyScopes; From 1bd9321a02cad9dde9b176eb449ed536beac3e98 Mon Sep 17 00:00:00 2001 From: vitek-karas Date: Wed, 8 Sep 2021 01:22:01 -0700 Subject: [PATCH 3/5] PR feedback --- src/linker/Linker.Steps/SweepStep.cs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/linker/Linker.Steps/SweepStep.cs b/src/linker/Linker.Steps/SweepStep.cs index 9e8e1e3d2ed1..67388c4581b5 100644 --- a/src/linker/Linker.Steps/SweepStep.cs +++ b/src/linker/Linker.Steps/SweepStep.cs @@ -256,7 +256,7 @@ protected virtual void SweepAssembly (AssemblyDefinition assembly) SweepAssemblyReferences (assembly); } - bool SweepAssemblyReferences (AssemblyDefinition assembly) + static bool SweepAssemblyReferences (AssemblyDefinition assembly) { // // We used to run over list returned by GetTypeReferences but @@ -266,7 +266,7 @@ bool SweepAssemblyReferences (AssemblyDefinition assembly) // assembly.MainModule.AssemblyReferences.Clear (); - var ars = new AssemblyReferencesCorrector (assembly, Context); + var ars = new AssemblyReferencesCorrector (assembly); return ars.Process (); } @@ -568,18 +568,14 @@ struct AssemblyReferencesCorrector { readonly AssemblyDefinition assembly; readonly DefaultMetadataImporter importer; -#pragma warning disable IDE0052 // Remove unread private members - readonly ITryResolveMetadata resolver; -#pragma warning restore IDE0052 // Remove unread private members HashSet updated; bool changedAnyScopes; - public AssemblyReferencesCorrector (AssemblyDefinition assembly, ITryResolveMetadata resolver) + public AssemblyReferencesCorrector (AssemblyDefinition assembly) { this.assembly = assembly; this.importer = new DefaultMetadataImporter (assembly.MainModule); - this.resolver = resolver; updated = null; changedAnyScopes = false; From 9b6049c635da0400b0584444334833307f46e26e Mon Sep 17 00:00:00 2001 From: vitek-karas Date: Wed, 8 Sep 2021 01:34:47 -0700 Subject: [PATCH 4/5] PR feedback --- src/linker/Linker.Steps/SweepStep.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/linker/Linker.Steps/SweepStep.cs b/src/linker/Linker.Steps/SweepStep.cs index 67388c4581b5..797b15781dcc 100644 --- a/src/linker/Linker.Steps/SweepStep.cs +++ b/src/linker/Linker.Steps/SweepStep.cs @@ -924,6 +924,11 @@ void UpdateScopeOfTypeReference (TypeReference type) // Resolve to type definition to remove any type forwarding imports // // Workaround for https://github.com/mono/linker/issues/2260 + // Context has a cache which stores ref->def mapping. This code runs during sweeping + // which can remove the type-def from its assembly, effectively making the ref unresolvable. + // But the cache doesn't know that, it would still "resolve" the type-ref to now defunct type-def. + // For this reason we can't use the context resolution here, and must force Cecil to perform + // real type resolution again (since it can fail, and that's OK). #pragma warning disable RS0030 // Do not used banned APIs TypeDefinition td = type.Resolve (); #pragma warning restore RS0030 // Do not used banned APIs From 90f02ead69351f74f934903086e138ae55e812d6 Mon Sep 17 00:00:00 2001 From: vitek-karas Date: Wed, 8 Sep 2021 01:38:00 -0700 Subject: [PATCH 5/5] Enable test targetting this issue --- .../OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs b/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs index 593465da536c..f4dea2347b31 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs @@ -15,9 +15,7 @@ namespace Mono.Linker.Tests.Cases.Inheritance.AbstractClasses.NoKeptCtor.Overrid [KeptAssembly ("library.dll")] [KeptAssembly ("librarywithnonempty.dll")] - // Uncomment after this bug is fixed - // https://github.com/mono/linker/issues/2260 - //[RemovedTypeInAssembly ("library.dll", typeof (Dependencies.OverrideOfAbstractIsKeptNonEmpty_UnusedType))] + [RemovedTypeInAssembly ("library.dll", typeof (Dependencies.OverrideOfAbstractIsKeptNonEmpty_UnusedType))] public class OverrideOfAbstractIsKeptNonEmpty { @@ -28,10 +26,6 @@ public static void Main () Dependencies.OverrideOfAbstractIsKeptNonEmpty_BaseType c = HelperToMarkLibraryAndRequireItsBase (); c.Method (); - - // Remove after this bug is fixed - // https://github.com/mono/linker/issues/2260 - new Dependencies.OverrideOfAbstractIsKeptNonEmpty_UnusedType (); } [Kept]