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

No TypeDefinitionDocuments entries compiled for enums/interfaces etc in pdb #100051

Closed
huangmin-ms opened this issue Mar 20, 2024 · 24 comments · Fixed by dotnet/cecil#185 or jbevain/cecil#948
Assignees
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers in-pr There is an active PR which will close this issue when it is merged linkable-framework Issues associated with delivering a linker friendly framework
Milestone

Comments

@huangmin-ms
Copy link

huangmin-ms commented Mar 20, 2024

Version Used:
4.8.0-3.23518.7
Steps to Reproduce:

  1. Download NuGet package from https://www.nuget.org/packages/Microsoft.NETCore.App.Runtime.win-x64/8.0.0
  2. Go to folder runtimes/win-x64/lib/net8.0 and find Microsoft.CSharp.dll.
  3. Download its pdb at http://msdl.microsoft.com/download/symbols/Microsoft.CSharp.pdb\6b0e3c54f1b8eadd4968a9c4faa40bdfFFFFFFFF\Microsoft.CSharp.pdb
  4. Go to CustomDebugInformation table and there's no TypeDefinitionDocuments entry for enum Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfoFlags

Case dotnet/roslyn#1:

Expected Behavior:
Document info for enum Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfoFlags should be found in CustomDebugInformation table

Actual Behavior:
No TypeDefinitionDocuments entry found for enum Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfoFlags

Case dotnet/roslyn#2:

System.Net.NetworkInformation.GatewayIPAddressInformation in System.Net.NetworkInformation.dll is missing from the Document table.

image
image

@huangmin-ms
Copy link
Author

@tmat FYI.

@jaredpar
Copy link
Member

Moving back to runtime. They are generating + publishing the PDBs here. If they feel this is in error then we can discuss and move back to roslyn if we believe there is an issue in the compiler

@jaredpar jaredpar transferred this issue from dotnet/roslyn Mar 21, 2024
Copy link
Contributor

Tagging subscribers to this area: @cston
See info in area-owners.md if you want to be subscribed.

@ericstj
Copy link
Member

ericstj commented Mar 26, 2024

There isn't anything wrong with the how runtime is building these. I believe the problem that @huangmin-ms is reporting is that when a type contains no method bodies - as is the case for an enum, abstract classes, some attribute classes, interfaces, etc - then there is no information in the PDB to map back to the source information for that type.

Is it possible for the compiler to emit this information? It would be helpful for the documentation's link to source, and it would also be helpful for "Go to definition" in the IDE. Currently if you try to "Go to definition" for these types the IDE generates source from metadata, where it will use sourelink information for other types in the same assembly. Perhaps if not source-link information per-se could this information be stored in some other way in the binary or PDB?

@jaredpar
Copy link
Member

when a type contains no method bodies - as is the case for an enum, abstract classes, some attribute classes, interfaces, etc - then there is no information in the PDB to map back to the source information for that type.

PDBs generally map executable code back to source locations. A type which has no code is hence not going to appear in the PDB. There is nothing there to map to.

Is it possible for the compiler to emit this information?

What should we be emitting? There is nothing to map to.

it would also be helpful for "Go to definition" in the IDE.

Possible but this isn't anything that we've seen requested from the IDE team.

This issues seems to be documenting the current design state of PDBs. It's unclear what the ask here is or what capability it provides.

@ericstj
Copy link
Member

ericstj commented Mar 26, 2024

@tmat previously provided a recommendation

CustomDebugInformation records with TypeDefinitionDocuments kind associate types with source documents for types that do not have any method with a user defined body (abstract types, interfaces, etc.).

I believe that is what @huangmin-ms is trying to implement here but finding that's not always the case. @huangmin-ms it might be helpful to share this as an isolated repo that uses source that replicates the issue. I suspect that's simply just setting up a project that uses source-link and then creating types with similar shapes to these. Maybe if we can find why the compiler sometimes creates these entries and sometimes does not it would lead us to a path where we can get more detail about the bug or feature request.

@tmat
Copy link
Member

tmat commented Mar 26, 2024

@jaredpar The feature was implemented by dotnet/roslyn#56278.

@tmat
Copy link
Member

tmat commented Mar 26, 2024

Confirmed that the feature works as expected for a simple dotnet app (dotnet new console).
The PDB contains correct TypeDefinitionDocuments entries for files that don't have method body.

@tmat
Copy link
Member

tmat commented Mar 26, 2024

@ericstj Any chance the binary has been post-processed by IL trimmer or some other tool like that?

@ericstj
Copy link
Member

ericstj commented Mar 26, 2024

@tmat were you able to try it for one of the reported types, for example https://github.com/dotnet/runtime/blob/main/src/libraries/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/CSharpArgumentInfoFlags.cs?

The linker runs, but it's supposed to preserve all public API. I'm not sure what value it has in mucking with this part of the PDB. They are also crossgen'ed in the runtime-pack which could remove the data (also I don't think it should). How can we check the PDB for the right information? Maybe we can examine the PDB at various stages of output to confirm the information is present at first compile then stripped at a later point.

@tmat
Copy link
Member

tmat commented Mar 26, 2024

I suspect that the linker omits unsupported custom debug information from the PDB.

You can use https://github.com/dotnet/metadata-tools to inspect the PDB. I have updated the tool to support displaying the latest custom debug info, the new build will be available shortly. But even the existing tools displays it - it just shows the guid (932E74BC-DBA9-4478-8D46-0F32A7BAB3D3) and raw blob without parsing it into human readable form.

@tmat
Copy link
Member

tmat commented Mar 26, 2024

I think the linker should report warning on unsupported custom debug information (if it indeed omits it).

@ericstj
Copy link
Member

ericstj commented Mar 27, 2024

You might be right, here's the output of MDV (I built your version) for trimmed and prior to trimming:
https://gist.github.com/ericstj/b1e73cbfee3a8e540634fecf6d670b0d
Untrimmed file lists Type Definition Documents. While the trimmed file does not.

We link with these arguments:

C:\Program Files\dotnet\dotnet.exe "C:\src\dotnet\runtime\artifacts\bin\ILLink.Tasks\Debug\net9.0\illink.dll" -a "C:\src\dotnet\runtime\artifacts\obj\Microsoft.CSharp\Debug\net9.0\PreTrim/Microsoft.CSharp.dll" library
-out "C:\src\dotnet\runtime\artifacts\obj\Microsoft.CSharp\Debug\net9.0"
--warnaserror-  --ignore-link-attributes true --skip-unresolved true --trim-mode skip --action skip --action link Microsoft.CSharp -b true --nowarn ;IL2008;IL2009;IL2121;IL2025;IL2035 -d "C:\src\dotnet\runtime\artifacts\bin\microsoft.netcore.app.ref\ref\net9.0"

If I run manually with --verbose I don't see any warnings related to stripping custom debug information. My best guess is that it's accidentally removing this from the PDB. @dotnet/illink can you see what might be going wrong here that strips this information from the PDB?

@ericstj ericstj added linkable-framework Issues associated with delivering a linker friendly framework area-Tools-ILLink .NET linker development as well as trimming analyzers and removed area-Microsoft.CSharp labels Mar 27, 2024
@ericstj
Copy link
Member

ericstj commented Mar 27, 2024

Maybe this is a mono.cecil issue. @tmat do you think this needs an update cc @jbevain? https://github.com/dotnet/cecil/blob/cff8545a8c0177b669e129516b75b844a740987d/Mono.Cecil.Cil/Symbols.cs#L467-L475
https://github.com/dotnet/cecil/blob/cff8545a8c0177b669e129516b75b844a740987d/Mono.Cecil/AssemblyReader.cs#L3163

Maybe not - I see there is a fallback for unknown guids. I debugged a little and I think the problem might be that GetCustomDebugInformation is never called for TokenType.TypeDef and all the infos are stored against that token type.

@tmat
Copy link
Member

tmat commented Mar 27, 2024

If I run manually with --verbose I don't see any warnings related to stripping custom debug information.

It's a bit tricky with reporting warning. On one hand, you don't want unknown CDI to always break build because we are adding them every now and then to support various compiler and debugging features. On the other hand, you want to know if they are missing and Cecil (or other tool) needs to be updated. I'd certainly expect the information to be reported in verbose mode. Perhaps you could have a CI leg that enables the verbose mode and checks for this.

@ericstj
Copy link
Member

ericstj commented Mar 28, 2024

We'd be happy to surface those warnings if they were there -- but they don't exist. It looks to me like the linker isn't even reading out the CustomDebugInformation for TypeDef tokens. It's only reading for Module, Document, LocalVariable, LocalConstant, and Method. I think we need to make cecil plumb the data into TypeDefinitions.

@tmat
Copy link
Member

tmat commented Mar 28, 2024

. It's only reading for Module, Document, LocalVariable, LocalConstant, and Method

That's not good. All metadata entities can have custom debug information.

@ericstj
Copy link
Member

ericstj commented Apr 3, 2024

@sbomer @agocke do you think this is something that you can look at in the linker / cecil? I spent a bit of time trying to see if it was a simple fix but I couldn't find a pattern to follow. Looks like we'd need to have it copy over CustomDebugInformation for TypeDef's - or ideally preserve all CustomDebugInformation for things included regardless of the token type.

@vitek-karas
Copy link
Member

There's a technical problem with the idea of producing warnings - the reading of the PDB is handled by Cecil and AFAIK it currently has no mechanism to provide any kind of feedback about the input - it either reads it or throws. And as you would expect it will skip over things it doesn't understand lot of the time (it was built to make things work most of the time).

Additionally, Cecil currently doesn't have any concept of "debug information on a TypeDefinition" - it only has debug info on MethodDefinition and ModuleDefinition.

Overall, I don't think it would be too difficult to add debug info for types and potentially other things as well (if we have it in the PDB). The ability to produce warnings would be a bigger change which would probably require some design work and thus time.

It looks to me like the linker isn't even reading out the CustomDebugInformation for TypeDef tokens.

Note that for this to work the linker doesn't necessarily need to know about these - it would be enough if Cecil can roundtrip the information correctly. Linker doesn't make a copy of the IL representation, it "trims" the representation produced by Cecil when reading the input, and then writes out what remains into the output. This is similar to other IL-level changes (like field RCA), where pretty much all changes were made in Cecil and then it just works.

@vitek-karas
Copy link
Member

ideally preserve all CustomDebugInformation for things included regardless of the token type

This would be much more difficult - Cecil doesn't work this way - it fully relies on storing everything in the object model, so we would have to add the ability to store the unrecognized stuff somewhere (module-def ?) and then it would also be difficult to update the tokens (again cecil doesn't have the map of old->new tokens, it fully relies on the OM to produce the new tokens).

@ericstj
Copy link
Member

ericstj commented Apr 5, 2024

I was imagining that Cecil's object-model tack on CustomDebugInformation to everything so that it would round trip. I could see how it could be added it to Types - just following a similar pattern to methods - but if it was added to everything maybe there's an easier way.

@vitek-karas
Copy link
Member

If I remember correctly Cecil doesn't have a base class for all definitions. Instead definition derives from reference - and references don't have debug info (or pretty much anything really). But I think adding it to all relevant definitions would make sense, it just has to be done one-by-one.

@agocke
Copy link
Member

agocke commented Apr 22, 2024

+ @sbomer for thoughts

@sbomer
Copy link
Member

sbomer commented Apr 22, 2024

Not much to add - I agree with @vitek-karas's observations. We should make a fix in cecil to keep custom debug info for TypeDefinition.

@sbomer sbomer added this to the 9.0.0 milestone Apr 22, 2024
@sbomer sbomer removed the untriaged New issue has not been triaged by the area owner label Apr 25, 2024
@sbomer sbomer self-assigned this Apr 25, 2024
@sbomer sbomer moved this to High Priority in AppModel Apr 25, 2024
@sbomer sbomer added the in-pr There is an active PR which will close this issue when it is merged label Jun 10, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers in-pr There is an active PR which will close this issue when it is merged linkable-framework Issues associated with delivering a linker friendly framework
Projects
Archived in project
7 participants