ILVerify.IResolver with AssemblyName

[KalleOlaviNiemitalo]

In the ILVerify.IResolver interface, the Resolve method has only string simpleName as a parameter. Can an implementation of this interface get more information about the assembly reference? Preferably the referenced AssemblyName like in AssemblyLoadContext.Load, and perhaps also a MetadataReader on the referencing module. I would use this to simulate .NET Framework assembly probing when verifying a set of assemblies that support .NET Framework.

runtime/src/coreclr/tools/ILVerification/IResolver.cs

Lines 10 to 16 in 3a25a7f

	public interface IResolver
	{
	/// <summary>
	/// This method should return the same instance when queried multiple times.
	/// </summary>
	PEReader Resolve(string simpleName);
	}

I found this comment that appears to explain why the AssemblyName is not passed to IResolver, but am not sure what it means.

runtime/src/coreclr/tools/ILVerification/ILVerifyTypeSystemContext.cs

Lines 31 to 36 in 3a25a7f

	public override ModuleDesc ResolveAssembly(AssemblyName name, bool throwIfNotFound = true)
	{
	// Note: we use simple names instead of full names to resolve, because we can't get a full name from an assembly without reading it
	string simpleName = name.Name;
	return ResolveAssemblyOrNetmodule(simpleName, simpleName, null, throwIfNotFound);
	}

[jkotas]

Yes, I think it should be possible to change this to use AssemblyName instead of string.

perhaps also a MetadataReader on the referencing module

What would you like do with the MetadadaReader of the referencing module?

[KalleOlaviNiemitalo]

Like this, then:

 namespace ILVerify
 {
     public interface IResolver 
     { 
-        PEReader Resolve(string simpleName);
+        PEReader ResolveAssembly(AssemblyName assemblyName);
+        PEReader ResolveModule(PEReader manifestModule, string moduleName);
     }
 }

PEReader manifestModule would allow relative file lookups for modules of the same assembly, if the resolver remembers the path from which it got each PEReader. (Already in 6.0.0, the resolver needs to remember the PEReader instances that it returns, so that it can return the same ones again, and so that it can eventually close them.)

Does ILVerify resolve the unmanaged modules that are referenced for Platform Invoke? I guess it doesn't, because it does not pass a DllImportSearchPath to the resolver. If it resolved them too, PEReader manifestModule could be non-null even when the ModuleRef comes from ImplMap.ImportScope, at least if the module is listed in the File table of the assembly.

If the resolver throws FileNotFoundException, then ILVerify should include the exception instance in the VerificationResult. This would let the resolver report which search paths it used, and whether it found an assembly file with a matching file name but an incompatible assembly version. I don't see a public way for the resolver to set FileNotFoundException.FusionLog, but it could still place the info in Exception.Message or Exception.Data.

[jkotas]

The interface shape you have suggested looks good to me.

Does ILVerify resolve the unmanaged modules that are referenced for Platform Invoke? I guess it doesn't,

Correct.

[KalleOlaviNiemitalo]

If IResolver is changed, then ResolverBase needs changes as well. Minimally:

 namespace ILVerify
 {
     public abstract class ResolverBase : IResolver 
     {
         public PEReader Resolve(string simpleName);
+        public PEReader ResolveAssembly(AssemblyName assemblyName);
+        public PEReader ResolveModule(PEReader manifestModule, string moduleName);
 
         protected abstract PEReader ResolveCore(string simpleName);
     }
 }

Alternatively (but using AssemblyName as the key in ResolverBase._resolverCache would require a custom IEqualityComparer<T>, and would be cumbersome if the resolver wants to return the same PEReader for the same file even when the referenced assembly version is different):

 namespace ILVerify
 {
     public abstract class ResolverBase : IResolver
     {
-        public PEReader Resolve(string simpleName);
+        public PEReader ResolveAssembly(AssemblyName assemblyName);
+        public PEReader ResolveModule(PEReader manifestModule, string moduleName);

-        protected abstract PEReader ResolveCore(string simpleName);
+        protected abstract PEReader ResolveAssemblyCore(AssemblyName assemblyName);
+        protected abstract PEReader ResolveModuleCore(PEReader manifestModule, string moduleName);
     }
 }

I have not found ResolverBase useful because it never calls PEReader.Dispose(), and doing that in a derived class requires a separate collection in addition to the ResolverBase._resolverCache that is already there. If I can get the AssemblyName passed in, then the caching by simpleName in ResolverBase will be even less useful. I think it is best to let ResolverBase be used just by the command-line tool and tests, and recommend that more advanced resolvers implement IResolver directly.

Incidentally, should ResolverBase._resolverCache be changed to use StringComparer.OrdinalIgnoreCase? ECMA-335 6th edition section II.22 (Metadata logical format: tables) specifies "case-blind" comparisons of AssemblyRef to Assembly, and ModuleRef to Module. That's also what AssemblyName.ReferenceMatchesDefinition does (although that method has other problems):

runtime/src/libraries/System.Private.CoreLib/src/System/Reflection/AssemblyName.cs

Line 309 in ce15dc7

return refName.Equals(defName, StringComparison.OrdinalIgnoreCase);

[jkotas]

I think it would be fine to delete ResolverBase in the ILVerification.dll library. It has very little value as you have pointed out.

Incidentally, should ResolverBase._resolverCache be changed to use StringComparer.OrdinalIgnoreCase?

Yes, I agree.

[KalleOlaviNiemitalo]

This was pretty much transformed to the issue #65573 so I'll mark the discussion as resolved.