Support Windows PDB format in dotnet-symbol for ngen and ReadyToRun binaries

[chrisnas]

Fix #5111.

Some ngen and ReadyToRun assemblies contain debugging information for both Core PDB and Windows PDB files.
This pull request implements the missing Windows PDB format support.

For example, ASP.NET Core Microsoft.AspNetCore.Antiforgery.dll is bound to Microsoft.AspNetCore.Antiforgery.pdb (Core) and Microsoft.AspNetCore.Antiforgery.ni.pdb (Windows). With the current version, an exception happens when trying to validate the format of the Windows PDB file:

C:\dev> dotnet symbol --symbols -d --recurse-subdirectories "C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App\10.0.8\Microsoft.AspNetCore.Antiforgery.dll"
Downloading from https://msdl.microsoft.com/download/symbols/
Generating SymbolStore lookup keys for `C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App\10.0.8\Microsoft.AspNetCore.Antiforgery.dll`...
SymbolChecksum: SHA256:412863983d88d4471e2394ff97b295152d9e7b475dc2458d7ef772688ec0d7e0
ERROR: Unable to read beyond the end of the stream.
ERROR: Unable to read beyond the end of the stream.

Note that the existing Core PDB file is not even fetched due to the exception (displayed twice due to cascading catch blocks)

With the fix, both versions are downloaded, validated and copied:

Downloading from https://msdl.microsoft.com/download/symbols/
Generating SymbolStore lookup keys for `C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App\10.0.8\Microsoft.AspNetCore.Antiforgery.dll`...
SymbolChecksum: SHA256:412863983d88d4471e2394ff97b295152d9e7b475dc2458d7ef772688ec0d7e0
Accepting Windows PDB (MSF); content was validated by the symbol server via the SymbolChecksum header
Cached: C:\Users\Christophe Nasarre\AppData\Local\Temp\SymbolCache\microsoft.aspnetcore.antiforgery.ni.pdb/2530d9bd64692552018c512f8a5ada981/microsoft.aspnetcore.antiforgery.ni.pdb
Writing: C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App\10.0.8\Microsoft.AspNetCore.Antiforgery.ni.pdb
SymbolChecksum: SHA256:412863983d88d4471e2394ff97b295152d9e7b475dc2458d7ef772688ec0d7e0
Testing checksum: SHA256:412863983d88d4471e2394ff97b295152d9e7b475dc2458d7ef772688ec0d7e0
Found checksum match SHA256:412863983d88d4471e2394ff97b295152d9e7b475dc2458d7ef772688ec0d7e0
Cached: C:\Users\Christophe Nasarre\AppData\Local\Temp\SymbolCache\microsoft.aspnetcore.antiforgery.pdb/98632841883d47d49e2394ff97b29515FFFFFFFF/microsoft.aspnetcore.antiforgery.pdb
Writing: C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App\10.0.8\Microsoft.AspNetCore.Antiforgery.pdb
PEFile `C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App\10.0.8\Microsoft.AspNetCore.Antiforgery.dll`: no perfmap records found. No perfmap keys will be produced.
Generated 2 SymbolStoreKeys for `C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App\10.0.8\Microsoft.AspNetCore.Antiforgery.dll`.

[steveisok]

@hoyosjs this seems fine, but please weigh in

[noahfalk]

I think this looks good but we don't want the code comments or logging to imply that we can rely on the server for validation. dotnet-symbol can't validate much about these files.

[noahfalk]

I don't think we can assume the server verified anything (it might even be a malicious server or man-in-the-middle attack). I don't think its a problem though. dotnet-symbol doesn't give security guarantees about the suitability/safety of the downloaded files. As far as I know the checksum tests are only here to provide some sanity checking and early warning against obviously bad files.

Lets remove anything that implies the server (or dotnet-symbol itself) is providing any guarantees around the file content.

[chrisnas]

✅ done

[noahfalk]

We can't assume the server did any verification for us.

[chrisnas]

✅ done

[noahfalk]

Suggested change

	tracer.Information($"Accepting Windows PDB ({pdbFile.ContainerKind}); content was validated by the symbol server via the SymbolChecksum header");
	tracer.Information($"Accepting Windows PDB ({pdbFile.ContainerKind}); No checksum validation is available for this file format");

[chrisnas]

✅ done
I've also handled the possible Exception thrown by PDBFile constructor/IsValid

[noahfalk]

This looks good to me but I'd like @hoyosjs to take a look too.