Fixed entry point omitted from the PDB when it's the same as input filename (#5236)

The args in the PDB and debug DXIL are based CodeGenOptions::HLSLArgs. This list was created by copying the raw unprocessed arg list and excluding any args equal to the input file name. When the entry point happens to be the same as the input filename, it would get excluded from this list and the PDB and debug module end up with corrupted arg list where -E has no value or wrong value.

Fixed by only excluding args that are InputClass.

tools/clang/tools/dxcompiler/dxcompilerobj.cpp

@@ -1571,15 +1571,31 @@ public:
     else
       compiler.getCodeGenOpts().HLSLSignaturePackingStrategy = (unsigned)DXIL::PackingStrategy::Default;
 
-    // Constructing vector of wide strings to pass in to codegen. Just passing
+    // Constructing vector of strings to pass in to codegen. Just passing
     // in pArguments will expose ownership of memory to both CodeGenOptions and
     // this caller, which can lead to unexpected behavior.
-    for (UINT32 i = 0; i != Opts.Args.getNumInputArgStrings(); ++i) {
-      auto arg = Opts.Args.getArgString(i);
-      if (Opts.InputFile.compare(arg) != 0) {
-        compiler.getCodeGenOpts().HLSLArguments.emplace_back(arg);
+    {
+      // Find all args that are of Option::InputClass and record their indices
+      // in a set. If there are multiple Option::InputClass arguments, exclude
+      // all of them. We only use the last one and there's no point recording
+      // the rest of them.
+      //
+      // This list is used to populate the argument list in debug module and
+      // PDB, which are for recompiling. The input filenames are not needed for
+      // it and should be excluded.
+      llvm::DenseSet<unsigned> InputArgIndices;
+      for (llvm::opt::Arg *arg : Opts.Args.getArgs()) {
+        if (arg->getOption().getKind() == llvm::opt::Option::InputClass)
+          InputArgIndices.insert(arg->getIndex());
+      }
+      for (unsigned i = 0; i < Opts.Args.getNumInputArgStrings(); ++i) {
+        if (InputArgIndices.count(i) == 0) { // Only include this arg if it's not in the set of Option::InputClass args.
+          StringRef argStr = Opts.Args.getArgString(i);
+          compiler.getCodeGenOpts().HLSLArguments.emplace_back(argStr);
+        }
       }
     }
+
     // Overrding default set of loop unroll.
     if (Opts.PreferFlowControl)
       compiler.getCodeGenOpts().UnrollLoops = false;

tools/clang/unittests/HLSL/CompilerTest.cpp

@@ -146,6 +146,7 @@ public:
   TEST_METHOD(CompileThenTestPdbUtilsStripped)
   TEST_METHOD(CompileThenTestPdbUtilsEmptyEntry)
   TEST_METHOD(CompileThenTestPdbUtilsRelativePath)
+  TEST_METHOD(CompileSameFilenameAndEntryThenTestPdbUtilsArgs)
   TEST_METHOD(CompileWithRootSignatureThenStripRootSignature)
   TEST_METHOD(CompileThenSetRootSignatureThenValidate)
   TEST_METHOD(CompileSetPrivateThenWithStripPrivate)
@@ -1967,6 +1968,85 @@ TEST_F(CompilerTest, CompileThenTestPdbUtilsRelativePath) {
   VERIFY_SUCCEEDED(pPdbUtils->Load(pPdb));
 }
 
+TEST_F(CompilerTest, CompileSameFilenameAndEntryThenTestPdbUtilsArgs) {
+  // This is a regression test for a bug where if entry point has the same
+  // value as the input filename, the entry point gets omitted from the arg
+  // list in debug module and PDB, making them useless for recompilation.
+  CComPtr<IDxcCompiler> pCompiler;
+  VERIFY_SUCCEEDED(CreateCompiler(&pCompiler));
+
+  std::wstring shader = LR"x(
+    [RootSignature("")] float PSMain() : SV_Target {
+      return 0;
+    }
+  )x";
+
+  CComPtr<IDxcUtils> pUtils;
+  VERIFY_SUCCEEDED(DxcCreateInstance(CLSID_DxcUtils, IID_PPV_ARGS(&pUtils)));
+
+  CComPtr<IDxcOperationResult> pOpResult;
+
+  std::wstring EntryPoint = L"PSMain";
+  CComPtr<IDxcBlobEncoding> pShaderBlob;
+  VERIFY_SUCCEEDED(pUtils->CreateBlob(shader.data(), shader.size() * sizeof(shader[0]), DXC_CP_UTF16, &pShaderBlob));
+
+  const WCHAR *OtherInputs[] = {
+    L"AnotherInput1",
+    L"AnotherInput2",
+    L"AnotherInput3",
+    L"AnotherInput4",
+  };
+
+  const WCHAR *Args[] = {
+    OtherInputs[0], OtherInputs[1],
+    L"-Od",
+    OtherInputs[2],
+    L"-Zi",
+    OtherInputs[3],
+  };
+  VERIFY_SUCCEEDED(pCompiler->Compile(pShaderBlob, EntryPoint.c_str(), EntryPoint.c_str(), L"ps_6_0", Args, _countof(Args), nullptr, 0, nullptr, &pOpResult));
+
+  HRESULT compileStatus = S_OK;
+  VERIFY_SUCCEEDED(pOpResult->GetStatus(&compileStatus));
+  VERIFY_SUCCEEDED(compileStatus);
+
+  CComPtr<IDxcBlob> pDxil;
+  VERIFY_SUCCEEDED(pOpResult->GetResult(&pDxil));
+  CComPtr<IDxcResult> pResult;
+  VERIFY_SUCCEEDED(pOpResult.QueryInterface(&pResult));
+  CComPtr<IDxcBlob> pPdb;
+  VERIFY_SUCCEEDED(pResult->GetOutput(DXC_OUT_PDB, IID_PPV_ARGS(&pPdb), nullptr));
+
+  IDxcBlob *PdbLikes[] = {
+    pDxil, pPdb,
+  };
+
+  for (IDxcBlob *pPdbLike : PdbLikes) {
+    CComPtr<IDxcPdbUtils2> pPdbUtils;
+    VERIFY_SUCCEEDED(DxcCreateInstance(CLSID_DxcPdbUtils, IID_PPV_ARGS(&pPdbUtils)));
+    VERIFY_SUCCEEDED(pPdbUtils->Load(pPdbLike));
+
+    CComPtr<IDxcBlobWide> pEntryPoint;
+    VERIFY_SUCCEEDED(pPdbUtils->GetEntryPoint(&pEntryPoint));
+    VERIFY_IS_NOT_NULL(pEntryPoint);
+    VERIFY_ARE_EQUAL(std::wstring(pEntryPoint->GetStringPointer(), pEntryPoint->GetStringLength()), EntryPoint);
+
+    std::set<std::wstring> ArgSet;
+    UINT uNumArgs = 0;
+    VERIFY_SUCCEEDED(pPdbUtils->GetArgCount(&uNumArgs));
+    for (UINT i = 0; i < uNumArgs; i++) {
+      CComPtr<IDxcBlobWide> pArg;
+      VERIFY_SUCCEEDED(pPdbUtils->GetArg(i, &pArg));
+      ArgSet.insert(std::wstring(pArg->GetStringPointer(), pArg->GetStringLength()));
+    }
+
+    for (const WCHAR *OtherInputs : OtherInputs) {
+      VERIFY_ARE_EQUAL(ArgSet.end(), ArgSet.find(OtherInputs));
+    }
+    VERIFY_ARE_NOT_EQUAL(ArgSet.end(), ArgSet.find(L"-Od"));
+    VERIFY_ARE_NOT_EQUAL(ArgSet.end(), ArgSet.find(L"-Zi"));
+  }
+}
 
 TEST_F(CompilerTest, CompileThenTestPdbUtilsEmptyEntry) {
   std::string main_source = R"x(