[sancov] Use LLVM Support library JSON writer in favor of individual implementation

[dgg5503]

Summary

In this diff, I've replaced the individual implementation of JSONWriter with json::OStream provided by llvm/Support/JSON.h.

Important Note: The output format of the JSON is considerably different compared to the original implementation. Important differences include:

• New line for each entry in an array (should make diffs cleaner)
• No space between keys and colon in attributed object entries.
• Attributes with empty strings will now print the attribute name and a quote pair rather than excluding the attribute altogether

Examples of these differences can be seen in the changes to the sancov tests which compare the JSON output.

Patch by Douglas Gliner.

[kcc]

I was actually hoping to get rid of this code entirely.
Why do you need this change?

[dgg5503]

In D68752#1705235, @kcc wrote:

I was actually hoping to get rid of this code entirely.
Why do you need this change?

Hi @kcc,

Are you referring to the JSON formatted coverage report or some other aspect of sancov? The original fix found at D51018 rev 2 essentially added support for Windows paths to coverage-report-server.py. This also required some changes to the JSON writer in sancov and sancov itself to ensure paths were properly normalized and escaped. I figured I might as well have sancov use the LLVM JSON support library to reduce maintenance and also fix any potential escaping issues for free. These changes branched from this suggestion.

The original reason for D51018 was to allow coverage dumped from libFuzzer using -dump-coverage to be viewed via coverage-report-server.py when the binary was compiled on Windows and therefore had Windows paths in the debug info. This feature has since been deprecated in libFuzzer, however, I would argue these patches are still relevant since you can still dump coverage standalone using the sanitizer run-time (see here).

llvm/test/tools/sancov/merge.test

 REQUIRES: x86_64-linux
 RUN: sancov -merge %p/Inputs/test-linux_x86_64.0.symcov| FileCheck --check-prefix=MERGE1 %s
 RUN: sancov -merge %p/Inputs/test-linux_x86_64.0.symcov %p/Inputs/test-linux_x86_64.1.symcov| FileCheck --check-prefix=MERGE2 %s
 
 MERGE1: {
-MERGE1-NEXT:   "covered-points" : ["4e132b", "4e1472", "4e1520", "4e1553", "4e1586"],
-MERGE1-NEXT:   "binary-hash" : "BB3CDD5045AED83906F6ADCC1C4DAF7E2596A6B5",
-MERGE1-NEXT:   "point-symbol-info" : {
-MERGE1-NEXT:     "test/tools/sancov/Inputs/foo.cpp" : {
-MERGE1-NEXT:       "foo()" : {
-MERGE1-NEXT:         "4e178c" : "5:0"
+MERGE1-NEXT:   "covered-points": [
+MERGE1-NEXT:     "4e132b",
+MERGE1-NEXT:     "4e1472",
+MERGE1-NEXT:     "4e1520",
+MERGE1-NEXT:     "4e1553",
+MERGE1-NEXT:     "4e1586"
+MERGE1-NEXT:   ],
+MERGE1-NEXT:   "binary-hash": "BB3CDD5045AED83906F6ADCC1C4DAF7E2596A6B5",
+MERGE1-NEXT:   "point-symbol-info": {
+MERGE1-NEXT:     "test/tools/sancov/Inputs/foo.cpp": {
+MERGE1-NEXT:       "foo()": {
+MERGE1-NEXT:         "4e178c": "5:0"
 MERGE1-NEXT:       }
 MERGE1-NEXT:     },
-MERGE1-NEXT:     "test/tools/sancov/Inputs/test.cpp" : {
-MERGE1-NEXT:       "bar(std::string)" : {
-MERGE1-NEXT:         "4e132b" : "12:0"
+MERGE1-NEXT:     "test/tools/sancov/Inputs/test.cpp": {
+MERGE1-NEXT:       "bar(std::string)": {
+MERGE1-NEXT:         "4e132b": "12:0"
 MERGE1-NEXT:       },
-MERGE1-NEXT:       "main" : {
-MERGE1-NEXT:         "4e1472" : "14:0",
-MERGE1-NEXT:         "4e14c2" : "16:9",
-MERGE1-NEXT:         "4e1520" : "17:5",
-MERGE1-NEXT:         "4e1553" : "17:5",
-MERGE1-NEXT:         "4e1586" : "17:5",
-MERGE1-NEXT:         "4e1635" : "19:1",
-MERGE1-NEXT:         "4e1690" : "17:5"
+MERGE1-NEXT:       "main": {
+MERGE1-NEXT:         "4e1472": "14:0",
+MERGE1-NEXT:         "4e14c2": "16:9",
+MERGE1-NEXT:         "4e1520": "17:5",
+MERGE1-NEXT:         "4e1553": "17:5",
+MERGE1-NEXT:         "4e1586": "17:5",
+MERGE1-NEXT:         "4e1635": "19:1",
+MERGE1-NEXT:         "4e1690": "17:5"
 MERGE1-NEXT:       }
 MERGE1-NEXT:     }
 MERGE1-NEXT:   }
 MERGE1-NEXT: }
 
 MERGE2: {
-MERGE2-NEXT:   "covered-points" : ["04e132b", "04e1472", "04e1520", "04e1553", "04e1586", "14e132b", "14e1472", "14e14c2", "14e1520", "14e1553", "14e1586", "14e178c"],
-MERGE2-NEXT:   "point-symbol-info" : {
-MERGE2-NEXT:     "test/tools/sancov/Inputs/foo.cpp" : {
-MERGE2-NEXT:      "foo()" : {
-MERGE2-NEXT:         "04e178c" : "5:0",
-MERGE2-NEXT:         "14e178c" : "5:0"
+MERGE2-NEXT:   "covered-points": [
+MERGE2-NEXT:     "04e132b",
+MERGE2-NEXT:     "04e1472",
+MERGE2-NEXT:     "04e1520",
+MERGE2-NEXT:     "04e1553",
+MERGE2-NEXT:     "04e1586",
+MERGE2-NEXT:     "14e132b",
+MERGE2-NEXT:     "14e1472",
+MERGE2-NEXT:     "14e14c2",
+MERGE2-NEXT:     "14e1520",
+MERGE2-NEXT:     "14e1553",
+MERGE2-NEXT:     "14e1586",
+MERGE2-NEXT:     "14e178c"
+MERGE2-NEXT:   ],
+MERGE2-NEXT:   "binary-hash": "",
+MERGE2-NEXT:   "point-symbol-info": {
+MERGE2-NEXT:     "test/tools/sancov/Inputs/foo.cpp": {
+MERGE2-NEXT:       "foo()": {
+MERGE2-NEXT:         "04e178c": "5:0",
+MERGE2-NEXT:         "14e178c": "5:0"
 MERGE2-NEXT:       }
 MERGE2-NEXT:     },
-MERGE2-NEXT:     "test/tools/sancov/Inputs/test.cpp" : {
-MERGE2-NEXT:       "bar(std::string)" : {
-MERGE2-NEXT:         "04e132b" : "12:0",
-MERGE2-NEXT:         "14e132b" : "12:0"
+MERGE2-NEXT:     "test/tools/sancov/Inputs/test.cpp": {
+MERGE2-NEXT:       "bar(std::string)": {
+MERGE2-NEXT:         "04e132b": "12:0",
+MERGE2-NEXT:         "14e132b": "12:0"
 MERGE2-NEXT:       },
-MERGE2-NEXT:       "main" : {
-MERGE2-NEXT:         "04e1472" : "14:0",
-MERGE2-NEXT:         "04e14c2" : "16:9",
-MERGE2-NEXT:         "04e1520" : "17:5",
-MERGE2-NEXT:         "04e1553" : "17:5",
-MERGE2-NEXT:         "04e1586" : "17:5",
-MERGE2-NEXT:         "04e1635" : "19:1",
-MERGE2-NEXT:         "04e1690" : "17:5",
-MERGE2-NEXT:         "14e1472" : "14:0",
-MERGE2-NEXT:         "14e14c2" : "16:9",
-MERGE2-NEXT:         "14e1520" : "17:5",
-MERGE2-NEXT:         "14e1553" : "17:5",
-MERGE2-NEXT:         "14e1586" : "17:5",
-MERGE2-NEXT:         "14e1635" : "19:1",
-MERGE2-NEXT:         "14e1690" : "17:5"
+MERGE2-NEXT:       "main": {
+MERGE2-NEXT:         "04e1472": "14:0",
+MERGE2-NEXT:         "04e14c2": "16:9",
+MERGE2-NEXT:         "04e1520": "17:5",
+MERGE2-NEXT:         "04e1553": "17:5",
+MERGE2-NEXT:         "04e1586": "17:5",
+MERGE2-NEXT:         "04e1635": "19:1",
+MERGE2-NEXT:         "04e1690": "17:5",
+MERGE2-NEXT:         "14e1472": "14:0",
+MERGE2-NEXT:         "14e14c2": "16:9",
+MERGE2-NEXT:         "14e1520": "17:5",
+MERGE2-NEXT:         "14e1553": "17:5",
+MERGE2-NEXT:         "14e1586": "17:5",
+MERGE2-NEXT:         "14e1635": "19:1",
+MERGE2-NEXT:         "14e1690": "17:5"
 MERGE2-NEXT:       }
 MERGE2-NEXT:     }
 MERGE2-NEXT:   }
 MERGE2-NEXT: }
-

llvm/test/tools/sancov/symbolize.test

 REQUIRES: x86_64-linux
 RUN: sancov -symbolize -strip_path_prefix="llvm/" %p/Inputs/test-linux_x86_64 %p/Inputs/test-linux_x86_64.0.sancov | FileCheck %s
 
 CHECK: {
-CHECK-NEXT:  "covered-points" : ["4e132b", "4e1472", "4e1520", "4e1553", "4e1586"],
-CHECK-NEXT:  "binary-hash" : "BB3CDD5045AED83906F6ADCC1C4DAF7E2596A6B5",
-CHECK-NEXT:  "point-symbol-info" : {
-CHECK-NEXT:   "test/tools/sancov/Inputs/test.cpp" : {
-CHECK-NEXT:     "bar(std::string)" : {
-CHECK-NEXT:       "4e132b" : "12:0"
+CHECK-NEXT:  "covered-points": [
+CHECK-NEXT:    "4e132b",
+CHECK-NEXT:    "4e1472",
+CHECK-NEXT:    "4e1520",
+CHECK-NEXT:    "4e1553",
+CHECK-NEXT:    "4e1586"
+CHECK-NEXT:  ],
+CHECK-NEXT:  "binary-hash": "BB3CDD5045AED83906F6ADCC1C4DAF7E2596A6B5",
+CHECK-NEXT:  "point-symbol-info": {
+CHECK-NEXT:    "test/tools/sancov/Inputs/test.cpp": {
+CHECK-NEXT:      "bar(std::string)": {
+CHECK-NEXT:        "4e132b": "12:0"
 CHECK-NEXT:      },
-CHECK-NEXT:      "main" : {
-CHECK-NEXT:        "4e1472" : "14:0",
-CHECK-NEXT:        "4e14c2" : "16:9",
-CHECK-NEXT:        "4e1520" : "17:5",
-CHECK-NEXT:        "4e1553" : "17:5",
-CHECK-NEXT:        "4e1586" : "17:5",
-CHECK-NEXT:        "4e1635" : "19:1",
-CHECK-NEXT:        "4e1690" : "17:5"
+CHECK-NEXT:      "main": {
+CHECK-NEXT:        "4e1472": "14:0",
+CHECK-NEXT:        "4e14c2": "16:9",
+CHECK-NEXT:        "4e1520": "17:5",
+CHECK-NEXT:        "4e1553": "17:5",
+CHECK-NEXT:        "4e1586": "17:5",
+CHECK-NEXT:        "4e1635": "19:1",
+CHECK-NEXT:        "4e1690": "17:5"
 CHECK-NEXT:      }
 CHECK-NEXT:    }
 CHECK-NEXT:  }
 CHECK-NEXT:}
-

llvm/test/tools/sancov/symbolize_noskip_dead_files.test

 REQUIRES: x86_64-linux
 RUN: sancov -symbolize -skip-dead-files=0 -strip_path_prefix="llvm/" %p/Inputs/test-linux_x86_64 %p/Inputs/test-linux_x86_64.0.sancov | FileCheck %s
 
 CHECK: {
-CHECK-NEXT:  "covered-points" : ["4e132b", "4e1472", "4e1520", "4e1553", "4e1586"],
-CHECK-NEXT:  "binary-hash" : "BB3CDD5045AED83906F6ADCC1C4DAF7E2596A6B5",
-CHECK-NEXT:  "point-symbol-info" : {
-CHECK-NEXT:    "test/tools/sancov/Inputs/foo.cpp" : {
-CHECK-NEXT:      "foo()" : {
-CHECK-NEXT:        "4e178c" : "5:0"
-CHECK-NEXT:     }
-CHECK-NEXT:   },
-CHECK-NEXT:   "test/tools/sancov/Inputs/test.cpp" : {
-CHECK-NEXT:     "bar(std::string)" : {
-CHECK-NEXT:       "4e132b" : "12:0"
+CHECK-NEXT:  "covered-points": [
+CHECK-NEXT:    "4e132b",
+CHECK-NEXT:    "4e1472",
+CHECK-NEXT:    "4e1520",
+CHECK-NEXT:    "4e1553",
+CHECK-NEXT:    "4e1586"
+CHECK-NEXT:  ],
+CHECK-NEXT:  "binary-hash": "BB3CDD5045AED83906F6ADCC1C4DAF7E2596A6B5",
+CHECK-NEXT:  "point-symbol-info": {
+CHECK-NEXT:    "test/tools/sancov/Inputs/foo.cpp": {
+CHECK-NEXT:      "foo()": {
+CHECK-NEXT:        "4e178c": "5:0"
+CHECK-NEXT:      }
+CHECK-NEXT:    },
+CHECK-NEXT:    "test/tools/sancov/Inputs/test.cpp": {
+CHECK-NEXT:      "bar(std::string)": {
+CHECK-NEXT:        "4e132b": "12:0"
 CHECK-NEXT:      },
-CHECK-NEXT:      "main" : {
-CHECK-NEXT:        "4e1472" : "14:0",
-CHECK-NEXT:        "4e14c2" : "16:9",
-CHECK-NEXT:        "4e1520" : "17:5",
-CHECK-NEXT:        "4e1553" : "17:5",
-CHECK-NEXT:        "4e1586" : "17:5",
-CHECK-NEXT:        "4e1635" : "19:1",
-CHECK-NEXT:        "4e1690" : "17:5"
+CHECK-NEXT:      "main": {
+CHECK-NEXT:        "4e1472": "14:0",
+CHECK-NEXT:        "4e14c2": "16:9",
+CHECK-NEXT:        "4e1520": "17:5",
+CHECK-NEXT:        "4e1553": "17:5",
+CHECK-NEXT:        "4e1586": "17:5",
+CHECK-NEXT:        "4e1635": "19:1",
+CHECK-NEXT:        "4e1690": "17:5"
 CHECK-NEXT:      }
 CHECK-NEXT:    }
 CHECK-NEXT:  }
 CHECK-NEXT:}
-

llvm/tools/sancov/sancov.cpp

 #include "llvm/Object/COFF.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/Signals.h"

 static raw_ostream &operator<<(raw_ostream &OS, const CoverageStats &Stats) {
   OS << "all-edges: " << Stats.AllPoints << "\n";
   OS << "cov-edges: " << Stats.CovPoints << "\n";
   OS << "all-functions: " << Stats.AllFns << "\n";
   OS << "cov-functions: " << Stats.CovFns << "\n";
   return OS;
 }
 
-// Helper for writing out JSON. Handles indents and commas using
-// scope variables for objects and arrays.
-class JSONWriter {
-public:
-  JSONWriter(raw_ostream &Out) : OS(Out) {}
-  JSONWriter(const JSONWriter &) = delete;
-  ~JSONWriter() { OS << "\n"; }
-
-  void operator<<(StringRef S) { printJSONStringLiteral(S, OS); }
-
-  // Helper RAII class to output JSON objects.
-  class Object {
-  public:
-    Object(JSONWriter *W, raw_ostream &OS) : W(W), OS(OS) {
-      OS << "{";
-      W->Indent++;
-    }
-    ~Object() {
-      W->Indent--;
-      OS << "\n";
-      W->indent();
-      OS << "}";
-    }
-
-    void key(StringRef Key) {
-      Index++;
-      if (Index > 0)
-        OS << ",";
-      OS << "\n";
-      W->indent();
-      printJSONStringLiteral(Key, OS);
-      OS << " : ";
-    }
-
-  private:
-    JSONWriter *W;
-    raw_ostream &OS;
-    int Index = -1;
-  };
-
-  Object object() { return {this, OS}; }
-
-  // Helper RAII class to output JSON arrays.
-  class Array {
-  public:
-    Array(raw_ostream &OS) : OS(OS) { OS << "["; }
-    ~Array() { OS << "]"; }
-    void next() {
-      Index++;
-      if (Index > 0)
-        OS << ", ";
-    }
-
-  private:
-    raw_ostream &OS;
-    int Index = -1;
-  };
-
-  Array array() { return {OS}; }
-
-private:
-  void indent() { OS.indent(Indent * 2); }
-
-  static void printJSONStringLiteral(StringRef S, raw_ostream &OS) {
-    if (S.find('"') == std::string::npos) {
-      OS << "\"" << S << "\"";
-      return;
-    }
-    OS << "\"";
-    for (char Ch : S.bytes()) {
-      if (Ch == '"')
-        OS << "\\";
-      OS << Ch;
-    }
-    OS << "\"";
-  }
-
-  raw_ostream &OS;
-  int Indent = 0;
-};
-
 // Output symbolized information for coverage points in JSON.
 // Format:
 // {
 //   '<file_name>' : {
 //     '<function_name>' : {
 //       '<point_id'> : '<line_number>:'<column_number'.
 //          ....
 //       }
 //    }
 // }
-static void operator<<(JSONWriter &W,
+static void operator<<(json::OStream &W,
                        const std::vector<CoveragePoint> &Points) {
   // Group points by file.
-  auto ByFile(W.object());
   std::map<std::string, std::vector<const CoveragePoint *>> PointsByFile;
   for (const auto &Point : Points) {
     for (const DILineInfo &Loc : Point.Locs) {
       PointsByFile[Loc.FileName].push_back(&Point);
     }
   }
 
   for (const auto &P : PointsByFile) {
     std::string FileName = P.first;
-    ByFile.key(FileName);
-
-    // Group points by function.
-    auto ByFn(W.object());
     std::map<std::string, std::vector<const CoveragePoint *>> PointsByFn;
     for (auto PointPtr : P.second) {
       for (const DILineInfo &Loc : PointPtr->Locs) {
         PointsByFn[Loc.FunctionName].push_back(PointPtr);
       }
     }
 
+    W.attributeObject(P.first, [&] {
+      // Group points by function.
       for (const auto &P : PointsByFn) {
         std::string FunctionName = P.first;
         std::set<std::string> WrittenIds;
 
-      ByFn.key(FunctionName);
-
-      // Output <point_id> : "<line>:<col>".
-      auto ById(W.object());
+        W.attributeObject(FunctionName, [&] {
           for (const CoveragePoint *Point : P.second) {
             for (const auto &Loc : Point->Locs) {
               if (Loc.FileName != FileName || Loc.FunctionName != FunctionName)
                 continue;
               if (WrittenIds.find(Point->Id) != WrittenIds.end())
                 continue;
 
+              // Output <point_id> : "<line>:<col>".
               WrittenIds.insert(Point->Id);
-          ById.key(Point->Id);
-          W << (utostr(Loc.Line) + ":" + utostr(Loc.Column));
+              W.attribute(Point->Id,
+                          (utostr(Loc.Line) + ":" + utostr(Loc.Column)));
             }
           }
+        });
       }
+    });
   }
 }
 
-static void operator<<(JSONWriter &W, const SymbolizedCoverage &C) {
-  auto O(W.object());
-
-  {
-    O.key("covered-points");
-    auto PointsArray(W.array());
-
+static void operator<<(json::OStream &W, const SymbolizedCoverage &C) {
+  W.object([&] {
+    W.attributeArray("covered-points", [&] {
       for (const std::string &P : C.CoveredIds) {
-      PointsArray.next();
-      W << P;
-    }
-  }
-
-  {
-    if (!C.BinaryHash.empty()) {
-      O.key("binary-hash");
-      W << C.BinaryHash;
-    }
-  }
-
-  {
-    O.key("point-symbol-info");
-    W << C.Points;
+        W.value(P);
       }
+    });
+    W.attribute("binary-hash", C.BinaryHash);
+    W.attributeObject("point-symbol-info", [&] { W << C.Points; });
+  });
 }
 
 static std::string parseScalarString(yaml::Node *N) {
   SmallString<64> StringStorage;
   yaml::ScalarNode *S = dyn_cast<yaml::ScalarNode>(N);
   failIf(!S, "expected string");
   return S->getValue(StringStorage);
 }
case NotCoveredFunctionsAction: {
     return 0;
   }
   case StatsAction: {
     outs() << computeStats(*Coverage);
     return 0;
   }
   case MergeAction:
   case SymbolizeAction: { // merge & symbolize are synonims.
-    JSONWriter W(outs());
+    json::OStream W(outs(), 2);
     W << *Coverage;
     return 0;
   }
   case HtmlReportAction:
     errs() << "-html-report option is removed: "
               "use -symbolize & coverage-report-server.py instead\n";
     return 1;
   case PrintAction:
   case PrintCovPointsAction:
     llvm_unreachable("unsupported action");
   }
 }