FileChecker improvements: VFS, %dxl, FileCheck -D, error reporting (#3576)

- VFS captures output files for duration of test, enabling:
- %dxl test IDxcLinker
- add -D to FileCheck args to supply defined variables
- report failing RUN command when not consumed by FileCheck or XFail

Author: tex3d

include/dxc/Test/DxcTestUtils.h

@@ -60,6 +60,9 @@ class FileCheckForTest {
   int Run();
 };
 
+// wstring because most uses need UTF-16: IDxcResult output names, include handler
+typedef std::map<std::wstring, CComPtr<IDxcBlob>> FileMap;
+
 // The result of running a single command in a run pipeline
 struct FileRunCommandResult {
   CComPtr<IDxcOperationResult> OpResult; // The operation result, if any.
@@ -109,6 +112,7 @@ class FileRunCommandPart {
   std::string Command;      // Command to run, eg %dxc
   std::string Arguments;    // Arguments to command
   LPCWSTR CommandFileName;  // File name replacement for %s
+  FileMap *pVFS = nullptr;  // Files in virtual file system
 
 private:
   FileRunCommandResult RunFileChecker(const FileRunCommandResult *Prior, LPCWSTR dumpName = nullptr);
@@ -117,6 +121,7 @@ class FileRunCommandPart {
   FileRunCommandResult RunOpt(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior);
   FileRunCommandResult RunD3DReflect(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior);
   FileRunCommandResult RunDxr(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior);
+  FileRunCommandResult RunLink(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior);
   FileRunCommandResult RunTee(const FileRunCommandResult *Prior);
   FileRunCommandResult RunXFail(const FileRunCommandResult *Prior);
   FileRunCommandResult RunDxilVer(dxc::DxcDllSupport& DllSupport, const FileRunCommandResult* Prior);

tools/clang/unittests/HLSLTestLib/FileCheckerTest.cpp

@@ -39,6 +39,7 @@
 #include "dxc/dxctools.h"
 #include "dxc/Support/HLSLOptions.h"
 #include "dxc/Support/Unicode.h"
+#include "dxc/Support/microcom.h"
 #include "dxc/DxilContainer/DxilContainer.h"
 #include "dxc/Test/D3DReflectionDumper.h"
 
@@ -107,6 +108,9 @@ FileRunCommandResult FileRunCommandPart::Run(dxc::DxcDllSupport &DllSupport, con
   else if (0 == _stricmp(Command.c_str(), "%dxr")) {
     return RunDxr(DllSupport, Prior);
   }
+  else if (0 == _stricmp(Command.c_str(), "%dxl")) {
+    return RunLink(DllSupport, Prior);
+  }
   else if (pPluginToolsPaths != nullptr) {
     auto it = pPluginToolsPaths->find(Command.c_str());
     if (it != pPluginToolsPaths->end()) {
@@ -248,6 +252,85 @@ static HRESULT GetDxilBitcode(dxc::DxcDllSupport &DllSupport, IDxcBlob *pCompile
   return S_OK;
 }
 
+// Simple virtual file system include handler for test, fall back to default include handler
+class IncludeHandlerVFSOverlayForTest : public IDxcIncludeHandler {
+private:
+  DXC_MICROCOM_TM_REF_FIELDS()
+
+public:
+  DXC_MICROCOM_TM_ADDREF_RELEASE_IMPL()
+  DXC_MICROCOM_TM_CTOR(IncludeHandlerVFSOverlayForTest)
+
+  HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, void **ppvObject) override {
+    return DoBasicQueryInterface<IDxcIncludeHandler>(this, iid, ppvObject);
+  }
+
+  const FileMap *pVFS = nullptr;
+  CComPtr<IDxcIncludeHandler> pInnerIncludeHandler;
+
+  HRESULT STDMETHODCALLTYPE LoadSource(
+      _In_ LPCWSTR pFilename,                                   // Candidate filename.
+      _COM_Outptr_result_maybenull_ IDxcBlob **ppIncludeSource  // Resultant source object for included file, nullptr if not found.
+      ) override {
+    if (!ppIncludeSource)
+      return E_INVALIDARG;
+    *ppIncludeSource = nullptr;
+    if (!pFilename)
+      return E_INVALIDARG;
+    try {
+      if (pVFS) {
+        auto it = pVFS->find(pFilename);
+        if (it != pVFS->end()) {
+          return it->second.QueryInterface(ppIncludeSource);
+        }
+      }
+      if (pInnerIncludeHandler) {
+        return pInnerIncludeHandler->LoadSource(pFilename, ppIncludeSource);
+      }
+      return E_FAIL;
+    }
+    CATCH_CPP_RETURN_HRESULT();
+  }
+};
+
+static IncludeHandlerVFSOverlayForTest *AllocVFSIncludeHandler(IUnknown *pUnkLibrary, const FileMap *pVFS) {
+  CComPtr<IncludeHandlerVFSOverlayForTest> pVFSIncludeHandler = IncludeHandlerVFSOverlayForTest::Alloc(DxcGetThreadMallocNoRef());
+  IFTBOOL(pVFSIncludeHandler, E_OUTOFMEMORY);
+  if (pUnkLibrary) {
+    CComPtr<IDxcIncludeHandler> pInnerIncludeHandler;
+    CComPtr<IDxcUtils> pUtils;
+    if (SUCCEEDED(pUnkLibrary->QueryInterface(IID_PPV_ARGS(&pUtils)))) {
+      IFT(pUtils->CreateDefaultIncludeHandler(&pInnerIncludeHandler));
+    } else {
+      CComPtr<IDxcLibrary> pLibrary;
+      if (SUCCEEDED(pUnkLibrary->QueryInterface(IID_PPV_ARGS(&pLibrary)))) {
+        IFT(pLibrary->CreateIncludeHandler(&pInnerIncludeHandler));
+      }
+    }
+    pVFSIncludeHandler->pInnerIncludeHandler = pInnerIncludeHandler;
+  }
+  pVFSIncludeHandler->pVFS = pVFS;
+  return pVFSIncludeHandler.Detach();
+}
+
+static void AddOutputsToFileMap(IUnknown *pUnkResult, FileMap *pVFS) {
+  // If there is IDxcResult, save named output blobs to Files.
+  if (pUnkResult && pVFS) {
+    CComPtr<IDxcResult> pResult;
+    if (SUCCEEDED(pUnkResult->QueryInterface(IID_PPV_ARGS(&pResult)))) {
+      for (unsigned i = 0; i < pResult->GetNumOutputs(); i++) {
+        CComPtr<IDxcBlob> pOutput;
+        CComPtr<IDxcBlobUtf16> pOutputName;
+        if (SUCCEEDED(pResult->GetOutput(pResult->GetOutputByIndex(i),
+                                IID_PPV_ARGS(&pOutput), &pOutputName)) &&
+            pOutput && pOutputName && pOutputName->GetStringLength() > 0) {
+          (*pVFS)[pOutputName->GetStringPointer()] = pOutput;
+        }
+      }
+    }
+  }
+}
+
 static HRESULT CompileForHash(hlsl::options::DxcOpts &opts, LPCWSTR CommandFileName, dxc::DxcDllSupport &DllSupport, std::vector<LPCWSTR> &flags, IDxcBlob **ppHashBlob, std::string &output) {
   CComPtr<IDxcLibrary> pLibrary;
   CComPtr<IDxcCompiler> pCompiler;
@@ -466,13 +549,12 @@ FileRunCommandResult FileRunCommandPart::RunDxc(dxc::DxcDllSupport &DllSupport,
   CComPtr<IDxcBlobEncoding> pSource;
   CComPtr<IDxcBlobEncoding> pDisassembly;
   CComPtr<IDxcBlob> pCompiledBlob;
-  CComPtr<IDxcIncludeHandler> pIncludeHandler;
 
   HRESULT resultStatus;
 
   IFT(DllSupport.CreateInstance(CLSID_DxcLibrary, &pLibrary));
   IFT(pLibrary->CreateBlobFromFile(CommandFileName, nullptr, &pSource));
-  IFT(pLibrary->CreateIncludeHandler(&pIncludeHandler));
+  CComPtr<IDxcIncludeHandler> pIncludeHandler = AllocVFSIncludeHandler(pLibrary, pVFS);
   IFT(DllSupport.CreateInstance(CLSID_DxcCompiler, &pCompiler));
   IFT(pCompiler->Compile(pSource, CommandFileName, entry.c_str(), profile.c_str(),
                           flags.data(), flags.size(), nullptr, 0, pIncludeHandler, &pResult));
@@ -698,11 +780,10 @@ FileRunCommandResult FileRunCommandPart::RunDxr(dxc::DxcDllSupport &DllSupport,
   CComPtr<IDxcOperationResult> pResult;
   CComPtr<IDxcBlobEncoding> pSource;
   CComPtr<IDxcBlob> pResultBlob;
-  CComPtr<IDxcIncludeHandler> pIncludeHandler;
 
   IFT(DllSupport.CreateInstance(CLSID_DxcLibrary, &pLibrary));
   IFT(pLibrary->CreateBlobFromFile(CommandFileName, nullptr, &pSource));
-  IFT(pLibrary->CreateIncludeHandler(&pIncludeHandler));
+  CComPtr<IDxcIncludeHandler> pIncludeHandler = AllocVFSIncludeHandler(pLibrary, pVFS);
   IFT(DllSupport.CreateInstance(CLSID_DxcRewriter, &pRewriter));
   IFT(pRewriter->RewriteWithOptions(pSource, CommandFileName,
                                     flags.data(), flags.size(), nullptr, 0,
@@ -725,6 +806,107 @@ FileRunCommandResult FileRunCommandPart::RunDxr(dxc::DxcDllSupport &DllSupport,
   return result;
 }
 
+FileRunCommandResult FileRunCommandPart::RunLink(dxc::DxcDllSupport &DllSupport, const FileRunCommandResult *Prior) {
+  hlsl::options::MainArgs args;
+  hlsl::options::DxcOpts opts;
+  FileRunCommandResult readOptsResult = ReadOptsForDxc(args, opts,
+    hlsl::options::HlslFlags::CoreOption);
+  if (readOptsResult.ExitCode) return readOptsResult;
+
+  std::wstring entry =
+      Unicode::UTF8ToUTF16StringOrThrow(opts.EntryPoint.str().c_str());
+  std::wstring profile =
+      Unicode::UTF8ToUTF16StringOrThrow(opts.TargetProfile.str().c_str());
+  std::vector<LPCWSTR> flags;
+
+  // Skip targets that require a newer compiler or validator.
+  // Some features may require newer compiler/validator than indicated by the
+  // shader model, but these should use %dxilver explicitly.
+  {
+    unsigned RequiredDxilMajor = 1, RequiredDxilMinor = 0;
+    llvm::StringRef stage;
+    IFTBOOL(ParseTargetProfile(opts.TargetProfile, stage, RequiredDxilMajor, RequiredDxilMinor), E_INVALIDARG);
+    if (RequiredDxilMinor != 0xF && stage.compare("rootsig") != 0) {
+      // Convert stage to minimum dxil/validator version:
+      RequiredDxilMajor = std::max(RequiredDxilMajor, (unsigned)6) - 5;
+      FileRunCommandResult result = CheckDxilVer(DllSupport, RequiredDxilMajor, RequiredDxilMinor, !opts.DisableValidation);
+      if (result.AbortPipeline) {
+        return result;
+      }
+    }
+  }
+
+  // For now, too many tests are sensitive to stripping the refleciton info
+  // from the main module, so use this flag to prevent this until tests
+  // can be updated.
+  // That is, unless the test explicitly requests -Qstrip_reflect_from_dxil or -Qstrip_reflect
+  if (!opts.StripReflectionFromDxil && !opts.StripReflection) {
+    flags.push_back(L"-Qkeep_reflect_in_dxil");
+  }
+
+  std::vector<std::wstring> argWStrings;
+  CopyArgsToWStrings(opts.Args, hlsl::options::CoreOption, argWStrings);
+  for (const std::wstring &a : argWStrings)
+    flags.push_back(a.data());
+
+  // Parse semicolon separated list of library names.
+  llvm::StringRef optLibraries = opts.Args.getLastArgValue(hlsl::options::OPT_INPUT);
+  auto libs_utf8 = strtok(optLibraries.str().c_str(), ";");
+  std::vector<std::wstring> libs_utf16;
+  for (auto name : libs_utf8)
+    libs_utf16.emplace_back(Unicode::UTF8ToUTF16StringOrThrow(name.c_str()));
+  std::vector<LPCWSTR> libNames;
+  for (auto &name : libs_utf16)
+    libNames.emplace_back(name.data());
+
+  CComPtr<IDxcLibrary> pLibrary;
+  CComPtr<IDxcLinker> pLinker;
+  CComPtr<IDxcCompiler> pCompiler;
+  CComPtr<IDxcOperationResult> pResult;
+  CComPtr<IDxcBlobEncoding> pDisassembly;
+  CComPtr<IDxcBlob> pCompiledBlob;
+
+  HRESULT resultStatus;
+
+  IFT(DllSupport.CreateInstance(CLSID_DxcLibrary, &pLibrary));
+  CComPtr<IDxcIncludeHandler> pIncludeHandler = AllocVFSIncludeHandler(pLibrary, pVFS);
+  IFT(DllSupport.CreateInstance(CLSID_DxcLinker, &pLinker));
+  IFT(DllSupport.CreateInstance(CLSID_DxcCompiler, &pCompiler));
+
+  for (auto name : libNames) {
+    CComPtr<IDxcBlob> pLibBlob;
+    IFT(pIncludeHandler->LoadSource(name, &pLibBlob));
+    IFT(pLinker->RegisterLibrary(name, pLibBlob));
+  }
+
+  IFT(pLinker->Link(entry.c_str(), profile.c_str(), libNames.data(),
+                    libNames.size(), flags.data(), flags.size(), &pResult));
+  IFT(pResult->GetStatus(&resultStatus));
+
+  FileRunCommandResult result = {};
+  if (SUCCEEDED(resultStatus)) {
+    IFT(pResult->GetResult(&pCompiledBlob));
+    if (!opts.AstDump) {
+      IFT(pCompiler->Disassemble(pCompiledBlob, &pDisassembly));
+      result.StdOut = BlobToUtf8(pDisassembly);
+    } else {
+      result.StdOut = BlobToUtf8(pCompiledBlob);
+    }
+    CComPtr<IDxcBlobEncoding> pStdErr;
+    IFT(pResult->GetErrorBuffer(&pStdErr));
+    result.StdErr = BlobToUtf8(pStdErr);
+    result.ExitCode = 0;
+  }
+  else {
+    IFT(pResult->GetErrorBuffer(&pDisassembly));
+    result.StdErr = BlobToUtf8(pDisassembly);
+    result.ExitCode = resultStatus;
+  }
+
+  result.OpResult = pResult;
+  return result;
+}
+
 FileRunCommandResult FileRunCommandPart::RunTee(const FileRunCommandResult *Prior) {
   if (Prior == nullptr) {
     return FileRunCommandResult::Error("tee requires a prior command");
@@ -938,6 +1120,8 @@ class FileRunTestResultImpl : public FileRunTestResult {
   dxc::DxcDllSupport &m_support;
   PluginToolsPaths *m_pPluginToolsPaths;
   LPCWSTR m_dumpName = nullptr;
+  // keep track of virtual files for duration of this test (for all RUN lines)
+  FileMap Files;
 
   void RunHashTestFromCommands(LPCSTR commands, LPCWSTR fileName) {
     std::vector<FileRunCommandPart> parts;
@@ -967,13 +1151,38 @@ class FileRunTestResultImpl : public FileRunTestResult {
       this->ErrorMessage = "FileCheck found no commands to run";
       return;
     }
-    
     FileRunCommandResult result;
-    FileRunCommandResult* previousResult = nullptr;
+    FileRunCommandResult *previousResult = nullptr;
+    FileRunCommandPart *pPrior = nullptr;
     for (FileRunCommandPart & part : parts) {
+      int priorExitCode = result.ExitCode;
+      part.pVFS = &Files;
       result = part.Run(m_support, previousResult, m_pPluginToolsPaths, dumpName);
+
+      // If there is IDxcResult, save named output blobs to Files.
+      AddOutputsToFileMap(result.OpResult, &Files);
+
+      // When current failing stage is FileCheck, print prior command,
+      // as well as FileCheck command that failed, to help identify
+      // failing commands in longer run chains.
+      if (result.ExitCode &&
+          (0 == _stricmp(part.Command.c_str(), "FileCheck") ||
+           0 == _stricmp(part.Command.c_str(), "%FileCheck"))) {
+        std::ostringstream oss;
+        if (pPrior) {
+          oss << "Prior (" << priorExitCode << "): "
+              << pPrior->Command << pPrior->Arguments << endl;
+        }
+        oss << "Error (" << result.ExitCode << "): "
+            << part.Command << part.Arguments << endl;
+        oss << result.StdErr;
+        result.StdErr = oss.str();
+      }
+
+      if (result.AbortPipeline)
+        break;
       previousResult = &result;
-      if (result.AbortPipeline) break;
+      pPrior = &part;
     }
 
     this->RunResult = result.ExitCode;
@@ -1001,7 +1210,10 @@ class FileRunTestResultImpl : public FileRunTestResult {
       RunFileCheckFromCommands(cmd.c_str(), fileName, dumpName);
       // If any of the RUN cmd fails then skip executing remaining cmds
       // and report the error
-      if (this->RunResult != 0) break;
+      if (this->RunResult != 0) {
+        this->ErrorMessage = cmd + "\n" + this->ErrorMessage;
+        break;
+      }
       runIdx += 1;
     }
   }