Fixes local constant signature blob writer in pdb for datetime valuetype

[tonyredondo]

Fixes #1

[wwh1004]

it is not a bug. could you tell me why you want to fix it?

[tonyredondo]

it is not a bug. could you tell me why you want to fix it?

I agree that technically is not a bug, certainly from a mdTypeRef token we cannot know if the type is a ValueType or not without resolving to the mdTypeDef.

Despite this I think the library should handle this special case, currently if we open any assembly with a method with a local decimal constant and try to save it including the pdb file without modifying anything, it’s going to fail with an exception. If we move that constant to a class level constant it will work, so it behave differently.

OK example:

using System;

namespace DnlibTest
{
    public class OkClass
    {
        const decimal decimalValue = 7922816251426433759354395033M;
    }
}

KO example:

using System;

namespace DnlibTest
{
    public class OkClass
    {
        private void MySpecialMethod()
        {
            const decimal decimalValue = 7922816251426433759354395033M;
        }
    }
}

So, from a customer standpoint of this library I really think both cases should be handled correctly, as you can see I'm not doing any modification to the target assembly, just opening it and saving it including the pdb.

In my case I created a library to do some ILRewriting to assemblies, I required loading the pdb because I use the SequencePoint from the instructions and create new ones and so on... This library is really helpful doing the job done, but I found this local constant of decimal type case when I was rewriting Newtonsoft.Json, exactly this: https://github.com/JamesNK/Newtonsoft.Json/blob/f7e7bd05d9280f17993500085202ff4ea150564a/Src/Newtonsoft.Json/Utilities/ConvertUtils.cs#L1282 . Not a niche case, is present in the most downloaded and used library in .NET :)

Yeah, I can use my fork or vendor dnlib into my project, but because I found this an obvious fix for this simple case, I thought submitting this.

[wwh1004]

but in fact, if you don't resolve reference assemblies correctly. not only System.Decimal couldn't be written, other enum types are the same.

public static unsafe class Program {
	private static int Main(string[] args) {
		const BindingFlags flags = BindingFlags.NonPublic | BindingFlags.Static;
		var context = ModuleDef.CreateModuleContext();
		using var module = ModuleDefMD.Load("ConsoleApp1.dll", context);
		module.Write("1.dll", new ModuleWriterOptions(module) { WritePdb = true });
		return 0;
	}
}

i think the proper way to write pdb is using AssemblyResolver to resolve references rather than handling some special cases.

[tonyredondo]

Sadly creating and passing the context doesn't fix the issue in this case, I tried the code you pasted, and I still have the same Exception:

[tonyredondo]

I agree that the best solution will be the library resolving all mdTypeRef correctly. Because yes, as you said for enum types will face the same issue.

[wwh1004]

that is because AssemblyResolver is out of data and it doesn’t support .net core. if i have time, i should update it.

[ElektroKill]

Hi, I think dnSpy already implements a more universal resolver. Maybe the one from dnSpy could be adapted for dnlib only https://github.com/dnSpy/dnSpy/blob/master/dnSpy/dnSpy/Documents/AssemblyResolver.cs

[wwh1004]

Thanks. I will refer to it. But it is open source under GPL license, so I must rewrite a new one.