Fix concurrent dxilconv pass initialization

docs/ReleaseNotes.md

@@ -24,6 +24,8 @@ line upon naming the release. Refer to previous for appropriate section names.
 
 #### Bug Fixes
 
+- Fixed a race that could crash concurrent first-time DXBC-to-DXIL conversions
+  [#8819](https://github.com/microsoft/DirectXShaderCompiler/issues/8819).
 - Fixed derivative operations being moved into divergent control flow, which
   could produce incorrect results
   [#8001](https://github.com/microsoft/DirectXShaderCompiler/issues/8001).

projects/dxilconv/include/DxilConvPasses/InitializePasses.h

@@ -0,0 +1,22 @@
+///////////////////////////////////////////////////////////////////////////////
+//                                                                           //
+// InitializePasses.h                                                        //
+// Copyright (C) Microsoft Corporation. All rights reserved.                 //
+// This file is distributed under the University of Illinois Open Source     //
+// License. See LICENSE.TXT for details.                                      //
+//                                                                           //
+///////////////////////////////////////////////////////////////////////////////
+
+#pragma once
+
+#include "dxc/Support/Global.h"
+
+namespace llvm {
+class PassRegistry;
+}
+
+void __cdecl initializeDxilConvPasses(llvm::PassRegistry &Registry);
+
+namespace hlsl {
+HRESULT SetupRegistryPassForDxilConvPasses();
+}

projects/dxilconv/lib/DxilConvPasses/DxilCleanup.cpp

@@ -1452,6 +1452,8 @@ char &llvm::DxilCleanupID = DxilCleanup::ID;
 
 INITIALIZE_PASS_BEGIN(DxilCleanup, "dxil-cleanup",
                       "Optimize DXIL after conversion from DXBC", true, false)
+INITIALIZE_PASS_DEPENDENCY(DCE)
+INITIALIZE_PASS_DEPENDENCY(PromotePass)
 INITIALIZE_PASS_END(DxilCleanup, "dxil-cleanup",
                     "Optimize DXIL after conversion from DXBC", true, false)
 

projects/dxilconv/lib/DxilConvPasses/InitializePasses.cpp

@@ -10,19 +10,25 @@
 //                                                                           //
 ///////////////////////////////////////////////////////////////////////////////
 
+#include "DxilConvPasses/InitializePasses.h"
 #include "DxilConvPasses/DxilCleanup.h"
 #include "DxilConvPasses/NormalizeDxil.h"
 #include "DxilConvPasses/ScopeNestInfo.h"
 #include "DxilConvPasses/ScopeNestedCFG.h"
 #include "dxc/Support/Global.h"
 #include "dxc/Support/WinIncludes.h"
 
+#include "llvm/Analysis/ReducibilityAnalysis.h"
+#include "llvm/InitializePasses.h"
 #include "llvm/PassRegistry.h"
 
 using namespace llvm;
 
 // Place to put our private pass initialization for opt.exe.
 void __cdecl initializeDxilConvPasses(PassRegistry &Registry) {
+  initializeDCEPass(Registry);
+  initializePromotePassPass(Registry);
+  initializeReducibilityAnalysisPass(Registry);
   initializeScopeNestedCFGPass(Registry);
   initializeScopeNestInfoWrapperPassPass(Registry);
   initializeNormalizeDxilPassPass(Registry);

projects/dxilconv/tools/dxilconv/dxilconv.cpp

@@ -26,6 +26,7 @@
 #include "dxcetw.h"
 
 #include "DxbcConverter.h"
+#include "DxilConvPasses/InitializePasses.h"
 
 // Defined in DxbcConverter.lib
 // (projects/dxilconv/lib/DxbcConverter/DxbcConverter.cpp)
@@ -87,16 +88,21 @@ DXC_API_IMPORT HRESULT __stdcall DxcCreateInstance2(IMalloc *pMalloc,
 // __declspec(nothrow)
 static HRESULT InitMaybeFail() throw() {
   HRESULT hr;
-  bool memSetup = false;
+  bool fsSetup = false, memSetup = false;
   IFC(DxcInitThreadMalloc());
   DxcSetThreadMallocToDefault();
   memSetup = true;
   if (::llvm::sys::fs::SetupPerThreadFileSystem()) {
     hr = E_FAIL;
     goto Cleanup;
   }
+  fsSetup = true;
+  IFC(hlsl::SetupRegistryPassForDxilConvPasses());
 Cleanup:
   if (FAILED(hr)) {
+    if (fsSetup) {
+      ::llvm::sys::fs::CleanupPerThreadFileSystem();
+    }
     if (memSetup) {
       DxcClearThreadMalloc();
       DxcCleanupThreadMalloc();

projects/dxilconv/unittests/CMakeLists.txt

@@ -2,8 +2,31 @@
 # This file is distributed under the University of Illinois Open Source License. See LICENSE.TXT for details.
 
 find_package(TAEF REQUIRED)
+find_package(D3D12 REQUIRED)
 #find_package(DiaSDK REQUIRED) # Used for constants and declarations.
 
+set(LLVM_OPTIONAL_SOURCES
+  DxilConvPassRegistryTestHelper.cpp
+  DxilConvTests.cpp
+)
+
+add_dxilconv_project_executable(dxilconv-pass-registry-test-helper
+  DxilConvPassRegistryTestHelper.cpp
+)
+
+target_link_libraries(dxilconv-pass-registry-test-helper PRIVATE
+  DxilConvPasses
+  LLVMAnalysis
+  LLVMCore
+  LLVMScalarOpts
+  LLVMSupport
+  ${D3D12_LIBRARIES}
+)
+
+target_include_directories(dxilconv-pass-registry-test-helper PRIVATE
+  ${D3D12_INCLUDE_DIRS}
+)
+
 add_dxilconv_project_test_library(dxilconv-tests SHARED
   DxilConvTests.cpp
 )
@@ -26,11 +49,15 @@ target_include_directories(dxilconv-tests PRIVATE
 # dxilconv-tests calls out to several external tools. Those need to be listed
 # here to ensure they build with dxilconv for our test target depencencies to be
 # correct.
-add_dependencies(dxilconv-tests dxilconv HLSLTestLib dxbc2dxil dxa opt)
+add_dependencies(dxilconv-tests dxilconv dxilconv-pass-registry-test-helper
+                 HLSLTestLib dxbc2dxil dxa opt)
 
 install(TARGETS dxilconv-tests
   RUNTIME DESTINATION bin)
 
+install(TARGETS dxilconv-pass-registry-test-helper
+  RUNTIME DESTINATION bin)
+
 # Add a .user file with settings for te.exe.
 file(TO_NATIVE_PATH "${CMAKE_CURRENT_SOURCE_DIR}" DOS_STYLE_SOURCE_DIR)
 file(TO_NATIVE_PATH "${TAEF_BIN_DIR}" DOS_TAEF_BIN_DIR)

projects/dxilconv/unittests/DxilConvPassRegistryTestHelper.cpp

@@ -0,0 +1,179 @@
+///////////////////////////////////////////////////////////////////////////////
+//                                                                           //
+// DxilConvPassRegistryTestHelper.cpp                                         //
+// Copyright (C) Microsoft Corporation. All rights reserved.                 //
+// This file is distributed under the University of Illinois Open Source     //
+// License. See LICENSE.TXT for details.                                      //
+//                                                                           //
+///////////////////////////////////////////////////////////////////////////////
+
+#include "dxc/Support/WinIncludes.h"
+
+#include "DxbcConverter.h"
+#include "DxilConvPasses/InitializePasses.h"
+#include "llvm/PassRegistry.h"
+
+#include <atlbase.h>
+#include <d3dcompiler.h>
+
+#include <atomic>
+#include <cstdio>
+#include <string>
+#include <thread>
+#include <vector>
+
+static int VerifyPassRegistration() {
+  HRESULT Result = hlsl::SetupRegistryPassForDxilConvPasses();
+  if (FAILED(Result))
+    return 1;
+
+  llvm::PassRegistry *Registry = llvm::PassRegistry::getPassRegistry();
+  const char *RequiredPasses[] = {
+      "dce",
+      "mem2reg",
+      "assumption-cache-tracker",
+      "red",
+      "loops",
+      "domtree",
+      "dxil-cleanup",
+      "normalizedxil",
+      "scopenested",
+      "scopenestinfo",
+  };
+  for (const char *PassName : RequiredPasses) {
+    if (!Registry->getPassInfo(llvm::StringRef(PassName))) {
+      std::fprintf(stderr, "Pass was not registered: %s\n", PassName);
+      return 1;
+    }
+  }
+  return 0;
+}
+
+static std::wstring GetDxilConvPath() {
+  wchar_t Path[MAX_PATH];
+  DWORD Length = GetModuleFileNameW(nullptr, Path, _countof(Path));
+  if (Length == 0 || Length == _countof(Path))
+    return {};
+
+  std::wstring Result(Path, Length);
+  size_t Separator = Result.find_last_of(L"\\/");
+  if (Separator == std::wstring::npos)
+    return {};
+  Result.resize(Separator + 1);
+  Result += L"dxilconv.dll";
+  return Result;
+}
+
+static bool CompileShaders(std::vector<CComPtr<ID3DBlob>> &Shaders) {
+  for (size_t Index = 0; Index < Shaders.size(); ++Index) {
+    std::string Source =
+        "float4 main(float4 position : SV_Position) : SV_Target {"
+        " return float4(position.x + " +
+        std::to_string(Index) + ".0f, position.y, 0.0f, 1.0f); }";
+    CComPtr<ID3DBlob> Errors;
+    HRESULT Result =
+        D3DCompile(Source.data(), Source.size(), "concurrent-init.hlsl", nullptr,
+                   nullptr, "main", "ps_5_0", D3DCOMPILE_OPTIMIZATION_LEVEL3, 0,
+                   &Shaders[Index], &Errors);
+    if (FAILED(Result)) {
+      if (Errors)
+        std::fwrite(Errors->GetBufferPointer(), 1, Errors->GetBufferSize(),
+                    stderr);
+      return false;
+    }
+  }
+  return true;
+}
+
+static int RunConcurrentConversions() {
+  constexpr unsigned WorkerCount = 256;
+  std::vector<CComPtr<ID3DBlob>> Shaders(WorkerCount);
+  if (!CompileShaders(Shaders))
+    return 1;
+
+  std::wstring DxilConvPath = GetDxilConvPath();
+  if (DxilConvPath.empty())
+    return 1;
+
+  HMODULE DxilConv = LoadLibraryW(DxilConvPath.c_str());
+  if (!DxilConv)
+    return 1;
+
+  auto CreateInstance = reinterpret_cast<DxcCreateInstanceProc>(
+      GetProcAddress(DxilConv, "DxcCreateInstance"));
+  if (!CreateInstance) {
+    FreeLibrary(DxilConv);
+    return 1;
+  }
+
+  HANDLE ReadyEvent = CreateEventW(nullptr, TRUE, FALSE, nullptr);
+  HANDLE StartEvent = CreateEventW(nullptr, TRUE, FALSE, nullptr);
+  if (!ReadyEvent || !StartEvent) {
+    if (ReadyEvent)
+      CloseHandle(ReadyEvent);
+    if (StartEvent)
+      CloseHandle(StartEvent);
+    FreeLibrary(DxilConv);
+    return 1;
+  }
+
+  std::atomic<unsigned> ReadyCount{0};
+  std::vector<HRESULT> Results(WorkerCount, E_PENDING);
+  std::vector<std::thread> Workers;
+  Workers.reserve(WorkerCount);
+
+  for (unsigned Index = 0; Index < WorkerCount; ++Index) {
+    Workers.emplace_back([&, Index]() {
+      IDxbcConverter *RawConverter = nullptr;
+      HRESULT Result = CreateInstance(
+          CLSID_DxbcConverter, __uuidof(IDxbcConverter),
+          reinterpret_cast<void **>(&RawConverter));
+      CComPtr<IDxbcConverter> Converter;
+      Converter.Attach(RawConverter);
+
+      if (ReadyCount.fetch_add(1, std::memory_order_release) + 1 == WorkerCount)
+        SetEvent(ReadyEvent);
+      WaitForSingleObject(StartEvent, INFINITE);
+
+      if (SUCCEEDED(Result)) {
+        void *Dxil = nullptr;
+        UINT32 DxilSize = 0;
+        LPWSTR Diagnostics = nullptr;
+        Result = Converter->Convert(
+            Shaders[Index]->GetBufferPointer(),
+            static_cast<UINT32>(Shaders[Index]->GetBufferSize()), nullptr, &Dxil,
+            &DxilSize, &Diagnostics);
+        CoTaskMemFree(Dxil);
+        CoTaskMemFree(Diagnostics);
+      }
+      Results[Index] = Result;
+    });
+  }
+
+  DWORD ReadyResult = WaitForSingleObject(ReadyEvent, 60000);
+  SetEvent(StartEvent);
+  for (std::thread &Worker : Workers)
+    Worker.join();
+
+  CloseHandle(StartEvent);
+  CloseHandle(ReadyEvent);
+  FreeLibrary(DxilConv);
+
+  if (ReadyResult != WAIT_OBJECT_0)
+    return 1;
+  for (HRESULT Result : Results) {
+    if (FAILED(Result))
+      return 1;
+  }
+  return 0;
+}
+
+int __cdecl wmain(int ArgCount, wchar_t **Arguments) {
+  if (ArgCount != 2)
+    return 1;
+  if (wcscmp(Arguments[1], L"--verify-pass-registration") == 0)
+    return VerifyPassRegistration();
+  if (wcscmp(Arguments[1], L"--concurrent-conversion") == 0)
+    return RunConcurrentConversions();
+  return 1;
+}

projects/dxilconv/unittests/DxilConvTests.cpp

@@ -168,6 +168,71 @@ class DxilConvTest {
   }
 };
 
+class DxilConvPassRegistryTest {
+public:
+  BEGIN_TEST_CLASS(DxilConvPassRegistryTest)
+  TEST_CLASS_PROPERTY(L"Parallel", L"false")
+  TEST_METHOD_PROPERTY(L"Priority", L"0")
+  END_TEST_CLASS()
+
+  TEST_METHOD(PassDependenciesRegistered);
+  TEST_METHOD(ConcurrentInitialization);
+
+private:
+  static DWORD RunHelper(LPCWSTR Arguments) {
+    HMODULE TestModule = GetModuleHandleW(L"dxilconv-tests.dll");
+    IFTBOOL(TestModule != nullptr, HRESULT_FROM_WIN32(GetLastError()));
+
+    wchar_t ModulePath[MAX_PATH];
+    DWORD Length =
+        GetModuleFileNameW(TestModule, ModulePath, _countof(ModulePath));
+    IFTBOOL(Length != 0 && Length != _countof(ModulePath),
+            HRESULT_FROM_WIN32(GetLastError()));
+
+    std::wstring HelperPath(ModulePath, Length);
+    size_t Separator = HelperPath.find_last_of(L"\\/");
+    IFTBOOL(Separator != std::wstring::npos, E_FAIL);
+    HelperPath.resize(Separator + 1);
+    HelperPath += L"dxilconv-pass-registry-test-helper.exe";
+
+    std::wstring CommandLine =
+        L"\"" + HelperPath + L"\" " + std::wstring(Arguments);
+    std::vector<wchar_t> MutableCommandLine(CommandLine.begin(),
+                                            CommandLine.end());
+    MutableCommandLine.push_back(L'\0');
+
+    STARTUPINFOW StartupInfo = {};
+    StartupInfo.cb = sizeof(StartupInfo);
+    PROCESS_INFORMATION ProcessInfo = {};
+    IFTBOOL(CreateProcessW(HelperPath.c_str(), MutableCommandLine.data(),
+                           nullptr, nullptr, FALSE, 0, nullptr, nullptr,
+                           &StartupInfo, &ProcessInfo),
+            HRESULT_FROM_WIN32(GetLastError()));
+
+    CloseHandle(ProcessInfo.hThread);
+    DWORD WaitResult = WaitForSingleObject(ProcessInfo.hProcess, 120000);
+    if (WaitResult != WAIT_OBJECT_0) {
+      TerminateProcess(ProcessInfo.hProcess, 1);
+      CloseHandle(ProcessInfo.hProcess);
+      IFT(E_FAIL);
+    }
+
+    DWORD ExitCode = 1;
+    BOOL GotExitCode = GetExitCodeProcess(ProcessInfo.hProcess, &ExitCode);
+    CloseHandle(ProcessInfo.hProcess);
+    IFTBOOL(GotExitCode, HRESULT_FROM_WIN32(GetLastError()));
+    return ExitCode;
+  }
+};
+
+TEST_F(DxilConvPassRegistryTest, PassDependenciesRegistered) {
+  VERIFY_ARE_EQUAL(0u, RunHelper(L"--verify-pass-registration"));
+}
+
+TEST_F(DxilConvPassRegistryTest, ConcurrentInitialization) {
+  VERIFY_ARE_EQUAL(0u, RunHelper(L"--concurrent-conversion"));
+}
+
 bool DxilConvTest::InitSupport() {
   if (!m_dllSupport.IsEnabled()) {
     VERIFY_SUCCEEDED(