Browse Source

Move unused global tracing flags into single usage, closes #157. (#185)

* Move unused global tracing flags into single usage, closes #157.

Normalizes API-provided names to include at least a current-relative
directory, simplifying file lookups. There are still restrictions on
how expressive include handlers enable the virtual file system to be,
these should be documented on the project wiki.

Handle some errors in BeginSourceFile paths where include handlers may
cause early errors, before compilation execution can begin.

Removes the prior scheme for directory handles, where the same handle
would be returned for any parent directory.

Future improvements would be to expand dxc paths before calling into
dxcompiler, and/or to fully implement OS API calls from console apps.

Improve hcttest to describe build config better and not override env var.
Marcelo Lopez Ruiz 8 years ago
parent
commit
254f253b14

+ 0 - 16
include/dxc/Support/Global.h

@@ -150,18 +150,6 @@ inline void OutputDebugFormatA(_In_ _Printf_format_string_ _Null_terminated_ con
 
 #define DXVERIFY_NOMSG(exp) DXASSERT(exp, "")
 
-// This should be improved with global enabled mask rather than a compile-time mask.
-#define DXTRACE_MASK_ENABLED  0
-#define DXTRACE_MASK_APIFS    1
-#define DXTRACE_ENABLED(subsystem) (DXTRACE_MASK_ENABLED & subsystem)
-
-// DXTRACE_FMT formats a debugger trace message if DXTRACE_MASK allows it.
-#define DXTRACE_FMT(subsystem, fmt, ...) do { \
-  if (DXTRACE_ENABLED(subsystem)) OutputDebugFormatA(fmt, __VA_ARGS__); \
-} while (0)
-/// DXTRACE_FMT_APIFS is used by the API-based virtual filesystem.
-#define DXTRACE_FMT_APIFS(fmt, ...) DXTRACE_FMT(DXTRACE_MASK_APIFS, fmt, __VA_ARGS__)
-
 #else
 
 // DXASSERT is disabled in free builds.
@@ -176,8 +164,4 @@ inline void OutputDebugFormatA(_In_ _Printf_format_string_ _Null_terminated_ con
 // DXVERIFY is patterned after NT_VERIFY and will evaluate the expression
 #define DXVERIFY_NOMSG(exp) do { (exp); _Analysis_assume_(exp); } while (0)
 
-// DXTRACE_FMT and the subsystem versions are disabled in free builds.
-#define DXTRACE_FMT(...) (void)(0)
-#define DXTRACE_FMT_APIFS(...) (void)(0)
-
 #endif // DBG

+ 190 - 127
tools/clang/tools/dxcompiler/dxcompilerobj.cpp

@@ -41,7 +41,6 @@
 #include "dxc/HLSL/HLSLExtensionsCodegenHelper.h"
 #include "dxc/HLSL/DxilRootSignature.h"
 
-#ifdef _DEBUG
 #if defined(_MSC_VER)
 #include <io.h>
 #ifndef STDOUT_FILENO
@@ -51,7 +50,6 @@
 # define STDERR_FILENO 2
 #endif
 #endif
-#endif
 
 #include "dxc/Support/WinIncludes.h"
 #include "dxc/HLSL/DxilContainer.h"
@@ -80,6 +78,26 @@
 
 #define CP_UTF16 1200
 
+#ifdef DBG
+
+// This should be improved with global enabled mask rather than a compile-time mask.
+#define DXTRACE_MASK_ENABLED  0
+#define DXTRACE_MASK_APIFS    1
+#define DXTRACE_ENABLED(subsystem) (DXTRACE_MASK_ENABLED & subsystem)
+
+// DXTRACE_FMT formats a debugger trace message if DXTRACE_MASK allows it.
+#define DXTRACE_FMT(subsystem, fmt, ...) do { \
+  if (DXTRACE_ENABLED(subsystem)) OutputDebugFormatA(fmt, __VA_ARGS__); \
+} while (0)
+/// DXTRACE_FMT_APIFS is used by the API-based virtual filesystem.
+#define DXTRACE_FMT_APIFS(fmt, ...) DXTRACE_FMT(DXTRACE_MASK_APIFS, fmt, __VA_ARGS__)
+
+#else
+
+#define DXTRACE_FMT_APIFS(...)
+
+#endif // DBG
+
 using namespace llvm;
 using namespace clang;
 using namespace hlsl;
@@ -97,22 +115,108 @@ HRESULT RunInternalValidator(_In_ IDxcValidator *pValidator,
                              _In_ IDxcBlob *pShader, UINT32 Flags,
                              _In_ IDxcOperationResult **ppResult);
 
-static const HANDLE StdOutHandle = (HANDLE)0x1;
-static const HANDLE StdErrHandle = (HANDLE)0x2;
-static const HANDLE SourceParentDirHandle = (HANDLE)0x11;
-static const HANDLE SourceHandle = (HANDLE)0x12;
-static const HANDLE OutputHandle = (HANDLE)0x13;
-static const size_t IncludedHandleOffset = 0x100;
+enum class HandleKind {
+  Special = 0,
+  File = 1,
+  FileDir = 2,
+  SearchDir = 3
+};
+enum class SpecialValue {
+  Unknown = 0,
+  StdOut = 1,
+  StdErr = 2,
+  Source = 3,
+  Output = 4
+};
+struct HandleBits {
+  unsigned Offset : 8;
+  unsigned Length : 8;
+  unsigned Kind : 4;
+};
+struct DxcArgsHandle {
+  DxcArgsHandle(HANDLE h) : Handle(h) {}
+  DxcArgsHandle(unsigned fileIndex)
+    : Bits{ fileIndex, 0, (unsigned)HandleKind::File } {}
+  DxcArgsHandle(HandleKind HK, unsigned fileIndex, unsigned dirLength)
+    : Bits{ fileIndex, dirLength, (unsigned)HK} {}
+  DxcArgsHandle(SpecialValue V)
+      : Bits{(unsigned)V, 0, (unsigned)HandleKind::Special} {}
+  union {
+    HANDLE Handle;
+    HandleBits Bits;
+  };
+  bool operator==(const DxcArgsHandle &Other) { return Handle == Other.Handle; }
+  HandleKind GetKind() const { return (HandleKind)Bits.Kind; }
+  bool IsFileKind() const { return GetKind() == HandleKind::File; }
+  bool IsSpecialUnknown() const { return Handle == 0; }
+  bool IsDirHandle() const {
+    return GetKind() == HandleKind::FileDir || GetKind() == HandleKind::SearchDir;
+  }
+  bool IsStdHandle() const {
+    return GetKind() == HandleKind::Special &&
+           (GetSpecialValue() == SpecialValue::StdErr ||
+            GetSpecialValue() == SpecialValue::StdOut);
+  }
+  unsigned GetFileIndex() const {
+    DXASSERT_NOMSG(IsFileKind());
+    return Bits.Offset;
+  }
+  SpecialValue GetSpecialValue() const {
+    DXASSERT_NOMSG(GetKind() == HandleKind::Special);
+    return (SpecialValue)Bits.Offset;
+  }
+  unsigned Length() const { return Bits.Length; }
+};
+
+static_assert(sizeof(DxcArgsHandle) == sizeof(HANDLE), "else can't transparently typecast");
+
+static const DxcArgsHandle UnknownHandle(SpecialValue::Unknown);
+static const DxcArgsHandle StdOutHandle(SpecialValue::StdOut);
+static const DxcArgsHandle StdErrHandle(SpecialValue::StdErr);
+static const DxcArgsHandle OutputHandle(SpecialValue::Output);
 
 /// Max number of included files (1:1 to their directories) or search directories.
 /// If programs include more than a handful, DxcArgsFileSystem will need to do better than linear scans.
 /// If this is fired, ERROR_OUT_OF_STRUCTURES will be returned by an attempt to open a file.
 static const size_t MaxIncludedFiles = 200;
-static const size_t IncludedDirHandleOffset = 0x1000;
-static const size_t SearchDirHandleOffset = 0x2000;
 
-static_assert(IncludedDirHandleOffset > IncludedHandleOffset + MaxIncludedFiles, "else files and dirs may overlap");
-static_assert(SearchDirHandleOffset > IncludedDirHandleOffset + MaxIncludedFiles, "else file dirs and search option dirs overlap");
+static bool IsAbsoluteOrCurDirRelativeW(LPCWSTR Path) {
+  if (!Path || !Path[0]) return FALSE;
+  // Current dir-relative path.
+  if (Path[0] == L'.') {
+    return Path[1] == L'\0' || Path[1] == L'/' || Path[1] == L'\\';
+  }
+  // Disk designator, then absolute path.
+  if (Path[1] == L':' && Path[2] == L'\\') {
+    return TRUE;
+  }
+  // UNC name
+  if (Path[0] == L'\\') {
+    return Path[1] == L'\\';
+  }
+
+  //
+  // NOTE: there are a number of cases we don't handle, as they don't play well with the simple
+  // file system abstraction we use:
+  // - current directory on disk designator (eg, D:file.ext), requires per-disk current dir
+  // - parent paths relative to current directory (eg, ..\\file.ext)
+  //
+  // The current-directory support is available to help in-memory handlers. On-disk handlers
+  // will typically have absolute paths to begin with.
+  //
+  return FALSE;
+}
+
+static void MakeAbsoluteOrCurDirRelativeW(LPCWSTR &Path, std::wstring &PathStorage) {
+  if (IsAbsoluteOrCurDirRelativeW(Path)) {
+    return;
+  }
+  else {
+    PathStorage = L"./";
+    PathStorage += Path;
+    Path = PathStorage.c_str();
+  }
+}
 
 static bool IsAbsoluteOrCurDirRelative(const Twine &T) {
   if (llvm::sys::path::is_absolute(T)) {
@@ -153,22 +257,24 @@ class DxcArgsFileSystem : public ::llvm::sys::fs::MSFileSystem {
 private:
   CComPtr<IDxcBlob> m_pSource;
   LPCWSTR m_pSourceName;
+  std::wstring m_pAbsSourceName; // absolute (or '.'-relative) source name
   CComPtr<IStream> m_pSourceStream;
   CComPtr<IStream> m_pOutputStream;
   CComPtr<AbstractMemoryStream> m_pStdOutStream;
   CComPtr<AbstractMemoryStream> m_pStdErrStream;
   LPCWSTR m_pOutputStreamName;
+  std::wstring m_pAbsOutputStreamName;
   CComPtr<IDxcIncludeHandler> m_includeLoader;
   std::vector<std::wstring> m_searchEntries;
-  bool    m_bDisplayIncludeProcess;
+  bool m_bDisplayIncludeProcess;
 
   // Some constraints of the current design: opening the same file twice
   // will return the same handle/structure, and thus the same file pointer.
   struct IncludedFile {
-    CComPtr<IDxcBlobEncoding> Blob;
+    CComPtr<IDxcBlob> Blob;
     CComPtr<IStream> BlobStream;
     std::wstring Name;
-    IncludedFile(std::wstring &&name, IDxcBlobEncoding *pBlob, IStream *pStream)
+    IncludedFile(std::wstring &&name, IDxcBlob *pBlob, IStream *pStream)
       : Name(name), Blob(pBlob), BlobStream(pStream) { }
   };
   llvm::SmallVector<IncludedFile, 4> m_includedFiles;
@@ -197,25 +303,25 @@ private:
     for (size_t i = 0; i < m_includedFiles.size(); ++i) {
       const std::wstring &fileName = m_includedFiles[i].Name;
       if (IsDirOf(lpDir, dirLen, fileName)) {
-        return IncludedDirIndexToHandle(i);
+        return DxcArgsHandle(HandleKind::FileDir, i, dirLen).Handle;
       }
     }
     for (size_t i = 0; i < m_searchEntries.size(); ++i) {
       if (IsDirPrefixOrSame(lpDir, dirLen, m_searchEntries[i])) {
-        return SearchDirIndexToHandle(i);
+        return DxcArgsHandle(HandleKind::SearchDir, i, dirLen).Handle;
       }
     }
     return INVALID_HANDLE_VALUE;
   }
   DWORD TryFindOrOpen(LPCWSTR lpFileName, size_t &index) {
-    if (m_includeLoader.p != nullptr) {
-      for (size_t i = 0; i < m_includedFiles.size(); ++i) {
-        if (0 == wcscmp(lpFileName, m_includedFiles[i].Name.data())) {
-          index = i;
-          return ERROR_SUCCESS;
-        }
+    for (size_t i = 0; i < m_includedFiles.size(); ++i) {
+      if (0 == wcscmp(lpFileName, m_includedFiles[i].Name.data())) {
+        index = i;
+        return ERROR_SUCCESS;
       }
+    }
 
+    if (m_includeLoader.p != nullptr) {
       if (m_includedFiles.size() == MaxIncludedFiles) {
         return ERROR_OUT_OF_STRUCTURES;
       }
@@ -241,7 +347,7 @@ private:
           std::string openFileStr;
           raw_string_ostream s(openFileStr);
           std::string fileName = Unicode::UTF16ToUTF8StringOrThrow(lpFileName);
-          s << "Opening file [" << fileName << "], stack top [" << index
+          s << "Opening file [" << fileName << "], stack top [" << (index-1)
             << "]\n";
           s.flush();
           ULONG cbWritten;
@@ -254,57 +360,24 @@ private:
     return ERROR_NOT_FOUND;
   }
   static HANDLE IncludedFileIndexToHandle(size_t index) {
-    return (HANDLE)(uintptr_t)(IncludedHandleOffset + index);
-  }
-  static HANDLE IncludedDirIndexToHandle(size_t index) {
-    return (HANDLE)(uintptr_t)(IncludedDirHandleOffset + index);
-  }
-  static HANDLE SearchDirIndexToHandle(size_t index) {
-    return (HANDLE)(uintptr_t)(SearchDirHandleOffset + index);
-  }
-  bool IsHandleIncludedFile(HANDLE handle) const {
-    size_t val = (size_t)handle;
-    if (val < IncludedHandleOffset) return false;
-    val -= IncludedHandleOffset;
-    if (val >= m_includedFiles.size()) return false;
-    return true;
-  }
-  bool IsHandleIncludedDir(HANDLE handle) const {
-    size_t val = (size_t)handle;
-    if (val < IncludedDirHandleOffset) return false;
-    val -= IncludedDirHandleOffset;
-    if (val >= m_includedFiles.size()) return false;
-    return true;
-  }
-  bool IsHandleSearch(HANDLE handle) const {
-    size_t val = (size_t)handle;
-    if (val < SearchDirHandleOffset) return false;
-    val -= SearchDirHandleOffset;
-    if (val >= m_searchEntries.size()) return false;
-    return true;
-  }
-  bool IsStdHandle(HANDLE handle) const {
-    return handle == StdOutHandle || handle == StdErrHandle;
-  }
-  bool IsDirHandle(HANDLE handle) const {
-    return handle == SourceParentDirHandle || IsHandleIncludedDir(handle) || IsHandleSearch(handle);
+    return DxcArgsHandle(index).Handle;
   }
   bool IsKnownHandle(HANDLE h) const {
-    return IsStdHandle(h) ||
-      h == SourceHandle || h == SourceParentDirHandle || h == OutputHandle ||
-      IsDirHandle(h) || IsHandleIncludedFile(h);
+    return !DxcArgsHandle(h).IsSpecialUnknown();
   }
   IncludedFile &HandleToIncludedFile(HANDLE handle) {
-    DXASSERT_NOMSG((size_t)handle >= IncludedHandleOffset);
-    size_t index = (size_t)handle - IncludedHandleOffset;
-    DXASSERT_NOMSG(index < m_includedFiles.size());
-    return m_includedFiles[index];
+    DxcArgsHandle argsHandle(handle);
+    DXASSERT_NOMSG(argsHandle.GetFileIndex() < m_includedFiles.size());
+    return m_includedFiles[argsHandle.GetFileIndex()];
   }
 
 public:
   DxcArgsFileSystem(_In_ IDxcBlob *pSource, LPCWSTR pSourceName, _In_opt_ IDxcIncludeHandler* pHandler)
       : m_pSource(pSource), m_pSourceName(pSourceName), m_includeLoader(pHandler), m_bDisplayIncludeProcess(false),
         m_pOutputStreamName(nullptr) {
+    MakeAbsoluteOrCurDirRelativeW(m_pSourceName, m_pAbsSourceName);
+    IFT(CreateReadOnlyBlobStream(m_pSource, &m_pSourceStream));
+    m_includedFiles.push_back(IncludedFile(std::wstring(m_pSourceName), m_pSource, m_pSourceStream));
   }
   void EnableDisplayIncludeProcess() {
     m_bDisplayIncludeProcess = true;
@@ -323,24 +396,22 @@ public:
     return S_OK;
   }
 
+  void GetStreamForFD(int fd, IStream** ppResult) {
+    return GetStreamForHandle(HandleFromFD(fd), ppResult);
+  }
   void GetStreamForHandle(HANDLE handle, IStream** ppResult) {
     CComPtr<IStream> stream;
-    if (handle == SourceHandle) {
-      if (m_pSourceStream == nullptr) {
-        CreateReadOnlyBlobStream(m_pSource, &m_pSourceStream);
-      }
-      stream = m_pSourceStream;
-    }
-    else if (handle == OutputHandle) {
+    DxcArgsHandle argsHandle(handle);
+    if (argsHandle == OutputHandle) {
       stream = m_pOutputStream;
     }
-    else if (handle == StdOutHandle) {
+    else if (argsHandle == StdOutHandle) {
       stream = m_pStdOutStream;
     }
-    else if (handle == StdErrHandle) {
+    else if (argsHandle == StdErrHandle) {
       stream = m_pStdErrStream;
     }
-    else if (IsHandleIncludedFile(handle)) {
+    else if (argsHandle.GetKind() == HandleKind::File) {
       stream = HandleToIncludedFile(handle).BlobStream;
     }
     *ppResult = stream.Detach();
@@ -349,7 +420,7 @@ public:
   void SetupForCompilerInstance(CompilerInstance &compiler) {
     DXASSERT(m_searchEntries.size() == 0, "else compiler instance being set twice");
     // Turn these into UTF-16 to avoid converting later, and ensure they
-    // are fully-qualified or reltive to the current directory.
+    // are fully-qualified or relative to the current directory.
     const std::vector<HeaderSearchOptions::Entry> &entries =
       compiler.getHeaderSearchOpts().UserEntries;
     if (entries.size() > MaxIncludedFiles) {
@@ -372,6 +443,7 @@ public:
     DXASSERT(m_pOutputStream.p == nullptr, "else multiple outputs registered");
     m_pOutputStream = pStream;
     m_pOutputStreamName = pName;
+    MakeAbsoluteOrCurDirRelativeW(m_pOutputStreamName, m_pAbsOutputStreamName);
     return S_OK;
   }
 
@@ -402,27 +474,14 @@ public:
     _In_      DWORD dwFlagsAndAttributes) throw() {
     DXTRACE_FMT_APIFS("DxcArgsFileSystem::CreateFileW %S\n", lpFileName);
     size_t sourceNameLen = wcslen(m_pSourceName);
+    std::wstring FileNameStore;
+    MakeAbsoluteOrCurDirRelativeW(lpFileName, FileNameStore);
     size_t fileNameLen = wcslen(lpFileName);
 
-    // Check for a substring match to source, implying a parent directory.
-    if (fileNameLen < sourceNameLen) {
-      if (0 == wcsncmp(m_pSourceName, lpFileName, fileNameLen) ||
-          0 == wcscmp(L".", lpFileName)) {
-        return SourceParentDirHandle;
-      }
-    }
-
-    // Check for a match to the source.
-    if (fileNameLen == sourceNameLen) {
-      if (0 == wcsncmp(m_pSourceName, lpFileName, fileNameLen)) {
-        return SourceHandle;
-      }
-    }
-
     // Check for a match to the output file.
     if (m_pOutputStreamName != nullptr &&
         0 == wcscmp(lpFileName, m_pOutputStreamName)) {
-      return OutputHandle;
+      return OutputHandle.Handle;
     }
 
     HANDLE dirHandle = TryFindDirHandle(lpFileName);
@@ -449,14 +508,16 @@ public:
   }
 
   __override BOOL GetFileInformationByHandle(_In_ HANDLE hFile, _Out_ LPBY_HANDLE_FILE_INFORMATION lpFileInformation) throw() {
+    DxcArgsHandle argsHandle(hFile);
     ZeroMemory(lpFileInformation, sizeof(*lpFileInformation));
     lpFileInformation->nFileIndexLow = (DWORD)(uintptr_t)hFile;
-    if (hFile == SourceHandle) {
+    if (argsHandle.IsFileKind()) {
+      IncludedFile &file = HandleToIncludedFile(hFile);
       lpFileInformation->dwFileAttributes = FILE_ATTRIBUTE_NORMAL;
-      lpFileInformation->nFileSizeLow = m_pSource->GetBufferSize();
+      lpFileInformation->nFileSizeLow = file.Blob->GetBufferSize();
       return TRUE;
     }
-    else if (hFile == OutputHandle) {
+    if (argsHandle == OutputHandle) {
       lpFileInformation->dwFileAttributes = FILE_ATTRIBUTE_NORMAL;
       STATSTG stat;
       HRESULT hr = m_pOutputStream->Stat(&stat, STATFLAG_NONAME);
@@ -467,29 +528,23 @@ public:
       lpFileInformation->nFileSizeLow = stat.cbSize.LowPart;
       return TRUE;
     }
-    else if (IsDirHandle(hFile)) {
+    else if (argsHandle.IsDirHandle()) {
       lpFileInformation->dwFileAttributes = FILE_ATTRIBUTE_DIRECTORY;
       lpFileInformation->nFileIndexHigh = 1;
       return TRUE;
     }
 
-    if (IsHandleIncludedFile(hFile)) {
-      IncludedFile &file = HandleToIncludedFile(hFile);
-      lpFileInformation->dwFileAttributes = FILE_ATTRIBUTE_NORMAL;
-      lpFileInformation->nFileSizeLow = file.Blob->GetBufferSize();
-      return TRUE;
-    }
-
     SetLastError(ERROR_INVALID_HANDLE);
     return FALSE;
   }
 
   __override DWORD GetFileType(_In_ HANDLE hFile) throw() {
-    if (IsStdHandle(hFile)) {
+    DxcArgsHandle argsHandle(hFile);
+    if (argsHandle.IsStdHandle()) {
       return FILE_TYPE_CHAR;
     }
     // Every other known handle is of type disk.
-    if (IsKnownHandle(hFile)) {
+    if (!argsHandle.IsSpecialUnknown()) {
       return FILE_TYPE_DISK;
     }
 
@@ -507,17 +562,11 @@ public:
   }
   __override DWORD GetFileAttributesW(_In_ LPCWSTR lpFileName) throw() {
     DXTRACE_FMT_APIFS("DxcArgsFileSystem::GetFileAttributesW %S\n", lpFileName);
+    std::wstring FileNameStore;
+    MakeAbsoluteOrCurDirRelativeW(lpFileName, FileNameStore);
     size_t sourceNameLen = wcslen(m_pSourceName);
     size_t fileNameLen = wcslen(lpFileName);
 
-    // Check for a substring match to the source, implying a parent directory.
-    if (fileNameLen < sourceNameLen) {
-      if (0 == wcsncmp(m_pSourceName, lpFileName, fileNameLen) ||
-          0 == wcscmp(L".", lpFileName)) {
-        return FILE_ATTRIBUTE_DIRECTORY;
-      }
-    }
-
     // Check for a match to the source.
     if (fileNameLen == sourceNameLen) {
       if (0 == wcsncmp(m_pSourceName, lpFileName, fileNameLen)) {
@@ -631,19 +680,27 @@ public:
     __debugbreak();
   }
 
-  // CRT APIs - handles and file numbers are equivalent.
+  // CRT APIs - handles and file numbers can be mapped directly.
+  HANDLE HandleFromFD(int fd) const {
+    if (fd == STDOUT_FILENO) return StdOutHandle.Handle;
+    if (fd == STDERR_FILENO) return StdErrHandle.Handle;
+    return (HANDLE)(uintptr_t)(fd);
+  }
   __override int open_osfhandle(intptr_t osfhandle, int flags) throw() {
-    return osfhandle;
+    DxcArgsHandle H((HANDLE)osfhandle);
+    if (H == StdOutHandle.Handle) return STDOUT_FILENO;
+    if (H == StdErrHandle.Handle) return STDERR_FILENO;
+    return (int)(intptr_t)H.Handle;
   }
   __override intptr_t get_osfhandle(int fd) throw() {
-    return fd;
+    return (intptr_t)HandleFromFD(fd);
   }
   __override int close(int fd) throw() {
     return 0;
   }
   __override long lseek(int fd, long offset, int origin) throw() {
     CComPtr<IStream> stream;
-    GetStreamForHandle((HANDLE)(uintptr_t)fd, &stream);
+    GetStreamForFD(fd, &stream);
     if (stream == nullptr) {
       errno = EBADF;
       return -1;
@@ -669,7 +726,7 @@ public:
   }
   __override int Read(int fd, _Out_bytecap_(count) void* buffer, unsigned int count) throw() {
     CComPtr<IStream> stream;
-    GetStreamForHandle((HANDLE)(uintptr_t)fd, &stream);
+    GetStreamForFD(fd, &stream);
     if (stream == nullptr) {
       errno = EBADF;
       return -1;
@@ -686,7 +743,7 @@ public:
   }
   __override int Write(int fd, _In_bytecount_(count) const void* buffer, unsigned int count) throw() {
     CComPtr<IStream> stream;
-    GetStreamForHandle((HANDLE)(uintptr_t)fd, &stream);
+    GetStreamForFD(fd, &stream);
     if (stream == nullptr) {
       errno = EBADF;
       return -1;
@@ -730,7 +787,7 @@ static void CreateOperationResultFromOutputs(
     _COM_Outptr_ IDxcOperationResult **ppResult) {
   CComPtr<IStream> pErrorStream;
   CComPtr<IDxcBlobEncoding> pErrorBlob;
-  msfPtr->GetStreamForHandle(StdOutHandle, &pErrorStream);
+  msfPtr->GetStreamForHandle(StdOutHandle.Handle, &pErrorStream);
   if (pErrorStream != nullptr) {
     CComPtr<IDxcBlob> pErrorStreamBlob;
     IFT(pErrorStream.QueryInterface(&pErrorStreamBlob));
@@ -2141,13 +2198,18 @@ public:
         llvm::LLVMContext llvmContext;
         EmitBCAction action(&llvmContext);
         FrontendInputFile file(utf8SourceName.m_psz, IK_HLSL);
-        action.BeginSourceFile(compiler, file);
-        action.Execute();
-        action.EndSourceFile();
+        bool compileOK;
+        if (action.BeginSourceFile(compiler, file)) {
+          action.Execute();
+          action.EndSourceFile();
+          compileOK = !compiler.getDiagnostics().hasErrorOccurred();
+        }
+        else {
+          compileOK = false;
+        }
         outStream.flush();
 
         // Don't do work to put in a container if an error has occurred
-        bool compileOK = !compiler.getDiagnostics().hasErrorOccurred();
         if (compileOK) {
           HRESULT valHR = S_OK;
 
@@ -2324,9 +2386,10 @@ public:
 
       FrontendInputFile file(utf8SourceName.m_psz, IK_HLSL);
       clang::PrintPreprocessedAction action;
-      action.BeginSourceFile(compiler, file);
-      action.Execute();
-      action.EndSourceFile();
+      if (action.BeginSourceFile(compiler, file)) {
+        action.Execute();
+        action.EndSourceFile();
+      }
       outStream.flush();
 
       // Add std err to warnings.

+ 33 - 0
tools/clang/unittests/HLSL/CompilerTest.cpp

@@ -286,6 +286,7 @@ public:
   TEST_METHOD(CompileWhenIncludeSystemMissingThenLoadAttempt)
   TEST_METHOD(CompileWhenIncludeFlagsThenIncludeUsed)
   TEST_METHOD(CompileWhenIncludeMissingThenFail)
+  TEST_METHOD(CompileWhenIncludeHasPathThenOK)
 
   TEST_METHOD(CompileWhenODumpThenPassConfig)
   TEST_METHOD(CompileWhenODumpThenOptimizerMatch)
@@ -1769,6 +1770,38 @@ TEST_F(CompilerTest, CompileWhenIncludeMissingThenFail) {
   VERIFY_FAILED(hr);
 }
 
+TEST_F(CompilerTest, CompileWhenIncludeHasPathThenOK) {
+  CComPtr<IDxcCompiler> pCompiler;
+  LPCWSTR Source = L"c:\\temp\\OddIncludes\\main.hlsl";
+  LPCWSTR Args[] = { L"/I", L"c:\\temp" };
+  LPCWSTR ArgsUp[] = { L"/I", L"c:\\Temp" };
+  VERIFY_SUCCEEDED(CreateCompiler(&pCompiler));
+  bool useUpValues[] = { false, true };
+  for (bool useUp : useUpValues) {
+    CComPtr<IDxcOperationResult> pResult;
+    CComPtr<IDxcBlobEncoding> pSource;
+#if TEST_ON_DISK
+    CComPtr<IDxcLibrary> pLibrary;
+    VERIFY_SUCCEEDED(m_dllSupport.CreateInstance(CLSID_DxcLibrary, &pLibrary));
+    VERIFY_SUCCEEDED(pLibrary->CreateIncludeHandler(&pInclude));
+    VERIFY_SUCCEEDED(pLibrary->CreateBlobFromFile(Source, nullptr, &pSource));
+#else
+    CComPtr<TestIncludeHandler> pInclude;
+    pInclude = new TestIncludeHandler(m_dllSupport);
+    pInclude->CallResults.emplace_back("// Empty");
+    CreateBlobFromText("#include \"include.hlsl\"\r\n"
+                       "float4 main() : SV_Target { return 0; }",
+                       &pSource);
+#endif
+
+    VERIFY_SUCCEEDED(pCompiler->Compile(pSource, Source, L"main",
+      L"ps_6_0", useUp ? ArgsUp : Args, _countof(Args), nullptr, 0, pInclude, &pResult));
+    HRESULT hr;
+    VERIFY_SUCCEEDED(pResult->GetStatus(&hr));
+    VERIFY_SUCCEEDED(hr);
+ }
+}
+
 static const char EmptyCompute[] = "[numthreads(8,8,1)] void main() { }";
 
 TEST_F(CompilerTest, CompileWhenODumpThenPassConfig) {

+ 12 - 2
utils/hct/hcttest.cmd

@@ -9,7 +9,10 @@ set TEST_CLANG=1
 set TEST_EXEC=1
 set TEST_CLANG_VERIF=0
 set TEST_EXTRAS=1
-set BUILD_CONFIG=Debug
+
+if "%BUILD_CONFIG%"=="" (
+  set BUILD_CONFIG=Debug
+)
 
 if "%1"=="clean" (
   set TEST_CLEAN=1
@@ -175,7 +178,7 @@ exit /b 0
 :showhelp
 
 echo Usage:
-echo   hcttest [target]
+echo   hcttest [-rel] [-arm or -x86 or -x64] [target]
 echo.
 echo target can be empty or a specific subset.
 echo.
@@ -185,6 +188,13 @@ echo 'clang' will only run clang tests.
 echo 'exec' will only run execution tests.
 echo 'v' will run the clang tests that are verified-based.
 echo.
+echo   -rel builds release rather than debug
+echo.
+echo current BUILD_ARCH=%BUILD_ARCH%.  Override with:
+echo   -x86 targets an x86 build (aka. Win32)
+echo   -x64 targets an x64 build (aka. Win64)
+echo   -arm targets an ARM build
+echo.
 echo Use the HCT_EXTRAS environment variable to add hcttest-before and hcttest-after hooks.
 echo.
 call :showtesample clang-hlsl-tests.dll /p:"HlslDataDir=%HLSL_SRC_DIR%\tools\clang\test\HLSL"