Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReadDebugHeader method change causing a SymbolsNotMatchingException #589

Closed
tephe opened this issue Jun 4, 2019 · 5 comments
Closed

ReadDebugHeader method change causing a SymbolsNotMatchingException #589

tephe opened this issue Jun 4, 2019 · 5 comments

Comments

@tephe
Copy link

tephe commented Jun 4, 2019

Hi,

In #349 the ReadDebugHeader method was changed, and we now check


if (directory.AddressOfRawData == 0) {
	entries [i] = new ImageDebugHeaderEntry (directory, Empty<byte>.Array);
	continue;
}

inside

void ReadDebugHeader ()
{
	if (image.Debug.IsZero) {
		image.DebugHeader = new ImageDebugHeader (Empty<ImageDebugHeaderEntry>.Array);
		return;
	}

	MoveTo (image.Debug);

	var entries = new ImageDebugHeaderEntry [(int) image.Debug.Size / ImageDebugDirectory.Size];

	for (int i = 0; i < entries.Length; i++) {
		var directory = new ImageDebugDirectory {
			Characteristics = ReadInt32 (),
			TimeDateStamp = ReadInt32 (),
			MajorVersion = ReadInt16 (),
			MinorVersion = ReadInt16 (),
			Type = (ImageDebugType) ReadInt32 (),
			SizeOfData = ReadInt32 (),
			AddressOfRawData = ReadInt32 (),
			PointerToRawData = ReadInt32 (),
		};

		if (directory.AddressOfRawData == 0) {
			entries [i] = new ImageDebugHeaderEntry (directory, Empty<byte>.Array);
			continue;
		}

		var position = Position;
		try {
			MoveTo ((uint) directory.PointerToRawData);
			var data = ReadBytes (directory.SizeOfData);
			entries [i] = new ImageDebugHeaderEntry (directory, data);
		} finally {
			Position = position;
		}
	}

	image.DebugHeader = new ImageDebugHeader (entries);
}

We have a scenario in which we use il-repack to merge dlls (together with pdbs). Currently il-repack is using the old version of Cecil and this part works. We want to unfork cecil in gluck/il-repack#236 and use the current version but it seems that symbol files for IKVM'ed dlls are not read correctly now.

I tracked down the issue to that part of the code. In our IKVM symbol file the AddressOfRawData is 0. But the PointerToRawData is correct and if we skip that check (using debugger) then the behavior works as before.

Is that check necessary? Can we remove it?

Unfortunately I can't add the dll plus pdb which are problematic but I will try to generate dummy dlls and attach them here.

Thank you

@jbevain
Copy link
Owner

jbevain commented Jun 4, 2019

The check is necessary as we've probably seen empty debug headers. It should however be possible to change the check to verify that PointerToRawData is not zero.

@tephe
Copy link
Author

tephe commented Jun 4, 2019

Coming back with the test dll.

I have managed to create a ikvm generated test dll with 7.1.4532.2 . Attaching the dll and pdb here. The problem can easily be reproduced with a test inside PdbTests and these assemblies.

[Test]
public void TestIkvmDll()
{
	TestModule ("ikvm-test-dll.dll", module => {
		var type = module.GetType ("com.test.TestClass");
		var method = type.GetMethod ("Foo");
		Assert.NotNull(type);
		Assert.NotNull(method);

	}, readOnly: !Platform.HasNativePdbSupport, symbolReaderProvider: typeof(PdbReaderProvider), symbolWriterProvider: typeof(PdbWriterProvider));
}

test-dll.zip

@jbevain jbevain closed this as completed in 749710a Jun 4, 2019
@jbevain
Copy link
Owner

jbevain commented Jun 4, 2019

Thanks for the test, unfortunately it's not self contained and requires to resolve at least IKVM.Runtime, so I can't take the test as it is. I've pushed a quick fix, but would love a self contained test.

@tephe
Copy link
Author

tephe commented Jun 4, 2019

Thank you for the quick fix!

Indeed I saw that the test was failing for the ReadImmediately use case with the fix you mentioned. I've added the IKVM.Runtime.dll as well in this commit, if you want to add a self contained test.

tephe@c5c49a4

@tephe
Copy link
Author

tephe commented Jun 5, 2019

Another quick question, do you know when 0.11 will be released? Perhaps even a beta version we could reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants