Writing modules that have TypeRefs defined in the same module results in a null ResolutionScope

[puff]

AsmResolver Version

5.3.0

.NET Version

.NET 6.0

Operating System

Windows

Describe the Bug

When writing a module has a TypeRef with its ResolutionScope as the same module, AsmResolver will set it as 0, breaking the reference (at least in dnSpy, and PEVerify).

How To Reproduce

WinFormsApp2.zip
Load an affected file, and write it.

var module3 = ModuleDefinition.FromFile("WinFormsApp2.dll");
module3.Write("WinFormsApp2-test.dll");

Expected Behavior

Writing the file should result in the correct module as its ResolutionScope.

Actual Behavior

Writing the file results in the TypeRef's ResolutionScope set to 0.
Original assembly

After writing with AsmResolver (no changes to the assembly were made)

This results in dnSpy being unable to resolve the reference, and PEVerify throwing an error on the type.

Additional Context

I tracked it down to this line:

AsmResolver/src/AsmResolver.DotNet/Builder/DotNetDirectoryBuffer.CodedIndices.cs

Lines 39 to 46 in 9d0fe83

	var token = scope.MetadataToken.Table switch
	{
	TableIndex.AssemblyRef => AddAssemblyReference(scope as AssemblyReference, allowDuplicates, preserveRid),
	TableIndex.TypeRef => AddTypeReference(scope as TypeReference, allowDuplicates, preserveRid),
	TableIndex.ModuleRef => AddModuleReference(scope as ModuleReference, allowDuplicates, preserveRid),
	TableIndex.Module => 0,
	_ => throw new ArgumentOutOfRangeException(nameof(scope))
	};

This sets the ResolutionScope token to 0 when the scope is a ModuleDefinition.
According to the ECMA II.22.38 section, this is invalid. A null (0) ResolutionScope corresponds to an ExportedType, and not a Module. However, as discussed in the discord, the CLR handles a null ResolutionScope by treating it as the current module, which is also in AsmResolver here:

AsmResolver/src/AsmResolver.DotNet/Serialized/SerializedTypeReference.cs

Lines 43 to 44 in 9d0fe83

	if (_row.ResolutionScope == 0)
	return _context.ParentModule;

Changing the line

AsmResolver/src/AsmResolver.DotNet/Builder/DotNetDirectoryBuffer.CodedIndices.cs

Line 44 in 9d0fe83

TableIndex.Module => 0,

to TableIndex.Module => scope.MetadataToken fixes the issue, but as discussed in the discord, this might not be enough. There are cases where a TypeRef has a ResolutionScope of 0 in obfuscated assemblies (such as koivm, or assemblies written with an old version of dnlib)

[Washi1337]

Overall I am in favor of complying more with the ECMA specification.

We cannot use scope.MetadataToken, because that may still result in 0 when the ModuleDefinition is created by the user (and thus does not have a non-zero token assigned yet). A better solution probably is new MetadataToken(TableIndex.Module, 1), there is only one module definition anyways per PE.

The bigger issue is distinguishing between module definition scopes and actual null-scopes for ITypeDescriptor::Scope. If we want to allow for this property to be null in a valid type signature, we run into problems with determining the value of TypeSignature::Module, which is one of the main places where Scope is used. TypeSignature::Module is an important property used throughout the entire package to get access to several core services such as type factory and determining whether signatures are fully imported or not. The implementation of this property will have to be changed in such a way that the right module definition is returned even if Scope is null.

Ideally, I would really like to keep TypeSignature::Module a computed property, both from a memory consumption perspective as well as a usability perspective.

[Washi1337]

Implemented in #466