Recommit: "[Debugify][Original DI] Test dbg var loc preservation""

Djordje Todorovic · Djordje Todorovic · commit b9076d119a5b · 2021-05-21T02:04:29.000-07:00
[Debugify][Original DI] Test dbg var loc preservation This is an improvement of [0]. This adds checking of original llvm.dbg.values()/declares() instructions in optimizations. We have picked a real issue that has been found with this (actually, picked one variable location missing from [1] and resolved the issue), and the result is the fix for that -- D100844. Before applying the D100844, using the options from [0] (but with this patch applied) on the compilation of GDB 7.11, the final HTML report for the debug-info issues can be found at [1] (please scroll down, and look for "Summary of Variable Location Bugs"). After applying the D100844, the numbers has improved a bit -- please take a look into [2]. [0] https://llvm.org/docs/HowToUpdateDebugInfo.html#\ test-original-debug-info-preservation-in-optimizations [1] https://djolertrk.github.io/di-check-before-adce-fix/ [2] https://djolertrk.github.io/di-check-after-adce-fix/ Differential Revision: https://reviews.llvm.org/D100845 The Unit test was failing because the pass from the test that modifies the IR, in its runOnFunction() didn't return 'true', so the expensive-check configuration triggered an assertion.
diff --git a/llvm/docs/HowToUpdateDebugInfo.rst b/llvm/docs/HowToUpdateDebugInfo.rst
@@ -387,6 +387,9 @@ as follows:
   # Test each pass and export the issues report into the JSON file.
   $ clang -Xclang -fverify-debuginfo-preserve -Xclang -fverify-debuginfo-preserve-export=sample.json -g -O2 sample.c
 
+Please do note that there are some known false positives, for source locations
+and debug intrinsic checking, so that will be addressed as a future work.
+
 Mutation testing for MIR-level transformations
 ----------------------------------------------
 
diff --git a/llvm/include/llvm/Transforms/Utils/Debugify.h b/llvm/include/llvm/Transforms/Utils/Debugify.h
@@ -25,6 +25,7 @@
 
 using DebugFnMap = llvm::DenseMap<llvm::StringRef, const llvm::DISubprogram *>;
 using DebugInstMap = llvm::DenseMap<const llvm::Instruction *, bool>;
+using DebugVarMap = llvm::DenseMap<const llvm::DILocalVariable *, unsigned>;
 using WeakInstValueMap =
     llvm::DenseMap<const llvm::Instruction *, llvm::WeakVH>;
 
@@ -37,6 +38,8 @@ struct DebugInfoPerPass {
   // This tracks value (instruction) deletion. If an instruction gets deleted,
   // WeakVH nulls itself.
   WeakInstValueMap InstToDelete;
+  // Maps variable into dbg users (#dbg values/declares for this variable).
+  DebugVarMap DIVariables;
 };
 
 /// Map pass names to a per-pass DebugInfoPerPass instance.
diff --git a/llvm/lib/Transforms/Utils/Debugify.cpp b/llvm/lib/Transforms/Utils/Debugify.cpp
@@ -304,18 +304,39 @@ bool llvm::collectDebugInfoMetadata(Module &M,
     // Collect the DISubprogram.
     auto *SP = F.getSubprogram();
     DIPreservationMap[NameOfWrappedPass].DIFunctions.insert({F.getName(), SP});
-    if (SP)
+    if (SP) {
       LLVM_DEBUG(dbgs() << "  Collecting subprogram: " << *SP << '\n');
+      for (const DINode *DN : SP->getRetainedNodes()) {
+        if (const auto *DV = dyn_cast<DILocalVariable>(DN)) {
+          DIPreservationMap[NameOfWrappedPass].DIVariables[DV] = 0;
+        }
+      }
+    }
 
     for (BasicBlock &BB : F) {
-      // Collect debug locations (!dbg).
-      // TODO: Collect dbg.values.
+      // Collect debug locations (!dbg) and debug variable intrinsics.
       for (Instruction &I : BB) {
         // Skip PHIs.
         if (isa<PHINode>(I))
           continue;
 
-        // Skip debug instructions.
+        // Collect dbg.values and dbg.declares.
+        if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&I)) {
+          if (!SP)
+            continue;
+          // Skip inlined variables.
+          if (I.getDebugLoc().getInlinedAt())
+            continue;
+          // Skip undef values.
+          if (DVI->isUndef())
+            continue;
+
+          auto *Var = DVI->getVariable();
+          DIPreservationMap[NameOfWrappedPass].DIVariables[Var]++;
+          continue;
+        }
+
+        // Skip debug instructions other than dbg.value and dbg.declare.
         if (isa<DbgInfoIntrinsic>(&I))
           continue;
 
@@ -435,6 +456,39 @@ static bool checkInstructions(const DebugInstMap &DILocsBefore,
   return Preserved;
 }
 
+// This checks the preservation of original debug variable intrinsics.
+static bool checkVars(const DebugVarMap &DIFunctionsBefore,
+                      const DebugVarMap &DIFunctionsAfter,
+                      StringRef NameOfWrappedPass, StringRef FileNameFromCU,
+                      bool ShouldWriteIntoJSON, llvm::json::Array &Bugs) {
+  bool Preserved = true;
+  for (const auto &V : DIFunctionsBefore) {
+    auto VarIt = DIFunctionsAfter.find(V.first);
+    if (VarIt == DIFunctionsAfter.end())
+      continue;
+
+    unsigned NumOfDbgValsAfter = VarIt->second;
+
+    if (V.second > NumOfDbgValsAfter) {
+      if (ShouldWriteIntoJSON)
+        Bugs.push_back(llvm::json::Object(
+            {{"metadata", "dbg-var-intrinsic"},
+             {"name", V.first->getName()},
+             {"fn-name", V.first->getScope()->getSubprogram()->getName()},
+             {"action", "drop"}}));
+      else
+        dbg() << "WARNING: " << NameOfWrappedPass
+              << " drops dbg.value()/dbg.declare() for " << V.first->getName()
+              << " from "
+              << "function " << V.first->getScope()->getSubprogram()->getName()
+              << " (file " << FileNameFromCU << ")\n";
+      Preserved = false;
+    }
+  }
+
+  return Preserved;
+}
+
 // Write the json data into the specifed file.
 static void writeJSON(StringRef OrigDIVerifyBugsReportFilePath,
                       StringRef FileNameFromCU, StringRef NameOfWrappedPass,
@@ -484,18 +538,40 @@ bool llvm::checkDebugInfoMetadata(Module &M,
     auto *SP = F.getSubprogram();
     DIPreservationAfter[NameOfWrappedPass].DIFunctions.insert(
         {F.getName(), SP});
-    if (SP)
+
+    if (SP) {
       LLVM_DEBUG(dbgs() << "  Collecting subprogram: " << *SP << '\n');
+      for (const DINode *DN : SP->getRetainedNodes()) {
+        if (const auto *DV = dyn_cast<DILocalVariable>(DN)) {
+          DIPreservationAfter[NameOfWrappedPass].DIVariables[DV] = 0;
+        }
+      }
+    }
 
     for (BasicBlock &BB : F) {
-      // Collect debug locations (!dbg attachments).
-      // TODO: Collect dbg.values.
+      // Collect debug locations (!dbg) and debug variable intrinsics.
       for (Instruction &I : BB) {
         // Skip PHIs.
         if (isa<PHINode>(I))
           continue;
 
-        // Skip debug instructions.
+        // Collect dbg.values and dbg.declares.
+        if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&I)) {
+          if (!SP)
+            continue;
+          // Skip inlined variables.
+          if (I.getDebugLoc().getInlinedAt())
+            continue;
+          // Skip undef values.
+          if (DVI->isUndef())
+            continue;
+
+          auto *Var = DVI->getVariable();
+          DIPreservationAfter[NameOfWrappedPass].DIVariables[Var]++;
+          continue;
+        }
+
+        // Skip debug instructions other than dbg.value and dbg.declare.
         if (isa<DbgInfoIntrinsic>(&I))
           continue;
 
@@ -522,6 +598,9 @@ bool llvm::checkDebugInfoMetadata(Module &M,
 
   auto InstToDelete = DIPreservationAfter[NameOfWrappedPass].InstToDelete;
 
+  auto DIVarsBefore = DIPreservationMap[NameOfWrappedPass].DIVariables;
+  auto DIVarsAfter = DIPreservationAfter[NameOfWrappedPass].DIVariables;
+
   bool ShouldWriteIntoJSON = !OrigDIVerifyBugsReportFilePath.empty();
   llvm::json::Array Bugs;
 
@@ -531,7 +610,11 @@ bool llvm::checkDebugInfoMetadata(Module &M,
   bool ResultForInsts = checkInstructions(
       DILocsBefore, DILocsAfter, InstToDelete, NameOfWrappedPass,
       FileNameFromCU, ShouldWriteIntoJSON, Bugs);
-  bool Result = ResultForFunc && ResultForInsts;
+
+  bool ResultForVars = checkVars(DIVarsBefore, DIVarsAfter, NameOfWrappedPass,
+                                 FileNameFromCU, ShouldWriteIntoJSON, Bugs);
+
+  bool Result = ResultForFunc && ResultForInsts && ResultForVars;
 
   StringRef ResultBanner = NameOfWrappedPass != "" ? NameOfWrappedPass : Banner;
   if (ShouldWriteIntoJSON && !Bugs.empty())
diff --git a/llvm/test/tools/llvm-original-di-preservation/Inputs/expected-sample.html b/llvm/test/tools/llvm-original-di-preservation/Inputs/expected-sample.html
@@ -127,5 +127,32 @@
         <td colspan='2'> No bugs found </td>
       </tr>
     </table>
+<br>
+<br>
+<table>
+  <caption><b>Variable Location Bugs found by the Debugify</b></caption>
+  <tr>
+      <th>File</th>
+    <th>LLVM Pass Name</th>
+    <th>Variable</th>
+    <th>Function</th>
+    <th>Action</th>
+  </tr>
+<tr>
+        <td colspan='4'> No bugs found </td>
+      </tr>
+    </table>
+<br>
+<table>
+  <caption><b>Summary of Variable Location Bugs</b></caption>
+  <tr>
+      <th>LLVM Pass Name</th>
+    <th>Number of bugs</th>
+  </tr>
+  <tr>
+<tr>
+        <td colspan='2'> No bugs found </td>
+      </tr>
+    </table>
 </body>
   </html>
diff --git a/llvm/unittests/Transforms/Utils/DebugifyTest.cpp b/llvm/unittests/Transforms/Utils/DebugifyTest.cpp
@@ -6,8 +6,10 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/AsmParser/Parser.h"
 #include "llvm/IR/DebugInfoMetadata.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Transforms/Utils/Debugify.h"
@@ -41,13 +43,39 @@ struct DebugInfoDrop : public FunctionPass {
 
     return false;
   }
+
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesCFG();
   }
 
   DebugInfoDrop() : FunctionPass(ID) {}
 };
 
+struct DebugValueDrop : public FunctionPass {
+  static char ID;
+  bool runOnFunction(Function &F) override {
+    SmallVector<DbgVariableIntrinsic *, 4> Dbgs;
+    for (BasicBlock &BB : F) {
+      // Remove dbg var intrinsics.
+      for (Instruction &I : BB) {
+        if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&I))
+          Dbgs.push_back(DVI);
+      }
+    }
+
+    for (auto &I : Dbgs)
+      I->eraseFromParent();
+
+    return true;
+  }
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.setPreservesCFG();
+  }
+
+  DebugValueDrop() : FunctionPass(ID) {}
+};
+
 struct DebugInfoDummyAnalysis : public FunctionPass {
   static char ID;
   bool runOnFunction(Function &F) override {
@@ -63,6 +91,7 @@ struct DebugInfoDummyAnalysis : public FunctionPass {
 }
 
 char DebugInfoDrop::ID = 0;
+char DebugValueDrop::ID = 0;
 char DebugInfoDummyAnalysis::ID = 0;
 
 TEST(DebugInfoDrop, DropOriginalDebugInfo) {
@@ -116,6 +145,59 @@ TEST(DebugInfoDrop, DropOriginalDebugInfo) {
   EXPECT_TRUE(StdOut.find(FinalResult) != std::string::npos);
 }
 
+TEST(DebugValueDrop, DropOriginalDebugValues) {
+  LLVMContext C;
+  std::unique_ptr<Module> M = parseIR(C, R"(
+    define i16 @f(i16 %a) !dbg !6 {
+      %b = add i16 %a, 1, !dbg !11
+      call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+      ret i16 0, !dbg !11
+    }
+    declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+    !llvm.dbg.cu = !{!0}
+    !llvm.module.flags = !{!5}
+
+    !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+    !1 = !DIFile(filename: "t.ll", directory: "/")
+    !2 = !{}
+    !5 = !{i32 2, !"Debug Info Version", i32 3}
+    !6 = distinct !DISubprogram(name: "f", linkageName: "f", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+    !7 = !DISubroutineType(types: !2)
+    !8 = !{!9}
+    !9 = !DILocalVariable(name: "b", scope: !6, file: !1, line: 1, type: !10)
+    !10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned)
+    !11 = !DILocation(line: 1, column: 1, scope: !6)
+  )");
+
+  DebugValueDrop *P = new DebugValueDrop();
+
+  DebugInfoPerPassMap DIPreservationMap;
+  DebugifyCustomPassManager Passes;
+  Passes.setDIPreservationMap(DIPreservationMap);
+  Passes.add(createDebugifyModulePass(DebugifyMode::OriginalDebugInfo, "",
+                                      &(Passes.getDebugInfoPerPassMap())));
+  Passes.add(P);
+  Passes.add(createCheckDebugifyModulePass(false, "", nullptr,
+                                           DebugifyMode::OriginalDebugInfo,
+                                           &(Passes.getDebugInfoPerPassMap())));
+
+  testing::internal::CaptureStderr();
+  Passes.run(*M);
+
+  std::string StdOut = testing::internal::GetCapturedStderr();
+
+  std::string ErrorForSP = "ERROR:  dropped DISubprogram of";
+  std::string WarningForLoc = "WARNING:  dropped DILocation of";
+  std::string WarningForVars = "WARNING:  drops dbg.value()/dbg.declare() for";
+  std::string FinalResult = "CheckModuleDebugify (original debuginfo): FAIL";
+
+  EXPECT_TRUE(StdOut.find(ErrorForSP) == std::string::npos);
+  EXPECT_TRUE(StdOut.find(WarningForLoc) == std::string::npos);
+  EXPECT_TRUE(StdOut.find(WarningForVars) != std::string::npos);
+  EXPECT_TRUE(StdOut.find(FinalResult) != std::string::npos);
+}
+
 TEST(DebugInfoDummyAnalysis, PreserveOriginalDebugInfo) {
   LLVMContext C;
   std::unique_ptr<Module> M = parseIR(C, R"(
@@ -160,10 +242,12 @@ TEST(DebugInfoDummyAnalysis, PreserveOriginalDebugInfo) {
 
   std::string ErrorForSP = "ERROR:  dropped DISubprogram of";
   std::string WarningForLoc = "WARNING:  dropped DILocation of";
+  std::string WarningForVars = "WARNING:  drops dbg.value()/dbg.declare() for";
   std::string FinalResult = "CheckModuleDebugify (original debuginfo): PASS";
 
   EXPECT_TRUE(StdOut.find(ErrorForSP) == std::string::npos);
   EXPECT_TRUE(StdOut.find(WarningForLoc) == std::string::npos);
+  EXPECT_TRUE(StdOut.find(WarningForVars) == std::string::npos);
   EXPECT_TRUE(StdOut.find(FinalResult) != std::string::npos);
 }
 
diff --git a/llvm/utils/llvm-original-di-preservation.py b/llvm/utils/llvm-original-di-preservation.py
