[Lldb-commits] [lldb] [lldb] Add register field enum class (PR #90063)

via lldb-commits lldb-commits at lists.llvm.org
Thu Apr 25 07:22:43 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)

<details>
<summary>Changes</summary>

This represents the enum type that can be assigned to a field using the `<enum>` element in the target XML.

https://sourceware.org/gdb/current/onlinedocs/gdb.html/Enum-Target-Types.html

Each enumerator has:
* A non-empty name
* A value that is within the range of the field it's applied to

The XML includes a "size" but we don't need that for anything and it's a pain to verify so I've left it out of our internal structures. When emitting XML we'll set size to the size of the register using the enum.

An Enumerator class is added to RegisterFlags and hooked up to the existing ToXML so lldb-server can use it to emit enums as well.

As enums are elements on the same level as flags, when emitting XML we'll do so via the registers. Before emitting a flags element we look at all the fields and see what enums they reference. Then print all of those if we haven't already done so.

Functions are added to dump enum information for `register info` to use to show the enum information.

---

Patch is 21.03 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90063.diff


3 Files Affected:

- (modified) lldb/include/lldb/Target/RegisterFlags.h (+65-7) 
- (modified) lldb/source/Target/RegisterFlags.cpp (+197-2) 
- (modified) lldb/unittests/Target/RegisterFlagsTest.cpp (+174-1) 


``````````diff
diff --git a/lldb/include/lldb/Target/RegisterFlags.h b/lldb/include/lldb/Target/RegisterFlags.h
index 9b343e445678ab..49219a52e13643 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -13,11 +13,42 @@
 #include <string>
 #include <vector>
 
+#include "llvm/ADT/StringSet.h"
+
 namespace lldb_private {
 
 class StreamString;
 class Log;
 
+class FieldEnum {
+public:
+  struct Enumerator {
+    uint64_t m_value;
+    // Short name for the value. Shown in tables and when printing the field's
+    // value. For example "RZ".
+    std::string m_name;
+
+    Enumerator(uint64_t value, std::string name)
+        : m_value(value), m_name(name) {}
+
+    void ToXML(StreamString &strm) const;
+  };
+
+  typedef std::vector<Enumerator> Enumerators;
+
+  FieldEnum(std::string id, const Enumerators &enumerators);
+
+  const Enumerators &GetEnumerators() const { return m_enumerators; }
+
+  const std::string &GetID() const { return m_id; }
+
+  void ToXML(StreamString &strm, unsigned size) const;
+
+private:
+  std::string m_id;
+  std::vector<Enumerator> m_enumerators;
+};
+
 class RegisterFlags {
 public:
   class Field {
@@ -26,17 +57,27 @@ class RegisterFlags {
     /// significant bit. The start bit must be <= the end bit.
     Field(std::string name, unsigned start, unsigned end);
 
+    /// Construct a field that also has some known enum values.
+    Field(std::string name, unsigned start, unsigned end,
+          const FieldEnum *enum_type);
+
     /// Construct a field that occupies a single bit.
-    Field(std::string name, unsigned bit_position)
-        : m_name(std::move(name)), m_start(bit_position), m_end(bit_position) {}
+    Field(std::string name, unsigned bit_position);
 
     /// Get size of the field in bits. Will always be at least 1.
-    unsigned GetSizeInBits() const { return m_end - m_start + 1; }
+    unsigned GetSizeInBits() const;
+
+    /// Identical to GetSizeInBits, but for the GDB client to use.
+    static unsigned GetSizeInBits(unsigned start, unsigned end);
 
     /// A mask that covers all bits of the field.
-    uint64_t GetMask() const {
-      return (((uint64_t)1 << (GetSizeInBits())) - 1) << m_start;
-    }
+    uint64_t GetMask() const;
+
+    /// The maximum unsigned value that could be contained in this field.
+    uint64_t GetMaxValue() const;
+
+    /// Identical to GetMaxValue but for the GDB client to use.
+    static uint64_t GetMaxValue(unsigned start, unsigned end);
 
     /// Extract value of the field from a whole register value.
     uint64_t GetValue(uint64_t register_value) const {
@@ -46,6 +87,7 @@ class RegisterFlags {
     const std::string &GetName() const { return m_name; }
     unsigned GetStart() const { return m_start; }
     unsigned GetEnd() const { return m_end; }
+    const FieldEnum *GetEnum() const { return m_enum_type; }
     bool Overlaps(const Field &other) const;
     void log(Log *log) const;
 
@@ -69,12 +111,15 @@ class RegisterFlags {
 
   private:
     std::string m_name;
+
     /// Start/end bit positions. Where start N, end N means a single bit
     /// field at position N. We expect that start <= end. Bit positions begin
     /// at 0.
     /// Start is the LSB, end is the MSB.
     unsigned m_start;
     unsigned m_end;
+
+    const FieldEnum *m_enum_type;
   };
 
   /// This assumes that:
@@ -89,6 +134,10 @@ class RegisterFlags {
   /// when runtime field detection is needed.
   void SetFields(const std::vector<Field> &fields);
 
+  /// Make a string where each line contains the name of a field that has
+  /// enum values, and lists what those values are.
+  std::string DumpEnums(uint32_t max_width) const;
+
   // Reverse the order of the fields, keeping their values the same.
   // For example a field from bit 31 to 30 with value 0b10 will become bits
   // 1 to 0, with the same 0b10 value.
@@ -118,9 +167,18 @@ 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.
+  /// Output XML that describes this set of flags.
+  /// EnumsToXML should have been called before this.
   void ToXML(StreamString &strm) const;
 
+  /// Enum types must be defined before use, and
+  /// GDBRemoteCommunicationServerLLGS view of the register types is based only
+  /// on the registers. So this method emits any enum types that the upcoming
+  /// set of fields may need. "seen" is a set of Enum IDs that we have already
+  /// printed, that is updated with any printed by this call. This prevents us
+  /// printing the same enum multiple times.
+  void EnumsToXML(StreamString &strm, llvm::StringSet<> &seen) const;
+
 private:
   const std::string m_id;
   /// Size in bytes
diff --git a/lldb/source/Target/RegisterFlags.cpp b/lldb/source/Target/RegisterFlags.cpp
index b1669b85fd2fe7..1dd1baa3ebba36 100644
--- a/lldb/source/Target/RegisterFlags.cpp
+++ b/lldb/source/Target/RegisterFlags.cpp
@@ -12,16 +12,41 @@
 
 #include "llvm/ADT/StringExtras.h"
 
+#include <limits>
 #include <numeric>
 #include <optional>
 
 using namespace lldb_private;
 
 RegisterFlags::Field::Field(std::string name, unsigned start, unsigned end)
-    : m_name(std::move(name)), m_start(start), m_end(end) {
+    : m_name(std::move(name)), m_start(start), m_end(end),
+      m_enum_type(nullptr) {
   assert(m_start <= m_end && "Start bit must be <= end bit.");
 }
 
+RegisterFlags::Field::Field(std::string name, unsigned bit_position)
+    : m_name(std::move(name)), m_start(bit_position), m_end(bit_position),
+      m_enum_type(nullptr) {}
+
+RegisterFlags::Field::Field(std::string name, unsigned start, unsigned end,
+                            const FieldEnum *enum_type)
+    : m_name(std::move(name)), m_start(start), m_end(end),
+      m_enum_type(enum_type) {
+  if (m_enum_type) {
+    // Check that all values fit into this field. The XML parser will also
+    // do this check so at runtime nothing should fail this check.
+    // We can also make enums in C++ at compile time, which might fail this
+    // check, so we catch them before it makes it into a release.
+    uint64_t max_value = GetMaxValue();
+    UNUSED_IF_ASSERT_DISABLED(max_value);
+    for (const auto &enumerator : m_enum_type->GetEnumerators()) {
+      UNUSED_IF_ASSERT_DISABLED(enumerator);
+      assert(enumerator.m_value <= max_value &&
+             "Enumerator value exceeds maximum value for this field");
+    }
+  }
+}
+
 void RegisterFlags::Field::log(Log *log) const {
   LLDB_LOG(log, "  Name: \"{0}\" Start: {1} End: {2}", m_name.c_str(), m_start,
            m_end);
@@ -53,6 +78,35 @@ unsigned RegisterFlags::Field::PaddingDistance(const Field &other) const {
   return lhs_start - rhs_end - 1;
 }
 
+unsigned RegisterFlags::Field::GetSizeInBits(unsigned start, unsigned end) {
+  return end - start + 1;
+}
+
+unsigned RegisterFlags::Field::GetSizeInBits() const {
+  return GetSizeInBits(m_start, m_end);
+}
+
+uint64_t RegisterFlags::Field::GetMaxValue(unsigned start, unsigned end) {
+  uint64_t max = std::numeric_limits<uint64_t>::max();
+  unsigned bits = GetSizeInBits(start, end);
+  // If the field is >= 64 bits the shift below would be undefined.
+  // We assume the GDB client has discarded any field that would fail this
+  // assert, it's only to check information we define directly in C++.
+  assert(bits <= 64 && "Cannot handle field with size > 64 bits");
+  if (bits < 64) {
+    max = ((uint64_t)1 << bits) - 1;
+  }
+  return max;
+}
+
+uint64_t RegisterFlags::Field::GetMaxValue() const {
+  return GetMaxValue(m_start, m_end);
+}
+
+uint64_t RegisterFlags::Field::GetMask() const {
+  return GetMaxValue() << m_start;
+}
+
 void RegisterFlags::SetFields(const std::vector<Field> &fields) {
   // We expect that the XML processor will discard anything describing flags but
   // with no fields.
@@ -190,6 +244,133 @@ std::string RegisterFlags::AsTable(uint32_t max_width) const {
   return table;
 }
 
+// Print enums as:
+// value = name, value2 = name2
+// Subject to the limits of the terminal width.
+static void DumpEnumerators(StreamString &strm, size_t indent,
+                            size_t current_width, uint32_t max_width,
+                            const FieldEnum::Enumerators &enumerators) {
+  for (auto it = enumerators.cbegin(); it != enumerators.cend(); ++it) {
+    StreamString enumerator_strm;
+    // The first enumerator of a line doesn't need to be separated.
+    if (current_width != indent)
+      enumerator_strm << ' ';
+
+    enumerator_strm.Printf("%" PRIu64 " = %s", it->m_value, it->m_name.c_str());
+
+    // Don't put "," after the last enumerator.
+    if (std::next(it) != enumerators.cend())
+      enumerator_strm << ",";
+
+    llvm::StringRef enumerator_string = enumerator_strm.GetString();
+    // If printing the next enumerator would take us over the width, start
+    // a new line. However, if we're printing the first enumerator of this
+    // line, don't start a new one. Resulting in there being at least one per
+    // line.
+    //
+    // This means for very small widths we get:
+    // A: 0 = foo,
+    //    1 = bar
+    // Instead of:
+    // A:
+    //    0 = foo,
+    //    1 = bar
+    if ((current_width + enumerator_string.size() > max_width) &&
+        current_width != indent) {
+      current_width = indent;
+      strm << '\n' << std::string(indent, ' ');
+      // We're going to a new line so we don't need a space before the
+      // name of the enumerator.
+      enumerator_string = enumerator_string.drop_front();
+    }
+
+    current_width += enumerator_string.size();
+    strm << enumerator_string;
+  }
+}
+
+std::string RegisterFlags::DumpEnums(uint32_t max_width) const {
+  StreamString strm;
+  bool printed_enumerators_once = false;
+
+  for (const auto &field : m_fields) {
+    const FieldEnum *enum_type = field.GetEnum();
+    if (!enum_type)
+      continue;
+
+    const FieldEnum::Enumerators &enumerators = enum_type->GetEnumerators();
+    if (enumerators.empty())
+      continue;
+
+    // Break between enumerators of different fields.
+    if (printed_enumerators_once)
+      strm << "\n\n";
+    else
+      printed_enumerators_once = true;
+
+    std::string name_string = field.GetName() + ": ";
+    size_t indent = name_string.size();
+    size_t current_width = indent;
+
+    strm << name_string;
+
+    DumpEnumerators(strm, indent, current_width, max_width, enumerators);
+  }
+
+  return strm.GetString().str();
+}
+
+void RegisterFlags::EnumsToXML(StreamString &strm,
+                               llvm::StringSet<> &seen) const {
+  for (const Field &field : m_fields)
+    if (const FieldEnum *enum_type = field.GetEnum()) {
+      const std::string &id = enum_type->GetID();
+      if (!seen.contains(id)) {
+        enum_type->ToXML(strm, GetSize());
+        seen.insert(id);
+      }
+    }
+}
+
+void FieldEnum::ToXML(StreamString &strm, unsigned size) const {
+  // Example XML:
+  // <enum id="foo" size="4">
+  //  <evalue name="bar" value="1"/>
+  // </enum>
+  // Note that "size" is only emitted for GDB compatibility, LLDB does not need
+  // it.
+
+  strm.Indent();
+  strm << "<enum id=\"" << GetID() << "\" ";
+  // This is the size of the underlying enum type if this were a C type.
+  // In other words, the size of the register in bytes.
+  strm.Printf("size=\"%d\"", size);
+
+  const Enumerators &enumerators = GetEnumerators();
+  if (enumerators.empty()) {
+    strm << "/>\n";
+    return;
+  }
+
+  strm << ">\n";
+  strm.IndentMore();
+  for (const auto &enumerator : enumerators) {
+    strm.Indent();
+    enumerator.ToXML(strm);
+    strm.PutChar('\n');
+  }
+  strm.IndentLess();
+  strm.Indent("</enum>\n");
+}
+
+void FieldEnum::Enumerator::ToXML(StreamString &strm) const {
+  std::string escaped_name;
+  llvm::raw_string_ostream escape_strm(escaped_name);
+  llvm::printHTMLEscaped(m_name, escape_strm);
+  strm.Printf("<evalue name=\"%s\" value=\"%" PRIu64 "\"/>",
+              escaped_name.c_str(), m_value);
+}
+
 void RegisterFlags::ToXML(StreamString &strm) const {
   // Example XML:
   // <flags id="cpsr_flags" size="4">
@@ -214,7 +395,9 @@ void RegisterFlags::ToXML(StreamString &strm) const {
 }
 
 void RegisterFlags::Field::ToXML(StreamString &strm) const {
-  // Example XML:
+  // Example XML with an enum:
+  // <field name="correct" start="0" end="0" type="some_enum">
+  // Without:
   // <field name="correct" start="0" end="0"/>
   strm.Indent();
   strm << "<field name=\"";
@@ -225,5 +408,17 @@ void RegisterFlags::Field::ToXML(StreamString &strm) const {
   strm << escaped_name << "\" ";
 
   strm.Printf("start=\"%d\" end=\"%d\"", GetStart(), GetEnd());
+
+  if (const FieldEnum *enum_type = GetEnum())
+    strm << " type=\"" << enum_type->GetID() << "\"";
+
   strm << "/>";
 }
+
+FieldEnum::FieldEnum(std::string id, const Enumerators &enumerators)
+    : m_id(id), m_enumerators(enumerators) {
+  for (const auto &enumerator : m_enumerators) {
+    UNUSED_IF_ASSERT_DISABLED(enumerator);
+    assert(enumerator.m_name.size() && "Enumerator name cannot be empty");
+  }
+}
\ No newline at end of file
diff --git a/lldb/unittests/Target/RegisterFlagsTest.cpp b/lldb/unittests/Target/RegisterFlagsTest.cpp
index c7a41920316553..f2c61c4988b038 100644
--- a/lldb/unittests/Target/RegisterFlagsTest.cpp
+++ b/lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -260,7 +260,88 @@ TEST(RegisterFlagsTest, AsTable) {
             max_many_columns.AsTable(23));
 }
 
-TEST(RegisterFieldsTest, ToXML) {
+TEST(RegisterFlagsTest, DumpEnums) {
+  ASSERT_EQ(RegisterFlags("", 8, {RegisterFlags::Field{"A", 0}}).DumpEnums(80),
+            "");
+
+  FieldEnum basic_enum("test", {{0, "an_enumerator"}});
+  ASSERT_EQ(RegisterFlags("", 8, {RegisterFlags::Field{"A", 0, 0, &basic_enum}})
+                .DumpEnums(80),
+            "A: 0 = an_enumerator");
+
+  // If width is smaller than the enumerator name, print it anyway.
+  ASSERT_EQ(RegisterFlags("", 8, {RegisterFlags::Field{"A", 0, 0, &basic_enum}})
+                .DumpEnums(5),
+            "A: 0 = an_enumerator");
+
+  // Mutliple values can go on the same line, up to the width.
+  FieldEnum more_enum("long_enum",
+                      {{0, "an_enumerator"},
+                       {1, "another_enumerator"},
+                       {2, "a_very_very_long_enumerator_has_its_own_line"},
+                       {3, "small"},
+                       {4, "small2"}});
+  ASSERT_EQ(RegisterFlags("", 8, {RegisterFlags::Field{"A", 0, 2, &more_enum}})
+                // Width is chosen to be exactly enough to allow 0 and 1
+                // enumerators on the first line.
+                .DumpEnums(45),
+            "A: 0 = an_enumerator, 1 = another_enumerator,\n"
+            "   2 = a_very_very_long_enumerator_has_its_own_line,\n"
+            "   3 = small, 4 = small2");
+
+  // If they all exceed width, one per line.
+  FieldEnum another_enum("another_enum", {{0, "an_enumerator"},
+                                          {1, "another_enumerator"},
+                                          {2, "a_longer_enumerator"}});
+  ASSERT_EQ(
+      RegisterFlags("", 8, {RegisterFlags::Field{"A", 0, 1, &another_enum}})
+          .DumpEnums(5),
+      "A: 0 = an_enumerator,\n"
+      "   1 = another_enumerator,\n"
+      "   2 = a_longer_enumerator");
+
+  // If the name is already > the width, put one value per line.
+  FieldEnum short_enum("short_enum", {{0, "a"}, {1, "b"}, {2, "c"}});
+  ASSERT_EQ(RegisterFlags("", 8,
+                          {RegisterFlags::Field{"AReallyLongFieldName", 0, 1,
+                                                &short_enum}})
+                .DumpEnums(10),
+            "AReallyLongFieldName: 0 = a,\n"
+            "                      1 = b,\n"
+            "                      2 = c");
+
+  // Fields are separated by a blank line. Indentation of lines split by width
+  // is set by the size of the fields name (as opposed to some max of all field
+  // names).
+  FieldEnum enum_1("enum_1", {{0, "an_enumerator"}, {1, "another_enumerator"}});
+  FieldEnum enum_2("enum_2",
+                   {{0, "Cdef_enumerator_1"}, {1, "Cdef_enumerator_2"}});
+  ASSERT_EQ(RegisterFlags("", 8,
+                          {RegisterFlags::Field{"Ab", 1, 1, &enum_1},
+                           RegisterFlags::Field{"Cdef", 0, 0, &enum_2}})
+                .DumpEnums(10),
+            "Ab: 0 = an_enumerator,\n"
+            "    1 = another_enumerator\n"
+            "\n"
+            "Cdef: 0 = Cdef_enumerator_1,\n"
+            "      1 = Cdef_enumerator_2");
+
+  // Having fields without enumerators shouldn't produce any extra newlines.
+  ASSERT_EQ(RegisterFlags("", 8,
+                          {
+                              RegisterFlags::Field{"A", 4, 4},
+                              RegisterFlags::Field{"B", 3, 3, &enum_1},
+                              RegisterFlags::Field{"C", 2, 2},
+                              RegisterFlags::Field{"D", 1, 1, &enum_1},
+                              RegisterFlags::Field{"E", 0, 0},
+                          })
+                .DumpEnums(80),
+            "B: 0 = an_enumerator, 1 = another_enumerator\n"
+            "\n"
+            "D: 0 = an_enumerator, 1 = another_enumerator");
+}
+
+TEST(RegisterFieldsTest, FlagsToXML) {
   StreamString strm;
 
   // RegisterFlags requires that some fields be given, so no testing of empty
@@ -307,4 +388,96 @@ TEST(RegisterFieldsTest, ToXML) {
             "  <field name=\"D"\" start=\"1\" end=\"1\"/>\n"
             "  <field name=\"E&\" start=\"0\" end=\"0\"/>\n"
             "</flags>\n");
+
+  // Should include enumerators as the "type".
+  strm.Clear();
+  FieldEnum enum_single("enum_single", {{0, "a"}});
+  RegisterFlags("Enumerators", 8,
+                {RegisterFlags::Field("NoEnumerators", 4),
+                 RegisterFlags::Field("OneEnumerator", 3, 3, &enum_single)})
+      .ToXML(strm);
+  ASSERT_EQ(strm.GetString(),
+            "<flags id=\"Enumerators\" size=\"8\">\n"
+            "  <field name=\"NoEnumerators\" start=\"4\" end=\"4\"/>\n"
+            "  <field name=\"OneEnumerator\" start=\"3\" end=\"3\" "
+            "type=\"enum_single\"/>\n"
+            "</flags>\n");
+}
+
+TEST(RegisterFlagsTest, EnumeratorToXML) {
+  StreamString strm;
+
+  FieldEnum::Enumerator(1234, "test").ToXML(strm);
+  ASSERT_EQ(strm.GetString(), "<evalue name=\"test\" value=\"1234\"/>");
+
+  // Special XML chars in names must be escaped.
+  std::array special_names = {
+      std::make_pair(FieldEnum::Enumerator(0, "A<"),
+                     "<evalue name=\"A<\" value=\"0\"/>"),
+      std::make_pair(FieldEnum::Enumerator(1, "B>"),
+                     "<evalue name=\"B>\" value=\"1\"/>"),
+      std::make_pair(FieldEnum::Enumerator(2, "C'"),
+                     "<evalue name=\"C'\" value=\"2\"/>"),
+      std::make_pair(FieldEnum::Enumerator(3, "D\""),
+                     "<evalue name=\"D"\" value=\"3\"/>"),
+      std::make_pair(FieldEnum::Enumerator(4, "E&"),
+                     "<evalue name=\"E&\" value=\"4\"/>"),
+  };
+
+  for (const auto &[enumerator, expected] : special_names) {
+    strm.Clear();
+    enumerator.ToXML(strm);
+    ASSERT_EQ(strm.GetString(), expected);
+  }
+}
+
+TEST(RegisterFlagsTest, EnumToXML) {
+  StreamString strm;
+
+  FieldEnum("empty_enum", {}).ToXML(strm, 4);
+  ASSERT_EQ(strm.GetString(), "<enum id=\"empty_enum\" size=\"4\"/>\n");
+
+  strm.Clear();
+  FieldEnum("single_enumerator", {FieldEnum::Enumerator(0, "zero")})
+      .ToXML(strm, 5);
+  ASSERT_EQ(strm.GetString(), "<enum id=\"single_enumerator\" size=\"5\">\n"
+                              "  <evalue name=\"zero\" value=\"0\"/>\n"
+                              "</enum>\n");
+
+  strm.Clear();
+  FieldEnum("multiple_enumerator",
+            {FieldEnum::Enumerator(0, "zero"), FieldEnum::Enumerator(1, "one")})
+      .ToXML(strm, 8);
+  ASSERT_EQ(strm.GetString(), "<enum id=\"multiple_enumerator\" size=\"8\">\n"
+                              "  <evalue name=\"zero\" value=\"0\"/>\n"
+                              "  <evalue name=\"one\" value=\"1\"/>\n"
+                              "</enum>\n");
 }
+
+TEST(RegisterFlagsTest, EnumsToXML) {
+  // This method should output all the enums used by the register flag set,
+  // only once.
+
+  StreamString strm;
+  FieldEnum enum_a("enum_a", {Fi...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/90063


More information about the lldb-commits mailing list