[Lldb-commits] [lldb] [lldb][lldb-server] Enable sending RegisterFlags as XML (PR #69951)

David Spickett via lldb-commits lldb-commits at lists.llvm.org
Tue Oct 24 03:14:36 PDT 2023


https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/69951

>From 5c8b9538e1e5646f19d5bb40ab19afaf2c68e804 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Mon, 9 Oct 2023 10:33:08 +0100
Subject: [PATCH 1/5] [lldb][lldb-server] Enable sending registerflags as XML

This adds ToXML methods to encode RegisterFlags and its fields
into XML according to GDB's target XML format:
https://sourceware.org/gdb/onlinedocs/gdb/Target-Description-Format.html#Target-Description-Format

lldb-server does not currently use libXML to build XML,
so this follows the existing code that uses strings. Indentation
is used so the result is still human readable.

```
<flags id=\"Foo\" size=\"4\">
  <field name=\"abc\" start=\"0\" end=\"0\"/>
</flags>
```

This is used by lldb-server when building target XML,
though no one sets any fields yet. That'll come in a later commit.
---
 lldb/include/lldb/Target/RegisterFlags.h      |  8 +++++
 .../GDBRemoteCommunicationServerLLGS.cpp      | 10 ++++++
 lldb/source/Target/RegisterFlags.cpp          | 32 +++++++++++++++++++
 lldb/unittests/Target/RegisterFlagsTest.cpp   | 32 +++++++++++++++++++
 4 files changed, 82 insertions(+)

diff --git a/lldb/include/lldb/Target/RegisterFlags.h b/lldb/include/lldb/Target/RegisterFlags.h
index d98bc0263e35e23..4edbe7255f621e5 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -10,6 +10,7 @@
 #define LLDB_TARGET_REGISTERFLAGS_H
 
 #include "lldb/Utility/Log.h"
+#include "lldb/Utility/StreamString.h"
 
 namespace lldb_private {
 
@@ -51,6 +52,10 @@ class RegisterFlags {
     /// covered by either field.
     unsigned PaddingDistance(const Field &other) const;
 
+    /// Output XML that describes this field, to be inserted into a target XML
+    /// file.
+    void ToXML(StreamString &strm) const;
+
     bool operator<(const Field &rhs) const {
       return GetStart() < rhs.GetStart();
     }
@@ -106,6 +111,9 @@ class RegisterFlags {
   /// be split into many tables as needed.
   std::string AsTable(uint32_t max_width) const;
 
+  // Output XML that describes this set of flags.
+  void ToXML(StreamString &strm) const;
+
 private:
   const std::string m_id;
   /// Size in bytes
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 23c2f18cd388a86..1a4d561146c3bfb 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3094,6 +3094,12 @@ GDBRemoteCommunicationServerLLGS::BuildTargetXml() {
       continue;
     }
 
+    if (reg_info->flags_type) {
+      response.IndentMore();
+      reg_info->flags_type->ToXML(response);
+      response.IndentLess();
+    }
+
     response.Indent();
     response.Printf("<reg name=\"%s\" bitsize=\"%" PRIu32
                     "\" regnum=\"%d\" ",
@@ -3113,6 +3119,10 @@ GDBRemoteCommunicationServerLLGS::BuildTargetXml() {
     if (!format.empty())
       response << "format=\"" << format << "\" ";
 
+    if (reg_info->flags_type) {
+      response << "type=\"" << reg_info->flags_type->GetID() << "\" ";
+    }
+
     const char *const register_set_name =
         reg_context.GetRegisterSetNameForRegisterAtIndex(reg_index);
     if (register_set_name)
diff --git a/lldb/source/Target/RegisterFlags.cpp b/lldb/source/Target/RegisterFlags.cpp
index 06fb45d777ec36f..a0019d15a2088b2 100644
--- a/lldb/source/Target/RegisterFlags.cpp
+++ b/lldb/source/Target/RegisterFlags.cpp
@@ -175,3 +175,35 @@ std::string RegisterFlags::AsTable(uint32_t max_width) const {
 
   return table;
 }
+
+void RegisterFlags::ToXML(StreamString &strm) const {
+  // Example XML:
+  // <flags id="cpsr_flags" size="4">
+  //   <field name="incorrect" start="0" end="0"/>
+  // </flags>
+  strm.Indent();
+  strm << "<flags id=\"" << GetID() << "\" ";
+  strm.Printf("size=\"%d\"", GetSize());
+  strm << ">";
+  for (const Field &field : m_fields) {
+    // Skip padding fields.
+    if (field.GetName().empty())
+      continue;
+
+    strm << "\n";
+    strm.IndentMore();
+    field.ToXML(strm);
+    strm.IndentLess();
+  }
+  strm.PutChar('\n');
+  strm.Indent("</flags>\n");
+}
+
+void RegisterFlags::Field::ToXML(StreamString &strm) const {
+  // Example XML:
+  // <field name="correct" start="0" end="0"/>
+  strm.Indent();
+  strm << "<field name=\"" << GetName() << "\" ";
+  strm.Printf("start=\"%d\" end=\"%d\"", GetStart(), GetEnd());
+  strm << "/>";
+}
diff --git a/lldb/unittests/Target/RegisterFlagsTest.cpp b/lldb/unittests/Target/RegisterFlagsTest.cpp
index 167e28d0cecb3bd..3b34e6b1e1c160c 100644
--- a/lldb/unittests/Target/RegisterFlagsTest.cpp
+++ b/lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -258,3 +258,35 @@ TEST(RegisterFlagsTest, AsTable) {
             "| really long name |",
             max_many_columns.AsTable(23));
 }
+
+TEST(RegisterFieldsTest, ToXML) {
+  StreamString strm;
+
+  // RegisterFlags requires that some fields be given, so no testing of empty
+  // input.
+
+  // Unnamed fields are padding that are ignored. This applies to fields passed
+  // in, and those generated to fill the other bits (31-1 here).
+  RegisterFlags("Foo", 4, {RegisterFlags::Field("", 0, 0)}).ToXML(strm);
+  ASSERT_EQ(strm.GetString(), "<flags id=\"Foo\" size=\"4\">\n"
+                              "</flags>\n");
+
+  strm.Clear();
+  RegisterFlags("Foo", 4, {RegisterFlags::Field("abc", 0, 0)}).ToXML(strm);
+  ASSERT_EQ(strm.GetString(), "<flags id=\"Foo\" size=\"4\">\n"
+                              "  <field name=\"abc\" start=\"0\" end=\"0\"/>\n"
+                              "</flags>\n");
+
+  strm.Clear();
+  // Should use the current indentation level as a starting point.
+  strm.IndentMore();
+  RegisterFlags(
+      "Bar", 5,
+      {RegisterFlags::Field("f1", 25, 32), RegisterFlags::Field("f2", 10, 24)})
+      .ToXML(strm);
+  ASSERT_EQ(strm.GetString(),
+            "  <flags id=\"Bar\" size=\"5\">\n"
+            "    <field name=\"f1\" start=\"25\" end=\"32\"/>\n"
+            "    <field name=\"f2\" start=\"10\" end=\"24\"/>\n"
+            "  </flags>\n");
+}

>From 0efbe8f21031e6dcb569964814df2e41560e7fc1 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 24 Oct 2023 08:36:37 +0000
Subject: [PATCH 2/5] Forward declare some things, add missing standard
 includes as a result.

---
 lldb/include/lldb/Target/RegisterFlags.h | 9 +++++++--
 lldb/source/Target/RegisterFlags.cpp     | 1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lldb/include/lldb/Target/RegisterFlags.h b/lldb/include/lldb/Target/RegisterFlags.h
index 4edbe7255f621e5..b30e938079a8237 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -9,11 +9,16 @@
 #ifndef LLDB_TARGET_REGISTERFLAGS_H
 #define LLDB_TARGET_REGISTERFLAGS_H
 
-#include "lldb/Utility/Log.h"
-#include "lldb/Utility/StreamString.h"
+#include <cassert>
+#include <stdint.h>
+#include <string>
+#include <vector>
 
 namespace lldb_private {
 
+class StreamString;
+class Log;
+
 class RegisterFlags {
 public:
   class Field {
diff --git a/lldb/source/Target/RegisterFlags.cpp b/lldb/source/Target/RegisterFlags.cpp
index a0019d15a2088b2..c6820115a344e5c 100644
--- a/lldb/source/Target/RegisterFlags.cpp
+++ b/lldb/source/Target/RegisterFlags.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Target/RegisterFlags.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 
 #include <numeric>

>From 92602331171efea94d801a3cecbf446ff197ced5 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 24 Oct 2023 08:43:12 +0000
Subject: [PATCH 3/5] Single line if.

---
 .../Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp    | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 1a4d561146c3bfb..187c23a206094c0 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3119,9 +3119,8 @@ GDBRemoteCommunicationServerLLGS::BuildTargetXml() {
     if (!format.empty())
       response << "format=\"" << format << "\" ";
 
-    if (reg_info->flags_type) {
+    if (reg_info->flags_type)
       response << "type=\"" << reg_info->flags_type->GetID() << "\" ";
-    }
 
     const char *const register_set_name =
         reg_context.GetRegisterSetNameForRegisterAtIndex(reg_index);

>From 846a7013c09d63f248e9f9a12e5ed2195a49d86c Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 24 Oct 2023 10:07:27 +0000
Subject: [PATCH 4/5] Escape/replace reserved XML characters in field names.

---
 lldb/include/lldb/Target/RegisterFlags.h    |  3 ++-
 lldb/source/Target/RegisterFlags.cpp        | 10 +++++++++-
 lldb/unittests/Target/RegisterFlagsTest.cpp | 18 ++++++++++++++++++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/lldb/include/lldb/Target/RegisterFlags.h b/lldb/include/lldb/Target/RegisterFlags.h
index b30e938079a8237..d685b5c78868ac9 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -58,7 +58,8 @@ class RegisterFlags {
     unsigned PaddingDistance(const Field &other) const;
 
     /// Output XML that describes this field, to be inserted into a target XML
-    /// file.
+    /// file. Reserved characters in field names like "<"" are replaced with
+    /// their XML safe equivalents like ">".
     void ToXML(StreamString &strm) const;
 
     bool operator<(const Field &rhs) const {
diff --git a/lldb/source/Target/RegisterFlags.cpp b/lldb/source/Target/RegisterFlags.cpp
index c6820115a344e5c..580f180cbf263e1 100644
--- a/lldb/source/Target/RegisterFlags.cpp
+++ b/lldb/source/Target/RegisterFlags.cpp
@@ -10,6 +10,8 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 
+#include "llvm/ADT/StringExtras.h"
+
 #include <numeric>
 #include <optional>
 
@@ -204,7 +206,13 @@ void RegisterFlags::Field::ToXML(StreamString &strm) const {
   // Example XML:
   // <field name="correct" start="0" end="0"/>
   strm.Indent();
-  strm << "<field name=\"" << GetName() << "\" ";
+  strm << "<field name=\"";
+
+  std::string escaped_name;
+  llvm::raw_string_ostream escape_strm(escaped_name);
+  llvm::printHTMLEscaped(GetName(), escape_strm);
+  strm << escaped_name << "\" ";
+
   strm.Printf("start=\"%d\" end=\"%d\"", GetStart(), GetEnd());
   strm << "/>";
 }
diff --git a/lldb/unittests/Target/RegisterFlagsTest.cpp b/lldb/unittests/Target/RegisterFlagsTest.cpp
index 3b34e6b1e1c160c..c7a419203165538 100644
--- a/lldb/unittests/Target/RegisterFlagsTest.cpp
+++ b/lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Target/RegisterFlags.h"
+#include "lldb/Utility/StreamString.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -289,4 +290,21 @@ TEST(RegisterFieldsTest, ToXML) {
             "    <field name=\"f1\" start=\"25\" end=\"32\"/>\n"
             "    <field name=\"f2\" start=\"10\" end=\"24\"/>\n"
             "  </flags>\n");
+
+  strm.Clear();
+  strm.IndentLess();
+  // Should replace any XML unsafe characters in field names.
+  RegisterFlags("Safe", 8,
+                {RegisterFlags::Field("A<", 4), RegisterFlags::Field("B>", 3),
+                 RegisterFlags::Field("C'", 2), RegisterFlags::Field("D\"", 1),
+                 RegisterFlags::Field("E&", 0)})
+      .ToXML(strm);
+  ASSERT_EQ(strm.GetString(),
+            "<flags id=\"Safe\" size=\"8\">\n"
+            "  <field name=\"A<\" start=\"4\" end=\"4\"/>\n"
+            "  <field name=\"B>\" start=\"3\" end=\"3\"/>\n"
+            "  <field name=\"C'\" start=\"2\" end=\"2\"/>\n"
+            "  <field name=\"D"\" start=\"1\" end=\"1\"/>\n"
+            "  <field name=\"E&\" start=\"0\" end=\"0\"/>\n"
+            "</flags>\n");
 }

>From 94ea66d1148f4429d8bff884edf09f7134b91c68 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Tue, 24 Oct 2023 10:14:16 +0000
Subject: [PATCH 5/5] Fix docs typo.

---
 lldb/include/lldb/Target/RegisterFlags.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/include/lldb/Target/RegisterFlags.h b/lldb/include/lldb/Target/RegisterFlags.h
index d685b5c78868ac9..e6086567ebf2e20 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -58,7 +58,7 @@ class RegisterFlags {
     unsigned PaddingDistance(const Field &other) const;
 
     /// Output XML that describes this field, to be inserted into a target XML
-    /// file. Reserved characters in field names like "<"" are replaced with
+    /// file. Reserved characters in field names like "<" are replaced with
     /// their XML safe equivalents like ">".
     void ToXML(StreamString &strm) const;
 



More information about the lldb-commits mailing list