-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Symbols generated by ILAsm do not contain a PdbChecksum debug directory entry #62484
Comments
Tagging subscribers to this area: @dotnet/area-system-runtime-compilerservices Issue DetailsThe System.Runtime.CompilerServices.Unsafe 6.0 package does not have symbols on MSDL or NuGet's symbol server.
|
I just checked if we emmited PDBs for this project and we do. I tested it out on the 6.0 branch and we do generate the PDB and also we create a symbols package with the PDBs in it. So I wonder if this was a publishing issue or if we need to adjust the way we do publishing. |
I think we're publishing correctly. I can see the symbols when I use symchk. What's interesting are some errors from symchk. Public symbol server (I see the same when using the internal server)
The interesting parts seem to be:
And although Line Numbers is misspelled, that's not the problem 😄 I wonder if symchk complains about the PDB in the same way from a local build, or after we run linker during the build? Perhaps isolating the error to a particular part of the build/publish pipeline can let us know where to find the issue? |
Well, I was able to reproduce the same error when running locally, so perhaps it's a bug in
This might be relevant: #5051 |
Also related #36664 |
This does appear to be unique to 6.0, but not in the way I expected. Pre-6.0 builds of System.Runtime.CompilerServices.Unsafe look like they don't have a PDB and symchk seems to ignore them:
So perhaps this is the first time ILAsm is emitting PDBs in a way that SymChk can find them (perhaps it's the fitst time we're consuming 99bbb19) and SymChk doesn't like the emitted PDBs. @hoyosjs @jkotas @mikem8361 is there someone from the runtime/debugger side who can help here and have a look? |
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsThe System.Runtime.CompilerServices.Unsafe 6.0 package does not have symbols on MSDL or NuGet's symbol server.
|
I'm having a conversation with the debugger team about this offline. To make certain we're looking at the same thing, @clairernovotny could you please confirm you're seeing the same thing around missing symbols that I am? I want to make sure that a fix to symchk / debugger apis it uses will also fix whatever mechanism you used to identify this problem to begin with. |
cc @dotnet/jit-contrib. |
Looking at this
I think this is just an artifact of properly publishing symbol packages, which slipped in 6.0.0. |
Closing this as the symbols were already in the symbol server. |
I see the issue -- the PDB is missing checksums and so it cannot be validated against the original DLL. This returns zero checksum records: |
I agree with @clairernovotny that we should keep this issue open until we reach an agreement on what's expected for symbols from an ILAsm-built assembly. It may require changes made to ILAsm, Debugger-APIs, and/or NuGetPackageExplorer. @hoyosjs @jkotas is zero checksum records "normal" for this case? What does that mean for debugging? |
PdbChecksums is an optional part of the DLL/PDB generation part. ILAsm doesn't generate it, so S.R.CS.Unsafe won't have it (I just confirmed this. Is this more of a feature request to have it? |
I mostly closed it as it was tracking availability of symbols - these were there all along and they are perfectly valid symbols. If we want to have further discussions on other components these symbols might benefit from, I am happy to do so. Yes. PDB checksum is something we can add - it requires us to change mostly ILAsm.
Yes. Roslyn generates these, but they are not required. It means that the debugger will only match by PDB identity (a GUID embedded in the debug directory of the PE) and any other validation might be ad hoc. The checksum allows to do a quicker/more thorough validation of the PDB identity (a hash check essentiallly) of the PDB vs the PE. |
I see that NPE is doing this based on a copy of code from Nuget introduced here: https://github.com/NuGet/NuGet.Jobs/pull/488/files#diff-b5769d9e8d1e2dae217a8b47008c93a9bf271f72b48d8c09adbd7aa30afbd728R166 It seems that it was intentional in that PR (based on the description) to fail a PDB that has no checksums. I'm not sure why. |
Oh, I think I understand why. That code is trying to make sure that the checksum in the PDB matches the binary, but it also fails if the PDB has no checksums. If we think it's valid and fine for the PDB to have no checksums then we should make a change to NPE (and NuGet.Jobs, and whereever else that logic exists) to special case a PDB with no-checksums. |
I think it's also a good idea for security to implement checksums, but that's a feature request. |
Since it is an optional feature, I am moving this to future. |
Not to resurrect a quiet thread, but I took a look at this today and I wanted to check in to see what this bug is actually tracking. This was actually the concerning part. This sig/age being 0 meant that the pdb wasn't really accessible:
This looks to be fixed (see below), and the rest seems like symchk weirdness. In the next 4-6 weeks I'm looking to replace our dependence on symchk in favor of something that understands devidv symbol policies and can actually successfully verify our PDBs. For now though, it looks like the current System.Runtime.CompilerServices.Unsafe package (6.0.0) is working properly. Here's the output from symverify:
What it's saying here is that all dlls of S.R.CS.Unsafe.dll in this package have a properly generated PDB signature embedded in each dll, and that the PE Image and associated pdbs are on the symbol servers (it's not listed above, but the files are also on MSDL). The checksum issue is less interesting/not concerning to me. These PDB/PE file checksums do not constitute a security boundary and are not used for any security process that I know of (and if they are they shouldn't be!). It does look like old copies of this package are busted, but we can't really go back and re-ship those PE Images with proper PDB signatures embedded in them. As a result, I think we are safe to close this issue as fixed since in 6.0.0 we do embed the right information into the DLL and those PDBs are accessible via both the internal and external symbol server. Are there any objections to closing this issue, or did I miss anything? @hoyosjs? @clairernovotny? Happy to investigate further if there's something I missed here or if there's more issues. |
@leculver The package you pointed has 4 DLLs. all 4 contain this in their debug directory:
there's no |
Ahh, that makes more sense! Thanks for the clarification. Do we know of any tools (other than symchk) which rejects pdbs with a 0 checksum? Or is the feature more for just completeness sake? If we have debuggers or tools which reject pdbs with a 0 checksum then I can add that validation as part of the tooling I'm building. |
Not that I know of, only small custom checks like what the NuGet package explorers which says symbols are missing if there's 0 checksum records. Debuggers like VS only do the cross-reference if such a record is there since it's an optional record - Roslyn tends to emit it. I didn't know that SymChk now fails it. The spec is at https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md#pdb-checksum-debug-directory-entry-type-19 |
Still work-in-progress, but I managed to add the PdbChecksum debug directory entry: #85344 |
The System.Runtime.CompilerServices.Unsafe 6.0 package does not have symbols on MSDL or NuGet's symbol server.
@ericstj
The text was updated successfully, but these errors were encountered: