Fix crash when printing instructions that have a metadata attached but no parent.

[amaiorano]

Applying patch from upstream LLVM:
llvm/llvm-project@b9b50aa#diff-69d28d03f6d27b8c800ff1dafb34ede6999a3165c109263e7da48dbbdf964f0aR9

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff d39324e0635130e834a68e33b0c603cf5fc9fb4f 69a47ca153f85af8bfcf53a5d03551e14933a561 -- unittests/IR/AsmWriterTest.cpp lib/IR/AsmWriter.cpp

View the diff from clang-format here.

diff --git a/unittests/IR/AsmWriterTest.cpp b/unittests/IR/AsmWriterTest.cpp
index c7e7bb5c..4cbfce47 100644
--- a/unittests/IR/AsmWriterTest.cpp
+++ b/unittests/IR/AsmWriterTest.cpp
@@ -6,8 +6,8 @@
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
-#include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Function.h"
+#include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Module.h"
@@ -34,4 +34,4 @@ TEST(AsmWriterTest, DebugPrintDetachedInstruction) {
   EXPECT_TRUE(r != std::string::npos);
 }
 
-}
+} // namespace

• Check this box to apply formatting changes to this branch.

[amaiorano]

To reproduce this, build dxc in debug (so that NDEBUG is not defined) with sanitizers enabled, then compile this shader:

void main() {
    while(true) {
        if (true) {
            break;
        }
        discard;
    }
}

My result:

$ ./bin/dxc -T ps_6_6 test.hlsl 
While deleting: void (i32, float)* %dx.hl.op..void (i32, float)
Use still stuck around after Def is destroyed:  call void @"dx.hl.op..void (i32, float)"(i32 120, float -1.000000e+00)/home/amaiorano/src/external/DirectXShaderCompiler/lib/IR/AsmWriter.cpp:3052:16: runtime error: member call on null pointer of type 'llvm::Module'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /<snip>/src/external/DirectXShaderCompiler/lib/IR/AsmWriter.cpp:3052:16 in 

With the fix here:

$ ./bin/dxc -T ps_6_6 test.hlsl 
While deleting: void (i32, float)* %dx.hl.op..void (i32, float)
Use still stuck around after Def is destroyed:  call void @"dx.hl.op..void (i32, float)"(i32 120, float -1.000000e+00), !dbg <0x503000001e18>
Error: assert(use_empty() && "Uses remain when a value is destroyed!")
File:
/<snip>/src/external/DirectXShaderCompiler/lib/IR/Value.cpp(83)
Func:   ~Value
Illegal instruction (core dumped)

This only happens when !defined(NDEBUG ) (usually debug builds) because we enter this code in the Value dtor:

Value::~Value() {
...
#ifndef NDEBUG      // Only in -g mode...
  // Check to make sure that there are no uses of this value that are still
  // around when the value is destroyed.  If there are, then we have a dangling
  // reference and something is wrong.  This code is here to print out where
  // the value is still being referenced.
  //
  if (!use_empty()) {
    dbgs() << "While deleting: " << *VTy << " %" << getName() << "\n";
    for (auto *U : users())
      dbgs() << "Use still stuck around after Def is destroyed:" << *U << "\n";
  }
#endif
...
}

During the call to operator<<, we end up in Value::print() here:

void Value::print(raw_ostream &ROS, ModuleSlotTracker &MST) const {
...
  if (const Instruction *I = dyn_cast<Instruction>(this)) {
    incorporateFunction(I->getParent() ? I->getParent()->getParent() : nullptr);
    AssemblyWriter W(OS, SlotTable, getModuleFromVal(I), nullptr);
    W.printInstruction(*I);
  } else if (const BasicBlock *BB = dyn_cast<BasicBlock>(this)) {

The call to getModuleFromVal(I) returns nullptr for the current Value, so the AssemblyWriter is created with a nullptr Module, stored in member AssemblyWriter::TheModule. Then, finally, while printing the instruction, we ultimately reach a point where the AssemblyWriter assumes the TheModule is valid, but it isn't:

  if (MDNames.empty())
    TheModule->getMDKindNames(MDNames);

And we get a nullptr deref in getMDKindNames:

void Module::getMDKindNames(SmallVectorImpl<StringRef> &Result) const {
  return Context.getMDKindNames(Result);
}

Knowing the problem, I found a fix that was landed in upstream LLVM after DXC was forked: llvm/llvm-project@b9b50aa, which I have backported here.

Note that this fixes the nullptr crash, but there's still a separate bug in DXC that needs fixing.