jbevain / cecil

Cecil is a library to inspect, modify and create .NET programs and libraries.
MIT License
2.72k stars 620 forks source link

Type resolution depends on the program that is running, not the assembly being processed. #863

Open SteveGilham opened 2 years ago

SteveGilham commented 2 years ago

An assembly built to target the .Net Framework expects to resolve system references in the GAC; however a .net (core) program using Cecil to manipulate that assembly resolves them to the trusted locations under dotnet/shared. The two files will not always match up.

Here's an example -- ClassLibrary.csproj builds a WPF-consuming assembly that references WindowsBase.dll v4.0.0.0 in the GAC. CecilGAC.csproj is a simple .net6.0 program that reads and rewrites the ClassLibrary1.dll assembly.

CecilGAC.zip

The program fails during write with

Mono.Cecil.ResolutionException: Failed to resolve System.Windows.Threading.DispatcherPriority
at Mono.Cecil.Mixin.CheckedResolve(TypeReference self)
...

because a WindowsBase.dll v4.0.0.0 has been found within C:\Program Files\dotnet\shared\Microsoft.NETCore.App; but that assembly is a stub containing only

internal class <Module>
{
}

However, if CecilGAC.csproj is modified to be

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net48</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies.net48" Version="1.0.2">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
    <PackageReference Include="Mono.Cecil" Version="0.11.4" />
  </ItemGroup>

  <ItemGroup>
    <ProjectReference Include="..\ClassLibrary1\ClassLibrary1.csproj" />
  </ItemGroup>

</Project>

building against .Net Framework v4.8, the write operation finds the expected file in the GAC, and the type resolution completes successfully.

Because a suitable strong-named assembly has been found in the ,net core case, a simple reactive approach such as overriding the AssemblyResolver to handle resolution failures by checking the GAC (if present) does not resolve the issue; the necessary fix would be to replace the #if NET_CORE compile-time logic with a runtime selection based on the [TargetFramework] attribute value of the assembly being manipulated.

[ADDED] An alternative would be to recognise such a pure stub assembly and consider it an assembly resolution failure; this form of stub seems to be a standard pattern for GUI-related assemblies.

SimonCropp commented 2 years ago

@SteveGilham how are you resolving types?

SteveGilham commented 2 years ago

The simple example provided is using the DefaultAssemblyResolver type -- it just reads and then immediately writes a sample assembly; and its behaviour changes depending on the target platform to which it is built, even though the sample assembly being processed is the same in either case.

The issue was originally observed (https://github.com/SteveGilham/altcover/issues/156) in a context using a .net core executable that added a ResolveFailure handler for the purposes of looking in the nuget cache for dependencies; however in this case, the handler is never invoked because an apparently suitable assembly was found by name.

SimonCropp commented 2 years ago

you need to implement your own AssemblyResolver that respects the runtime of the target assembly. Cecil cannot do this for you. for example here is the AssemblyResolver in fody https://github.com/Fody/Fody/blob/master/FodyIsolated/AssemblyResolver.cs that uses the msbuild context to resolve assemblies

SteveGilham commented 2 years ago

Cecil cannot do this for you.

That's why I'm making this issue report. It could and should avoid such platform-inconsistent behaviour, and can do it simply by changing the compile-time logic check to being a run-time one.

SteveGilham commented 2 years ago

PR #864 does the core of the work, sufficient to let one write an assembly resolver that sub-classes e.g. DefaultAssemblyResolver and re-uses the existing tested search implementations and can say "that assembly under dotnet/shared is a stub, let's look in the GAC instead".

Going on from here, there are possibilities such as dropping the remaining #if block in favour of a strategy pattern, with the default strategy being chosen at compile time as now, and making the selection at run time available. That could build into the initialization of the default resolver (in ModuleDefinition.get_AssemblyResolver to select a strategy based on the module's [TargetFramework] attribute. Alternatively making an abstract resolver type with concrete BaseDotNetAssemblyResolver and BaseFrameworkAssemblyResolver and a generic DefaultAssemblyReader type based on the strategy as a class.

Zastai commented 2 years ago

Is there no chicken/egg issue with trying to have the behaviour of the resolver depend on [TargetFramework]? For one thing, I would expect it would need to resolve TargetFrameworkAttribute before being able to look at the target framework value.

SteveGilham commented 2 years ago

You can get the attribute type name and examine the associated binary blob data w/o any resolver as the following example demonstrates

using System;
using System.IO;
using System.Linq;
using System.Reflection;

using Mono.Cecil;

namespace CecilGAC
{
  public class NonResolver : IAssemblyResolver
  {
    public AssemblyDefinition Resolve(AssemblyNameReference name)
    {
      throw new NotImplementedException();
    }

    public AssemblyDefinition Resolve(AssemblyNameReference name, ReaderParameters parameters)
    {
      throw new NotImplementedException();
    }

    public void Dispose()
    {
    }
  }

  internal class Program
  {
    private static void Main(string[] args)
    {
      var parameters = new ReaderParameters();
      parameters.AssemblyResolver = new NonResolver();

      var def = AssemblyDefinition.ReadAssembly([assembly path goes here], parameters);
      foreach (var a in def.CustomAttributes)
      {
        Console.WriteLine(a.AttributeType.FullName);
        var b = a.GetBlob();
        var s = System.Text.Encoding.ASCII.GetString(b);
        Console.WriteLine(s);
      }
    }
  }
}