Skip to content

Conversation

@ElektroKill
Copy link
Contributor

Add support for the following CustomDebugInfos:

Copy link
Contributor

@wtfsck wtfsck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PdbTypeDefinitionDocumentsDebugInfo stores MDTokens of PDB documents. Those tokens won't necessarily be correct when writing the PDB. It also makes it difficult/impossible to add new PDB documents to a PdbTypeDefinitionDocumentsDebugInfo since we don't know the metadata tokens before we write the PDB file.

@ElektroKill
Copy link
Contributor Author

PdbTypeDefinitionDocumentsDebugInfo stores MDTokens of PDB documents. Those tokens won't necessarily be correct when writing the PDB. It also makes it difficult/impossible to add new PDB documents to a PdbTypeDefinitionDocumentsDebugInfo since we don't know the metadata tokens before we write the PDB file.

This is indeed the case, I think it's a good idea to change the collection in the PdbTypeDefinitionDocumentsDebugInfo to store PdbDocument objects as this allows easy implementation of writing code since Metadata exposes a GetRid(PdbDocument) method. However, I am puzzled as to how to implement reading for this debug info. PdbDocument objects are only created when the PdbState object is created based on the SymbolReader. The custom debug info reader code does not have access to any way to resolve a metadata token to its associated PdbDocument, it can only access SymbolDocument objects.

I'm currently looking for a good way to allow the reader to use PdbDocument objects but for now I do not have many ideas :/

@wtfsck
Copy link
Contributor

wtfsck commented Nov 13, 2022

The custom debug info reader code does not have access to any way to resolve a metadata token to its associated PdbDocument, it can only access SymbolDocument objects.

I'm currently looking for a good way to allow the reader to use PdbDocument objects but for now I do not have many ideas :/

I haven't looked at that code in a while, but can't you pass in some new context that the portable pdb reader can use? Some property with some interface that has a method it needs and can call, and nothing else.

@ElektroKill
Copy link
Contributor Author

I've taken another look at the code and I've come up with two potential ways to implement this so that PdbTypeDefinitionDocumentsDebugInfo can expose a list of PdbDocument.

Both ways will require some modification to PdbState to store a RID -> PdbDocument mapping since there is currently no way to reliably resolve RIDs since PdbDocument does not store its original RID and the collection of PdbDocuments in PdbState is an IEnumerable<T> derived from the Values property of a dictionary which is not an ordered collection. Due to how PdbDocument objects are created in PdbState the SymbolDocument type would have to be updated to store the RID that was used to derive it so that the PdbState constructor can build a dictionary of RID -> PdbDocument when building PdbDocument objects.
It is worth mentioning that the aforementioned logic would be specific to PortablePDB reading as WindowsPDB does not utilize RIDs nor does it store CustomDebugInfo structures.

Now onto the two different ways of implementing this.

  1. Since the PortablePdbCustomDebugInfoReader class has access to the ModuleDef the PDB belongs to, it could access the ModuleDef.PdbState property and use the newly added logic mentioned above to lookup the RID to a PdbDocument object. This implementation has the advantage of being simple but has one major disadvantage. This relies on the assumption that the custom debug info is read after the PdbState is constructed. This should always be the case as the TypeDefinitionDocuments custom debug info is only emitted for TypeDef members for which custom debug info is lazy loaded and the debug info is not accessible until the PDB finishes loading. However, it is possible that via external modification of the binary this debug info could be added to a Document member since those members also support custom debug info. In this case, reading of the debug info would fail as the PdbState property has not been initialized yet as custom debug info for Document members is processed inside the PortablePdbReader.
  2. PdbTypeDefinitionDocumentsDebugInfo could be modified in such a way to lazy initialize the document list object. The PdbTypeDefinitionDocumentsDebugInfo could have an overridable initialize method which creates a new empty list by default.
public IList<PdbDocument> Documents {
	get {
		if (documents is null)
			InitializeCustomDebugInfos();
		return documents;
	}
}

protected IList<PdbDocument> documents;

protected virtual void InitializeDocuments() => Interlocked.CompareExchange(ref documents, new List<PdbDocument>(), null);

The PdbTypeDefinitionDocumentsDebugInfo type could then be derived from to form a PdbTypeDefinitionDocumentsDebugInfoMD which is only created inside the PdbReader. It would store the ModuleDef object, a list of RIDs as well as override the initialize method to contain logic to translate the RIDs to PdbDocument objects using the data added to PdbState. This implementation does not suffer from the same issue as the previously mentioned approach as the Documents are resolved only when the user accesses and at that point, the PdbState would have been created already.

These implementation ideas are of course open to discussion, please let me know which implementation you would prefer and I will go through with it :p

@wtfsck
Copy link
Contributor

wtfsck commented Nov 14, 2022

Yeah, the 2nd one sounds better. Also you can add an MDToken to PdbDocument/SymbolDocument if needed, even if it's not used by Windows PDBs. PdbState could read that property.

@ElektroKill ElektroKill requested a review from wtfsck November 15, 2022 18:09
@ElektroKill ElektroKill requested a review from wtfsck November 17, 2022 22:16
@wtfsck wtfsck merged commit e43bd76 into 0xd4d:master Nov 18, 2022
@wtfsck
Copy link
Contributor

wtfsck commented Nov 18, 2022

Thanks, merged!

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

Successfully merging this pull request may close these issues.

Support TypeDefinitionDocuments custom debug information

2 participants