[PATCH] D69461: [XRay] Sanitize DOT labels in graph output

[Alex Cameron via Phabricator via llvm-commits]

tetsuo-cpp created this revision.
tetsuo-cpp added a reviewer: dberris.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=39701

This patch is to convert certain characters to their XML escape sequences when generating labels for a DOT graph.

I had trouble reproducing the exact issue described on the tracker. I ran `llvm-xray graph` on a log from a test program that included function templates but wasn't able to get the `dot` tool to complain about the `<` and `>` characters. The documentation also suggests that the escape sequences should only be necessary when using HTML string labels which XRay doesn't use (`label=<...>` as opposed to `label="..."`). Perhaps newer versions of Graphviz silently handle this in the case of quoted-string labels.

In any case, the generated labels still look correct after this patch and should also fix the reporter's issue.

I was a bit unsure how to add a test for this since the existing tests seem to only care about `func-id` rather than giving an actual name. If you could give me a hint on the best way to go about this, that'd be much appreciated!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69461

Files:
  llvm/tools/llvm-xray/xray-graph.cpp


Index: llvm/tools/llvm-xray/xray-graph.cpp
===================================================================
--- llvm/tools/llvm-xray/xray-graph.cpp
+++ llvm/tools/llvm-xray/xray-graph.cpp
@@ -170,6 +170,30 @@
   S.Sum += L;
 }
 
+// Labels in a DOT graph must be legal XML strings so it's necessary to escape
+// certain characters.
+static std::string escapeString(const std::string &Label) {
+  std::string Str;
+  Str.reserve(Label.size());
+  for (const auto C : Label) {
+    switch (C) {
+    case '&':
+      Str.append("&");
+      break;
+    case '<':
+      Str.append("<");
+      break;
+    case '>':
+      Str.append(">");
+      break;
+    default:
+      Str.push_back(C);
+      break;
+    }
+  }
+  return Str;
+}
+
 // Evaluates an XRay record and performs accounting on it.
 //
 // If the record is an ENTER record it pushes the FuncID and TSC onto a
@@ -405,8 +429,9 @@
     if (V.first == 0)
       continue;
     OS << "F" << V.first << " [label=\"" << (VT != StatType::NONE ? "{" : "")
-       << (VA.SymbolName.size() > 40 ? VA.SymbolName.substr(0, 40) + "..."
-                                     : VA.SymbolName);
+       << escapeString(VA.SymbolName.size() > 40
+                           ? VA.SymbolName.substr(0, 40) + "..."
+                           : VA.SymbolName);
     if (VT != StatType::NONE)
       OS << "|" << VA.S.getString(VT) << "}\"";
     else


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69461.226534.patch
Type: text/x-patch
Size: 1411 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191026/64cbe2a4/attachment.bin>