Workaround NRE in SweepStep (#2264)

Basically a workaround for #2260. This simply goes back to using the `Resolve` method and avoid the cache for now.
dotnet · Sep 8, 2021 · a9888c2 · a9888c2
1 parent 43fb979
commit a9888c2
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 13 deletions.
diff --git 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,16 +568,14 @@ struct AssemblyReferencesCorrector
 		{
 			readonly AssemblyDefinition assembly;
 			readonly DefaultMetadataImporter importer;
-			readonly ITryResolveMetadata resolver;
 
 			HashSet<TypeReference> 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;
@@ -925,7 +923,15 @@ 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
+				// 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
 				if (td == null) {
 					//
 					// This can happen when not all assembly refences were provided and we

diff --git a/...nheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs b/...nheritance.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]