diff --git a/ICSharpCode.Decompiler.Tests/TypeSystem/TypeForwarderResolutionTests.cs b/ICSharpCode.Decompiler.Tests/TypeSystem/TypeForwarderResolutionTests.cs new file mode 100644 index 000000000..b6ebed6b3 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TypeSystem/TypeForwarderResolutionTests.cs @@ -0,0 +1,284 @@ +// Copyright (c) 2026 Siegfried Pammer +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.IO; +using System.Linq; +using System.Threading.Tasks; + +using ICSharpCode.Decompiler.Metadata; +using ICSharpCode.Decompiler.Tests.Helpers; +using ICSharpCode.Decompiler.TypeSystem; + +using NUnit.Framework; + +namespace ICSharpCode.Decompiler.Tests.TypeSystem +{ + /// + /// A facade defines no types; it forwards them onwards. Resolving its references relative to the + /// assembly being decompiled - which is what every probe does by default - can take a chain of + /// forwarders out of the framework it started in and into an unrelated set of facades, where it + /// forwards in circles until the cycle guard gives up and the type is lost. That is issue #2054, + /// where a .NET Standard 2.0 assembly sitting among .NET Framework 4.6.1 facades lost + /// System.Linq.Enumerable and every LINQ call decompiled as a cast-laden static call. + /// + /// The layout below is that graph in miniature: the input directory holds a facade chain that + /// closes on itself, and only the second directory holds an assembly that defines the type. + /// + [TestFixture] + public class TypeForwarderResolutionTests + { + /// Forwards to Mid, which only the framework directory has. + const string ShimIL = @" +.assembly extern Mid { .ver 1:0:0:0 } +.assembly Shim { .ver 1:0:0:0 } +.class extern forwarder Ns.T +{ + .assembly extern Mid +} +"; + + /// The framework's facade: forwards to Impl, which exists in both directories. + const string MidIL = @" +.assembly extern Impl { .ver 1:0:0:0 } +.assembly Mid { .ver 1:0:0:0 } +.class extern forwarder Ns.T +{ + .assembly extern Impl +} +"; + + /// + /// The copy next to the input assembly: a facade that forwards back to Shim and closes the + /// cycle. Deliberately the higher version, so that picking the right assembly cannot be an + /// accident of the highest-version-wins deduplication of referenced assemblies. + /// + const string ImplCycleIL = @" +.assembly extern Shim { .ver 1:0:0:0 } +.assembly Impl { .ver 2:0:0:0 } +.class extern forwarder Ns.T +{ + .assembly extern Shim +} +"; + + /// The copy next to Mid: the only assembly in the graph that defines the type. + const string ImplRealIL = @" +.assembly extern System.Runtime { .ver 8:0:0:0 } +.assembly Impl { .ver 1:0:0:0 } +.class public auto ansi beforefieldinit Ns.T + extends [System.Runtime]System.Object +{ +} +"; + + /// + /// Exists in both directories and defines its type in both, so it is nobody's facade. Main + /// references it directly; an ordinary reference must keep resolving next to the assembly + /// being decompiled. + /// + const string DupIL = @" +.assembly extern System.Runtime { .ver 8:0:0:0 } +.assembly Dup { .ver {VERSION}:0:0:0 } +.class public auto ansi beforefieldinit Ns.D + extends [System.Runtime]System.Object +{ +} +"; + + /// + /// A second cycle, for a type nothing in the graph declares: Shim2 -> Mid2 -> Impl2 -> Shim2. + /// + const string Shim2IL = @" +.assembly extern Mid2 { .ver 1:0:0:0 } +.assembly Shim2 { .ver 1:0:0:0 } +.class extern forwarder Ns.U +{ + .assembly extern Mid2 +} +"; + + const string Mid2IL = @" +.assembly extern Impl2 { .ver 1:0:0:0 } +.assembly Mid2 { .ver 1:0:0:0 } +.class extern forwarder Ns.U +{ + .assembly extern Impl2 +} +"; + + /// The copy next to the input assembly, closing the cycle. + const string Impl2CycleIL = @" +.assembly extern Shim2 { .ver 1:0:0:0 } +.assembly Impl2 { .ver 2:0:0:0 } +.class extern forwarder Ns.U +{ + .assembly extern Shim2 +} +"; + + /// + /// The copy next to Mid2. It ends the chain - it forwards nothing - but it does not declare + /// Ns.U either, so the repair must leave it alone rather than load it over the Impl2 that the + /// input directory holds. + /// + const string Impl2DecoyIL = @" +.assembly extern System.Runtime { .ver 8:0:0:0 } +.assembly Impl2 { .ver 1:0:0:0 } +.class public auto ansi beforefieldinit Ns.SomethingElse + extends [System.Runtime]System.Object +{ +} +"; + + const string MainIL = @" +.assembly extern System.Runtime { .ver 8:0:0:0 } +.assembly extern Shim { .ver 1:0:0:0 } +.assembly extern Dup { .ver 1:0:0:0 } +.assembly extern Shim2 { .ver 1:0:0:0 } +.assembly Main { } + +.class public auto ansi beforefieldinit Consumer + extends [System.Runtime]System.Object +{ + .method public hidebysig instance void UseForwarded(class [Shim]Ns.T t) cil managed + { + ret + } + .method public hidebysig instance void UseDuplicated(class [Dup]Ns.D d) cil managed + { + ret + } + + .method public hidebysig instance void UseUndeclared(class [Shim2]Ns.U u) cil managed + { + ret + } +} +"; + + string inputDirectory; + string frameworkDirectory; + string mainAssemblyPath; + + [OneTimeSetUp] + public async Task SetUp() + { + string root = Path.Combine(Path.GetTempPath(), "ILSpy-TypeForwarderResolution-" + Guid.NewGuid().ToString("N")); + inputDirectory = Path.Combine(root, "input"); + frameworkDirectory = Path.Combine(root, "framework"); + Directory.CreateDirectory(inputDirectory); + Directory.CreateDirectory(frameworkDirectory); + + await AssembleAsync(inputDirectory, "Shim", ShimIL).ConfigureAwait(false); + await AssembleAsync(inputDirectory, "Impl", ImplCycleIL).ConfigureAwait(false); + await AssembleAsync(inputDirectory, "Dup", DupIL.Replace("{VERSION}", "2")).ConfigureAwait(false); + await AssembleAsync(frameworkDirectory, "Mid", MidIL).ConfigureAwait(false); + await AssembleAsync(frameworkDirectory, "Impl", ImplRealIL).ConfigureAwait(false); + await AssembleAsync(frameworkDirectory, "Dup", DupIL.Replace("{VERSION}", "1")).ConfigureAwait(false); + await AssembleAsync(inputDirectory, "Shim2", Shim2IL).ConfigureAwait(false); + await AssembleAsync(inputDirectory, "Impl2", Impl2CycleIL).ConfigureAwait(false); + await AssembleAsync(frameworkDirectory, "Mid2", Mid2IL).ConfigureAwait(false); + await AssembleAsync(frameworkDirectory, "Impl2", Impl2DecoyIL).ConfigureAwait(false); + + mainAssemblyPath = await AssembleAsync(inputDirectory, "Main", MainIL).ConfigureAwait(false); + } + + [OneTimeTearDown] + public void TearDown() + { + Directory.Delete(Path.GetDirectoryName(inputDirectory), recursive: true); + } + + static Task AssembleAsync(string directory, string name, string il) + { + string sourceFile = Path.Combine(directory, name + ".il"); + File.WriteAllText(sourceFile, il); + return Tester.AssembleIL(sourceFile, AssemblerOptions.Library); + } + + DecompilerTypeSystem CreateTypeSystem() + { + var mainModule = new PEFile(mainAssemblyPath); + // The target framework decides which probe runs first, and the bug only shows on the + // .NET Core path finder, which the .NET Standard identifier selects. + var resolver = new UniversalAssemblyResolver(mainAssemblyPath, throwOnError: false, + ".NETStandard,Version=v2.0"); + resolver.AddSearchDirectory(frameworkDirectory); + return new DecompilerTypeSystem(mainModule, resolver); + } + + IParameter GetParameterOf(DecompilerTypeSystem typeSystem, string methodName) + { + var consumer = typeSystem.MainModule.GetTypeDefinition(new TopLevelTypeName(string.Empty, "Consumer")); + Assert.That(consumer, Is.Not.Null, "the fixture assembly must declare Consumer"); + var method = consumer.Methods.SingleOrDefault(m => m.Name == methodName); + Assert.That(method, Is.Not.Null, $"Consumer must declare {methodName}"); + return method.Parameters.Single(); + } + + [Test] + public void ForwarderChainLeavingTheInputDirectoryResolvesInTheDirectoryItReached() + { + var parameter = GetParameterOf(CreateTypeSystem(), "UseForwarded"); + + using (Assert.EnterMultipleScope()) + { + Assert.That(parameter.Type.Kind, Is.Not.EqualTo(TypeKind.Unknown), + "the forwarded type must resolve; the chain used to cycle back into the input directory"); + Assert.That(parameter.Type.FullName, Is.EqualTo("Ns.T")); + Assert.That(parameter.Type.GetDefinition().ParentModule.FullAssemblyName, Does.Contain("Version=1.0.0.0"), + "it must come from the assembly that defines it, not from the higher-versioned facade next to the input"); + } + } + + [Test] + public void ChainThatCannotBeRepairedLeavesTheResolvedAssembliesAlone() + { + // The repaired chain ends at an assembly that forwards nothing - but declares nothing + // either. Loading it would displace the assembly of the same name the input directory + // holds, on the strength of an assumption that does not hold, so the repair declines and + // the type stays unresolved exactly as it was. + var typeSystem = CreateTypeSystem(); + var parameter = GetParameterOf(typeSystem, "UseUndeclared"); + + using (Assert.EnterMultipleScope()) + { + Assert.That(parameter.Type.Kind, Is.EqualTo(TypeKind.Unknown), + "nothing declares Ns.U, so no repair can find it"); + var impl2 = typeSystem.Modules.SingleOrDefault(m => m.AssemblyName == "Impl2"); + Assert.That(impl2, Is.Not.Null, "Impl2 must still be loaded"); + Assert.That(impl2.FullAssemblyName, Does.Contain("Version=2.0.0.0"), + "the copy next to the input assembly must not be displaced by one that declares nothing"); + } + } + + [Test] + public void OrdinaryReferenceStillResolvesNextToTheAssemblyBeingDecompiled() + { + var parameter = GetParameterOf(CreateTypeSystem(), "UseDuplicated"); + + using (Assert.EnterMultipleScope()) + { + Assert.That(parameter.Type.FullName, Is.EqualTo("Ns.D")); + Assert.That(parameter.Type.GetDefinition().ParentModule.FullAssemblyName, Does.Contain("Version=2.0.0.0"), + "a reference from an assembly that is not a facade keeps resolving next to the input assembly"); + } + } + } +} diff --git a/ICSharpCode.Decompiler/Metadata/AssemblyReferences.cs b/ICSharpCode.Decompiler/Metadata/AssemblyReferences.cs index 834334aa8..baf120fec 100644 --- a/ICSharpCode.Decompiler/Metadata/AssemblyReferences.cs +++ b/ICSharpCode.Decompiler/Metadata/AssemblyReferences.cs @@ -220,9 +220,25 @@ namespace ICSharpCode.Decompiler.Metadata { readonly System.Reflection.Metadata.AssemblyReference entry; - public MetadataReader Metadata { get; } public AssemblyReferenceHandle Handle { get; } + /// + /// The module that declares this reference. Assembly resolution is otherwise anchored on the + /// assembly being decompiled, no matter which assembly is asking; a resolver that has to tell + /// the two apart - to keep a chain of type forwarders inside the framework it started in, + /// say - needs to know who made the reference. + /// + public MetadataFile ReferencingModule { get; } + + public MetadataReader Metadata => ReferencingModule.Metadata; + + /// + /// Asks the resolver to look next to before anywhere else. + /// Set while repairing a chain of type forwarders that resolution took out of the framework + /// it was walking through; ordinary references leave it alone and resolve as they always did. + /// + public bool PreferNextToReferencingModule { get; } + public bool IsWindowsRuntime => (entry.Flags & AssemblyFlags.WindowsRuntime) != 0; public bool IsRetargetable => (entry.Flags & AssemblyFlags.Retargetable) != 0; @@ -325,26 +341,17 @@ namespace ICSharpCode.Decompiler.Metadata } } - public AssemblyReference(MetadataReader metadata, AssemblyReferenceHandle handle) - { - if (metadata == null) - throw new ArgumentNullException(nameof(metadata)); - if (handle.IsNil) - throw new ArgumentNullException(nameof(handle)); - Metadata = metadata; - Handle = handle; - entry = metadata.GetAssemblyReference(handle); - } - - public AssemblyReference(MetadataFile module, AssemblyReferenceHandle handle) + public AssemblyReference(MetadataFile module, AssemblyReferenceHandle handle, + bool preferNextToReferencingModule = false) { if (module == null) throw new ArgumentNullException(nameof(module)); if (handle.IsNil) throw new ArgumentNullException(nameof(handle)); - Metadata = module.Metadata; Handle = handle; - entry = Metadata.GetAssemblyReference(handle); + ReferencingModule = module; + PreferNextToReferencingModule = preferNextToReferencingModule; + entry = module.Metadata.GetAssemblyReference(handle); } public override string ToString() diff --git a/ICSharpCode.Decompiler/Metadata/MetadataFile.cs b/ICSharpCode.Decompiler/Metadata/MetadataFile.cs index 398cf3fe0..c96fbc144 100644 --- a/ICSharpCode.Decompiler/Metadata/MetadataFile.cs +++ b/ICSharpCode.Decompiler/Metadata/MetadataFile.cs @@ -130,7 +130,7 @@ namespace ICSharpCode.Decompiler.Metadata var value = assemblyReferences; if (value.IsDefault) { - value = Metadata.AssemblyReferences.Select(r => new AssemblyReference(this.Metadata, r)).ToImmutableArray(); + value = Metadata.AssemblyReferences.Select(r => new AssemblyReference(this, r)).ToImmutableArray(); assemblyReferences = value; } return value; diff --git a/ICSharpCode.Decompiler/Metadata/ReferenceLoadInfo.cs b/ICSharpCode.Decompiler/Metadata/ReferenceLoadInfo.cs index 0581caf48..be380dbad 100644 --- a/ICSharpCode.Decompiler/Metadata/ReferenceLoadInfo.cs +++ b/ICSharpCode.Decompiler/Metadata/ReferenceLoadInfo.cs @@ -22,6 +22,15 @@ using System.Linq; namespace ICSharpCode.Decompiler.Metadata { + /// + /// Implemented by assembly resolvers that keep a log of how each reference was resolved, so that + /// the type system can report a reference it could not follow to the same place. + /// + public interface IReferenceLoadInfoProvider + { + ReferenceLoadInfo LoadInfo { get; } + } + public class ReferenceLoadInfo { readonly Dictionary loadedAssemblyReferences = new Dictionary(); diff --git a/ICSharpCode.Decompiler/Metadata/UniversalAssemblyResolver.cs b/ICSharpCode.Decompiler/Metadata/UniversalAssemblyResolver.cs index f72c95396..b4a1f0bc9 100644 --- a/ICSharpCode.Decompiler/Metadata/UniversalAssemblyResolver.cs +++ b/ICSharpCode.Decompiler/Metadata/UniversalAssemblyResolver.cs @@ -298,6 +298,11 @@ namespace ICSharpCode.Decompiler.Metadata } string? file; +#if !VSADDIN + file = FindNextToReferencingModule(name); + if (file != null) + return file; +#endif switch (targetFrameworkIdentifier) { case TargetFrameworkIdentifier.NET: @@ -307,20 +312,45 @@ namespace ICSharpCode.Decompiler.Metadata goto default; file = dotNetCorePathFinder.Value.TryResolveDotNetCore(name); if (file != null) - return file; + break; goto default; case TargetFrameworkIdentifier.Silverlight: if (IsZeroOrAllOnes(targetFrameworkVersion)) goto default; file = ResolveSilverlight(name, targetFrameworkVersion); if (file != null) - return file; + break; goto default; default: - return ResolveInternal(name); + file = ResolveInternal(name); + break; } + + return file; } +#if !VSADDIN + /// + /// Every other probe searches relative to the assembly being decompiled, whichever assembly + /// is asking. A caller that is following a chain of type forwarders needs the opposite: the + /// next assembly in the chain has to come from where the chain currently is, or the chain + /// leaves the framework it reached and can end up going in circles (issue #2054). Only a + /// caller that knows it is repairing such a chain asks for this, by setting + /// . + /// + string? FindNextToReferencingModule(IAssemblyReference name) + { + if (name is not AssemblyReference { PreferNextToReferencingModule: true, ReferencingModule: { } referrer }) + return null; + // An entry of a package or a single-file bundle carries its path inside the container, + // which would be probed relative to the current working directory. + if (!Path.IsPathRooted(referrer.FileName)) + return null; + string? directory = Path.GetDirectoryName(referrer.FileName); + return directory == null ? null : SearchDirectory(name, directory); + } +#endif + DotNetCorePathFinder InitDotNetCorePathFinder() { DotNetCorePathFinder dotNetCorePathFinder; diff --git a/ICSharpCode.Decompiler/TypeSystem/DecompilerTypeSystem.cs b/ICSharpCode.Decompiler/TypeSystem/DecompilerTypeSystem.cs index f641da40a..87812d19d 100644 --- a/ICSharpCode.Decompiler/TypeSystem/DecompilerTypeSystem.cs +++ b/ICSharpCode.Decompiler/TypeSystem/DecompilerTypeSystem.cs @@ -273,8 +273,16 @@ namespace ICSharpCode.Decompiler.TypeSystem "System.Runtime.CompilerServices.Unsafe" }; + /// + /// Where the resolver keeps one, the log that records how each reference was resolved. The + /// type system reports the forwarder chains it cannot follow there, next to the resolution + /// messages for the same reference. + /// + internal ReferenceLoadInfo ReferenceLoadInfo { get; private set; } + private async Task InitializeAsync(MetadataFile mainModule, IAssemblyResolver assemblyResolver) { + ReferenceLoadInfo = (assemblyResolver as IReferenceLoadInfoProvider)?.LoadInfo; DecompilerEventSource.Log.TypeSystemInitStart(mainModule.Name); int referencedAssembliesResolved = 0; try @@ -287,6 +295,170 @@ namespace ICSharpCode.Decompiler.TypeSystem } } + /// + /// Walks the type forwarders of every loaded assembly and looks for chains that come back to + /// an assembly they already passed through. Such a chain never reaches a definition, so the + /// type it forwards is lost. It is walked a second time with each hop resolved next to the + /// assembly forwarding it, which keeps it inside the framework it reached, and the assembly + /// that ends the repaired chain is returned so the caller can load it. + /// + /// + /// Only chains that are already broken are walked twice: a chain that reaches a definition is + /// left exactly as it resolved, so nothing that decompiles correctly today changes. + /// + static async Task> RepairCyclicTypeForwardersAsync( + List referencedAssemblies, IAssemblyResolver assemblyResolver) + { + // The chain is followed the way the type system follows it: by assembly short name, over + // the assemblies that are loaded, keeping the highest version of each name. + var loadedByName = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var file in referencedAssemblies) + { + if (!file.IsAssembly) + continue; + if (!loadedByName.TryGetValue(file.Name, out var existing) + || file.Metadata.GetAssemblyDefinition().Version > existing.Metadata.GetAssemblyDefinition().Version) + { + loadedByName[file.Name] = file; + } + } + + var repaired = new HashSet(); + // The same chain carries every type a facade forwards; walking one of them settles it. + var alreadyWalked = new HashSet<(MetadataFile, string)>(); + foreach (var file in referencedAssemblies.ToArray()) + { + var metadata = file.Metadata; + foreach (var handle in metadata.ExportedTypes) + { + var exportedType = metadata.GetExportedType(handle); + // Only a row that names another assembly can start a chain that leaves this one. + // A row implemented by an AssemblyFile stays inside this assembly - the type lives + // in one of its other modules - and a nested type is implemented by its enclosing + // exported type, so it travels with the chain of the enclosing name. + if (exportedType.Implementation.Kind != SRM.HandleKind.AssemblyReference) + continue; + var typeName = exportedType.GetFullTypeName(metadata); + var reference = (SRM.AssemblyReferenceHandle)exportedType.Implementation; + string targetName = metadata.GetString(metadata.GetAssemblyReference(reference).Name); + if (!alreadyWalked.Add((file, targetName))) + continue; + if (!ChainIsCyclic(file, typeName, targetName, loadedByName)) + continue; + var definition = await FollowChainNextToForwardersAsync(file, typeName, reference, assemblyResolver) + .ConfigureAwait(false); + if (definition != null && !loadedByName.ContainsValue(definition)) + { + repaired.Add(definition); + } + } + } + return repaired; + } + + /// + /// Whether following through the loaded assemblies returns to one + /// it already passed through. A chain that ends anywhere else - at an assembly that does not + /// forward the type onwards, or at a name nothing resolves to - is not this method's business. + /// + static bool ChainIsCyclic(MetadataFile start, FullTypeName typeName, string targetName, + Dictionary loadedByName) + { + var visited = new HashSet { start }; + for (int hop = 0; hop < MaxTypeForwarderHops; hop++) + { + if (!loadedByName.TryGetValue(targetName, out var next)) + return false; + if (!visited.Add(next)) + return true; + var forwarder = next.GetTypeForwarder(typeName); + if (forwarder.IsNil) + return false; + var exportedType = next.Metadata.GetExportedType(forwarder); + // Anything but another assembly ends the chain here: an AssemblyFile row puts the + // type in a sibling module of this assembly, and a nested type row points back at + // its enclosing type rather than onwards. + if (exportedType.Implementation.Kind != SRM.HandleKind.AssemblyReference) + return false; + var reference = (SRM.AssemblyReferenceHandle)exportedType.Implementation; + targetName = next.Metadata.GetString(next.Metadata.GetAssemblyReference(reference).Name); + } + return false; + } + + /// + /// Follows the chain again with every hop resolved next to the assembly that forwards it, and + /// returns the assembly the chain ends at - the one that holds the definition, where the + /// repair worked. Null where it still leads nowhere. + /// + static async Task FollowChainNextToForwardersAsync(MetadataFile start, + FullTypeName typeName, SRM.AssemblyReferenceHandle reference, IAssemblyResolver assemblyResolver) + { + var current = start; + var visited = new HashSet { start }; + for (int hop = 0; hop < MaxTypeForwarderHops; hop++) + { + MetadataFile next; + try + { + next = await assemblyResolver.ResolveAsync( + new AssemblyReference(current, reference, preferNextToReferencingModule: true)) + .ConfigureAwait(false); + } + catch (Exception ex) when (!(ex is OperationCanceledException)) + { + return null; + } + if (next == null || !visited.Add(next)) + return null; + var forwarder = next.GetTypeForwarder(typeName); + if (forwarder.IsNil) + { + // The chain ends here, which is only worth anything if the type is really + // declared here: an assembly that neither forwards nor defines it would + // otherwise be loaded, and displace the assembly it shares its name with. + return DefinesType(next, typeName) ? next : null; + } + var exportedType = next.Metadata.GetExportedType(forwarder); + if (exportedType.Implementation.Kind == SRM.HandleKind.AssemblyFile) + { + // The type is declared in another module of this assembly, which the loader pulls + // in along with it, so the chain ends here and ends well. + return next; + } + if (exportedType.Implementation.Kind != SRM.HandleKind.AssemblyReference) + { + // A nested type, implemented by its enclosing exported type. The chain that + // matters is the enclosing type's, and that one is walked in its own right. + return null; + } + current = next; + reference = (SRM.AssemblyReferenceHandle)exportedType.Implementation; + } + return null; + } + + /// + /// Whether the file declares the type itself. Walking every type definition is affordable + /// here because it only happens for a chain that is already known to be broken. + /// + static bool DefinesType(MetadataFile file, FullTypeName typeName) + { + var metadata = file.Metadata; + foreach (var handle in metadata.TypeDefinitions) + { + if (handle.GetFullTypeName(metadata) == typeName) + return true; + } + return false; + } + + /// + /// Chains are a handful of hops long in practice; the cap only stops a malformed assembly + /// from walking forever. + /// + const int MaxTypeForwarderHops = 16; + /// The number of references in the final set passed to Init(): distinct /// resolved assemblies (same-name lower-version duplicates dropped) plus resolved /// non-assembly modules. @@ -372,6 +544,16 @@ namespace ICSharpCode.Decompiler.TypeSystem } } + // A chain of type forwarders is followed by assembly name, and every name is resolved + // relative to the assembly being decompiled - so a chain that leaves for another + // framework can be pulled straight back and end up at an assembly it already visited. + // Nothing in the closure defines the type then, and it is lost (issue #2054). Such a + // chain is already broken, so walking it again costs nothing: this time each hop is + // resolved next to the assembly that forwards it, and whatever that turns up is added. + var repairedFiles = await RepairCyclicTypeForwardersAsync(referencedAssemblies, assemblyResolver) + .ConfigureAwait(false); + referencedAssemblies.AddRange(repairedFiles); + if (!(identifier == TargetFrameworkIdentifier.NET && version >= new Version(7, 0))) { typeSystemOptions &= ~TypeSystemOptions.NativeIntegersWithoutAttribute; @@ -379,7 +561,7 @@ namespace ICSharpCode.Decompiler.TypeSystem var mainModuleWithOptions = mainModule.WithOptions(typeSystemOptions); // create IModuleReferences for all references var referencedAssembliesWithOptions = new List(referencedAssemblies.Count); - Dictionary referenceAssemblyVersionMap = new(); + Dictionary referenceAssemblyVersionMap = new(); foreach (var file in referencedAssemblies) { // if the file is an assembly, we need to make sure to deduplicate all assemblies, @@ -387,18 +569,22 @@ namespace ICSharpCode.Decompiler.TypeSystem if (file.IsAssembly) { var newFileVersion = file.Metadata.GetAssemblyDefinition().Version; + // A file the forwarder repair found holds the definition the chain was looking + // for, which the assembly it shares its name with does not - version order says + // nothing about that, so it wins outright. + bool isRepaired = repairedFiles.Contains(file); if (referenceAssemblyVersionMap.TryGetValue(file.Name, out var info)) { - if (newFileVersion >= info.version) + if (isRepaired || (newFileVersion >= info.version && !info.repaired)) { referencedAssembliesWithOptions[info.insertionIndex] = file.WithOptions(typeSystemOptions); - referenceAssemblyVersionMap[file.Name] = (newFileVersion, info.insertionIndex); + referenceAssemblyVersionMap[file.Name] = (newFileVersion, info.insertionIndex, isRepaired); } continue; } else { - referenceAssemblyVersionMap[file.Name] = (file.Metadata.GetAssemblyDefinition().Version, referencedAssembliesWithOptions.Count); + referenceAssemblyVersionMap[file.Name] = (newFileVersion, referencedAssembliesWithOptions.Count, isRepaired); } } referencedAssembliesWithOptions.Add(file.WithOptions(typeSystemOptions)); diff --git a/ICSharpCode.Decompiler/TypeSystem/MetadataModule.cs b/ICSharpCode.Decompiler/TypeSystem/MetadataModule.cs index 0002b5d0a..e2f094fa6 100644 --- a/ICSharpCode.Decompiler/TypeSystem/MetadataModule.cs +++ b/ICSharpCode.Decompiler/TypeSystem/MetadataModule.cs @@ -366,7 +366,7 @@ namespace ICSharpCode.Decompiler.TypeSystem IModule ResolveModuleUncached(AssemblyReferenceHandle handle) { - var asmRef = new Metadata.AssemblyReference(metadata, handle); + var asmRef = new Metadata.AssemblyReference(MetadataFile, handle); return Compilation.FindModuleByReference(asmRef); } @@ -941,6 +941,16 @@ namespace ICSharpCode.Decompiler.TypeSystem return td; } } + else + { + // The chain of forwarders came back to this module, so nothing in it defines the + // type and the assemblies involved disagree about where it lives - mismatched + // facades from two frameworks, typically. Recorded once per reference: a facade + // forwards hundreds of types, and they all fail together. + (Compilation as DecompilerTypeSystem)?.ReferenceLoadInfo?.AddMessageOnce( + module.FullAssemblyName, MessageKind.Warning, + $"Could not follow the type forwarders for {typeName.ReflectionName}: they lead back to {AssemblyName}."); + } } return new UnknownType(typeName); diff --git a/ICSharpCode.ILSpyX/LoadedAssembly.cs b/ICSharpCode.ILSpyX/LoadedAssembly.cs index e2775843a..d9f30d9d7 100644 --- a/ICSharpCode.ILSpyX/LoadedAssembly.cs +++ b/ICSharpCode.ILSpyX/LoadedAssembly.cs @@ -578,7 +578,7 @@ namespace ICSharpCode.ILSpyX return debugInfoProvider; } - sealed class MyAssemblyResolver : IAssemblyResolver + sealed class MyAssemblyResolver : IAssemblyResolver, IReferenceLoadInfoProvider { readonly LoadedAssembly parent; readonly bool loadOnDemand; @@ -609,6 +609,12 @@ namespace ICSharpCode.ILSpyX this.referenceLoadInfo = parent.LoadedAssemblyReferencesInfo; } + /// + /// The log the resolution messages go to, so the type system can report the forwarder + /// chains it cannot follow to the same place. + /// + public ReferenceLoadInfo LoadInfo => referenceLoadInfo; + public MetadataFile? Resolve(IAssemblyReference reference) { return ResolveAsync(reference).GetAwaiter().GetResult(); diff --git a/ILSpy.ReadyToRun/ReadyToRunLanguage.cs b/ILSpy.ReadyToRun/ReadyToRunLanguage.cs index e3737aeef..90f159be9 100644 --- a/ILSpy.ReadyToRun/ReadyToRunLanguage.cs +++ b/ILSpy.ReadyToRun/ReadyToRunLanguage.cs @@ -302,7 +302,11 @@ namespace ICSharpCode.ILSpy.ReadyToRun public IAssemblyMetadata FindAssembly(MetadataReader metadataReader, AssemblyReferenceHandle assemblyReferenceHandle, string parentFile) { - return GetAssemblyMetadata(assemblyResolver.Resolve(new Decompiler.Metadata.AssemblyReference(metadataReader, assemblyReferenceHandle))); + // Only the reader is handed in, so the reference is resolved by name: an + // AssemblyReference is tied to the file that declares it. + var reference = metadataReader.GetAssemblyReference(assemblyReferenceHandle); + var name = Decompiler.Metadata.AssemblyNameReference.Parse(reference.GetFullAssemblyName(metadataReader)); + return GetAssemblyMetadata(assemblyResolver.Resolve(name)); } public IAssemblyMetadata FindAssembly(string simpleName, string parentFile) diff --git a/ILSpy/Metadata/MetadataTableTreeNode.cs b/ILSpy/Metadata/MetadataTableTreeNode.cs index 6414ee726..4ba133d1a 100644 --- a/ILSpy/Metadata/MetadataTableTreeNode.cs +++ b/ILSpy/Metadata/MetadataTableTreeNode.cs @@ -94,7 +94,7 @@ namespace ICSharpCode.ILSpy.Metadata output.Write(metadata.GetString(moduleReference.Name)); break; case HandleKind.AssemblyReference: - var asmRef = new ICSharpCode.Decompiler.Metadata.AssemblyReference(metadata, (AssemblyReferenceHandle)handle); + var asmRef = new ICSharpCode.Decompiler.Metadata.AssemblyReference(module, (AssemblyReferenceHandle)handle); output.Write(asmRef.ToString()); break; case HandleKind.Parameter: