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

SourceLink returning incorrect PDB info #111138

Open
333fred opened this issue Nov 18, 2024 · 13 comments · May be fixed by dotnet/cecil#270 or jbevain/cecil#965
Open

SourceLink returning incorrect PDB info #111138

333fred opened this issue Nov 18, 2024 · 13 comments · May be fixed by dotnet/cecil#270 or jbevain/cecil#965
Labels
area-Infrastructure-coreclr untriaged New issue has not been triaged by the area owner

Comments

@333fred
Copy link
Member

333fred commented Nov 18, 2024

Version Used: 4.13.0-2.24561.3 (24e1dd6b)

Steps to Reproduce:

  1. Paste this code into a .NET 9 template:
IEnumerable<string>? s = null;
  1. Make sure dotnet.navigation.navigateToSourceLinkAndEmbeddedSources is enabled in vscode. Or, if using vs, make sure sourcelink is enabled there.
  2. Go-to-def on IEnumerable

Expected Behavior:

SourceLink opens https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/IEnumerable.cs

Actual Behavior:

SourceLink opens https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/TypeUnloadedException.cs.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Nov 18, 2024
@dibarbet
Copy link
Member

dibarbet commented Nov 19, 2024

This issue appears to be specific to type definitions that are not an implementation, for example an interface like IEnumerable<T> or delegate Action<T>

Considering the attached DLL and PDB pair (see bottom of message)

  1. the typedef table (metadata) row 2624 has token 33557056
  2. the customdebuginformation (debug metadata) at row 948 has parent 33557056 (matching 1) - this entry has the comprressed integer that points to the TypeInitializationException document in the documents table (wrong)

Previously, non implementation types were not ever added to the Documents table at all. And this appears to be the case in this instance as well, IEnumerable.cs is not present.
Image

However - support for non implementation types in the documents table was added in dotnet/roslyn#56278 (3 yrs ago),
so they should be in the documents table.

I think the next step is to see if the compiler is writing the correct output into the pdb (with both the item added to the documents table and the customdebuginformation pointing to it).

dllandpdb.zip

@davidwengier
Copy link
Member

I verified the compiler changes in dotnet/roslyn#56278 seem to still be working fine:

A TypeDef for an inteface (02000003)
Image

Gets CustomDebugInformation record added for it
Image

and a Document record, which is pointed to by the value in the CustomDebugInformation:
Image

The System.Private.CoreLib.pdb has a CustomDebugInformation record for IEnumerable<T>, but not a Document record, which is odd given both were added in the same PR. I wonder if some part of the runtime build process strips out parts of a PDB, or combines parts of different PDBs, or something weird.

@stephentoub @tannergooding @jkoritzinsky do you know if the runtime build does anything interesting here?

@arunchndr arunchndr removed the untriaged New issue has not been triaged by the area owner label Nov 26, 2024
@davidwengier
Copy link
Member

Another report on Reddit, this time IList going to UInt64: https://www.reddit.com/r/csharp/comments/1h2jgh4/when_i_click_on_go_to_definition_in_ilistint_a_it/

Gentle nudge @stephentoub @tannergooding @jkoritzinsky if any of you have any knowledge of possible post-processing PDBs go through in the runtime repo?

@jkoritzinsky
Copy link
Member

Pdb2Pdb in arcade is used as part of publishing. That's the only post processing I know of.

@davidwengier
Copy link
Member

@tmat do you have any idea what might be happening here?

The compiler only emits TypeDefinitionDocuments info for portable PDBs so the fact that its present indicates to me that the PDB is portable, so I wouldn't think Pdb2Pdb is relevant, but I don't actually know how it's used. I guess if Pdb2Pdb converts to Windows PDBs, and those are what is being downloaded from symbol servers, then maybe it is maintaining the contents of the CustomDebugInformation table, but inadvertently stripping out some of the contents of the Document table?

@tmat
Copy link
Member

tmat commented Jan 6, 2025

Seems like dup of #100051

@333fred
Copy link
Member Author

333fred commented Jan 6, 2025

@tmat are you saying the issue isn't actually fixed, since @sbomer closed that for .NET 9?

@dibarbet
Copy link
Member

dibarbet commented Jan 6, 2025

At this point I'm fairly confident the issue is with how the runtime is creating the pdbs and there isn't any action to take on the Roslyn side. Transferring to the runtime repo.

We're still getting pdbs with 9.0.100 that are missing the interface types in the document table (and custom debug info pointing to the wrong document). Is it possible the bug in cecil (linked above by Tomas) was fixed, but the runtime is still on an older version of it with the issue?

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jan 6, 2025
@dibarbet dibarbet transferred this issue from dotnet/roslyn Jan 6, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 6, 2025
@dibarbet dibarbet removed their assignment Jan 6, 2025
Copy link
Contributor

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

@tommcdon tommcdon added area-Infrastructure and removed area-Diagnostics-coreclr needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jan 9, 2025
Copy link
Contributor

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

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

@tommcdon
Copy link
Member

tommcdon commented Jan 9, 2025

I wonder if some part of the runtime build process strips out parts of a PDB, or combines parts of different PDBs, or something weird.
At this point I'm fairly confident the issue is with how the runtime is creating the pdbs and there isn't any action to take on the Roslyn side.

Moving to area-infrastructure-coreclr to take a first look

@akoeplinger
Copy link
Member

It does seem related to ILLink. If I look at artifacts\obj\coreclr\System.Private.CoreLib\windows.x64.Debug\PreTrim\System.Private.CoreLib.dll then IEnumerable.cs is there:

Image

but it's missing in the trimmed version. this is main so it should have the above Cecil fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Infrastructure-coreclr untriaged New issue has not been triaged by the area owner
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

10 participants