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

[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.

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
 ----------------------------------------------
 

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.

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).
+      // Collect debug locations (!dbg) and debug variable intrinsics.
-      // TODO: Collect dbg.values.
       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).
+      // Collect debug locations (!dbg) and debug variable intrinsics.
-      // TODO: Collect dbg.values.
       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())

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>

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,6 +43,7 @@ struct DebugInfoDrop : public FunctionPass {
 
     return false;
   }
+
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesCFG();
   }
@@ -48,6 +51,31 @@ struct DebugInfoDrop : public FunctionPass {
   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);
 }
 

utils/llvm-original-di-preservation.py

@@ -23,10 +23,16 @@ class DISPBug:
     self.action = action
     self.fn_name = fn_name
 
+class DIVarBug:
+  def __init__(self, action, name, fn_name):
+    self.action = action
+    self.name = name
+    self.fn_name = fn_name
+
 # Report the bugs in form of html.
-def generate_html_report(di_location_bugs, di_subprogram_bugs, \
+def generate_html_report(di_location_bugs, di_subprogram_bugs, di_var_bugs, \
                          di_location_bugs_summary, di_sp_bugs_summary, \
-                         html_file):
+                         di_var_bugs_summary, html_file):
   fileout = open(html_file, "w")
 
   html_header = """ <html>
@@ -145,7 +151,7 @@ def generate_html_report(di_location_bugs, di_subprogram_bugs, \
 
   at_least_one_bug_found = False
 
-  # Handle loction bugs.
+  # Handle fn bugs.
   for file, per_file_bugs in di_subprogram_bugs.items():
     for llvm_pass, per_pass_bugs in per_file_bugs.items():
       # No SP bugs for the pass.
@@ -212,6 +218,89 @@ def generate_html_report(di_location_bugs, di_subprogram_bugs, \
     """
   table_di_sp_sum += "</table>\n"
 
+  # Create the table for Variable bugs.
+  table_title_di_var = "Variable Location Bugs found by the Debugify"
+  table_di_var = """<table>
+  <caption><b>{}</b></caption>
+  <tr>
+  """.format(table_title_di_var)
+
+  header_di_var = ["File", "LLVM Pass Name", "Variable", "Function", "Action"]
+
+  for column in header_di_var:
+    table_di_var += "    <th>{0}</th>\n".format(column.strip())
+  table_di_var += "  </tr>\n"
+
+  at_least_one_bug_found = False
+
+  # Handle var bugs.
+  for file, per_file_bugs in di_var_bugs.items():
+    for llvm_pass, per_pass_bugs in per_file_bugs.items():
+      # No SP bugs for the pass.
+      if len(per_pass_bugs) == 0:
+        continue
+      at_least_one_bug_found = True
+      row = []
+      table_di_var += "  </tr>\n"
+      # Get the bugs info.
+      for x in per_pass_bugs:
+        row.append("    <tr>\n")
+        row.append(file)
+        row.append(llvm_pass)
+        row.append(x.name)
+        row.append(x.fn_name)
+        row.append(x.action)
+        row.append("    </tr>\n")
+      # Dump the bugs info into the table.
+      for column in row:
+        # The same file-pass pair can have multiple bugs.
+        if (column == "    <tr>\n" or column == "    </tr>\n"):
+          table_di_var += column
+          continue
+        table_di_var += "    <td>{0}</td>\n".format(column.strip())
+      table_di_var += "  <tr>\n"
+
+  if not at_least_one_bug_found:
+    table_di_var += """<tr>
+        <td colspan='4'> No bugs found </td>
+      </tr>
+    """
+  table_di_var += "</table>\n"
+
+  # Create the summary table for the sp bugs.
+  table_title_di_var_sum = "Summary of Variable Location Bugs"
+  table_di_var_sum = """<table>
+  <caption><b>{}</b></caption>
+  <tr>
+  """.format(table_title_di_var_sum)
+
+  header_di_var_sum = ["LLVM Pass Name", "Number of bugs"]
+
+  for column in header_di_var_sum:
+    table_di_var_sum += "    <th>{0}</th>\n".format(column.strip())
+  table_di_var_sum += "  </tr>\n"
+
+  # Print the summary.
+  row = []
+  for llvm_pass, num in sorted(di_var_bugs_summary.items()):
+    row.append("    <tr>\n")
+    row.append(llvm_pass)
+    row.append(str(num))
+    row.append("    </tr>\n")
+  for column in row:
+    if (column == "    <tr>\n" or column == "    </tr>\n"):
+      table_di_var_sum += column
+      continue
+    table_di_var_sum += "    <td>{0}</td>\n".format(column.strip())
+  table_di_var_sum += "  <tr>\n"
+
+  if not at_least_one_bug_found:
+    table_di_var_sum += """<tr>
+        <td colspan='2'> No bugs found </td>
+      </tr>
+    """
+  table_di_var_sum += "</table>\n"
+
   # Finish the html page.
   html_footer = """</body>
   </html>"""
@@ -227,6 +316,11 @@ def generate_html_report(di_location_bugs, di_subprogram_bugs, \
   fileout.writelines(table_di_sp)
   fileout.writelines(new_line)
   fileout.writelines(table_di_sp_sum)
+  fileout.writelines(new_line)
+  fileout.writelines(new_line)
+  fileout.writelines(table_di_var)
+  fileout.writelines(new_line)
+  fileout.writelines(table_di_var_sum)
   fileout.writelines(html_footer)
   fileout.close()
 
@@ -288,10 +382,12 @@ def Main():
   # Use the defaultdict in order to make multidim dicts.
   di_location_bugs = defaultdict(lambda: defaultdict(dict))
   di_subprogram_bugs = defaultdict(lambda: defaultdict(dict))
+  di_variable_bugs = defaultdict(lambda: defaultdict(dict))
 
   # Use the ordered dict to make a summary.
   di_location_bugs_summary = OrderedDict()
   di_sp_bugs_summary = OrderedDict()
+  di_var_bugs_summary = OrderedDict()
 
   # Map the bugs into the file-pass pairs.
   for bugs_per_pass in debug_info_bugs:
@@ -302,6 +398,8 @@ def Main():
 
     di_loc_bugs = []
     di_sp_bugs = []
+    di_var_bugs = []
+
     for bug in bugs:
       bugs_metadata = bug["metadata"]
       if bugs_metadata == "DILocation":
@@ -326,16 +424,28 @@ def Main():
           di_sp_bugs_summary[bugs_pass] += 1
         else:
           di_sp_bugs_summary[bugs_pass] = 1
+      elif bugs_metadata == "dbg-var-intrinsic":
+        action = bug["action"]
+        fn_name = bug["fn-name"]
+        name = bug["name"]
+        di_var_bugs.append(DIVarBug(action, name, fn_name))
+
+        # Fill the summary dict.
+        if bugs_pass in di_var_bugs_summary:
+          di_var_bugs_summary[bugs_pass] += 1
         else:
-        print ("error: Only DILocation and DISubprogram are supported.")
+          di_var_bugs_summary[bugs_pass] = 1
+      else:
+        print ("error: Unsupported metadata.")
         sys.exit(1)
 
     di_location_bugs[bugs_file][bugs_pass] = di_loc_bugs
     di_subprogram_bugs[bugs_file][bugs_pass] = di_sp_bugs
+    di_variable_bugs[bugs_file][bugs_pass] = di_var_bugs
 
-  generate_html_report(di_location_bugs, di_subprogram_bugs, \
+  generate_html_report(di_location_bugs, di_subprogram_bugs, di_variable_bugs, \
                        di_location_bugs_summary, di_sp_bugs_summary, \
-                       opts.html_file)
+                       di_var_bugs_summary, opts.html_file)
 
 if __name__ == "__main__":
   Main()