[analyzer] JsonSupport: Escape escapes

[Charusso]

Summary

-

[NoQ]

Hmm, this doesn't seem to solve my problem in D62761. Let me write some actual test case so that you knew it's fixed when it's fixed.

[Charusso]

In D63462#1549144, @NoQ wrote:

Hmm, this doesn't seem to solve my problem in D62761. Let me write some actual test case so that you knew it's fixed when it's fixed.

Well, I have did the same:

if (is_dot_node(line)):
        node_id = str_between(line, 'Node', ' [shape=record')
        label = get_label(line)
        label = (label.replace('\\"', '"').replace('\\\\', '\\') ...

I believe this is the correct behavior. What I have pointed out is the opposite: when we have a unicode character we have to escape it, so it would be that \\\\ case. Rewrite that to '\\' is cool and safe.

[NoQ]

I mean, i'm removing backslashes but you're adding more backslashes. Therefore i think we're talking about different issues.

[Charusso]

In D63462#1549225, @NoQ wrote:

I mean, i'm removing backslashes but you're adding more backslashes. Therefore i think we're talking about different issues.

You *have to* remove backslashes, because sometimes we have \\\\ so two of them packed together. Then when you remove it, it will be only one (\\ ), which is handled by JSON properly. Sadly it is very strict and cannot predict two should not be a problem.

[NoQ]

My problem is demonstrated (and solved) by D63519. If i revert my changes but apply this patch instead, my test keeps failing.

[Charusso]

In D63462#1549241, @NoQ wrote:

My problem is demonstrated (and solved) by D63519. If i revert my changes but apply this patch instead, my test keeps failing.

That is what we want. .replace('\\\\', '\\') \ is perfect, and should work with that patch as I use it as well.

[NoQ]

I guess let's add a test for the unicode problem that you're seeing.

[Charusso]

• Test case added.

[NoQ]

Thx!

I guess it makes sense to add a test into test/Analysis/exploded-graph-rewriter/escapes.c as well, so that to learn if we can actually parse it later.

[NoQ]

See also https://xkcd.com/1638/

[Charusso]

• More test.

[Charusso]

In D63462#1556831, @NoQ wrote:

See also https://xkcd.com/1638/

Well, after a month of escaping we are still have problems, so it is truly comes to your brain.

[Charusso]

• A working one.

cfe/trunk/include/clang/Basic/JsonSupport.h

 inline std::string JsonFormat(StringRef RawSR, bool AddQuotes) {
   if (RawSR.empty())
     return "null";
 
   // Trim special characters.
   std::string Str = RawSR.trim().str();
   size_t Pos = 0;
 
+  // Escape backslashes.
+  while (true) {
+    Pos = Str.find('\\', Pos);
+    if (Pos == std::string::npos)
+      break;
+
+    // Prevent bad conversions.
+    size_t TempPos = (Pos != 0) ? Pos - 1 : 0;
+
+    // See whether the current backslash is not escaped.
+    if (TempPos != Str.find("\\\\", Pos)) {
+      Str.insert(Pos, "\\");
+      ++Pos; // As we insert the backslash move plus one.
+    }
+
+    ++Pos;
+  }
+
   // Escape double quotes.
+  Pos = 0;
   while (true) {
     Pos = Str.find('\"', Pos);
     if (Pos == std::string::npos)
       break;
 
     // Prevent bad conversions.
     size_t TempPos = (Pos != 0) ? Pos - 1 : 0;
 
-    // See whether the current double quote is escaped.
-    if (TempPos != Str.find("\\\"", TempPos)) {
+    // See whether the current double quote is not escaped.
+    if (TempPos != Str.find("\\\"", Pos)) {
       Str.insert(Pos, "\\");
       ++Pos; // As we insert the escape-character move plus one.
     }
 
     ++Pos;
   }
 
   // Remove new-lines.

cfe/trunk/test/Analysis/dump_egraph.c

 // RUN: %clang_analyze_cc1 -analyzer-checker=core \
 // RUN:  -analyzer-dump-egraph=%t.dot %s
 // RUN: cat %t.dot | FileCheck %s
 
 // RUN: %clang_analyze_cc1 -analyzer-checker=core \
 // RUN:  -analyzer-dump-egraph=%t.dot \
 // RUN:  -trim-egraph %s
 // RUN: cat %t.dot | FileCheck %s
 
 // REQUIRES: asserts
 
 int getJ();
 
 int foo() {
   int *x = 0, *y = 0;
+  char c = '\x13';
+
   return *x + *y;
 }
 
 // CHECK: \"program_points\": [\l    \{ \"kind\": \"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null, \"term_kind\": null, \"tag\": null \}\l  ],\l  \"program_state\": null
 
 // CHECK: \"program_points\": [\l    \{ \"kind\": \"BlockEntrance\", \"block_id\": 1
 
 // CHECK: \"has_report\": true
 
-// CHECK: \"pretty\": \"*x\", \"location\": \{ \"line\": 16, \"column\": 10, \"file\": \"{{(.+)}}dump_egraph.c\" \}
+// CHECK: \"pretty\": \"*x\", \"location\": \{ \"line\": 18, \"column\": 10, \"file\": \"{{(.+)}}dump_egraph.c\" \}
+
+// CHECK: \"pretty\": \"'\\\\x13'\"
 

cfe/trunk/test/Analysis/exploded-graph-rewriter/escapes.c

 
 void escapes() {
   // CHECK: <td align="left"><b>Store: </b></td>
   // CHECK-SAME: <td align="left">foo</td><td align="left">0</td>
   // CHECK-SAME: <td align="left">&amp;Element\{"foo",0 S64b,char\}</td>
   // CHECK: <td align="left"><b>Environment: </b></td>
   // CHECK-SAME: <td align="left">"foo"</td>
   // CHECK-SAME: <td align="left">&amp;Element\{"foo",0 S64b,char\}</td>
-  const char *const foo = "foo";
+  const char *const foo = "\x66\x6f\x6f";
 
   // CHECK: <font color="cyan3">BinaryOperator</font>
   // CHECK-SAME: <td align="left">1 \| 2</td>
   // CHECK-SAME: <td align="left">3 S32b</td>
   int x = 1 | 2;
 }