[libcxx-commits] [libcxx] [libc++] Fix mi-mode in GDB pretty printers (PR #120951)

Sv. Lockal via libcxx-commits libcxx-commits at lists.llvm.org
Mon Dec 23 07:07:34 PST 2024


https://github.com/AngryLoki updated https://github.com/llvm/llvm-project/pull/120951

>From d58559ef748ab8da915a00eaaea393b0ac479ffe Mon Sep 17 00:00:00 2001
From: "Sv. Lockal" <lockalsash at gmail.com>
Date: Mon, 23 Dec 2024 09:00:39 +0000
Subject: [PATCH] [libc++] Fix mi-mode in GDB pretty printers

GDB/MI requires unique names for each child, otherwise fails with "Duplicate variable object name".
Additionally wrapped containers printers were flattened for cleaner visualization in IDEs and CLI.

Fixes #62340
---
 .../libcxx/gdb/gdb_pretty_printer_test.py     | 56 +++++++++++++----
 .../libcxx/gdb/gdb_pretty_printer_test.sh.cpp | 63 +++++++++++++------
 libcxx/utils/gdb/libcxx/printers.py           | 41 +++++++-----
 3 files changed, 112 insertions(+), 48 deletions(-)

diff --git a/libcxx/test/libcxx/gdb/gdb_pretty_printer_test.py b/libcxx/test/libcxx/gdb/gdb_pretty_printer_test.py
index 07154d001d3a1d..254a61a8c633e7 100644
--- a/libcxx/test/libcxx/gdb/gdb_pretty_printer_test.py
+++ b/libcxx/test/libcxx/gdb/gdb_pretty_printer_test.py
@@ -15,6 +15,7 @@
 """
 
 from __future__ import print_function
+import json
 import re
 import gdb
 import sys
@@ -29,6 +30,7 @@
 # we exit.
 has_run_tests = False
 
+has_execute_mi = tuple(map(int, gdb.VERSION.split("."))) >= (14, 2)
 
 class CheckResult(gdb.Command):
     def __init__(self):
@@ -42,36 +44,43 @@ def invoke(self, arg, from_tty):
 
             # Stack frame is:
             # 0. StopForDebugger
-            # 1. ComparePrettyPrintToChars or ComparePrettyPrintToRegex
+            # 1. CompareListChildrenToChars, ComparePrettyPrintToChars or ComparePrettyPrintToRegex
             # 2. TestCase
             compare_frame = gdb.newest_frame().older()
             testcase_frame = compare_frame.older()
             test_loc = testcase_frame.find_sal()
+            test_loc_str = test_loc.symtab.filename + ":" + str(test_loc.line)
             # Use interactive commands in the correct context to get the pretty
             # printed version
 
-            value_str = self._get_value_string(compare_frame, testcase_frame)
+            frame_name = compare_frame.name()
+            if frame_name.startswith("CompareListChildren"):
+                if has_execute_mi:
+                    value = self._get_children(compare_frame)
+                else:
+                    print("SKIPPED: " + test_loc_str)
+                    return
+            else:
+                value = self._get_value(compare_frame, testcase_frame)
 
-            # Ignore the convenience variable name and newline
-            value = value_str[value_str.find("= ") + 2 : -1]
             gdb.newest_frame().select()
             expectation_val = compare_frame.read_var("expectation")
             check_literal = expectation_val.string(encoding="utf-8")
-            if "PrettyPrintToRegex" in compare_frame.name():
+            if "PrettyPrintToRegex" in frame_name:
                 test_fails = not re.search(check_literal, value)
             else:
                 test_fails = value != check_literal
 
             if test_fails:
                 global test_failures
-                print("FAIL: " + test_loc.symtab.filename + ":" + str(test_loc.line))
+                print("FAIL: " + test_loc_str)
                 print("GDB printed:")
                 print("   " + repr(value))
                 print("Value should match:")
                 print("   " + repr(check_literal))
                 test_failures += 1
             else:
-                print("PASS: " + test_loc.symtab.filename + ":" + str(test_loc.line))
+                print("PASS: " + test_loc_str)
 
         except RuntimeError as e:
             # At this point, lots of different things could be wrong, so don't try to
@@ -81,9 +90,28 @@ def invoke(self, arg, from_tty):
             print(str(e))
             test_failures += 1
 
-    def _get_value_string(self, compare_frame, testcase_frame):
+    def _get_children(self, compare_frame):
+        compare_frame.select()
+        gdb.execute_mi("-var-create", "value", "*", "value")
+        r = gdb.execute_mi("-var-list-children", "--simple-values", "value")
+        gdb.execute_mi("-var-delete", "value")
+        children = r["children"]
+        if r["displayhint"] == "map":
+            r = [
+                {
+                    "key": json.loads(children[2 * i]["value"]),
+                    "value": json.loads(children[2 * i + 1]["value"]),
+                }
+                for i in range(len(children) // 2)
+            ]
+        else:
+            r = [json.loads(el["value"]) for el in children]
+        return json.dumps(r, sort_keys=True)
+
+    def _get_value(self, compare_frame, testcase_frame):
         compare_frame.select()
-        if "ComparePrettyPrint" in compare_frame.name():
+        frame_name = compare_frame.name()
+        if frame_name.startswith("ComparePrettyPrint"):
             s = gdb.execute("p value", to_string=True)
         else:
             value_str = str(compare_frame.read_var("value"))
@@ -91,8 +119,10 @@ def _get_value_string(self, compare_frame, testcase_frame):
             testcase_frame.select()
             s = gdb.execute("p " + clean_expression_str, to_string=True)
         if sys.version_info.major == 2:
-            return s.decode("utf-8")
-        return s
+            s = s.decode("utf-8")
+
+        # Ignore the convenience variable name and newline
+        return s[s.find("= ") + 2 : -1]
 
 
 def exit_handler(event=None):
@@ -112,6 +142,10 @@ def exit_handler(event=None):
 # Disable terminal paging
 gdb.execute("set height 0")
 gdb.execute("set python print-stack full")
+
+if has_execute_mi:
+    gdb.execute_mi("-enable-pretty-printing")
+
 test_failures = 0
 CheckResult()
 test_bp = gdb.Breakpoint("StopForDebugger")
diff --git a/libcxx/test/libcxx/gdb/gdb_pretty_printer_test.sh.cpp b/libcxx/test/libcxx/gdb/gdb_pretty_printer_test.sh.cpp
index 9d4b7039402a49..ff951d94db0a49 100644
--- a/libcxx/test/libcxx/gdb/gdb_pretty_printer_test.sh.cpp
+++ b/libcxx/test/libcxx/gdb/gdb_pretty_printer_test.sh.cpp
@@ -129,6 +129,12 @@ void CompareExpressionPrettyPrintToRegex(
   StopForDebugger(&value, &expectation);
 }
 
+template <typename TypeToPrint>
+void CompareListChildrenToChars(TypeToPrint value, const char* expectation) {
+  MarkAsLive(value);
+  StopForDebugger(&value, &expectation);
+}
+
 namespace example {
   struct example_struct {
     int a = 0;
@@ -361,28 +367,28 @@ void multimap_test() {
 
 void queue_test() {
   std::queue<int> i_am_empty;
-  ComparePrettyPrintToChars(i_am_empty,
-      "std::queue wrapping = {std::deque is empty}");
+  ComparePrettyPrintToChars(i_am_empty, "std::queue wrapping: std::deque is empty");
 
   std::queue<int> one_two_three(std::deque<int>{1, 2, 3});
-    ComparePrettyPrintToChars(one_two_three,
-        "std::queue wrapping = {"
-        "std::deque with 3 elements = {1, 2, 3}}");
+  ComparePrettyPrintToChars(
+      one_two_three,
+      "std::queue wrapping: "
+      "std::deque with 3 elements = {1, 2, 3}");
 }
 
 void priority_queue_test() {
   std::priority_queue<int> i_am_empty;
-  ComparePrettyPrintToChars(i_am_empty,
-      "std::priority_queue wrapping = {std::vector of length 0, capacity 0}");
+  ComparePrettyPrintToChars(i_am_empty, "std::priority_queue wrapping: std::vector of length 0, capacity 0");
 
   std::priority_queue<int> one_two_three;
   one_two_three.push(11111);
   one_two_three.push(22222);
   one_two_three.push(33333);
 
-  ComparePrettyPrintToRegex(one_two_three,
-      R"(std::priority_queue wrapping = )"
-      R"({std::vector of length 3, capacity 3 = {33333)");
+  ComparePrettyPrintToRegex(
+      one_two_three,
+      R"(std::priority_queue wrapping: )"
+      R"(std::vector of length 3, capacity 3 = {33333)");
 
   ComparePrettyPrintToRegex(one_two_three, ".*11111.*");
   ComparePrettyPrintToRegex(one_two_three, ".*22222.*");
@@ -410,25 +416,22 @@ void set_test() {
 
 void stack_test() {
   std::stack<int> test0;
-  ComparePrettyPrintToChars(test0,
-                            "std::stack wrapping = {std::deque is empty}");
+  ComparePrettyPrintToChars(test0, "std::stack wrapping: std::deque is empty");
   test0.push(5);
   test0.push(6);
-  ComparePrettyPrintToChars(
-      test0, "std::stack wrapping = {std::deque with 2 elements = {5, 6}}");
+  ComparePrettyPrintToChars(test0, "std::stack wrapping: std::deque with 2 elements = {5, 6}");
   std::stack<bool> test1;
   test1.push(true);
   test1.push(false);
-  ComparePrettyPrintToChars(
-      test1,
-      "std::stack wrapping = {std::deque with 2 elements = {true, false}}");
+  ComparePrettyPrintToChars(test1, "std::stack wrapping: std::deque with 2 elements = {true, false}");
 
   std::stack<std::string> test2;
   test2.push("Hello");
   test2.push("World");
-  ComparePrettyPrintToChars(test2,
-                            "std::stack wrapping = {std::deque with 2 elements "
-                            "= {\"Hello\", \"World\"}}");
+  ComparePrettyPrintToChars(
+      test2,
+      "std::stack wrapping: std::deque with 2 elements "
+      "= {\"Hello\", \"World\"}");
 }
 
 void multiset_test() {
@@ -662,6 +665,25 @@ void streampos_test() {
   ComparePrettyPrintToRegex(test1, "^std::fpos with stream offset:5( with state: {count:0 value:0})?$");
 }
 
+void mi_mode_test() {
+  std::map<int, std::string> one_two_three_map;
+  one_two_three_map.insert({1, "one"});
+  one_two_three_map.insert({2, "two"});
+  one_two_three_map.insert({3, "three"});
+  CompareListChildrenToChars(
+      one_two_three_map, R"([{"key": 1, "value": "one"}, {"key": 2, "value": "two"}, {"key": 3, "value": "three"}])");
+
+  std::unordered_map<int, std::string> one_two_three_umap;
+  one_two_three_umap.insert({3, "three"});
+  one_two_three_umap.insert({2, "two"});
+  one_two_three_umap.insert({1, "one"});
+  CompareListChildrenToChars(
+      one_two_three_umap, R"([{"key": 3, "value": "three"}, {"key": 2, "value": "two"}, {"key": 1, "value": "one"}])");
+
+  std::deque<int> one_two_three_deque{1, 2, 3};
+  CompareListChildrenToChars(one_two_three_deque, "[1, 2, 3]");
+}
+
 int main(int, char**) {
   framework_self_test();
 
@@ -694,5 +716,6 @@ int main(int, char**) {
   unordered_set_iterator_test();
   pointer_negative_test();
   streampos_test();
+  mi_mode_test();
   return 0;
 }
diff --git a/libcxx/utils/gdb/libcxx/printers.py b/libcxx/utils/gdb/libcxx/printers.py
index 8e74b93e7b121f..31c27a1959cb20 100644
--- a/libcxx/utils/gdb/libcxx/printers.py
+++ b/libcxx/utils/gdb/libcxx/printers.py
@@ -446,10 +446,13 @@ def _list_it(self):
         num_emitted = 0
         current_addr = self.start_ptr
         start_index = self.first_block_start_index
+        i = 0
         while num_emitted < self.size:
             end_index = min(start_index + self.size - num_emitted, self.block_size)
             for _, elem in self._bucket_it(current_addr, start_index, end_index):
-                yield "", elem
+                key_name = "[%d]" % i
+                i += 1
+                yield key_name, elem
             num_emitted += end_index - start_index
             current_addr = gdb.Value(addr_as_long(current_addr) + _pointer_size).cast(
                 self.node_type
@@ -494,8 +497,8 @@ def to_string(self):
 
     def _list_iter(self):
         current_node = self.first_node
-        for _ in range(self.size):
-            yield "", current_node.cast(self.nodetype).dereference()["__value_"]
+        for i in range(self.size):
+            yield "[%d]" % i, current_node.cast(self.nodetype).dereference()["__value_"]
             current_node = current_node.dereference()["__next_"]
 
     def __iter__(self):
@@ -512,15 +515,14 @@ class StdQueueOrStackPrinter(object):
     """Print a std::queue or std::stack."""
 
     def __init__(self, val):
-        self.val = val
-        self.underlying = val["c"]
+        self.typename = _remove_generics(_prettify_typename(val.type))
+        self.visualizer = gdb.default_visualizer(val["c"])
 
     def to_string(self):
-        typename = _remove_generics(_prettify_typename(self.val.type))
-        return "%s wrapping" % typename
+        return "%s wrapping: %s" % (self.typename, self.visualizer.to_string())
 
     def children(self):
-        return iter([("", self.underlying)])
+        return self.visualizer.children()
 
     def display_hint(self):
         return "array"
@@ -530,19 +532,18 @@ class StdPriorityQueuePrinter(object):
     """Print a std::priority_queue."""
 
     def __init__(self, val):
-        self.val = val
-        self.underlying = val["c"]
+        self.typename = _remove_generics(_prettify_typename(val.type))
+        self.visualizer = gdb.default_visualizer(val["c"])
 
     def to_string(self):
         # TODO(tamur): It would be nice to print the top element. The technical
         # difficulty is that, the implementation refers to the underlying
         # container, which is a generic class. libstdcxx pretty printers do not
         # print the top element.
-        typename = _remove_generics(_prettify_typename(self.val.type))
-        return "%s wrapping" % typename
+        return "%s wrapping: %s" % (self.typename, self.visualizer.to_string())
 
     def children(self):
-        return iter([("", self.underlying)])
+        return self.visualizer.children()
 
     def display_hint(self):
         return "array"
@@ -622,13 +623,16 @@ def _traverse(self):
         """Traverses the binary search tree in order."""
         current = self.util.root
         skip_left_child = False
+        i = 0
         while True:
             if not skip_left_child and self.util.left_child(current):
                 current = self.util.left_child(current)
                 continue
             skip_left_child = False
             for key_value in self._get_key_value(current):
-                yield "", key_value
+                key_name = "[%d]" % i
+                i += 1
+                yield key_name, key_value
             right_child = self.util.right_child(current)
             if right_child:
                 current = right_child
@@ -784,10 +788,13 @@ def __init__(self, val):
 
     def _list_it(self, sentinel_ptr):
         next_ptr = sentinel_ptr["__next_"]
+        i = 0
         while str(next_ptr.cast(_void_pointer_type)) != "0x0":
             next_val = next_ptr.cast(self.cast_type).dereference()
             for key_value in self._get_key_value(next_val):
-                yield "", key_value
+                key_name = "[%d]" % i
+                i += 1
+                yield key_name, key_value
             next_ptr = next_val["__next_"]
 
     def to_string(self):
@@ -851,8 +858,8 @@ def children(self):
         return self if self.addr else iter(())
 
     def __iter__(self):
-        for key_value in self._get_key_value():
-            yield "", key_value
+        for i, key_value in enumerate(self._get_key_value()):
+            yield "[%d]" % i, key_value
 
 
 class StdUnorderedSetIteratorPrinter(AbstractHashMapIteratorPrinter):



More information about the libcxx-commits mailing list