Fix ilasm PdbChecksum and the PDB ID under -DET

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)

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);

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;

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
 }
 

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
 
 //*****************************************************************************

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)
 {

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();

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

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);
 

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,