[lldb][lldb-server] Enable sending RegisterFlags as XML

[DavidSpickett]

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 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.

[DavidSpickett]

For https://discourse.llvm.org/t/rfc-adding-register-field-information-to-lldb-server/74143.

May be helpful to look at DavidSpickett@93bdc63 which will be the next PR and adds the first register information using this.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/69951.diff

4 Files Affected:

• (modified) lldb/include/lldb/Target/RegisterFlags.h (+8)
• (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp (+10)
• (modified) lldb/source/Target/RegisterFlags.cpp (+32)
• (modified) lldb/unittests/Target/RegisterFlagsTest.cpp (+32)

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");
+}

[DavidSpickett]

And if you're not familiar with lldb-server and XML, as I wasn't, it doesn't use libXML to generate it. That's why I'm building strings to fit with the existing target.xml code.

[JDevlieghere]

And if you're not familiar with lldb-server and XML, as I wasn't, it doesn't use libXML to generate it. That's why I'm building strings to fit with the existing target.xml code.

It's somewhat implied, but is the goal to (continue to) avoid that dependency? Other parts of LLDB do rely on libxml2 but I can see how you'd want to limit the dependencies of lldb-server.

[DavidSpickett]

It's somewhat implied, but is the goal to (continue to) avoid that dependency? Other parts of LLDB do rely on libxml2 but I can see how you'd want to limit the dependencies of lldb-server.

Yeah I agree with how lldb-server does it now, and don't want to change that.

Just mentioning it because without looking at the whole file, it might seem weird to be inserting raw strings into something that is elsewhere built with libXML.

[bulbazord]

LGTM. I didn't realize lldb-server didn't use libXML, learned something new today. :)