diff --git a/src/coreclr/ilasm/portable_pdb.cpp b/src/coreclr/ilasm/portable_pdb.cpp index 3aa26fe1597a5f..7fa5b1fb2e92f4 100644 --- a/src/coreclr/ilasm/portable_pdb.cpp +++ b/src/coreclr/ilasm/portable_pdb.cpp @@ -160,15 +160,36 @@ HRESULT PortablePdbWriter::BuildPdbStream(IMetaDataEmit3* peEmitter, mdMethodDef return hr; } -HRESULT PortablePdbWriter::ComputeSha256PdbStreamChecksum(BYTE(&checksum)[32]) +HRESULT PortablePdbWriter::ComputeSha256PdbChecksum(BYTE(&checksum)[32]) { - return m_ilasmPdbWriter->ComputeSha256PdbStreamChecksum(Sha256Hash, checksum); + // The checksum is the hash of the entire PDB file with its 20-byte PDB ID zeroed + // (see "PDB Checksum Debug Directory Entry" in docs/design/specs/PE-COFF.md). + // Serialize the PDB the same way it is saved later, with the ID temporarily zeroed, + // then restore the ID. + HRESULT hr = S_OK; + HRESULT restoreHr = S_OK; + DWORD pdbSize = 0; + BYTE* pdbData = NULL; + + if (FAILED(hr = m_ilasmPdbWriter->ChangePdbStreamId(GUID(), 0))) goto exit; + if (FAILED(hr = m_pdbEmitter->GetSaveSize(cssAccurate, &pdbSize))) goto exit; + pdbData = new BYTE[pdbSize]; + if (FAILED(hr = m_pdbEmitter->SaveToMemory(pdbData, pdbSize))) goto exit; + hr = Sha256Hash(pdbData, pdbSize, checksum, sizeof(checksum)); + +exit: + restoreHr = m_ilasmPdbWriter->ChangePdbStreamId(m_pdbStream.id.pdbGuid, m_pdbStream.id.pdbTimeStamp); + if (SUCCEEDED(hr)) + hr = restoreHr; + delete[] pdbData; + return hr; } -HRESULT PortablePdbWriter::ChangePdbStreamGuid(REFGUID newGuid) +HRESULT PortablePdbWriter::ChangePdbStreamId(REFGUID newGuid, const ULONG newTimestamp) { m_pdbStream.id.pdbGuid = newGuid; - return m_ilasmPdbWriter->ChangePdbStreamGuid(newGuid); + m_pdbStream.id.pdbTimeStamp = newTimestamp; + return m_ilasmPdbWriter->ChangePdbStreamId(newGuid, newTimestamp); } HRESULT PortablePdbWriter::DefineDocument(char* name, GUID* language) diff --git a/src/coreclr/ilasm/portable_pdb.h b/src/coreclr/ilasm/portable_pdb.h index 5fd69d0fbb6034..00a9c0fc8c0efb 100644 --- a/src/coreclr/ilasm/portable_pdb.h +++ b/src/coreclr/ilasm/portable_pdb.h @@ -50,8 +50,8 @@ class PortablePdbWriter void SetTimestamp(const ULONG newTimestamp); Document* GetCurrentDocument(); HRESULT BuildPdbStream(IMetaDataEmit3* peEmitter, mdMethodDef entryPoint); - HRESULT ComputeSha256PdbStreamChecksum(BYTE (&checksum)[32]); - HRESULT ChangePdbStreamGuid(REFGUID newGuid); + HRESULT ComputeSha256PdbChecksum(BYTE (&checksum)[32]); + HRESULT ChangePdbStreamId(REFGUID newGuid, const ULONG newTimestamp); HRESULT DefineDocument(char* name, GUID* language); HRESULT DefineSequencePoints(Method* method); HRESULT DefineLocalScope(Method* method); diff --git a/src/coreclr/ilasm/writer.cpp b/src/coreclr/ilasm/writer.cpp index a985d2ec196c63..7076922965b8ba 100644 --- a/src/coreclr/ilasm/writer.cpp +++ b/src/coreclr/ilasm/writer.cpp @@ -79,8 +79,8 @@ HRESULT Assembler::InitMetaData() if (m_fDeterministic) { - // When build determinism is enabled, the PDB checksum is computed with these fields set to zero. - // The GUID and timestamp will be updated after. + // In deterministic mode, the random GUID and timestamp set by Init() must not reach the output. + // The deterministic PDB ID is written in CreatePEFile, once the metadata hash is known. m_pPortablePdbWriter->SetGuid(GUID()); m_pPortablePdbWriter->SetTimestamp(0); } @@ -1480,16 +1480,14 @@ HRESULT Assembler::CreatePEFile(_In_ __nullterminated WCHAR *pwzOutputFilename) if (FAILED(hr = m_pPortablePdbWriter->BuildPdbStream(m_pEmitter, entryPoint))) goto exit; BYTE pdbChecksum[32]; - if (FAILED(hr = m_pPortablePdbWriter->ComputeSha256PdbStreamChecksum(pdbChecksum))) goto exit; + if (FAILED(hr = m_pPortablePdbWriter->ComputeSha256PdbChecksum(pdbChecksum))) goto exit; if (m_fDeterministic) { // Now that the PDB checksum has been computed, update the GUID and timestamp _ASSERTE(*(m_pPortablePdbWriter->GetGuid()) == GUID()); - if (FAILED(hr = m_pPortablePdbWriter->ChangePdbStreamGuid(deterministicGuid))) goto exit; - _ASSERTE(m_pPortablePdbWriter->GetTimestamp() == 0); - m_pPortablePdbWriter->SetTimestamp(deterministicTimestamp); + if (FAILED(hr = m_pPortablePdbWriter->ChangePdbStreamId(deterministicGuid, deterministicTimestamp))) goto exit; } if (FAILED(hr=CreateDebugDirectory(pdbChecksum))) goto exit; diff --git a/src/coreclr/md/compiler/emit.cpp b/src/coreclr/md/compiler/emit.cpp index 50ff3c1ad2e9d0..7c518799dc5e43 100644 --- a/src/coreclr/md/compiler/emit.cpp +++ b/src/coreclr/md/compiler/emit.cpp @@ -2116,29 +2116,16 @@ STDMETHODIMP RegMeta::DefineLocalVariable( // S_OK or error. } // RegMeta::DefineLocalVariable //******************************************************************************* -// ComputeSha256PdbStreamChecksum +// ChangePdbStreamId //******************************************************************************* -STDMETHODIMP RegMeta::ComputeSha256PdbStreamChecksum( - HRESULT (*computeSha256)(BYTE* pSrc, DWORD srcSize, BYTE* pDst, DWORD dstSize), - BYTE (&checksum)[32]) +STDMETHODIMP RegMeta::ChangePdbStreamId( + REFGUID newGuid, + ULONG newTimestamp) { #ifdef FEATURE_METADATA_EMIT_IN_DEBUGGER return E_NOTIMPL; #else //!FEATURE_METADATA_EMIT_IN_DEBUGGER - return m_pStgdb->m_pPdbHeap->ComputeSha256Checksum(computeSha256, checksum); -#endif //!FEATURE_METADATA_EMIT_IN_DEBUGGER -} - -//******************************************************************************* -// ChangePdbStreamGuid -//******************************************************************************* -STDMETHODIMP RegMeta::ChangePdbStreamGuid( - REFGUID newGuid) -{ -#ifdef FEATURE_METADATA_EMIT_IN_DEBUGGER - return E_NOTIMPL; -#else //!FEATURE_METADATA_EMIT_IN_DEBUGGER - return m_pStgdb->m_pPdbHeap->SetDataGuid(newGuid); + return m_pStgdb->m_pPdbHeap->SetDataId(newGuid, newTimestamp); #endif //!FEATURE_METADATA_EMIT_IN_DEBUGGER } diff --git a/src/coreclr/md/compiler/regmeta.h b/src/coreclr/md/compiler/regmeta.h index b16a1326b02502..9a5bcbf3208904 100644 --- a/src/coreclr/md/compiler/regmeta.h +++ b/src/coreclr/md/compiler/regmeta.h @@ -1133,12 +1133,9 @@ class RegMeta : //***************************************************************************** // IILAsmPortablePdbWriter methods //***************************************************************************** - STDMETHODIMP ComputeSha256PdbStreamChecksum( // S_OK or error. - HRESULT (*computeSha256)(BYTE* pSrc, DWORD srcSize, BYTE* pDst, DWORD dstSize), // [IN] - BYTE (&checksum)[32]); // [OUT] 256-bit Pdb checksum - - STDMETHODIMP ChangePdbStreamGuid( // S_OK or error. - REFGUID newGuid); // [IN] GUID to use as the PDB GUID + STDMETHODIMP ChangePdbStreamId( // S_OK or error. + REFGUID newGuid, // [IN] GUID to use as the PDB GUID + ULONG newTimestamp); // [IN] Timestamp to use as the PDB stamp #endif // FEATURE_METADATA_EMIT_PORTABLE_PDB //***************************************************************************** diff --git a/src/coreclr/md/enc/pdbheap.cpp b/src/coreclr/md/enc/pdbheap.cpp index d402cddc0a7316..1e406a4d2e5f75 100644 --- a/src/coreclr/md/enc/pdbheap.cpp +++ b/src/coreclr/md/enc/pdbheap.cpp @@ -68,23 +68,25 @@ HRESULT PdbHeap::SetData(PORT_PDB_STREAM* data) __checkReturn -HRESULT PdbHeap::SetDataGuid(REFGUID newGuid) +HRESULT PdbHeap::SetDataId(REFGUID newGuid, ULONG newTimestamp) { _ASSERTE(m_size >= sizeof(PDB_ID)); - if (memcpy_s(m_data, m_size, &newGuid, sizeof(GUID))) + PDB_ID id; + id.pdbGuid = newGuid; + id.pdbTimeStamp = newTimestamp; + +#if BIGENDIAN + SwapGuid(&id.pdbGuid); + id.pdbTimeStamp = VAL32(id.pdbTimeStamp); +#endif + + if (memcpy_s(m_data, m_size, &id, sizeof(id))) return E_FAIL; return S_OK; } -__checkReturn -HRESULT PdbHeap::ComputeSha256Checksum(HRESULT (*computeSha256)(BYTE* pSrc, DWORD srcSize, BYTE* pDst, DWORD dstSize), BYTE (&checksum)[32]) -{ - _ASSERTE(m_size >= sizeof(PDB_ID)); - return computeSha256(m_data, m_size, (BYTE*)&checksum, sizeof(checksum)); -} - __checkReturn HRESULT PdbHeap::SaveToStream(IStream* stream) { diff --git a/src/coreclr/md/inc/pdbheap.h b/src/coreclr/md/inc/pdbheap.h index 88bae893f86d7e..1c6387b2ce52cb 100644 --- a/src/coreclr/md/inc/pdbheap.h +++ b/src/coreclr/md/inc/pdbheap.h @@ -21,8 +21,7 @@ class PdbHeap ~PdbHeap(); __checkReturn HRESULT SetData(PORT_PDB_STREAM* data); - __checkReturn HRESULT SetDataGuid(REFGUID newGuid); - __checkReturn HRESULT ComputeSha256Checksum(HRESULT (*computeSha256)(BYTE* pSrc, DWORD srcSize, BYTE* pDst, DWORD dstSize), BYTE (&checksum)[32]); + __checkReturn HRESULT SetDataId(REFGUID newGuid, ULONG newTimestamp); __checkReturn HRESULT SaveToStream(IStream* stream); BOOL IsEmpty(); ULONG GetSize(); diff --git a/src/coreclr/md/inc/portablepdbmdi.h b/src/coreclr/md/inc/portablepdbmdi.h index fffdfbe95a5ff9..3a598744f0c4d3 100644 --- a/src/coreclr/md/inc/portablepdbmdi.h +++ b/src/coreclr/md/inc/portablepdbmdi.h @@ -100,12 +100,9 @@ EXTERN_GUID(IID_IILAsmPortablePdbWriter, 0x8b2db1f0, 0x91f5, 0x4c99, 0xbb, 0x07, #define INTERFACE IILAsmPortablePdbWriter DECLARE_INTERFACE_(IILAsmPortablePdbWriter, IUnknown) { - STDMETHOD(ComputeSha256PdbStreamChecksum)( // S_OK or error. - HRESULT (*computeSha256)(BYTE* pSrc, DWORD srcSize, BYTE* pDst, DWORD dstSize), // [IN] - BYTE (&checksum)[32]) PURE; // [OUT] 256-bit Pdb checksum - - STDMETHOD(ChangePdbStreamGuid)( // S_OK or error. - REFGUID newGuid) PURE; // [IN] GUID to use as the PDB GUID + STDMETHOD(ChangePdbStreamId)( // S_OK or error. + REFGUID newGuid, // [IN] GUID to use as the PDB GUID + ULONG newTimestamp) PURE; // [IN] Timestamp to use as the PDB stamp }; #ifdef __cplusplus diff --git a/src/tests/ilasm/PortablePdb/IlasmPortablePdbTester.cs b/src/tests/ilasm/PortablePdb/IlasmPortablePdbTester.cs index e4089f9812e377..85bcaf29feac93 100644 --- a/src/tests/ilasm/PortablePdb/IlasmPortablePdbTester.cs +++ b/src/tests/ilasm/PortablePdb/IlasmPortablePdbTester.cs @@ -5,7 +5,9 @@ using System.Linq; using Xunit; using System.Collections.Generic; +using System.Collections.Immutable; using System.Reflection.Metadata; +using System.Security.Cryptography; namespace IlasmPortablePdbTests { @@ -27,15 +29,17 @@ public IlasmPortablePdbTester() IlasmFile = IlasmFileName + NativeExtension; } - // Tests whether pe file includes portable pdb codeview debug directory - // and its contents against the generated portable pdb metadata file + // Tests whether pe file includes portable pdb codeview and pdb checksum debug directory entries + // and their contents against the generated portable pdb file, with and without deterministic output [Theory] - [InlineData("TestPdbDebugDirectory1.il")] - [InlineData("TestPdbDebugDirectory2.il")] - public void TestPortablePdbDebugDirectory(string ilSource) + [InlineData("TestPdbDebugDirectory1.il", false)] + [InlineData("TestPdbDebugDirectory1.il", true)] + [InlineData("TestPdbDebugDirectory2.il", false)] + [InlineData("TestPdbDebugDirectory2.il", true)] + public void TestPortablePdbDebugDirectory(string ilSource, bool deterministic) { var ilasm = IlasmPortablePdbTesterCommon.GetIlasmFullPath(CoreRootVar, IlasmFile); - IlasmPortablePdbTesterCommon.Assemble(ilasm, ilSource, TestDir, out string dll, out string pdb); + IlasmPortablePdbTesterCommon.Assemble(ilasm, ilSource, TestDir, out string dll, out string pdb, deterministic); using (var peStream = new FileStream(dll, FileMode.Open, FileAccess.Read)) { @@ -51,22 +55,32 @@ public void TestPortablePdbDebugDirectory(string ilSource) Assert.Equal(1, portablePdbDbgEntry.Age); Assert.Equal(pdb, portablePdbDbgEntry.Path); + var pdbChecksumEntry = Assert.Single(dbgDirEntries, entry => entry.Type == DebugDirectoryEntryType.PdbChecksum); + var pdbChecksum = peReader.ReadPdbChecksumDebugDirectoryData(pdbChecksumEntry); + Assert.Equal("SHA256", pdbChecksum.AlgorithmName); + + var pdbImage = File.ReadAllBytes(pdb); + using (var pdbImageReaderProvider = MetadataReaderProvider.FromPortablePdbImage(ImmutableArray.Create(pdbImage))) + { + var pdbHeader = pdbImageReaderProvider.GetMetadataReader().DebugMetadataHeader; + Assert.NotNull(pdbHeader); + + // check pdb id (guid and stamp) against the codeview entry + var pdbId = new BlobContentId(pdbHeader.Id); + Assert.Equal(portablePdbDbgEntry.Guid, pdbId.Guid); + Assert.Equal(dbgEntry.Stamp, pdbId.Stamp); + + // check pdb checksum: the hash of the entire pdb file with its 20-byte pdb id zeroed + Array.Clear(pdbImage, pdbHeader.IdStartOffset, pdbHeader.Id.Length); + Assert.Equal(SHA256.HashData(pdbImage), pdbChecksum.Checksum.ToArray()); + } + using (var pdbReaderProvider = IlasmPortablePdbTesterCommon.GetMetadataReaderProvider(dll, pdb, peReader, false)) { var portablePdbMdReader = pdbReaderProvider.GetMetadataReader(); Assert.NotNull(portablePdbMdReader); // check pdb stream Assert.NotNull(portablePdbMdReader.DebugMetadataHeader); - // check pdb guid - var pdbGuid = portablePdbDbgEntry.Guid.ToByteArray(); - var pdbId = portablePdbMdReader.DebugMetadataHeader.Id.ToArray(); - int i = 0; - foreach (var pdbGuidByte in pdbGuid) - { - Assert.True(i < pdbId.Length); - Assert.Equal(pdbGuidByte, pdbId[i++]); - } - Assert.Equal(i, pdbGuid.Length); var peMdReader = peReader.GetMetadataReader(); Assert.NotNull(peMdReader); diff --git a/src/tests/ilasm/PortablePdb/IlasmPortablePdbTesterCommon.cs b/src/tests/ilasm/PortablePdb/IlasmPortablePdbTesterCommon.cs index f31fb44f5f06b6..0b6cb4735bec67 100644 --- a/src/tests/ilasm/PortablePdb/IlasmPortablePdbTesterCommon.cs +++ b/src/tests/ilasm/PortablePdb/IlasmPortablePdbTesterCommon.cs @@ -21,7 +21,7 @@ public static string GetIlasmFullPath(string coreRootVar, string ilasmFile) return ilasmFullPath; } - public static void Assemble(string ilasmFullPath, string ilSrc, string testDir, out string dll, out string pdb) + public static void Assemble(string ilasmFullPath, string ilSrc, string testDir, out string dll, out string pdb, bool deterministic = false) { var currentDirectory = Environment.CurrentDirectory; var ilSrcFullPath = Path.Combine(currentDirectory, testDir, ilSrc); @@ -33,7 +33,7 @@ public static void Assemble(string ilasmFullPath, string ilSrc, string testDir, var dllFullPath = Path.Combine(currentDirectory, testDir, dllFileName); var pdbFullPath = Path.Combine(currentDirectory, testDir, pdbFileName); - var ilasmArgs = $"{CommonIlasmArguments} -output={dllFullPath} {ilSrcFullPath}"; + var ilasmArgs = $"{CommonIlasmArguments}{(deterministic ? " -det" : string.Empty)} -output={dllFullPath} {ilSrcFullPath}"; var ilasmPsi = new ProcessStartInfo { UseShellExecute = false,