[Debugify][OriginalDIMode] Make HTML reporting infrastructure more resilient

[ntesic]

Summary

Debugify in OriginalDebugInfo mode (verify-each-debuginfo-preserve), when used in
parallel builds of large projects, can produce incorrect report.
More precisely, simultaneous writes to JSON report file,
could form incorrect JSON objects, which describe found Debug Info bugs.

This patch uses the lock/unlock mechanism to protect JSON report file and also
makes script llvm/utils/llvm-original-di-preservation.py resilient to corrupted
lines in the report file. So, it ensures the creation of HTML report.

[StephenTozer]

llvm/utils/llvm-original-di-preservation.py

381

Nit, this declaration of skipped_lines looks unnecessary

409–414

Maybe for this section we could also use a skipped_bugs counter, so that we can still see there are file errors when the individual bugs are incorrect, even if every line is still a valid json object with all the expected top level fields present?

[StephenTozer]

Change SGTM, except for the minor inline comments.

[ntesic]

Addressed comments.
@StephenTozer Thanks for the comments, I agree with all the suggestions.

llvm/lib/Transforms/Utils/Debugify.cpp

static void writeJSON(StringRef OrigDIVerifyBugsReportFilePath,
   raw_fd_ostream OS_FILE{OrigDIVerifyBugsReportFilePath, EC,
                          sys::fs::OF_Append | sys::fs::OF_TextWithCRLF};
   if (EC) {
     errs() << "Could not open file: " << EC.message() << ", "
            << OrigDIVerifyBugsReportFilePath << '\n';
     return;
   }
 
+  if (auto L = OS_FILE.lock()) {
     OS_FILE << "{\"file\":\"" << FileNameFromCU << "\", ";
 
-  StringRef PassName = NameOfWrappedPass != "" ? NameOfWrappedPass : "no-name";
+    StringRef PassName =
+        NameOfWrappedPass != "" ? NameOfWrappedPass : "no-name";
     OS_FILE << "\"pass\":\"" << PassName << "\", ";
 
     llvm::json::Value BugsToPrint{std::move(Bugs)};
     OS_FILE << "\"bugs\": " << BugsToPrint;
 
     OS_FILE << "}\n";
   }
+  OS_FILE.close();
+}
 
 bool llvm::checkDebugInfoMetadata(Module &M,
                                   iterator_range<Module::iterator> Functions,
                                   DebugInfoPerPass &DebugInfoBeforePass,
                                   StringRef Banner, StringRef NameOfWrappedPass,
                                   StringRef OrigDIVerifyBugsReportFilePath) {
   LLVM_DEBUG(dbgs() << Banner << ": (after) " << NameOfWrappedPass << '\n');
 

llvm/test/tools/llvm-original-di-preservation/Inputs/corrupted.json

+{"file":"debug.c", "pass":"Simplify the CFG", "bugs": [[{"action":"drop","fn-name":"debug_class_type_samep","metadata":"dbg-var-intrinsic","name":"pf1"},{"action":"drop","fn-name":"debug_class_type_samep","metadata":"dbg-var-intrinsic","name":"pf2"},{"action":"drop","fn-name":"debug_class_type_samep","metadata":"dbg-var-intrinsic","name":"pv1"},{"action":"drop","fn-name":"debug_class_type_samep","metadata":"dbg-var-intrinsic","name":"pv2"}]]}
+{"file":"debug.c", "pass":"Combine redundant instructions", "bugs": [[{"action":"drop","fn-name":"debug_write_block","metadata":"dbg-var-intrinsic","name":"n"},{"action":"drop","fn-name":"debug_write_block","metadata":"dbg-var-intrinsic","name":"b"}]]}
+{"file":"ieee.c", "pass":"Interprocedural Sparse Conditional Constant Propagation", "bugs": [[{"action":"not-generate","bb-name":"land.lhs.true251","fn-name":"parse_ieee_bb","instr":"br","metadata":"DILocation"},{"action":"not-generate","bb-name":"cond.false70","fn-n{"file":"stabs.c", "pass":"Simplify the CFG", "bugs": [[{"action":"not-generate","bb-name":"land.lhs.true76","fn-name":"parse_stab","instr":"icmp","metadata":"DILocation"}]]}
+ame":"ieee_end_class_type","instr":"br","metadata":"DILocation"},{"action":"not-generate","bb-name":"lor.lhs.false79","fn-name":"ieee_end_class_type","instr":"br","metadata":"DILocation"}]]}
+{"file":"wrstabs.c", "pass":"Simplify the CFG", "bugs": [[{"action":"drop","fn-name":"stab_bool_type","metadata":"dbg-var-intrinsic","name":"tindex"}]]}
+{"file":"prdbg.c", "pass":"Simplify the CFG", "bugs": [[{"action":"drop","fn-name":"pr_tag_type","metadata":"dbg-var-intrinsic","name":"t"},{"action":"drop","fn-name":"pr_tag_type","metadata":"dbg-var-intrinsic","name":"tag"}]]}
+{"file":"objdump.c", "pass":"Simplify the CFG", "bugs": [[{"action":"drop","fn-name":"dump_section_header","metadata":"dbg-var-intrinsic","name":"comma"},{"action":"drop","fn-name":"dump_section_header","metadata":"dbg-var-intrinsic","name":"ls"}]]}
+{"file":"objdump.c", "bad-field": [[{"action":"drop","fn-name":"dump_section_header","metadata":"dbg-var-intrinsic","name":"comma"},{"action":"drop","fn-name":"dump_section_header","metadata":"dbg-var-intrinsic","name":"ls"}]]}
+{"file":"wrstabs.c", "pass":"Simplify the CFG", "bugs": [[{"bad-bug":"stab_function_type"}]]}
+{"file":"wrstabs.c", "pass":"Simplify the CFG", "bugs": [[{"action":"drop","bb-name":"if.end","fn-name":"stab_function_type","instr":"br","metadata":"DILocation"}]]}

llvm/test/tools/llvm-original-di-preservation/Inputs/expected-skipped.html

+ <html>
+  <head>
+  <style>
+  table, th, td {
+    border: 1px solid black;
+  }
+  table.center {
+    margin-left: auto;
+    margin-right: auto;
+  }
+  </style>
+  </head>
+  <body>
+  <table>
+  <caption><b>Location Bugs found by the Debugify</b></caption>
+  <tr>
+      <th>File</th>
+    <th>LLVM Pass Name</th>
+    <th>LLVM IR Instruction</th>
+    <th>Function Name</th>
+    <th>Basic Block Name</th>
+    <th>Action</th>
+  </tr>
+  </tr>
+    <tr>
+    <td>wrstabs.c</td>
+    <td>Simplify the CFG</td>
+    <td>br</td>
+    <td>stab_function_type</td>
+    <td>if.end</td>
+    <td>drop</td>
+    </tr>
+  <tr>
+</table>
+<br>
+<table>
+  <caption><b>Summary of Location Bugs</b></caption>
+  <tr>
+      <th>LLVM Pass Name</th>
+    <th>Number of bugs</th>
+  </tr>
+    <tr>
+    <td>Simplify the CFG</td>
+    <td>1</td>
+    </tr>
+  <tr>
+</table>
+<br>
+<br>
+<table>
+  <caption><b>SP Bugs found by the Debugify</b></caption>
+  <tr>
+      <th>File</th>
+    <th>LLVM Pass Name</th>
+    <th>Function Name</th>
+    <th>Action</th>
+  </tr>
+<tr>
+        <td colspan='4'> No bugs found </td>
+      </tr>
+    </table>
+<br>
+<table>
+  <caption><b>Summary of SP 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>
+<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>
+    <tr>
+    <td>debug.c</td>
+    <td>Simplify the CFG</td>
+    <td>pf1</td>
+    <td>debug_class_type_samep</td>
+    <td>drop</td>
+    </tr>
+    <tr>
+    <td>debug.c</td>
+    <td>Simplify the CFG</td>
+    <td>pf2</td>
+    <td>debug_class_type_samep</td>
+    <td>drop</td>
+    </tr>
+    <tr>
+    <td>debug.c</td>
+    <td>Simplify the CFG</td>
+    <td>pv1</td>
+    <td>debug_class_type_samep</td>
+    <td>drop</td>
+    </tr>
+    <tr>
+    <td>debug.c</td>
+    <td>Simplify the CFG</td>
+    <td>pv2</td>
+    <td>debug_class_type_samep</td>
+    <td>drop</td>
+    </tr>
+  <tr>
+  </tr>
+    <tr>
+    <td>debug.c</td>
+    <td>Combine redundant instructions</td>
+    <td>n</td>
+    <td>debug_write_block</td>
+    <td>drop</td>
+    </tr>
+    <tr>
+    <td>debug.c</td>
+    <td>Combine redundant instructions</td>
+    <td>b</td>
+    <td>debug_write_block</td>
+    <td>drop</td>
+    </tr>
+  <tr>
+  </tr>
+    <tr>
+    <td>prdbg.c</td>
+    <td>Simplify the CFG</td>
+    <td>t</td>
+    <td>pr_tag_type</td>
+    <td>drop</td>
+    </tr>
+    <tr>
+    <td>prdbg.c</td>
+    <td>Simplify the CFG</td>
+    <td>tag</td>
+    <td>pr_tag_type</td>
+    <td>drop</td>
+    </tr>
+  <tr>
+  </tr>
+    <tr>
+    <td>objdump.c</td>
+    <td>Simplify the CFG</td>
+    <td>comma</td>
+    <td>dump_section_header</td>
+    <td>drop</td>
+    </tr>
+    <tr>
+    <td>objdump.c</td>
+    <td>Simplify the CFG</td>
+    <td>ls</td>
+    <td>dump_section_header</td>
+    <td>drop</td>
+    </tr>
+  <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>
+    <td>Combine redundant instructions</td>
+    <td>2</td>
+    </tr>
+    <tr>
+    <td>Simplify the CFG</td>
+    <td>9</td>
+    </tr>
+  <tr>
+</table>
+</body>
+  </html>
 No newline at end of file

llvm/test/tools/llvm-original-di-preservation/basic.test

-RUN: %llvm-original-di-preservation %p/Inputs/sample.json %t.html
+RUN: %llvm-original-di-preservation %p/Inputs/sample.json %t.html | FileCheck %s
 RUN: diff -w %p/Inputs/expected-sample.html %t.html
+CHECK-NOT: Skipped lines:
+
+RUN: %llvm-original-di-preservation %p/Inputs/corrupted.json %t2.html | FileCheck %s -check-prefix=CORRUPTED
+RUN: diff -w %p/Inputs/expected-skipped.html %t2.html
+CORRUPTED: Skipped lines: 3
+CORRUPTED: Skipped bugs: 1

llvm/utils/llvm-original-di-preservation.py

def generate_html_report(di_location_bugs, di_subprogram_bugs, di_var_bugs, \
   fileout.close()
 
   print("The " + html_file + " generated.")
 
 # Read the JSON file.
 def get_json(file):
   json_parsed = None
   di_checker_data = []
+  skipped_lines = 0
 
   # The file contains json object per line.
   # An example of the line (formatted json):
   # {
   #  "file": "simple.c",
   #  "pass": "Deduce function attributes in RPO",
   #  "bugs": [
   #    [
def get_json(file):
   #    ]
   #  ]
   #}
   with open(file) as json_objects_file:
     for json_object_line in json_objects_file:
       try:
         json_object = loads(json_object_line)
       except:
-        print ("error: No valid di-checker data found.")
-        sys.exit(1)
+        skipped_lines += 1
+      else:
         di_checker_data.append(json_object)
 
-  return di_checker_data
+  return (di_checker_data, skipped_lines)
 
 # Parse the program arguments.
 def parse_program_args(parser):
   parser.add_argument("file_name", type=str, help="json file to process")
   parser.add_argument("html_file", type=str, help="html file to output data")
 
   return parser.parse_args()
 
 def Main():
   parser = argparse.ArgumentParser()
   opts = parse_program_args(parser)
 
   if not opts.html_file.endswith('.html'):
     print ("error: The output file must be '.html'.")
     sys.exit(1)
 
-  debug_info_bugs = get_json(opts.file_name)
+  (debug_info_bugs, skipped_lines) = get_json(opts.file_name)
 
   # 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()
 
+  skipped_bugs = 0
   # Map the bugs into the file-pass pairs.
   for bugs_per_pass in debug_info_bugs:
+    try:
       bugs_file = bugs_per_pass["file"]
       bugs_pass = bugs_per_pass["pass"]
-
       bugs = bugs_per_pass["bugs"][0]
+    except:
+      skipped_lines += 1
+      continue
 
     di_loc_bugs = []
     di_sp_bugs = []
     di_var_bugs = []
 
     for bug in bugs:
+      try:
         bugs_metadata = bug["metadata"]
+      except:
+        skipped_bugs += 1
+        continue
+
       if bugs_metadata == "DILocation":
+        try:
           action = bug["action"]
           bb_name = bug["bb-name"]
           fn_name = bug["fn-name"]
           instr = bug["instr"]
+        except:
+          skipped_bugs += 1
+          continue
         di_loc_bugs.append(DILocBug(action, bb_name, fn_name, instr))
 
         # Fill the summary dict.
         if bugs_pass in di_location_bugs_summary:
           di_location_bugs_summary[bugs_pass] += 1
         else:
           di_location_bugs_summary[bugs_pass] = 1
       elif bugs_metadata == "DISubprogram":
+        try:
           action = bug["action"]
           name = bug["name"]
+        except:
+          skipped_bugs += 1
+          continue
         di_sp_bugs.append(DISPBug(action, name))
 
         # Fill the summary dict.
         if bugs_pass in di_sp_bugs_summary:
           di_sp_bugs_summary[bugs_pass] += 1
         else:
           di_sp_bugs_summary[bugs_pass] = 1
       elif bugs_metadata == "dbg-var-intrinsic":
+        try:
           action = bug["action"]
           fn_name = bug["fn-name"]
           name = bug["name"]
+        except:
+          skipped_bugs += 1
+          continue
         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:
           di_var_bugs_summary[bugs_pass] = 1
       else:
-        print ("error: Unsupported metadata.")
-        sys.exit(1)
+        # Unsupported metadata.
+        skipped_bugs += 1
+        continue
 
     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, di_variable_bugs, \
                        di_location_bugs_summary, di_sp_bugs_summary, \
                        di_var_bugs_summary, opts.html_file)
 
+  if skipped_lines > 0:
+    print ("Skipped lines: " + str(skipped_lines))
+  if skipped_bugs > 0:
+    print ("Skipped bugs: " + str(skipped_bugs))
+
 if __name__ == "__main__":
   Main()
   sys.exit(0)