Fix PDB generation to not require embedded hash inside DxilContainer (#2272)

This would break when official DXIL.dll was used, since it would exclude
the shader hash part that the validator wouldn't recognize, and is not
required for a valid shader.  This prevented shader signing for the
output of newer compiler versions.

include/dxc/DXIL/DxilPDB.h

@@ -10,6 +10,7 @@
 ///////////////////////////////////////////////////////////////////////////////
 
 #include "dxc/Support/WinIncludes.h"
+#include "llvm/ADT/ArrayRef.h"
 
 struct IDxcBlob;
 struct IStream;
@@ -19,6 +20,6 @@ namespace hlsl {
 namespace pdb {
 
   HRESULT LoadDataFromStream(IMalloc *pMalloc, IStream *pIStream, IDxcBlob **pOutContainer);
-  HRESULT WriteDxilPDB(IMalloc *pMalloc, IDxcBlob *pContainer, IDxcBlob **ppOutBlob); 
+  HRESULT WriteDxilPDB(IMalloc *pMalloc, IDxcBlob *pContainer, llvm::ArrayRef<BYTE> HashData, IDxcBlob **ppOutBlob);
 }
 }

include/dxc/DxilContainer/DxilContainerAssembler.h

@@ -50,7 +50,8 @@ void SerializeDxilContainerForModule(hlsl::DxilModule *pModule,
                                      AbstractMemoryStream *pModuleBitcode,
                                      AbstractMemoryStream *pStream,
                                      llvm::StringRef DebugName,
-                                     SerializeDxilFlags Flags);
+                                     SerializeDxilFlags Flags,
+                                     DxilShaderHash *pShaderHashOut = nullptr);
 void SerializeDxilContainerForRootSignature(hlsl::RootSignatureHandle *pRootSigHandle,
                                      AbstractMemoryStream *pStream);
 

lib/DXIL/DxilPDB.cpp

@@ -266,6 +266,7 @@ SmallVector<char, 0> WritePdbStream(ArrayRef<BYTE> Hash) {
   Header.Version = (uint32_t)PdbStreamVersion::VC70;
   Header.Age = 1;
   Header.Signature = 0;
+  DXASSERT_NOMSG(Hash.size() == sizeof(Header.UniqueId));
   memcpy(Header.UniqueId, Hash.data(), std::min(Hash.size(), sizeof(Header.UniqueId)));
 
   SmallVector<char, 0> Result;
@@ -291,33 +292,11 @@ SmallVector<char, 0> WritePdbStream(ArrayRef<BYTE> Hash) {
   return Result;
 }
 
-static HRESULT FindShaderHash(IDxcBlob *pContainer, BYTE *pDigest, UINT32 uCapacity, UINT32 *pBytesWritten) {
+HRESULT hlsl::pdb::WriteDxilPDB(IMalloc *pMalloc, IDxcBlob *pContainer, ArrayRef<BYTE> HashData, IDxcBlob **ppOutBlob) {
   if (!hlsl::IsValidDxilContainer((hlsl::DxilContainerHeader *)pContainer->GetBufferPointer(), pContainer->GetBufferSize()))
     return E_FAIL;
 
-  hlsl::DxilContainerHeader *DxilHeader = (hlsl::DxilContainerHeader *)pContainer->GetBufferPointer();
-  for (unsigned i = 0; i < DxilHeader->PartCount; i++) {
-    hlsl::DxilPartHeader *PartHeader = GetDxilContainerPart(DxilHeader, i);
-    if (PartHeader->PartFourCC == hlsl::DFCC_ShaderHash) {
-      hlsl::DxilShaderHash *HashHeader = (hlsl::DxilShaderHash *)(PartHeader+1);
-      if (sizeof(HashHeader->Digest) > uCapacity)
-        return E_FAIL;
-      memcpy(pDigest, HashHeader->Digest, sizeof(HashHeader->Digest));
-      *pBytesWritten = sizeof(HashHeader->Digest);
-      return S_OK;
-    }
-  }
-  return E_FAIL;
-}
-
-HRESULT hlsl::pdb::WriteDxilPDB(IMalloc *pMalloc, IDxcBlob *pContainer, IDxcBlob **ppOutBlob) {
-  if (!hlsl::IsValidDxilContainer((hlsl::DxilContainerHeader *)pContainer->GetBufferPointer(), pContainer->GetBufferSize()))
-    return E_FAIL;
-
-  BYTE HashData[16];
-  UINT32 uHashSize = 0;
-  IFR(FindShaderHash(pContainer, HashData, sizeof(HashData), &uHashSize));
-  SmallVector<char, 0> PdbStream = WritePdbStream({ HashData, HashData+uHashSize });
+  SmallVector<char, 0> PdbStream = WritePdbStream(HashData);
 
   MSFWriter Writer;
   Writer.AddEmptyStream();     // Old Directory

lib/DxilContainer/DxilContainerAssembler.cpp

@@ -1464,7 +1464,8 @@ void hlsl::SerializeDxilContainerForModule(DxilModule *pModule,
                                            AbstractMemoryStream *pModuleBitcode,
                                            AbstractMemoryStream *pFinalStream,
                                            llvm::StringRef DebugName,
-                                           SerializeDxilFlags Flags) {
+                                           SerializeDxilFlags Flags,
+                                           DxilShaderHash *pShaderHashOut) {
   // TODO: add a flag to update the module and remove information that is not part
   // of DXIL proper and is used only to assemble the container.
 
@@ -1606,7 +1607,7 @@ void hlsl::SerializeDxilContainerForModule(DxilModule *pModule,
   CComPtr<AbstractMemoryStream> pHashStream;
   std::string DebugNameStr; // Used if constructing name based on hash
   DxilShaderHash HashContent;
-  if (Flags & SerializeDxilFlags::IncludeDebugNamePart) {
+  if (pShaderHashOut || Flags & SerializeDxilFlags::IncludeDebugNamePart) {
     // If the debug name should be specific to the sources, base the name on the debug
     // bitcode, which will include the source references, line numbers, etc. Otherwise,
     // do it exclusively on the target shader bitcode.
@@ -1625,33 +1626,35 @@ void hlsl::SerializeDxilContainerForModule(DxilModule *pModule,
     md5.update(Data);
     md5.final(HashContent.Digest);
 
-    if (DebugName.empty()) {
-      md5.stringifyResult(HashContent.Digest, Hash);
-      DebugNameStr += Hash;
-      DebugNameStr += ".pdb";
-      DebugName = DebugNameStr;
-    }
-
-    // Calculate the size of the blob part.
-    const uint32_t DebugInfoContentLen = PSVALIGN4(
-        sizeof(DxilShaderDebugName) + DebugName.size() + 1); // 1 for null
+    if (Flags & SerializeDxilFlags::IncludeDebugNamePart) {
+      if (DebugName.empty()) {
+        md5.stringifyResult(HashContent.Digest, Hash);
+        DebugNameStr += Hash;
+        DebugNameStr += ".pdb";
+        DebugName = DebugNameStr;
+      }
 
-    writer.AddPart(DFCC_ShaderDebugName, DebugInfoContentLen,
-      [DebugName]
-      (AbstractMemoryStream *pStream)
-    {
-      DxilShaderDebugName NameContent;
-      NameContent.Flags = 0;
-      NameContent.NameLength = DebugName.size();
-      IFT(WriteStreamValue(pStream, NameContent));
+      // Calculate the size of the blob part.
+      const uint32_t DebugInfoContentLen = PSVALIGN4(
+          sizeof(DxilShaderDebugName) + DebugName.size() + 1); // 1 for null
 
-      ULONG cbWritten;
-      IFT(pStream->Write(DebugName.begin(), DebugName.size(), &cbWritten));
-      const char Pad[] = { '\0','\0','\0','\0' };
-      // Always writes at least one null to align size
-      unsigned padLen = (4 - ((sizeof(DxilShaderDebugName) + cbWritten) & 0x3));
-      IFT(pStream->Write(Pad, padLen, &cbWritten));
-    });
+      writer.AddPart(DFCC_ShaderDebugName, DebugInfoContentLen,
+        [DebugName]
+        (AbstractMemoryStream *pStream)
+      {
+        DxilShaderDebugName NameContent;
+        NameContent.Flags = 0;
+        NameContent.NameLength = DebugName.size();
+        IFT(WriteStreamValue(pStream, NameContent));
+
+        ULONG cbWritten;
+        IFT(pStream->Write(DebugName.begin(), DebugName.size(), &cbWritten));
+        const char Pad[] = { '\0','\0','\0','\0' };
+        // Always writes at least one null to align size
+        unsigned padLen = (4 - ((sizeof(DxilShaderDebugName) + cbWritten) & 0x3));
+        IFT(pStream->Write(Pad, padLen, &cbWritten));
+      });
+    }
 
     if (bSupportsShaderHash) {
       writer.AddPart(DFCC_ShaderHash, sizeof(HashContent),
@@ -1661,6 +1664,10 @@ void hlsl::SerializeDxilContainerForModule(DxilModule *pModule,
         IFT(WriteStreamValue(pStream, HashContent));
       });
     }
+
+    if (pShaderHashOut) {
+      memcpy(pShaderHashOut, &HashContent, sizeof(DxilShaderHash));
+    }
   }
 
   // Compute padded bitcode size.

tools/clang/tools/dxcompiler/dxcompilerobj.cpp

@@ -481,6 +481,7 @@ public:
     CComPtr<IDxcBlobEncoding> utf8Source;
     CComPtr<AbstractMemoryStream> pOutputStream;
     CComHeapPtr<wchar_t> DebugBlobName;
+    DxilShaderHash ShaderHashContent;
 
     DxcEtw_DXCompilerCompile_Start();
     pSourceName = (pSourceName && *pSourceName) ? pSourceName : L"hlsl.hlsl"; // declared optional, so pick a default
@@ -743,11 +744,13 @@ public:
           if (needsValidation) {
             valHR = dxcutil::ValidateAndAssembleToContainer(
                 action.takeModule(), pOutputBlob, m_pMalloc, SerializeFlags,
-                pOutputStream, opts.IsDebugInfoEnabled(), opts.GetPDBName(), compiler.getDiagnostics());
+                pOutputStream, opts.IsDebugInfoEnabled(), opts.GetPDBName(), compiler.getDiagnostics(),
+                (SerializeFlags & SerializeDxilFlags::IncludeDebugNamePart) ? &ShaderHashContent : nullptr);
           } else {
             dxcutil::AssembleToContainer(action.takeModule(),
-                                                 pOutputBlob, m_pMalloc,
-                                                 SerializeFlags, pOutputStream);
+                                         pOutputBlob, m_pMalloc,
+                                         SerializeFlags, pOutputStream,
+                (SerializeFlags & SerializeDxilFlags::IncludeDebugNamePart) ? &ShaderHashContent : nullptr);
           }
 
           // Callback after valid DXIL is produced
@@ -791,7 +794,7 @@ public:
           CComPtr<IDxcBlob> pDebugBitcodeBlob;
           DXVERIFY_NOMSG(SUCCEEDED(pOutputStream.QueryInterface(&pDebugBitcodeBlob)));
           DXVERIFY_NOMSG(SUCCEEDED(CreateContainerForPDB(m_pMalloc, pOutputBlob, pDebugBitcodeBlob, &pStrippedContainer)));
-          DXVERIFY_NOMSG(SUCCEEDED((hlsl::pdb::WriteDxilPDB(m_pMalloc, pStrippedContainer, ppDebugBlob)))); 
+          DXVERIFY_NOMSG(SUCCEEDED((hlsl::pdb::WriteDxilPDB(m_pMalloc, pStrippedContainer, ShaderHashContent.Digest, ppDebugBlob))));
         }
         if (ppDebugBlobName) {
           *ppDebugBlobName = DebugBlobName.Detach();

tools/clang/tools/dxcompiler/dxcutil.cpp

@@ -74,11 +74,13 @@ public:
   void WrapModuleInDxilContainer(IMalloc *pMalloc,
                                  AbstractMemoryStream *pModuleBitcode,
                                  CComPtr<IDxcBlob> &pDxilContainerBlob,
-                                 SerializeDxilFlags Flags) {
+                                 SerializeDxilFlags Flags,
+                                 DxilShaderHash *pShaderHashOut) {
     CComPtr<AbstractMemoryStream> pContainerStream;
     IFT(CreateMemoryStream(pMalloc, &pContainerStream));
     SerializeDxilContainerForModule(&m_llvmModule->GetOrCreateDxilModule(),
-                                    pModuleBitcode, pContainerStream, m_debugName, Flags);
+                                    pModuleBitcode, pContainerStream, m_debugName, Flags,
+                                    pShaderHashOut);
 
     pDxilContainerBlob.Release();
     IFT(pContainerStream.QueryInterface(&pDxilContainerBlob));
@@ -120,12 +122,13 @@ void AssembleToContainer(std::unique_ptr<llvm::Module> pM,
                          CComPtr<IDxcBlob> &pOutputBlob,
                          IMalloc *pMalloc,
                          SerializeDxilFlags SerializeFlags,
-                         CComPtr<AbstractMemoryStream> &pOutputStream) {
+                         CComPtr<AbstractMemoryStream> &pOutputStream,
+                         DxilShaderHash *pShaderHashOut) {
   // Take ownership of the module from the action.
   DxilCompilerLLVMModuleOutput llvmModule(std::move(pM));
 
   llvmModule.WrapModuleInDxilContainer(pMalloc, pOutputStream, pOutputBlob,
-                                       SerializeFlags);
+                                       SerializeFlags, pShaderHashOut);
 }
 
 void ReadOptsAndValidate(hlsl::options::MainArgs &mainArgs,
@@ -157,7 +160,7 @@ HRESULT ValidateAndAssembleToContainer(
     std::unique_ptr<llvm::Module> pM, CComPtr<IDxcBlob> &pOutputBlob,
     IMalloc *pMalloc, SerializeDxilFlags SerializeFlags,
     CComPtr<AbstractMemoryStream> &pOutputStream, bool bDebugInfo, llvm::StringRef DebugName,
-    clang::DiagnosticsEngine &Diag) {
+    clang::DiagnosticsEngine &Diag, DxilShaderHash *pShaderHashOut) {
   HRESULT valHR = S_OK;
 
   // Take ownership of the module from the action.
@@ -187,7 +190,7 @@ HRESULT ValidateAndAssembleToContainer(
   }
 
   llvmModule.WrapModuleInDxilContainer(pMalloc, pOutputStream, pOutputBlob,
-                                       SerializeFlags);
+                                       SerializeFlags, pShaderHashOut);
 
   CComPtr<IDxcOperationResult> pValResult;
   // Important: in-place edit is required so the blob is reused and thus

tools/clang/tools/dxcompiler/dxcutil.h

@@ -30,6 +30,7 @@ class Twine;
 
 namespace hlsl {
 enum class SerializeDxilFlags : uint32_t;
+struct DxilShaderHash;
 class AbstractMemoryStream;
 namespace options {
 class MainArgs;
@@ -42,13 +43,14 @@ HRESULT ValidateAndAssembleToContainer(
     std::unique_ptr<llvm::Module> pM, CComPtr<IDxcBlob> &pOutputContainerBlob,
     IMalloc *pMalloc, hlsl::SerializeDxilFlags SerializeFlags,
     CComPtr<hlsl::AbstractMemoryStream> &pModuleBitcode, bool bDebugInfo, llvm::StringRef DebugName,
-    clang::DiagnosticsEngine &Diag);
+    clang::DiagnosticsEngine &Diag, hlsl::DxilShaderHash *pShaderHashOut = nullptr);
 void GetValidatorVersion(unsigned *pMajor, unsigned *pMinor);
 void AssembleToContainer(std::unique_ptr<llvm::Module> pM,
                          CComPtr<IDxcBlob> &pOutputContainerBlob,
                          IMalloc *pMalloc,
                          hlsl::SerializeDxilFlags SerializeFlags,
-                         CComPtr<hlsl::AbstractMemoryStream> &pModuleBitcode);
+                         CComPtr<hlsl::AbstractMemoryStream> &pModuleBitcode,
+                         hlsl::DxilShaderHash *pShaderHashOut = nullptr);
 HRESULT Disassemble(IDxcBlob *pProgram, llvm::raw_string_ostream &Stream);
 void ReadOptsAndValidate(hlsl::options::MainArgs &mainArgs,
                          hlsl::options::DxcOpts &opts,