Change behavior of copy action to not rewrite assemblies (dotnet/link…

…er#1869) Co-authored-by: Marek Safar <[email protected]> Commit migrated from dotnet/linker@1d96189
dotnet · Apr 1, 2021 · da7b83f · da7b83f
1 parent 7d12e2a
commit da7b83f
Show file tree

Hide file tree

Showing 43 changed files with 755 additions and 143 deletions.
diff --git a/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/tools/illink/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs
@@ -966,14 +966,15 @@ public override bool HandleCall (MethodBody callingMethodBody, MethodReference c
 						}
 						foreach (var typeNameValue in methodParams[0].UniqueValues ()) {
 							if (typeNameValue is KnownStringValue knownStringValue) {
-								TypeReference foundTypeRef = _context.TypeNameResolver.ResolveTypeName (knownStringValue.Contents);
+								TypeReference foundTypeRef = _context.TypeNameResolver.ResolveTypeName (knownStringValue.Contents, out AssemblyDefinition typeAssembly);
 								TypeDefinition foundType = foundTypeRef?.ResolveToMainTypeDefinition ();
 								if (foundType == null) {
 									// Intentionally ignore - it's not wrong for code to call Type.GetType on non-existing name, the code might expect null/exception back.
 									reflectionContext.RecordHandledPattern ();
 								} else {
 									reflectionContext.RecordRecognizedPattern (foundType, () => _markStep.MarkTypeVisibleToReflection (foundTypeRef, new DependencyInfo (DependencyKind.AccessedViaReflection, callingMethodDefinition), callingMethodDefinition));
 									methodReturnValue = MergePointValue.MergeValues (methodReturnValue, new SystemTypeValue (foundType));
+									_context.MarkingHelpers.MarkMatchingExportedType (foundType, typeAssembly, new DependencyInfo (DependencyKind.AccessedViaReflection, foundType));
 								}
 							} else if (typeNameValue == NullValue.Instance) {
 								reflectionContext.RecordHandledPattern ();
@@ -1977,14 +1978,15 @@ void RequireDynamicallyAccessedMembers (ref ReflectionPatternContext reflectionC
 				} else if (uniqueValue is SystemTypeValue systemTypeValue) {
 					MarkTypeForDynamicallyAccessedMembers (ref reflectionContext, systemTypeValue.TypeRepresented, requiredMemberTypes);
 				} else if (uniqueValue is KnownStringValue knownStringValue) {
-					TypeReference typeRef = _context.TypeNameResolver.ResolveTypeName (knownStringValue.Contents);
+					TypeReference typeRef = _context.TypeNameResolver.ResolveTypeName (knownStringValue.Contents, out AssemblyDefinition typeAssembly);
 					TypeDefinition foundType = typeRef?.ResolveToMainTypeDefinition ();
 					if (foundType == null) {
 						// Intentionally ignore - it's not wrong for code to call Type.GetType on non-existing name, the code might expect null/exception back.
 						reflectionContext.RecordHandledPattern ();
 					} else {
 						MarkType (ref reflectionContext, typeRef);
 						MarkTypeForDynamicallyAccessedMembers (ref reflectionContext, foundType, requiredMemberTypes);
+						_context.MarkingHelpers.MarkMatchingExportedType (foundType, typeAssembly, new DependencyInfo (DependencyKind.DynamicallyAccessedMember, foundType));
 					}
 				} else if (uniqueValue == NullValue.Instance) {
 					// Ignore - probably unreachable path as it would fail at runtime anyway.

diff --git a/src/tools/illink/src/linker/Linker.Steps/DescriptorMarker.cs b/src/tools/illink/src/linker/Linker.Steps/DescriptorMarker.cs
@@ -45,6 +45,9 @@ protected override void ProcessAssembly (AssemblyDefinition assembly, XPathNavig
 			if (GetTypePreserve (nav) == TypePreserve.All) {
 				foreach (var type in assembly.MainModule.Types)
 					MarkAndPreserveAll (type);
+
+				foreach (var exportedType in assembly.MainModule.ExportedTypes)
+					_context.MarkingHelpers.MarkExportedType (exportedType, assembly.MainModule, new DependencyInfo (DependencyKind.XmlDescriptor, assembly.MainModule));
 			} else {
 				ProcessTypes (assembly, nav, warnOnUnresolvedTypes);
 				ProcessNamespaces (assembly, nav);

diff --git a/src/tools/illink/src/linker/Linker.Steps/LinkAttributesParser.cs b/src/tools/illink/src/linker/Linker.Steps/LinkAttributesParser.cs
@@ -264,7 +264,7 @@ CustomAttributeArgument[] ReadCustomAttributeArguments (XPathNodeIterator iterat
 				if (!typeref.IsTypeOf ("System", "Type"))
 					goto default;
 
-				TypeReference type = _context.TypeNameResolver.ResolveTypeName (svalue);
+				TypeReference type = _context.TypeNameResolver.ResolveTypeName (svalue, out _);
 				if (type == null) {
 					_context.LogError ($"Could not resolve custom attribute type value '{svalue}'", 1044, _xmlDocumentLocation);
 					return null;
@@ -284,7 +284,7 @@ TypeReference ResolveArgumentType (XPathNodeIterator iterator)
 				if (string.IsNullOrEmpty (typeName))
 					typeName = "System.String";
 
-				TypeReference typeref = _context.TypeNameResolver.ResolveTypeName (typeName);
+				TypeReference typeref = _context.TypeNameResolver.ResolveTypeName (typeName, out _);
 				if (typeref == null) {
 					_context.LogError ($"The type '{typeName}' used with attribute value '{iterator.Current.Value}' could not be found", 1041, _xmlDocumentLocation);
 					return null;

diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
@@ -189,6 +189,7 @@ public MarkStep ()
 		}
 
 		public AnnotationStore Annotations => _context.Annotations;
+		public MarkingHelpers MarkingHelpers => _context.MarkingHelpers;
 		public Tracer Tracer => _context.Tracer;
 
 		public virtual void Process (LinkContext context)
@@ -365,40 +366,12 @@ private void MarkEntireTypeInternal (TypeDefinition type, bool includeBaseTypes,
 
 		void Process ()
 		{
-			while (ProcessPrimaryQueue () || ProcessMarkedPending () || ProcessLazyAttributes () || ProcessLateMarkedAttributes () || MarkFullyPreservedAssemblies () || ProcessInternalsVisibleAttributes ()) {
-
-				// deal with [TypeForwardedTo] pseudo-attributes
-
-				// This marks all exported types that resolve to a marked type. Note that it
-				// may mark unused exported types that happen to resolve to a type which was
-				// marked from a different reference. This seems incorrect.
-				// Note also that we may still remove type forwarder assemblies with marked exports in SweepStep,
-				// if they don't contain other used code.
-				// https://github.com/mono/linker/issues/1740
-				foreach (AssemblyDefinition assembly in _context.GetAssemblies ()) {
-					if (!assembly.MainModule.HasExportedTypes)
-						continue;
-
-					foreach (var exported in assembly.MainModule.ExportedTypes) {
-						bool isForwarder = exported.IsForwarder;
-						var declaringType = exported.DeclaringType;
-						while (!isForwarder && (declaringType != null)) {
-							isForwarder = declaringType.IsForwarder;
-							declaringType = declaringType.DeclaringType;
-						}
-
-						if (!isForwarder)
-							continue;
-						TypeDefinition type = exported.Resolve ();
-						if (type == null)
-							continue;
-						if (!Annotations.IsMarked (type))
-							continue;
-						var di = new DependencyInfo (DependencyKind.ExportedType, type);
-						_context.MarkingHelpers.MarkExportedType (exported, assembly.MainModule, di);
-					}
-				}
-			}
+			while (ProcessPrimaryQueue () ||
+				ProcessMarkedPending () ||
+				ProcessLazyAttributes () ||
+				ProcessLateMarkedAttributes () ||
+				MarkFullyPreservedAssemblies () ||
+				ProcessInternalsVisibleAttributes ()) ;
 
 			ProcessPendingTypeChecks ();
 		}
@@ -858,6 +831,8 @@ void MarkDynamicDependency (DynamicDependency dynamicDependency, IMemberDefiniti
 					_context.LogWarning ($"Unresolved type '{typeName}' in DynamicDependencyAttribute", 2036, context);
 					return;
 				}
+
+				MarkingHelpers.MarkMatchingExportedType (type, assembly, new DependencyInfo (DependencyKind.DynamicDependency, type));
 			} else if (dynamicDependency.Type is TypeReference typeReference) {
 				type = typeReference.Resolve ();
 				if (type == null) {
@@ -947,14 +922,20 @@ protected virtual void MarkUserDependency (IMemberDefinition context, CustomAttr
 				assembly = null;
 			}
 
-			TypeDefinition td;
+			TypeDefinition td = null;
 			if (args.Count >= 2 && args[1].Value is string typeName) {
-				td = _context.TypeNameResolver.ResolveTypeName (assembly ?? (context as MemberReference).Module.Assembly, typeName)?.Resolve ();
+				AssemblyDefinition assemblyDef = assembly ?? (context as MemberReference).Module.Assembly;
+				TypeReference tr = _context.TypeNameResolver.ResolveTypeName (assemblyDef, typeName);
+				if (tr != null)
+					td = tr.Resolve ();
+
 				if (td == null) {
 					_context.LogWarning (
 						$"Could not resolve dependency type '{typeName}' specified in a `PreserveDependency` attribute", 2004, context);
 					return;
 				}
+
+				MarkingHelpers.MarkMatchingExportedType (td, assemblyDef, new DependencyInfo (DependencyKind.PreservedDependency, ca));
 			} else {
 				td = context.DeclaringType.Resolve ();
 			}
@@ -1357,15 +1338,20 @@ void MarkEntireAssembly (AssemblyDefinition assembly)
 		{
 			Debug.Assert (Annotations.IsProcessed (assembly));
 
+			ModuleDefinition module = assembly.MainModule;
 			MarkCustomAttributes (assembly, new DependencyInfo (DependencyKind.AssemblyOrModuleAttribute, assembly), null);
-			MarkCustomAttributes (assembly.MainModule, new DependencyInfo (DependencyKind.AssemblyOrModuleAttribute, assembly.MainModule), null);
+			MarkCustomAttributes (module, new DependencyInfo (DependencyKind.AssemblyOrModuleAttribute, module), null);
 
-			if (assembly.MainModule.HasExportedTypes) {
-				// TODO: This needs more work accross all steps
+			foreach (TypeDefinition type in module.Types)
+				MarkEntireType (type, includeBaseTypes: false, includeInterfaceTypes: false, new DependencyInfo (DependencyKind.TypeInAssembly, assembly), null);
+
+			foreach (ExportedType exportedType in module.ExportedTypes) {
+				MarkingHelpers.MarkExportedType (exportedType, module, new DependencyInfo (DependencyKind.ExportedType, assembly));
+				MarkingHelpers.MarkForwardedScope (new TypeReference (exportedType.Namespace, exportedType.Name, module, exportedType.Scope));
 			}
 
-			foreach (TypeDefinition type in assembly.MainModule.Types)
-				MarkEntireType (type, includeBaseTypes: false, includeInterfaceTypes: false, new DependencyInfo (DependencyKind.TypeInAssembly, assembly), null);
+			foreach (TypeReference typeReference in module.GetTypeReferences ())
+				MarkingHelpers.MarkForwardedScope (typeReference);
 		}
 
 		void ProcessModuleType (AssemblyDefinition assembly)
@@ -1867,21 +1853,24 @@ protected virtual void MarkTypeConverterLikeDependency (CustomAttribute attribut
 			if (args.Count < 1)
 				return;
 
-			TypeDefinition tdef = null;
+			TypeDefinition typeDefinition = null;
 			switch (attribute.ConstructorArguments[0].Value) {
 			case string s:
-				tdef = _context.TypeNameResolver.ResolveTypeName (s)?.Resolve ();
+				typeDefinition = _context.TypeNameResolver.ResolveTypeName (s, out AssemblyDefinition assemblyDefinition)?.Resolve ();
+				if (typeDefinition != null)
+					MarkingHelpers.MarkMatchingExportedType (typeDefinition, assemblyDefinition, new DependencyInfo (DependencyKind.CustomAttribute, provider));
+
 				break;
 			case TypeReference type:
-				tdef = type.Resolve ();
+				typeDefinition = type.Resolve ();
 				break;
 			}
 
-			if (tdef == null)
+			if (typeDefinition == null)
 				return;
 
 			Tracer.AddDirectDependency (attribute, new DependencyInfo (DependencyKind.CustomAttribute, provider), marked: false);
-			MarkMethodsIf (tdef.Methods, predicate, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), sourceLocationMember);
+			MarkMethodsIf (typeDefinition.Methods, predicate, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), sourceLocationMember);
 		}
 
 		void MarkTypeWithDebuggerDisplayAttribute (TypeDefinition type, CustomAttribute attribute)

diff --git a/src/tools/illink/src/linker/Linker.Steps/OutputStep.cs b/src/tools/illink/src/linker/Linker.Steps/OutputStep.cs
@@ -271,42 +271,24 @@ FileInfo GetOriginalAssemblyFileInfo (AssemblyDefinition assembly)
 
 		protected virtual void CopyAssembly (AssemblyDefinition assembly, string directory)
 		{
-			// Special case.  When an assembly has embedded pdbs, link symbols is not enabled, and the assembly's action is copy,
-			// we want to match the behavior of assemblies with the other symbol types and end up with an assembly that does not have symbols.
-			// In order to do that, we can't simply copy files.  We need to write the assembly without symbols
-			if (assembly.MainModule.HasSymbols && !Context.LinkSymbols && assembly.MainModule.SymbolReader is EmbeddedPortablePdbReader) {
-				WriteAssembly (assembly, directory, new WriterParameters ());
-				return;
-			}
-
 			FileInfo fi = GetOriginalAssemblyFileInfo (assembly);
 			string target = Path.GetFullPath (Path.Combine (directory, fi.Name));
 			string source = fi.FullName;
+
 			if (source == target)
 				return;
 
-			CopyFileAndRemoveReadOnly (source, target);
-
+			File.Copy (source, target, true);
 			if (!Context.LinkSymbols)
 				return;
 
 			var mdb = source + ".mdb";
 			if (File.Exists (mdb))
-				CopyFileAndRemoveReadOnly (mdb, target + ".mdb");
+				File.Copy (mdb, target + ".mdb", true);
 
 			var pdb = Path.ChangeExtension (source, "pdb");
 			if (File.Exists (pdb))
-				CopyFileAndRemoveReadOnly (pdb, Path.ChangeExtension (target, "pdb"));
-		}
-
-		static void CopyFileAndRemoveReadOnly (string src, string dest)
-		{
-			File.Copy (src, dest, true);
-
-			System.IO.FileAttributes attrs = File.GetAttributes (dest);
-
-			if ((attrs & System.IO.FileAttributes.ReadOnly) == System.IO.FileAttributes.ReadOnly)
-				File.SetAttributes (dest, attrs & ~System.IO.FileAttributes.ReadOnly);
+				File.Copy (pdb, Path.ChangeExtension (target, "pdb"), true);
 		}
 
 		protected virtual string GetAssemblyFileName (AssemblyDefinition assembly, string directory)

diff --git a/src/tools/illink/src/linker/Linker.Steps/SweepStep.cs b/src/tools/illink/src/linker/Linker.Steps/SweepStep.cs
@@ -97,13 +97,13 @@ void UpdateAssemblyReferencesToRemovedAssemblies (AssemblyDefinition assembly)
 		{
 			var action = Annotations.GetAction (assembly);
 			switch (action) {
-			case AssemblyAction.Skip:
+			case AssemblyAction.Copy:
 			case AssemblyAction.Delete:
 			case AssemblyAction.Link:
 			case AssemblyAction.Save:
+			case AssemblyAction.Skip:
 				return;
 
-			case AssemblyAction.Copy:
 			case AssemblyAction.CopyUsed:
 			case AssemblyAction.AddBypassNGen:
 			case AssemblyAction.AddBypassNGenUsed:
@@ -121,7 +121,6 @@ void UpdateAssemblyReferencesToRemovedAssemblies (AssemblyDefinition assembly)
 
 					switch (action) {
 					case AssemblyAction.CopyUsed:
-					case AssemblyAction.Copy:
 						//
 						// Assembly has a reference to another assembly which has been fully removed. This can
 						// happen when for example the reference assembly is 'copy-used' and it's not needed.
@@ -172,22 +171,14 @@ protected void ProcessAssemblyAction (AssemblyDefinition assembly)
 				break;
 
 			case AssemblyAction.CopyUsed:
-				Annotations.SetAction (assembly, AssemblyAction.Copy);
-				goto case AssemblyAction.Copy;
+				AssemblyAction assemblyAction = AssemblyAction.Copy;
+				if (!Context.KeepTypeForwarderOnlyAssemblies && SweepTypeForwarders (assembly))
+					assemblyAction = AssemblyAction.Save;
 
-			case AssemblyAction.Copy:
-				//
-				// Facade assemblies can have unused forwarders pointing to
-				// removed type (when facades are kept)
-				//
-				//		main.exe -> facade.dll -> lib.dll
-				//		link     |  copy       |  link
-				//
-				// when main.exe has unused reference to type in lib.dll
-				//
-				if (SweepTypeForwarders (assembly))
-					Annotations.SetAction (assembly, AssemblyAction.Save);
+				Annotations.SetAction (assembly, assemblyAction);
+				break;
 
+			case AssemblyAction.Copy:
 				break;
 
 			case AssemblyAction.Link:

diff --git a/src/tools/illink/src/linker/Linker/MarkingHelpers.cs b/src/tools/illink/src/linker/Linker/MarkingHelpers.cs
@@ -11,12 +11,33 @@ public MarkingHelpers (LinkContext context)
 			_context = context;
 		}
 
-		public void MarkExportedType (ExportedType type, ModuleDefinition module, in DependencyInfo reason)
+		public void MarkMatchingExportedType (TypeDefinition typeToMatch, AssemblyDefinition assembly, in DependencyInfo reason)
 		{
-			if (!_context.Annotations.MarkProcessed (type, reason))
+			if (typeToMatch == null || assembly == null)
 				return;
-			if (_context.KeepTypeForwarderOnlyAssemblies)
-				_context.Annotations.Mark (module, new DependencyInfo (DependencyKind.ModuleOfExportedType, type));
+
+			if (assembly.MainModule.GetMatchingExportedType (typeToMatch, out var exportedType))
+				MarkExportedType (exportedType, assembly.MainModule, reason);
+		}
+
+		public void MarkExportedType (ExportedType exportedType, ModuleDefinition module, in DependencyInfo reason)
+		{
+			if (!_context.Annotations.MarkProcessed (exportedType, reason))
+				return;
+
+			_context.Annotations.Mark (module, reason);
+		}
+
+		public void MarkForwardedScope (TypeReference typeReference)
+		{
+			if (typeReference == null)
+				return;
+
+			if (typeReference.Scope is AssemblyNameReference) {
+				var assembly = _context.Resolve (typeReference.Scope);
+				if (assembly != null && assembly.MainModule.GetMatchingExportedType (typeReference.Resolve (), out var exportedType))
+					MarkExportedType (exportedType, assembly.MainModule, new DependencyInfo (DependencyKind.ExportedType, typeReference));
+			}
 		}
 	}
 }
diff --git a/src/tools/illink/src/linker/Linker/ModuleDefinitionExtensions.cs b/src/tools/illink/src/linker/Linker/ModuleDefinitionExtensions.cs
@@ -11,6 +11,21 @@ public static bool IsCrossgened (this ModuleDefinition module)
 				(module.Attributes & ModuleAttributes.ILLibrary) != 0;
 		}
 
+		public static bool GetMatchingExportedType (this ModuleDefinition module, TypeDefinition typeDefinition, out ExportedType exportedType)
+		{
+			exportedType = null;
+			if (!module.HasExportedTypes || typeDefinition == null)
+				return false;
+
+			foreach (var et in module.ExportedTypes)
+				if (et.Resolve () == typeDefinition) {
+					exportedType = et;
+					return true;
+				}
+
+			return false;
+		}
+
 		public static TypeDefinition ResolveType (this ModuleDefinition module, string typeFullName)
 		{
 			if (typeFullName == null)

diff --git a/src/tools/illink/src/linker/Linker/TypeNameResolver.cs b/src/tools/illink/src/linker/Linker/TypeNameResolver.cs
@@ -13,8 +13,9 @@ public TypeNameResolver (LinkContext context)
 			_context = context;
 		}
 
-		public TypeReference ResolveTypeName (string typeNameString)
+		public TypeReference ResolveTypeName (string typeNameString, out AssemblyDefinition typeAssembly)
 		{
+			typeAssembly = null;
 			if (string.IsNullOrEmpty (typeNameString))
 				return null;
 
@@ -28,13 +29,19 @@ public TypeReference ResolveTypeName (string typeNameString)
 			}
 
 			if (parsedTypeName is AssemblyQualifiedTypeName assemblyQualifiedTypeName) {
-				return ResolveTypeName (null, assemblyQualifiedTypeName);
+				typeAssembly = _context.TryResolve (assemblyQualifiedTypeName.AssemblyName.Name);
+				if (typeAssembly == null)
+					return null;
+
+				return ResolveTypeName (typeAssembly, assemblyQualifiedTypeName.TypeName);
 			}
 
 			foreach (var assemblyDefinition in _context.GetReferencedAssemblies ()) {
 				var foundType = ResolveTypeName (assemblyDefinition, parsedTypeName);
-				if (foundType != null)
+				if (foundType != null) {
+					typeAssembly = assemblyDefinition;
 					return foundType;
+				}
 			}
 
 			return null;