[analyzer] Handle `\l` symbol in string literals in exploded-graph-rewriter

[ASDenysPetrov]

Summary

Handle \l separately because a string literal can be in code like "string\\literal" with the \l inside. Also on Windows macros FILE produces specific delimiters \ and a directory or file may starts with the letter l.

Fix: Use regex on for replacing all \l (like ,\l, }\l, [\l) except \\l, because a literal as a rule containes multiple \ before \l.

[vsavchenko]

Even though it doesn't seem that necessary, I would still vote to comply with clang-format in tests as well.
Other then that looks good to me!

Additionally, I want to make a note that we are not really working hard on keeping all the scripts and the analyzer itself to be fully functional on Windows. It can limit the user base and also prevent new developers from participating in the project. So, thank you for cleaning that up and for making it easier for other people to join our community :)

[NoQ]

Nice catch!

clang/utils/analyzer/exploded-graph-rewriter.py

383

Behavior of this script should not depend on the host platform. Neither should it depend on the target platform if the graph is obtained via cross-compilation. It should only depend on the graph itself, as it's a simple dot file converter. The graph itself may, of course, depend on the target platform (maybe even on the host platform).

So it sounds like this fix should be on the C++ side.

[NoQ]

In D82092#2100973, @vsavchenko wrote:

Additionally, I want to make a note that we are not really working hard on keeping all the scripts and the analyzer itself to be fully functional on Windows.

Yup. See also D76768 - i.e., well, i guess we tried.

[NoQ]

clang/utils/analyzer/exploded-graph-rewriter.py

383

Like, i mean, i see no reason why it should be impossible to copy the original dot file from one machine to another and still be able to run the script. It's probably not a super-important use case but i don't see why would we consciously break that.

[ASDenysPetrov]

Fixed clang-format.

[ASDenysPetrov]

In D82092#2100973, @vsavchenko wrote:

Even though it doesn't seem that necessary, I would still vote to comply with clang-format in tests as well.

Yes, I just forgot to format it and was going to load it after.

Additionally, I want to make a note that we are not really working hard on keeping all the scripts and the analyzer itself to be fully functional on Windows.

I think it depends on the platform you prefer, so that's natural. I also try to use Ubuntu on VM to check any differences.

clang/utils/analyzer/exploded-graph-rewriter.py

383

So it sounds like this fix should be on the C++ side.

The first I did was C++ side. BTW an explicit literal "string\\literal" can also cause this issue.
But then I found that I am not sure what the solution could be. How can we let the script skip this particular case without modifiing the string? How can we modify the string to keep it logically valid? The only solution I see is to make the replacement selection more accurate.

Like, i mean, i see no reason why it should be impossible to copy the original dot file from one machine to another and still be able to run the script.

Correct! I'll keep the regex part for all platforms.

I've just checked the old code on Ubuntu using the sample. char text[] = "string\\literal"; And it also fails as on Windows.
When I used the patch It passed. So this is not only a Windows related issue.

[ASDenysPetrov]

Expanded the patch for all platforms.

[ASDenysPetrov]

clang/utils/analyzer/exploded-graph-rewriter.py

432

This makes the graph data being look from

to

.

[NoQ]

I see what you did here! Great!

clang/test/Analysis/exploded-graph-rewriter/l_name_starts_with_l.cpp

26

I still wouldn't mind testing the actual output.

[ASDenysPetrov]

Thanks for the quick review, guys!

clang/test/Analysis/exploded-graph-rewriter/l_name_starts_with_l.cpp

26

I'll add more checks before the load.

clang/test/Analysis/exploded-graph-rewriter/l_name_starts_with_l.cpp

+// CAUTION: The name of this file should start with `l` for proper tests.
+// FIXME: Figure out how to use %clang_analyze_cc1 with our lit.local.cfg.
+// RUN: %clang_cc1 -analyze -triple x86_64-unknown-linux-gnu \
+// RUN:                     -analyzer-checker=core \
+// RUN:                     -analyzer-dump-egraph=%t.dot %s
+// RUN: %exploded_graph_rewriter %t.dot | FileCheck %s
+// REQUIRES: asserts
+
+void test1() {
+  // Here __FILE__ macros produces a string with `\` delimiters on Windows
+  // and the name of the file starts with `l`.
+  char text[] = __FILE__;
+}
+
+void test2() {
+  // Here `\l` is in the middle of the literal.
+  char text[] = "string\\literal";
+}
+
+void test() {
+  test1();
+  test2();
+}
+
+// This test is passed if exploded_graph_rewriter handles dot file without errors.
+// CHECK: digraph "ExplodedGraph"
+// CHECK: clang\\test\\Analysis\\exploded-graph-rewriter\\l_name_starts_with_l.cpp";
+// CHECK: char text[] = "string\\literal";

clang/utils/analyzer/exploded-graph-rewriter.py

def add_raw_line(self, raw_line):
         result = self.node_re.match(raw_line)
         if result is not None:
             logging.debug('Classified as node line.')
             node_id = result.group(1)
             if len(self.nodes) == 0:
                 self.root_id = node_id
             # Note: when writing tests you don't need to escape everything,
             # even though in a valid dot file everything is escaped.
-            node_label = result.group(2).replace('\\l', '') \
-                                        .replace(' ', '') \
+            node_label = result.group(2).replace(' ', '') \
                                         .replace('\\"', '"') \
                                         .replace('\\{', '{') \
                                         .replace('\\}', '}') \
                                         .replace('\\\\', '\\') \
                                         .replace('\\|', '|') \
                                         .replace('\\<', '\\\\<') \
                                         .replace('\\>', '\\\\>') \
                                         .rstrip(',')
+            # Handle `\l` separately because a string literal can be in code
+            # like "string\\literal" with the `\l` inside.
+            # Also on Windows macros __FILE__ produces specific delimiters `\`
+            # and a directory or file may starts with the letter `l`.
+            # Find all `\l` (like `,\l`, `}\l`, `[\l`) except `\\l`,
+            # because the literal as a rule containes multiple `\` before `\l`.
+            node_label = re.sub(r'(?<!\\)\\l', '', node_label)
             logging.debug(node_label)
             json_node = json.loads(node_label)
             self.nodes[node_id].construct(node_id, json_node)
             return
         logging.debug('Skipping.')
 
 
 #===-----------------------------------------------------------------------===#
def output(self):
             return ''.join(self._output)
 
     def _dump(self, s):
         s = s.replace('&', '&amp;') \
              .replace('{', '\\{') \
              .replace('}', '\\}') \
              .replace('\\<', '&lt;') \
              .replace('\\>', '&gt;') \
-             .replace('\\l', '<br />') \
              .replace('|', '\\|')
+        s = re.sub(r'(?<!\\)\\l', '<br />', s)
         if self._gray_mode:
             s = re.sub(r'<font color="[a-z0-9]*">', '', s)
             s = re.sub(r'</font>', '', s)
         self._dump_raw(s)
 
     @staticmethod
     def _diff_plus_minus(is_added):
         if is_added is None: