Fixed a bug reading version string from PDB. Implemented IDxcVersionInfo3 for DxcCompiler. (#3570)

Author: adam-yang

include/dxc/dxcapi.h

@@ -576,7 +576,7 @@ struct IDxcVersionInfo2 : public IDxcVersionInfo {
 };
 
 CROSS_PLATFORM_UUIDOF(IDxcVersionInfo3, "5e13e843-9d25-473c-9ad2-03b2d0b44b1e")
-struct IDxcVersionInfo3 : public IDxcVersionInfo2 {
+struct IDxcVersionInfo3 : public IUnknown {
   virtual HRESULT STDMETHODCALLTYPE GetCustomVersionString(
     _Outptr_result_z_ char **pVersionString // Custom version string for compiler. (Must be CoTaskMemFree()'d!)
   ) = 0;

tools/clang/tools/dxcompiler/dxcompilerobj.cpp

@@ -51,6 +51,7 @@
 #include "dxillib.h"
 #include "dxcshadersourceinfo.h"
 #include "dxcompileradapter.h"
+#include "dxcversion.inc"
 #include <algorithm>
 #include <cfloat>
 
@@ -97,6 +98,8 @@ struct CompilerVersionPartWriter {
   hlsl::DxilCompilerVersion m_Header = {};
   CComHeapPtr<char> m_CommitShaStorage;
   llvm::StringRef m_CommitSha = "";
+  CComHeapPtr<char> m_CustomStringStorage;
+  llvm::StringRef m_CustomString = "";
 
   void Init(IDxcVersionInfo *pVersionInfo) {
     m_Header = {};
@@ -115,7 +118,14 @@ struct CompilerVersionPartWriter {
       IFT(pVersionInfo2->GetCommitInfo(&CommitCount, &m_CommitShaStorage));
       m_CommitSha = llvm::StringRef(m_CommitShaStorage.m_pData, strlen(m_CommitShaStorage.m_pData));
       m_Header.CommitCount = CommitCount;
-      m_Header.VersionStringListSizeInBytes = m_CommitSha.size() + /*null term*/1 + /*another null term for the empty custom string*/1;
+      m_Header.VersionStringListSizeInBytes += m_CommitSha.size() + /*null term*/1;
+    }
+
+    CComPtr<IDxcVersionInfo3> pVersionInfo3;
+    if (SUCCEEDED(pVersionInfo->QueryInterface(&pVersionInfo3))) {
+      IFT(pVersionInfo3->GetCustomVersionString(&m_CustomStringStorage));
+      m_CustomString = llvm::StringRef(m_CustomStringStorage, strlen(m_CustomStringStorage.m_pData));
+      m_Header.VersionStringListSizeInBytes += m_CustomString.size() + /*null term*/1;
     }
   }
 
@@ -150,7 +160,9 @@ struct CompilerVersionPartWriter {
     // Null terminator for the commit sha
     IFT(pStream->Write(&padByte, sizeof(padByte), &cbWritten));
 
-    // Null terminator for the empty version string
+    // Write the custom version string.
+    IFT(pStream->Write(m_CustomString.data(), m_CustomString.size(), &cbWritten));
+    // Null terminator for the custom version string.
     IFT(pStream->Write(&padByte, sizeof(padByte), &cbWritten));
 
     // Write padding
@@ -537,10 +549,9 @@ static void CreateDefineStrings(
 class DxcCompiler : public IDxcCompiler3,
                     public IDxcLangExtensions2,
                     public IDxcContainerEvent,
+                    public IDxcVersionInfo3,
 #ifdef SUPPORT_QUERY_GIT_COMMIT_INFO
                     public IDxcVersionInfo2
-#else
-                    public IDxcVersionInfo
 #endif // SUPPORT_QUERY_GIT_COMMIT_INFO
 {
 private:
@@ -577,6 +588,7 @@ class DxcCompiler : public IDxcCompiler3,
 #ifdef SUPPORT_QUERY_GIT_COMMIT_INFO
       ,IDxcVersionInfo2
 #endif // SUPPORT_QUERY_GIT_COMMIT_INFO
+      ,IDxcVersionInfo3
      >
      (this, iid, ppvObject);
     if (FAILED(hr)) {
@@ -1393,6 +1405,19 @@ class DxcCompiler : public IDxcCompiler3,
     *pMinor = DXIL::kDxilMinor;
     return S_OK;
   }
+  HRESULT STDMETHODCALLTYPE GetCustomVersionString(
+    _Outptr_result_z_ char **pVersionString // Custom version string for compiler. (Must be CoTaskMemFree()'d!)
+  ) override
+  {
+    size_t size = strlen(RC_FILE_VERSION);
+    char *const result = (char *)CoTaskMemAlloc(size + 1);
+    if (result == nullptr)
+      return E_OUTOFMEMORY;
+    std::strcpy(result, RC_FILE_VERSION);
+    *pVersionString = result;
+    return S_OK;
+  }
+
 #ifdef SUPPORT_QUERY_GIT_COMMIT_INFO
   HRESULT STDMETHODCALLTYPE GetCommitInfo(_Out_ UINT32 *pCommitCount,
                                           _Out_ char **pCommitHash) override {
@@ -1410,6 +1435,7 @@ class DxcCompiler : public IDxcCompiler3,
     return S_OK;
   }
 #endif // SUPPORT_QUERY_GIT_COMMIT_INFO
+
   HRESULT STDMETHODCALLTYPE GetFlags(_Out_ UINT32 *pFlags) override {
     if (pFlags == nullptr)
       return E_INVALIDARG;

tools/clang/tools/dxcompiler/dxcpdbutils.cpp

@@ -144,7 +144,10 @@ static void ComputeFlagsBasedOnArgs(ArrayRef<std::wstring> args, std::vector<std
   }
 }
 
-struct DxcPdbVersionInfo : public IDxcVersionInfo3 {
+struct DxcPdbVersionInfo :
+  public IDxcVersionInfo2,
+  public IDxcVersionInfo3
+{
 private:
   DXC_MICROCOM_TM_REF_FIELDS()
 
@@ -453,31 +456,34 @@ struct DxcPdbUtils : public IDxcPdbUtils, public IDxcPixDxilDebugInfoFactory
         m_HasVersionInfo = true;
 
         const char *ptr = (const char *)(header+1);
-        unsigned commitShaLength = 0;
         unsigned i = 0;
 
-        const char *commitSha = (const char *)(header+1) + i;
-        for (; i < header->VersionStringListSizeInBytes; i++) {
-          if (ptr[i] == 0) {
-            commitShaLength = i;
-            i++;
-            break;
+        {
+          unsigned commitShaLength = 0;
+          const char *commitSha = (const char *)(header+1) + i;
+          for (; i < header->VersionStringListSizeInBytes; i++) {
+            if (ptr[i] == 0) {
+              i++;
+              break;
+            }
+            commitShaLength++;
           }
+          m_VersionCommitSha.assign(commitSha, commitShaLength);
         }
 
-        const char *versionString = (const char *)(header+1) + i;
-        unsigned versionStringLength = 0;
-        for (; i < header->VersionStringListSizeInBytes; i++) {
-          if (ptr[i] == 0) {
-            commitShaLength = i;
-            i++;
-            break;
+        {
+          const char *versionString = (const char *)(header+1) + i;
+          unsigned versionStringLength = 0;
+          for (; i < header->VersionStringListSizeInBytes; i++) {
+            if (ptr[i] == 0) {
+              i++;
+              break;
+            }
+            versionStringLength++;
           }
+          m_VersionString.assign(versionString, versionStringLength);
         }
 
-        m_VersionCommitSha.assign(commitSha, commitShaLength);
-        m_VersionString.assign(versionString, versionStringLength);
-
       } break;
 
       case hlsl::DFCC_ShaderSourceInfo:

tools/clang/unittests/HLSL/CompilerTest.cpp

@@ -1052,9 +1052,11 @@ static void VerifyPdbUtil(dxc::DxcDllSupport &dllSupport,
       // IDxcVersionInfo3
       CComHeapPtr<char> VersionString;
       VERIFY_SUCCEEDED(pVersion3->GetCustomVersionString(&VersionString));
+      VERIFY_IS_TRUE(VersionString && strlen(VersionString) != 0);
 
-      CComPtr<IDxcVersionInfo3> pCompilerVersion3;
-      if (SUCCEEDED(pCompiler->QueryInterface(&pCompilerVersion3))) {
+      {
+        CComPtr<IDxcVersionInfo3> pCompilerVersion3;
+        VERIFY_SUCCEEDED(pCompiler->QueryInterface(&pCompilerVersion3));
         CComHeapPtr<char> CompilerVersionString;
         VERIFY_SUCCEEDED(pCompilerVersion3->GetCustomVersionString(&CompilerVersionString));
         VERIFY_IS_TRUE(0 == strcmp(CompilerVersionString, VersionString));