diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypeMapMetadata.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypeMapMetadata.cs index 5eec852b659c90..88290a47d718cc 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypeMapMetadata.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypeMapMetadata.cs @@ -59,6 +59,7 @@ protected override int CompareToImpl(MethodDesc other, TypeSystemComparer compar private readonly Dictionary _associatedTypeMap = []; private readonly Dictionary _externalTypeMap = []; + private readonly List _targetModules = []; private ThrowingMethodStub _externalTypeMapExceptionStub; private ThrowingMethodStub _associatedTypeMapExceptionStub; @@ -104,6 +105,57 @@ public void SetAssociatedTypeMapException(ModuleDesc stubModule, TypeSystemExcep _associatedTypeMapExceptionStub ??= new ThrowingMethodStub(stubModule.GetGlobalModuleType(), TypeMapGroup, externalTypeMap: false, exception); } + public void MergePendingMap(Map pendingMap) + { + // Don't waste time adding entries from the pending map if we already have an exception stub, + // as the exception stub means the map is invalid and the entries won't be used anyway. + if (_associatedTypeMapExceptionStub is null) + { + if (pendingMap._associatedTypeMapExceptionStub is not null) + { + _associatedTypeMapExceptionStub = pendingMap._associatedTypeMapExceptionStub; + } + else + { + foreach (KeyValuePair kvp in pendingMap._associatedTypeMap) + { + AddAssociatedTypeMapEntry(kvp.Key, kvp.Value); + } + } + } + else if (_associatedTypeMapExceptionStub.Exception is not TypeSystemException.FileNotFoundException && + pendingMap._associatedTypeMapExceptionStub?.Exception is TypeSystemException.FileNotFoundException) + { + // If the pending map has a FileNotFoundException, it takes precedence over our existing exception stub, so use it instead. + _associatedTypeMapExceptionStub = pendingMap._associatedTypeMapExceptionStub; + } + + // Don't waste time adding entries from the pending map if we already have an exception stub, + // as the exception stub means the map is invalid and the entries won't be used anyway. + if (_externalTypeMapExceptionStub is null) + { + if (pendingMap._externalTypeMapExceptionStub is not null) + { + _externalTypeMapExceptionStub = pendingMap._externalTypeMapExceptionStub; + } + else + { + foreach (KeyValuePair kvp in pendingMap._externalTypeMap) + { + AddExternalTypeMapEntry(kvp.Key, kvp.Value.type, kvp.Value.trimmingTarget); + } + } + } + else if (_externalTypeMapExceptionStub.Exception is not TypeSystemException.FileNotFoundException && + pendingMap._externalTypeMapExceptionStub?.Exception is TypeSystemException.FileNotFoundException) + { + // If the pending map has a FileNotFoundException, it takes precedence over our existing exception stub, so use it instead. + _externalTypeMapExceptionStub = pendingMap._externalTypeMapExceptionStub; + } + + _targetModules.AddRange(pendingMap._targetModules); + } + public IExternalTypeMapNode GetExternalTypeMapNode() { if (_externalTypeMapExceptionStub is not null) @@ -121,6 +173,16 @@ public IProxyTypeMapNode GetProxyTypeMapNode() } return new ProxyTypeMapNode(TypeMapGroup, _associatedTypeMap); } + + public void AddTargetModule(ModuleDesc targetModule) + { + _targetModules.Add(targetModule); + } + + /// + /// The modules targeted with TypeMapAssemblyTarget attributes for this type map group. This is only populated when TypeMapMetadata is created with TypeMapAssemblyTargetsMode.Record. When TypeMapMetadata is created with TypeMapAssemblyTargetsMode.Traverse, this will be empty as the target assemblies will be traversed to include their type maps instead of just being recorded as targets. + /// + public IEnumerable TargetModules => _targetModules; } public static readonly TypeMapMetadata Empty = new TypeMapMetadata(new Dictionary(), "No type maps"); @@ -141,22 +203,55 @@ private TypeMapMetadata(IReadOnlyDictionary states, string diagno public string DiagnosticName { get; } - public static TypeMapMetadata CreateFromAssembly(EcmaAssembly assembly, CompilerTypeSystemContext typeSystemContext) + public static TypeMapMetadata CreateFromAssembly(EcmaAssembly assembly, ModuleDesc throwHelperEmitModule, TypeMapAssemblyTargetsMode assemblyTargetsMode) { Dictionary typeMapStates = []; - HashSet scannedAssemblies = []; + // The pendingMaps collection represents assemblies that have been scanned, but the provided assembly + // has not been added as a target yet for the specified type map group. + // This can occur when an assembly is the target of a TypeMapAssemblyTarget attribute for a different group. + Dictionary<(EcmaAssembly assembly, TypeDesc typeMapGroup), (TypeDesc scannedDuringGroup, Map map)> pendingMaps = []; + HashSet<(EcmaAssembly assembly, TypeDesc typeMapGroup)> scannedAssemblies = []; - Queue assembliesToScan = new Queue(); - assembliesToScan.Enqueue(assembly); + Queue<(EcmaAssembly assembly, TypeDesc typeMapGroup)> assembliesToScan = []; + assembliesToScan.Enqueue((assembly, null)); while (assembliesToScan.Count > 0) { - EcmaAssembly currentAssembly = assembliesToScan.Dequeue(); - if (scannedAssemblies.Contains(currentAssembly)) - continue; + (EcmaAssembly currentAssembly, TypeDesc currentTypeMapGroup) = assembliesToScan.Dequeue(); - scannedAssemblies.Add(currentAssembly); + // A null currentTypeMapGroup means we're looking at all type maps in the assembly, not just traversing one. + // Otherwise, we're searching for a specific one. + // Don't rescan specific assembly + type map group combos as the results will be identical. + if (currentTypeMapGroup is not null) + { + if (scannedAssemblies.Contains((currentAssembly, currentTypeMapGroup))) + { + if (pendingMaps.TryGetValue((currentAssembly, currentTypeMapGroup), out var pendingMap)) + { + // We have already scanned this assembly, but we haven't added the results to the type map state for the specified type map group. + // Merge in the results. + if (!typeMapStates.TryGetValue(currentTypeMapGroup, out Map typeMapState)) + { + typeMapStates[currentTypeMapGroup] = typeMapState = new Map(currentTypeMapGroup); + } + typeMapState.MergePendingMap(pendingMap.map); + foreach (ModuleDesc targetModule in pendingMap.map.TargetModules) + { + Debug.Assert(assemblyTargetsMode == TypeMapAssemblyTargetsMode.Traverse, "We should only have pending maps with target modules when we're traversing for type map groups, as opposed to just recording targets."); + // If the pending map has target modules, + // then we need to ensure that they also get included in the metadata. + assembliesToScan.Enqueue(((EcmaAssembly)targetModule, currentTypeMapGroup)); + } + pendingMaps.Remove((currentAssembly, currentTypeMapGroup)); + } + // We've already scanned this assembly for this type map group. + // There's no more work to do for this case. + continue; + } + } + // Either we haven't seen this assembly + group combo before + // or we are scanning all type map groups in the assembly. foreach (CustomAttributeHandle attrHandle in currentAssembly.MetadataReader.GetCustomAttributes(EntityHandle.AssemblyDefinition)) { CustomAttribute attr = currentAssembly.MetadataReader.GetCustomAttribute(attrHandle); @@ -185,20 +280,55 @@ public static TypeMapMetadata CreateFromAssembly(EcmaAssembly assembly, Compiler TypeDesc typeMapGroup = type.Instantiation[0]; + Map typeMapState; + + if (currentTypeMapGroup is null || typeMapGroup == currentTypeMapGroup) + { + // This attribute is definitely included in the type map. + if (!typeMapStates.TryGetValue(typeMapGroup, out typeMapState)) + { + typeMapStates[typeMapGroup] = typeMapState = new Map(typeMapGroup); + } + } + else + { + // This attribute may be included in the type map, but this type map group isn't the one that triggered us to scan this assembly, + // so we need to save it as a pending map in case its type map group is triggered later when we scan another assembly. + if (!pendingMaps.TryGetValue((currentAssembly, typeMapGroup), out var pendingMap)) + { + typeMapState = new Map(typeMapGroup); + pendingMaps[(currentAssembly, typeMapGroup)] = (currentTypeMapGroup, typeMapState); + } + else + { + if (pendingMap.scannedDuringGroup != currentTypeMapGroup) + { + // We already scanned this assembly for this type map group while + // processing a different type map group. + // Skip processing it again to avoid hitting duplicate entries. + continue; + } + typeMapState = pendingMap.map; + } + } + + // Mark this assembly + type map group as scanned to avoid redundant work in the future. + scannedAssemblies.Add((currentAssembly, typeMapGroup)); + try { switch (attrKind) { case TypeMapAttributeKind.TypeMapAssemblyTarget: - ProcessTypeMapAssemblyTargetAttribute(attrValue); + ProcessTypeMapAssemblyTargetAttribute(attrValue, typeMapState); break; case TypeMapAttributeKind.TypeMap: - ProcessTypeMapAttribute(attrValue, typeMapGroup); + ProcessTypeMapAttribute(attrValue, typeMapState); break; case TypeMapAttributeKind.TypeMapAssociation: - ProcessTypeMapAssociationAttribute(attrValue, typeMapGroup); + ProcessTypeMapAssociationAttribute(attrValue, typeMapState); break; default: @@ -216,17 +346,24 @@ public static TypeMapMetadata CreateFromAssembly(EcmaAssembly assembly, Compiler if (attrKind is TypeMapAttributeKind.TypeMapAssemblyTarget or TypeMapAttributeKind.TypeMap) { - value.SetExternalTypeMapException(typeSystemContext.GeneratedAssembly, ex); + value.SetExternalTypeMapException(throwHelperEmitModule, ex); } if (attrKind is TypeMapAttributeKind.TypeMapAssemblyTarget or TypeMapAttributeKind.TypeMapAssociation) { - value.SetAssociatedTypeMapException(typeSystemContext.GeneratedAssembly, ex); + value.SetAssociatedTypeMapException(throwHelperEmitModule, ex); } } } - void ProcessTypeMapAssemblyTargetAttribute(CustomAttributeValue attrValue) + if (currentTypeMapGroup is not null) + { + // Mark this assembly + type map group as scanned to avoid redundant work in the future. + // We can hit this case when the type map group has no entries in the current assembly. + scannedAssemblies.Add((currentAssembly, currentTypeMapGroup)); + } + + void ProcessTypeMapAssemblyTargetAttribute(CustomAttributeValue attrValue, Map typeMapState) { if (attrValue.FixedArguments is not [{ Value: string assemblyName }]) { @@ -235,30 +372,29 @@ void ProcessTypeMapAssemblyTargetAttribute(CustomAttributeValue attrVa } EcmaAssembly targetAssembly = (EcmaAssembly)assembly.Context.ResolveAssembly(AssemblyNameInfo.Parse(assemblyName), throwIfNotFound: true); + typeMapState.AddTargetModule(targetAssembly); - assembliesToScan.Enqueue(targetAssembly); + if (assemblyTargetsMode == TypeMapAssemblyTargetsMode.Traverse + && (currentTypeMapGroup is null || currentTypeMapGroup == typeMapState.TypeMapGroup)) + { + // If we're traversing for the current type map group, enqueue the target to be scanned. + // Otherwise, we'll pull the targets to be scanned when the pending group is scanned. + assembliesToScan.Enqueue((targetAssembly, typeMapState.TypeMapGroup)); + } } - void ProcessTypeMapAttribute(CustomAttributeValue attrValue, TypeDesc typeMapGroup) + void ProcessTypeMapAttribute(CustomAttributeValue attrValue, Map typeMapState) { switch (attrValue.FixedArguments) { case [{ Value: string typeName }, { Value: TypeDesc targetType }]: { - if (!typeMapStates.TryGetValue(typeMapGroup, out Map typeMapState)) - { - typeMapStates[typeMapGroup] = typeMapState = new Map(typeMapGroup); - } typeMapState.AddExternalTypeMapEntry(typeName, targetType, null); break; } case [{ Value: string typeName }, { Value: TypeDesc targetType }, { Value: TypeDesc trimTargetType }]: { - if (!typeMapStates.TryGetValue(typeMapGroup, out Map typeMapState)) - { - typeMapStates[typeMapGroup] = typeMapState = new Map(typeMapGroup); - } typeMapState.AddExternalTypeMapEntry(typeName, targetType, trimTargetType); break; } @@ -269,7 +405,7 @@ void ProcessTypeMapAttribute(CustomAttributeValue attrValue, TypeDesc } } - void ProcessTypeMapAssociationAttribute(CustomAttributeValue attrValue, TypeDesc typeMapGroup) + void ProcessTypeMapAssociationAttribute(CustomAttributeValue attrValue, Map typeMapState) { // If attribute is TypeMapAssociationAttribute, we need to extract the generic argument (type map group) // and process it. @@ -279,11 +415,6 @@ void ProcessTypeMapAssociationAttribute(CustomAttributeValue attrValue return; } - if (!typeMapStates.TryGetValue(typeMapGroup, out Map typeMapState)) - { - typeMapStates[typeMapGroup] = typeMapState = new Map(typeMapGroup); - } - typeMapState.AddAssociatedTypeMapEntry(type, associatedType); } } @@ -291,4 +422,16 @@ void ProcessTypeMapAssociationAttribute(CustomAttributeValue attrValue return new TypeMapMetadata(typeMapStates, $"Type maps rooted at {assembly}"); } } + + public enum TypeMapAssemblyTargetsMode + { + /// + /// Traverse the TypeMapAssemblyTarget attributes and include type maps from the target assemblies. + /// + Traverse, + /// + /// Record the TypeMapAssemblyTarget attributes but do not traverse them to include type maps from the target assemblies. + /// + Record + } } diff --git a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/TrimmingDriver.cs b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/TrimmingDriver.cs index 68b25cb1428b51..acf155339d2055 100644 --- a/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/TrimmingDriver.cs +++ b/src/coreclr/tools/aot/ILCompiler.Trimming.Tests/TestCasesRunner/TrimmingDriver.cs @@ -150,12 +150,12 @@ public ILScanResults Trim(ILCompilerOptions options, TrimmingCustomizations? cus if (options.TypeMapEntryAssembly is not null && typeSystemContext.ResolveAssembly(AssemblyNameInfo.Parse(options.TypeMapEntryAssembly), throwIfNotFound: true) is EcmaAssembly typeMapEntryAssembly) { - typeMapManager = new UsageBasedTypeMapManager(TypeMapMetadata.CreateFromAssembly(typeMapEntryAssembly, typeSystemContext)); + typeMapManager = new UsageBasedTypeMapManager(TypeMapMetadata.CreateFromAssembly(typeMapEntryAssembly, typeSystemContext.GeneratedAssembly, TypeMapAssemblyTargetsMode.Traverse)); } else if (entrypointModule is { Assembly: EcmaAssembly entryAssembly }) { // Pass null for typeMappingEntryAssembly to use default entry assembly behavior in tests - typeMapManager = new UsageBasedTypeMapManager(TypeMapMetadata.CreateFromAssembly(entryAssembly, typeSystemContext)); + typeMapManager = new UsageBasedTypeMapManager(TypeMapMetadata.CreateFromAssembly(entryAssembly, typeSystemContext.GeneratedAssembly, TypeMapAssemblyTargetsMode.Traverse)); } CompilationBuilder builder = new RyuJitCompilationBuilder(typeSystemContext, compilationGroup) diff --git a/src/coreclr/tools/aot/ILCompiler/Program.cs b/src/coreclr/tools/aot/ILCompiler/Program.cs index 82652ac08f4a3b..9786ffc489b473 100644 --- a/src/coreclr/tools/aot/ILCompiler/Program.cs +++ b/src/coreclr/tools/aot/ILCompiler/Program.cs @@ -202,7 +202,7 @@ public int Run() compilationRoots.Add(new SingleMethodRootProvider(singleMethod)); if (singleMethod.OwningType is MetadataType { Module.Assembly: EcmaAssembly assembly }) { - typeMapManager = new UsageBasedTypeMapManager(TypeMapMetadata.CreateFromAssembly(assembly, typeSystemContext)); + typeMapManager = new UsageBasedTypeMapManager(TypeMapMetadata.CreateFromAssembly(assembly, typeSystemContext.GeneratedAssembly, TypeMapAssemblyTargetsMode.Traverse)); } } else @@ -319,12 +319,12 @@ public int Run() if (typeMappingEntryAssembly is not null) { var typeMapEntryAssembly = (EcmaAssembly)typeSystemContext.ResolveAssembly(AssemblyNameInfo.Parse(typeMappingEntryAssembly), throwIfNotFound: true); - typeMapManager = new UsageBasedTypeMapManager(TypeMapMetadata.CreateFromAssembly(typeMapEntryAssembly, typeSystemContext)); + typeMapManager = new UsageBasedTypeMapManager(TypeMapMetadata.CreateFromAssembly(typeMapEntryAssembly, typeSystemContext.GeneratedAssembly, TypeMapAssemblyTargetsMode.Traverse)); } else if (entrypointModule is { Assembly: EcmaAssembly entryAssembly }) { // Fall back to entryassembly if not specified - typeMapManager = new UsageBasedTypeMapManager(TypeMapMetadata.CreateFromAssembly(entryAssembly, typeSystemContext)); + typeMapManager = new UsageBasedTypeMapManager(TypeMapMetadata.CreateFromAssembly(entryAssembly, typeSystemContext.GeneratedAssembly, TypeMapAssemblyTargetsMode.Traverse)); } } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/Dependencies/TypeMapReferencedAssembly.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/Dependencies/TypeMapReferencedAssembly.cs index 1ccad3565ad20f..ef0ead884ac1db 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/Dependencies/TypeMapReferencedAssembly.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/Dependencies/TypeMapReferencedAssembly.cs @@ -14,6 +14,8 @@ [assembly: TypeMapAssociation(typeof(ProxySource2), typeof(ProxyTarget2))] [assembly: TypeMapAssemblyTarget("library2")] +[assembly: TypeMap("UnimportantString", typeof(TargetTypeUnconditional3))] + namespace Mono.Linker.Tests.Cases.Reflection.Dependencies { public class TypeMapReferencedAssembly @@ -29,11 +31,13 @@ public static void Run() // Mark expected type map universe _ = TypeMapping.GetOrCreateExternalTypeMapping(); _ = TypeMapping.GetOrCreateProxyTypeMapping(); + _ = TypeMapping.GetOrCreateExternalTypeMapping(); } } public class UsedTypeMapUniverse; public class UnusedTypeMapUniverse; + public class UsedWithoutAssemblyTargetUniverse; } namespace Mono.Linker.Tests.Cases.Reflection.Dependencies.Library @@ -48,4 +52,5 @@ public class ProxySource2; public class ProxyTarget2; public class TrimTarget1; public class TrimTarget2; + public class TargetTypeUnconditional3; } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeMap.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeMap.cs index dbc3009a23b065..558306f20960eb 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeMap.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/Reflection/TypeMap.cs @@ -18,6 +18,7 @@ [assembly: KeptAttributeAttribute(typeof(TypeMapAssociationAttribute))] [assembly: KeptAttributeAttribute(typeof(TypeMapAssociationAttribute))] [assembly: KeptAttributeAttribute(typeof(TypeMapAttribute))] +[assembly: KeptAttributeAttribute(typeof(TypeMapAttribute))] [assembly: TypeMap("TrimTargetIsTarget", typeof(TargetAndTrimTarget), typeof(TargetAndTrimTarget))] [assembly: TypeMap("TrimTargetIsUnrelated", typeof(TargetType), typeof(TrimTarget))] [assembly: TypeMap(nameof(AllocatedNoTypeCheckClassTarget), typeof(AllocatedNoTypeCheckClassTarget), typeof(AllocatedNoTypeCheckClass))] @@ -53,13 +54,17 @@ [assembly: TypeMap("ClassWithStaticMethod", typeof(TargetType4), typeof(ClassWithStaticMethod))] [assembly: TypeMap("ClassWithStaticMethodAndField", typeof(TargetType5), typeof(ClassWithStaticMethodAndField))] +[assembly: TypeMap("UnimportantString", typeof(PreservedTargetType))] + [assembly: KeptAttributeAttribute(typeof(TypeMapAssemblyTargetAttribute))] [assembly: TypeMapAssemblyTarget("library")] // TypeMapAssemblyTarget is kept regardless of which type map the program needs (External or Proxy) [assembly: KeptAttributeAttribute(typeof(TypeMapAssemblyTargetAttribute))] [assembly: KeptAttributeAttribute(typeof(TypeMapAssemblyTargetAttribute))] +[assembly: KeptAttributeAttribute(typeof(TypeMapAssemblyTargetAttribute))] [assembly: TypeMapAssemblyTarget("library")] [assembly: TypeMapAssemblyTarget("library")] +[assembly: TypeMapAssemblyTarget("library")] [assembly: TypeMapAssemblyTarget("library")] // Should be removed namespace Mono.Linker.Tests.Cases.Reflection @@ -84,6 +89,15 @@ namespace Mono.Linker.Tests.Cases.Reflection [KeptMemberInAssembly("library.dll", typeof(ProxySource1), ".ctor()")] [KeptTypeInAssembly("library.dll", typeof(ProxyTarget1))] + // For correctness, NativeAOT only preserves type map entries in a given assembly when the type map is referenced + // and the given assembly is referenced with TypeMapAssemblyTargetAttribute with the given type map group. + // This is required for the correct runtime behavior. + // For simplicity, we do not do the same for ILLinker as the runtime behavior will be correct in CoreCLR regardless + // and in the vast majority of user scenarios, assemblies will be correctly referenced with TypeMapAssemblyTargetAttribute. + // Nearly every case where this behavior would kick in is a bug in user code. + [KeptTypeInAssembly("library.dll", typeof(TargetTypeUnconditional3), Tool = Tool.Trimmer)] + [KeptAttributeInAssembly("library.dll", typeof(TypeMapAttribute))] + [KeptAttributeInAssembly("library.dll", typeof(TypeMapAttribute))] [KeptAttributeInAssembly("library.dll", typeof(TypeMapAssociationAttribute))] [KeptAttributeInAssembly("library.dll", typeof(TypeMapAssemblyTargetAttribute))] @@ -522,4 +536,7 @@ class UsedExternalTarget2; class UsedProxyTarget; [Kept] class UsedProxyTarget2; + + [Kept] + class PreservedTargetType; }