Allow ExternalTypeMap to have duplicates entries with identical mapping but different trimTarget

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExternalTypeMapNode.cs

@@ -13,9 +13,9 @@ namespace ILCompiler.DependencyAnalysis
 {
     internal sealed class ExternalTypeMapNode : DependencyNodeCore<NodeFactory>, IExternalTypeMapNode
     {
-        private readonly IEnumerable<KeyValuePair<string, (TypeDesc targetType, TypeDesc trimmingTargetType)>> _mapEntries;
+        private readonly IEnumerable<KeyValuePair<string, (TypeDesc targetType, List<TypeDesc> trimmingTargetType)>> _mapEntries;
 
-        public ExternalTypeMapNode(TypeDesc typeMapGroup, IEnumerable<KeyValuePair<string, (TypeDesc targetType, TypeDesc trimmingTargetType)>> mapEntries)
+        public ExternalTypeMapNode(TypeDesc typeMapGroup, IEnumerable<KeyValuePair<string, (TypeDesc targetType, List<TypeDesc> trimmingTargetTypes)>> mapEntries)
         {
             _mapEntries = mapEntries;
             TypeMapGroup = typeMapGroup;
@@ -35,13 +35,27 @@ public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDep
         {
             foreach (var entry in _mapEntries)
             {
-                var (targetType, trimmingTargetType) = entry.Value;
-                if (trimmingTargetType is not null)
+                bool unconditional = false;
+                foreach (var trimTarget in entry.Value.trimmingTargetType)
                 {
-                    yield return new CombinedDependencyListEntry(
-                        context.MetadataTypeSymbol(targetType),
-                        context.NecessaryTypeSymbol(trimmingTargetType),
-                        "Type in external type map is cast target");
+                    if (trimTarget is null)
+                    {
+                        unconditional = true;
+                        break;
+                    }
+                }
+                if (unconditional)
+                    continue;
+                foreach (var trimmingTargetType in entry.Value.trimmingTargetType)
+                {
+                    var targetType = entry.Value.targetType;
+                    if (trimmingTargetType is not null)
+                    {
+                        yield return new CombinedDependencyListEntry(
+                            context.MetadataTypeSymbol(targetType),
+                            context.NecessaryTypeSymbol(trimmingTargetType),
+                            "Type in external type map is cast target");
+                    }
                 }
             }
         }
@@ -50,12 +64,16 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
         {
             foreach (var entry in _mapEntries)
             {
-                var (targetType, trimmingTargetType) = entry.Value;
-                if (trimmingTargetType is null)
+                var (targetType, trimmingTargetTypes) = entry.Value;
+                foreach (var trimTarget in trimmingTargetTypes)
                 {
-                    yield return new DependencyListEntry(
-                        context.MetadataTypeSymbol(targetType),
-                        "External type map entry target type");
+                    if (trimTarget is null)
+                    {
+                        yield return new DependencyListEntry(
+                            context.MetadataTypeSymbol(targetType),
+                            "External type map entry target type");
+                        break;
+                    }
                 }
             }
         }
@@ -75,14 +93,18 @@ public int CompareToImpl(ISortableNode other, CompilerComparer comparer)
         {
             foreach (var entry in _mapEntries)
             {
-                var (targetType, trimmingTargetType) = entry.Value;
+                var (targetType, trimmingTargetTypes) = entry.Value;
 
-                if (trimmingTargetType is null
-                    || factory.NecessaryTypeSymbol(trimmingTargetType).Marked)
+                foreach (var trimmingTargetType in trimmingTargetTypes)
                 {
-                    IEETypeNode targetNode = factory.MetadataTypeSymbol(targetType);
-                    Debug.Assert(targetNode.Marked);
-                    yield return (entry.Key, targetNode);
+                    if (trimmingTargetType is null
+                        || factory.NecessaryTypeSymbol(trimmingTargetType).Marked)
+                    {
+                        IEETypeNode targetNode = factory.MetadataTypeSymbol(targetType);
+                        Debug.Assert(targetNode.Marked);
+                        yield return (entry.Key, targetNode);
+                        break;
+                    }
                 }
             }
         }

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypeMapMetadata.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Reflection.Metadata;
 using ILCompiler.DependencyAnalysis;
 using Internal.IL;
@@ -58,7 +59,7 @@ protected override int CompareToImpl(MethodDesc other, TypeSystemComparer compar
             }
 
             private readonly Dictionary<TypeDesc, TypeDesc> _associatedTypeMap = [];
-            private readonly Dictionary<string, (TypeDesc type, TypeDesc trimmingTarget)> _externalTypeMap = [];
+            private readonly Dictionary<string, (TypeDesc type, List<TypeDesc> trimmingTargets)> _externalTypeMap = [];
             private ThrowingMethodStub _externalTypeMapExceptionStub;
             private ThrowingMethodStub _associatedTypeMapExceptionStub;
 
@@ -78,9 +79,21 @@ public void AddAssociatedTypeMapEntry(TypeDesc type, TypeDesc associatedType)
             }
             public void AddExternalTypeMapEntry(string typeName, TypeDesc type, TypeDesc trimmingTarget)
             {
-                if (!_externalTypeMap.TryAdd(typeName, (type, trimmingTarget)))
+                if (_externalTypeMap.TryGetValue(typeName, out var currentValue))
                 {
-                    ThrowHelper.ThrowBadImageFormatException();
+                    if (currentValue.type != type)
+                    {
+                        // Conflicting type map entry
+                        ThrowHelper.ThrowBadImageFormatException();
+                    }
+                    else
+                    {
+                        currentValue.trimmingTargets.Add(trimmingTarget);
+                    }
+                }
+                else
+                {
+                    _externalTypeMap[typeName] = (type, [trimmingTarget]);
                 }
             }
 

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TypeMapLazyDictionary.cs

@@ -265,6 +265,13 @@ private unsafe struct TypeNameUtf8
         {
             public required void* Utf8TypeName { get; init; }
             public required int Utf8TypeNameLen { get; init; }
+
+            public bool Equals(TypeNameUtf8 other)
+            {
+                ReadOnlySpan<byte> thisSpan = new ReadOnlySpan<byte>(Utf8TypeName, Utf8TypeNameLen);
+                ReadOnlySpan<byte> otherSpan = new ReadOnlySpan<byte>(other.Utf8TypeName, other.Utf8TypeNameLen);
+                return thisSpan.SequenceEqual(otherSpan);
+            }
         }
 
         [RequiresUnreferencedCode("Lazy TypeMap isn't supported for Trimmer scenarios")]
@@ -282,6 +289,8 @@ public DelayedType(TypeNameUtf8 typeNameUtf8, RuntimeAssembly fallbackAssembly)
                 _type = null;
             }
 
+            public TypeNameUtf8 TypeName => _typeNameUtf8;
+
             public unsafe Type GetOrLoadType()
             {
                 if (_type is null)
@@ -328,12 +337,16 @@ protected override bool TryGetOrLoadType(string key, [NotNullWhen(true)] out Typ
             public void Add(string key, TypeNameUtf8 targetType, RuntimeAssembly fallbackAssembly)
             {
                 int hash = ComputeHashCode(key);
-                if (_lazyData.ContainsKey(hash))
+                // Allow duplicates that have the same string -> mapping. They may have different trimTargets.
+                // Warn if the mapping conflicts with an existing mapping.
+                if (!_lazyData.TryGetValue(hash, out DelayedType? existing))
+                {
+                    _lazyData.Add(hash, new DelayedType(targetType, fallbackAssembly));
+                }
+                else if (!existing.TypeName.Equals(targetType))
                 {
                     ThrowHelper.ThrowAddingDuplicateWithKeyArgumentException(key);
                 }
-
-                _lazyData.Add(hash, new DelayedType(targetType, fallbackAssembly));
             }
         }
 

src/libraries/System.Runtime.InteropServices/tests/TrimmingTests/TypeMap.cs

@@ -8,6 +8,8 @@
 
 [assembly: TypeMap<UsedTypeMap>("TrimTargetIsTarget", typeof(TargetAndTrimTarget), typeof(TargetAndTrimTarget))]
 [assembly: TypeMap<UsedTypeMap>("TrimTargetIsUnrelated", typeof(TargetType), typeof(TrimTarget))]
+[assembly: TypeMap<UsedTypeMap>("DuplicateMappingWithDifferentTrimTargets", typeof(TargetType2), typeof(TrimTarget2))]
+[assembly: TypeMap<UsedTypeMap>("DuplicateMappingWithDifferentTrimTargets", typeof(TargetType2), typeof(TrimTarget3))]
 [assembly: TypeMap<UsedTypeMap>("TrimTargetIsUnreferenced", typeof(UnreferencedTargetType), typeof(UnreferencedTrimTarget))]
 [assembly: TypeMapAssociation<UsedTypeMap>(typeof(SourceClass), typeof(ProxyType))]
 
@@ -27,6 +29,14 @@
     {
         Console.WriteLine("Type deriving from TrimTarget instantiated.");
     }
+    else if (t is TrimTarget2)
+    {
+        Console.WriteLine("Type deriving from TrimTarget2 instantiated.");
+    }
+    else if (t is TrimTarget3)
+    {
+        Console.WriteLine("Type deriving from TrimTarget3 instantiated.");
+    }
 
     Console.WriteLine("Hash code of SourceClass instance: " + new SourceClass().GetHashCode());
     return -1;
@@ -95,6 +105,12 @@
     return 10;
 }
 
+if (!usedTypeMap.TryGetValue("DuplicateMappingWithDifferentTrimTargets", out Type duplicatedTarget))
+{
+    Console.WriteLine("Could not find duplicated target type");
+    return 11;
+}
+
 return 100;
 
 [MethodImpl(MethodImplOptions.NoInlining)]
@@ -106,7 +122,10 @@ static Type GetTypeWithoutTrimAnalysis(string typeName)
 class UsedTypeMap;
 class TargetAndTrimTarget;
 class TargetType;
+class TargetType2;
 class TrimTarget;
+class TrimTarget2;
+class TrimTarget3;
 class UnreferencedTargetType;
 class UnreferencedTrimTarget;
 class SourceClass;

src/tests/Interop/TypeMap/GroupTypes.cs

@@ -17,7 +17,9 @@ public class I2<U> {}
 
 public class TypicalUseCase { }
 
-public class DuplicateTypeNameKey { }
+public class ValidDuplicateTypeNameKey { }
+
+public class InvalidDuplicateTypeNameKey { }
 
 public class InvalidTypeNameKey { }
 

src/tests/Interop/TypeMap/TypeMapApp.cs

@@ -61,11 +61,15 @@
 [assembly: TypeMap<InvalidTypeNameKey>(null!, typeof(object))]
 [assembly: TypeMapAssociation<InvalidTypeNameKey>(null!, typeof(object))]
 
-[assembly: TypeMap<DuplicateTypeNameKey>("1", typeof(object))]
-[assembly: TypeMap<DuplicateTypeNameKey>("1", typeof(object))]
+[assembly: TypeMap<ValidDuplicateTypeNameKey>("1", typeof(object))]
+[assembly: TypeMap<ValidDuplicateTypeNameKey>("1", typeof(object), typeof(object))]
+[assembly: TypeMap<ValidDuplicateTypeNameKey>("1", typeof(object), typeof(string))]
 
-[assembly: TypeMapAssociation<DuplicateTypeNameKey>(typeof(DupType_MapObject), typeof(object))]
-[assembly: TypeMapAssociation<DuplicateTypeNameKey>(typeof(DupType_MapString), typeof(string))]
+[assembly: TypeMap<InvalidDuplicateTypeNameKey>("1", typeof(object))]
+[assembly: TypeMap<InvalidDuplicateTypeNameKey>("1", typeof(string))]
+
+[assembly: TypeMapAssociation<InvalidDuplicateTypeNameKey>(typeof(DupType_MapObject), typeof(object))]
+[assembly: TypeMapAssociation<InvalidDuplicateTypeNameKey>(typeof(DupType_MapString), typeof(string))]
 
 // Redefine the same type as in the TypeMapLib2 assembly
 // This is testing the duplicate type name key for the
@@ -178,19 +182,28 @@ public static void Validate_ProxyTypeMapping()
     }
 
     [Fact]
-    public static void Validate_ExternalTypeMapping_DuplicateTypeKey()
+    public static void Validate_ExternalTypeMapping_InvalidDuplicateTypeKey()
     {
-        Console.WriteLine(nameof(Validate_ExternalTypeMapping_DuplicateTypeKey));
+        Console.WriteLine(nameof(Validate_ExternalTypeMapping_InvalidDuplicateTypeKey));
 
-        AssertExtensions.ThrowsAny<ArgumentException, BadImageFormatException>(() => TypeMapping.GetOrCreateExternalTypeMapping<DuplicateTypeNameKey>());
+        AssertExtensions.ThrowsAny<ArgumentException, BadImageFormatException>(() => TypeMapping.GetOrCreateExternalTypeMapping<InvalidDuplicateTypeNameKey>());
     }
 
+    [Fact]
+    public static void Validate_ExternalTypeMapping_ValidDuplicateTypeKey()
+    {
+        Console.WriteLine(nameof(Validate_ExternalTypeMapping_ValidDuplicateTypeKey));
+
+        var mapping = TypeMapping.GetOrCreateExternalTypeMapping<ValidDuplicateTypeNameKey>();
+        Assert.Equal(typeof(object), mapping["1"]);
+    }
     [Fact]
     public static void Validate_ProxyTypeMapping_DuplicateTypeKey()
     {
         Console.WriteLine(nameof(Validate_ProxyTypeMapping_DuplicateTypeKey));
 
-        IReadOnlyDictionary<Type, Type> map = TypeMapping.GetOrCreateProxyTypeMapping<DuplicateTypeNameKey>();
+        // Invalid external mapping shouldn't impact proxy mapping
+        IReadOnlyDictionary<Type, Type> map = TypeMapping.GetOrCreateProxyTypeMapping<InvalidDuplicateTypeNameKey>();
 
         Assert.Equal(typeof(object), map[typeof(DupType_MapObject)]);
         Assert.Equal(typeof(string), map[typeof(DupType_MapString)]);