address Tex

Author: bob80905

include/dxc/Support/dxcapi.extval.h

@@ -14,16 +14,16 @@ class DxcDllExtValidationLoader : public DllLoader {
   dxc::SpecificDllLoader DxCompilerSupport;
   dxc::SpecificDllLoader DxilExtValSupport;
   std::string DxilDllPath;
-  enum {
+
+public:
+  std::string getDxilDllPath() { return DxilDllPath; }
+  enum InitializationFailures {
     FailedNone = 0,
     FailedCompilerLoad,
     FailedDxilPath,
     FailedDxilLoad,
   } FailureReason = FailedNone;
-
-public:
-  std::string getDxilDllPath() { return DxilDllPath; }
-  int getFailureReason() { return FailureReason; }
+  InitializationFailures getFailureReason() { return FailureReason; }
   bool dxilDllFailedToLoad() {
     return !DxilDllPath.empty() && !DxilExtValSupport.IsEnabled();
   }
@@ -34,6 +34,7 @@ class DxcDllExtValidationLoader : public DllLoader {
                               IUnknown **pResult) override;
 
   HRESULT initialize();
+  HRESULT InitializeForDll(LPCSTR dllName, LPCSTR fnName);
 
   bool IsEnabled() const override { return DxCompilerSupport.IsEnabled(); }
 };

lib/DxcSupport/dxcapi.extval.cpp

@@ -406,7 +406,7 @@ HRESULT DxcDllExtValidationLoader::CreateInstance2Impl(
 }
 
 HRESULT
-DxcDllExtValidationLoader::initialize() {
+DxcDllExtValidationLoader::InitializeForDll(LPCSTR dllName, LPCSTR fnName) {
   // Load dxcompiler.dll
   HRESULT Result =
       DxCompilerSupport.InitializeForDll(kDxCompilerLib, "DxcCreateInstance");
@@ -442,4 +442,8 @@ DxcDllExtValidationLoader::initialize() {
   return Result;
 }
 
+HRESULT DxcDllExtValidationLoader::initialize() {
+  return InitializeForDll(kDxCompilerLib, "DxcCreateInstance");
+}
+
 } // namespace dxc

tools/clang/tools/dxclib/dxc.cpp

@@ -1459,16 +1459,16 @@ int dxc::main(int argc, const char **argv_) {
       HRESULT dllResult = dxcSupport.initialize();
       if (DXC_FAILED(dllResult)) {
         switch (dxcSupport.getFailureReason()) {
-        case 1: {
+        case dxcSupport.FailedCompilerLoad: {
           fprintf(stderr, "dxcompiler.dll failed to load\n");
           break;
         }
-        case 2: {
+        case dxcSupport.FailedDxilPath: {
           fprintf(stderr, "dxil.dll path %s could not be found",
                   dxcSupport.getDxilDllPath().c_str());
           break;
         }
-        case 3: {
+        case dxcSupport.FailedDxilLoad: {
           fprintf(stderr, "%s failed to load",
                   dxcSupport.getDxilDllPath().c_str());
           break;

tools/clang/tools/dxv/dxv.cpp

@@ -163,8 +163,29 @@ int main(int argc, const char **argv) {
     }
 
     DxcDllExtValidationLoader dxcSupport;
-    std::string dllLogString;
-    IFT(dxcSupport.initialize());
+    HRESULT dllResult = dxcSupport.initialize();
+    if (DXC_FAILED(dllResult)) {
+      switch (dxcSupport.getFailureReason()) {
+      case dxcSupport.InitializationFailures::FailedCompilerLoad: {
+        fprintf(stderr, "dxcompiler.dll failed to load\n");
+        break;
+      }
+      case dxcSupport.InitializationFailures::FailedDxilPath: {
+        fprintf(stderr, "dxil.dll path %s could not be found",
+                dxcSupport.getDxilDllPath().c_str());
+        break;
+      }
+      case dxcSupport.InitializationFailures::FailedDxilLoad: {
+        fprintf(stderr, "%s failed to load",
+                dxcSupport.getDxilDllPath().c_str());
+        break;
+      }
+      default: {
+        llvm_unreachable("unexpected failure reason");
+      }
+      }
+      return dllResult;
+    }
 
     DxvContext context(dxcSupport);
     pStage = "Validation";

tools/clang/unittests/HLSL/ValidationTest.cpp

@@ -12,6 +12,7 @@
 
 #include "dxc/Support/WinIncludes.h"
 #include <algorithm>
+#include <filesystem>
 #include <memory>
 #include <string>
 #include <vector>
@@ -4221,13 +4222,17 @@ void SetEnvVarW(const std::wstring &VarName, const std::wstring &VarValue) {
   _wputenv_s(VarName.c_str(), VarValue.c_str());
 }
 
-// For now, 3 things are tested:
+// For now, 4 things are tested:
 // 1. The environment variable is not set. GetDxilDllPath() is empty and
 // DxilDllFailedToLoad() returns false
 // 2. Given a bogus path in the environment variable, and an initialized
 // DxcDllExtValidationSupport object, GetDxilDllPath()
 // retrieves the bogus path despite DxcDllExtValidationSupport failing to load
 // it as a dll, and DxilDllFailedToLoad() returns true.
+// 3. Given a valid path in the environment variable to a non-dll file,
+// and an initialized DxcDllExtValidationSupport object, GetDxilDllPath()
+// retrieves the valid path despite DxcDllExtValidationSupport failing to load
+// it as a dll, and DxilDllFailedToLoad() returns true.
 // 3. CLSID_DxcCompiler, CLSID_DxcLinker, CLSID_DxcValidator
 // may be created through DxcDllExtValidationSupport.
 // This is all to simply test that the new class, DxcDllExtValidationSupport,
@@ -4236,6 +4241,7 @@ void SetEnvVarW(const std::wstring &VarName, const std::wstring &VarValue) {
 TEST_F(ValidationTest, UnitTestExtValidationSupport) {
   dxc::DxcDllExtValidationLoader ExtSupportEmpty;
   dxc::DxcDllExtValidationLoader ExtSupportBogus;
+  dxc::DxcDllExtValidationLoader ExtSupportValidNonDLLPath;
 
   // capture any existing value in the environment variable,
   // so that it can be restored after the test
@@ -4250,23 +4256,41 @@ TEST_F(ValidationTest, UnitTestExtValidationSupport) {
   VERIFY_SUCCEEDED(ExtSupportEmpty.initialize());
 
   VERIFY_IS_FALSE(ExtSupportEmpty.dxilDllFailedToLoad());
-  std::string EmptyPath = ExtSupportBogus.getDxilDllPath();
+  std::string EmptyPath = ExtSupportEmpty.getDxilDllPath();
   VERIFY_ARE_EQUAL_STR(EmptyPath.c_str(), "");
 
   // 2. Test with a bogus path in the environment variable
   SetEnvVarW(L"DXC_DXIL_DLL_PATH", L"bogus");
 
   if (!ExtSupportBogus.IsEnabled()) {
     VERIFY_FAILED(ExtSupportBogus.initialize());
+    VERIFY_ARE_EQUAL(ExtSupportBogus.getFailureReason(),
+                     ExtSupportBogus.FailedDxilPath);
   }
 
-  // validate that m_dllExtSupport2 was able to capture the environment
+  // validate that ExtSupportBogus was able to capture the environment
   // variable's value, and that loading the bogus path was unsuccessful
   std::string BogusPath = ExtSupportBogus.getDxilDllPath();
   VERIFY_ARE_EQUAL_STR(BogusPath.c_str(), "bogus");
   VERIFY_IS_TRUE(ExtSupportBogus.dxilDllFailedToLoad());
 
-  // 3. Test production of class IDs CLSID_DxcCompiler, CLSID_DxcLinker,
+  // 3. Test with a valid path to this file in the environment variable
+  std::filesystem::path p = std::filesystem::absolute(__FILE__);
+  SetEnvVarW(L"DXC_DXIL_DLL_PATH", p.wstring());
+
+  if (!ExtSupportValidNonDLLPath.IsEnabled()) {
+    VERIFY_FAILED(ExtSupportValidNonDLLPath.initialize());
+    VERIFY_ARE_EQUAL(ExtSupportValidNonDLLPath.getFailureReason(),
+                     ExtSupportValidNonDLLPath.FailedDxilLoad);
+  }
+
+  // validate that ExtSupportValidNonDLLPath was able to capture the environment
+  // variable's value, and that loading the valid non-dll path was unsuccessful
+  std::string ValidNonDLLPath = ExtSupportValidNonDLLPath.getDxilDllPath();
+  VERIFY_ARE_EQUAL_STR(ValidNonDLLPath.c_str(), __FILE__);
+  VERIFY_IS_TRUE(ExtSupportValidNonDLLPath.dxilDllFailedToLoad());
+
+  // 4. Test production of class IDs CLSID_DxcCompiler, CLSID_DxcLinker,
   // and CLSID_DxcValidator through DxcDllExtValidationSupport.
   CComPtr<IDxcCompiler> Compiler;
   CComPtr<IDxcLinker> Linker;