Revert "[lldb] Parse and display register field enums (#95768)"

This reverts commit ba60d8a.

Author: DavidSpickett

lldb/include/lldb/Target/RegisterFlags.h

@@ -32,15 +32,10 @@ class FieldEnum {
         : m_value(value), m_name(std::move(name)) {}
 
     void ToXML(Stream &strm) const;
-
-    void DumpToLog(Log *log) const;
   };
 
   typedef std::vector<Enumerator> Enumerators;
 
-  // GDB also includes a "size" that is the size of the underlying register.
-  // We will not store that here but instead use the size of the register
-  // this gets attached to when emitting XML.
   FieldEnum(std::string id, const Enumerators &enumerators);
 
   const Enumerators &GetEnumerators() const { return m_enumerators; }
@@ -49,8 +44,6 @@ class FieldEnum {
 
   void ToXML(Stream &strm, unsigned size) const;
 
-  void DumpToLog(Log *log) const;
-
 private:
   std::string m_id;
   Enumerators m_enumerators;

lldb/source/Core/DumpRegisterInfo.cpp

@@ -111,11 +111,6 @@ void lldb_private::DoDumpRegisterInfo(
   };
   DumpList(strm, "    In sets: ", in_sets, emit_set);
 
-  if (flags_type) {
+  if (flags_type)
     strm.Printf("\n\n%s", flags_type->AsTable(terminal_width).c_str());
-
-    std::string enumerators = flags_type->DumpEnums(terminal_width);
-    if (enumerators.size())
-      strm << "\n\n" << enumerators;
-  }
 }

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

@@ -4179,134 +4179,21 @@ struct GdbServerTargetInfo {
   RegisterSetMap reg_set_map;
 };
 
-static FieldEnum::Enumerators ParseEnumEvalues(const XMLNode &enum_node) {
-  Log *log(GetLog(GDBRLog::Process));
-  // We will use the last instance of each value. Also we preserve the order
-  // of declaration in the XML, as it may not be numerical.
-  // For example, hardware may intially release with two states that softwware
-  // can read from a register field:
-  // 0 = startup, 1 = running
-  // If in a future hardware release, the designers added a pre-startup state:
-  // 0 = startup, 1 = running, 2 = pre-startup
-  // Now it makes more sense to list them in this logical order as opposed to
-  // numerical order:
-  // 2 = pre-startup, 1 = startup, 0 = startup
-  // This only matters for "register info" but let's trust what the server
-  // chose regardless.
-  std::map<uint64_t, FieldEnum::Enumerator> enumerators;
-
-  enum_node.ForEachChildElementWithName(
-      "evalue", [&enumerators, &log](const XMLNode &enumerator_node) {
-        std::optional<llvm::StringRef> name;
-        std::optional<uint64_t> value;
-
-        enumerator_node.ForEachAttribute(
-            [&name, &value, &log](const llvm::StringRef &attr_name,
-                                  const llvm::StringRef &attr_value) {
-              if (attr_name == "name") {
-                if (attr_value.size())
-                  name = attr_value;
-                else
-                  LLDB_LOG(log, "ProcessGDBRemote::ParseEnumEvalues "
-                                "Ignoring empty name in evalue");
-              } else if (attr_name == "value") {
-                uint64_t parsed_value = 0;
-                if (llvm::to_integer(attr_value, parsed_value))
-                  value = parsed_value;
-                else
-                  LLDB_LOG(log,
-                           "ProcessGDBRemote::ParseEnumEvalues "
-                           "Invalid value \"{0}\" in "
-                           "evalue",
-                           attr_value.data());
-              } else
-                LLDB_LOG(log,
-                         "ProcessGDBRemote::ParseEnumEvalues Ignoring "
-                         "unknown attribute "
-                         "\"{0}\" in evalue",
-                         attr_name.data());
-
-              // Keep walking attributes.
-              return true;
-            });
-
-        if (value && name)
-          enumerators.insert_or_assign(
-              *value, FieldEnum::Enumerator(*value, name->str()));
-
-        // Find all evalue elements.
-        return true;
-      });
-
-  FieldEnum::Enumerators final_enumerators;
-  for (auto [_, enumerator] : enumerators)
-    final_enumerators.push_back(enumerator);
-
-  return final_enumerators;
-}
-
-static void
-ParseEnums(XMLNode feature_node,
-           llvm::StringMap<std::unique_ptr<FieldEnum>> &registers_enum_types) {
-  Log *log(GetLog(GDBRLog::Process));
-
-  // The top level element is "<enum...".
-  feature_node.ForEachChildElementWithName(
-      "enum", [log, &registers_enum_types](const XMLNode &enum_node) {
-        std::string id;
-
-        enum_node.ForEachAttribute([&id](const llvm::StringRef &attr_name,
-                                         const llvm::StringRef &attr_value) {
-          if (attr_name == "id")
-            id = attr_value;
-
-          // There is also a "size" attribute that is supposed to be the size in
-          // bytes of the register this applies to. However:
-          // * LLDB doesn't need this information.
-          // * It  is difficult to verify because you have to wait until the
-          //   enum is applied to a field.
-          //
-          // So we will emit this attribute in XML for GDB's sake, but will not
-          // bother ingesting it.
-
-          // Walk all attributes.
-          return true;
-        });
-
-        if (!id.empty()) {
-          FieldEnum::Enumerators enumerators = ParseEnumEvalues(enum_node);
-          if (!enumerators.empty()) {
-            LLDB_LOG(log,
-                     "ProcessGDBRemote::ParseEnums Found enum type \"{0}\"",
-                     id);
-            registers_enum_types.insert_or_assign(
-                id, std::make_unique<FieldEnum>(id, enumerators));
-          }
-        }
-
-        // Find all <enum> elements.
-        return true;
-      });
-}
-
-static std::vector<RegisterFlags::Field> ParseFlagsFields(
-    XMLNode flags_node, unsigned size,
-    const llvm::StringMap<std::unique_ptr<FieldEnum>> &registers_enum_types) {
+static std::vector<RegisterFlags::Field> ParseFlagsFields(XMLNode flags_node,
+                                                          unsigned size) {
   Log *log(GetLog(GDBRLog::Process));
   const unsigned max_start_bit = size * 8 - 1;
 
   // Process the fields of this set of flags.
   std::vector<RegisterFlags::Field> fields;
-  flags_node.ForEachChildElementWithName("field", [&fields, max_start_bit, &log,
-                                                   &registers_enum_types](
-                                                      const XMLNode
-                                                          &field_node) {
+  flags_node.ForEachChildElementWithName("field", [&fields, max_start_bit,
+                                                   &log](const XMLNode
+                                                             &field_node) {
     std::optional<llvm::StringRef> name;
     std::optional<unsigned> start;
     std::optional<unsigned> end;
-    std::optional<llvm::StringRef> type;
 
-    field_node.ForEachAttribute([&name, &start, &end, &type, max_start_bit,
+    field_node.ForEachAttribute([&name, &start, &end, max_start_bit,
                                  &log](const llvm::StringRef &attr_name,
                                        const llvm::StringRef &attr_value) {
       // Note that XML in general requires that each of these attributes only
@@ -4353,7 +4240,8 @@ static std::vector<RegisterFlags::Field> ParseFlagsFields(
                    attr_value.data());
         }
       } else if (attr_name == "type") {
-        type = attr_value;
+        // Type is a known attribute but we do not currently use it and it is
+        // not required.
       } else {
         LLDB_LOG(
             log,
@@ -4366,55 +4254,14 @@ static std::vector<RegisterFlags::Field> ParseFlagsFields(
     });
 
     if (name && start && end) {
-      if (*start > *end)
+      if (*start > *end) {
         LLDB_LOG(
             log,
             "ProcessGDBRemote::ParseFlagsFields Start {0} > end {1} in field "
             "\"{2}\", ignoring",
             *start, *end, name->data());
-      else {
-        if (RegisterFlags::Field::GetSizeInBits(*start, *end) > 64)
-          LLDB_LOG(log,
-                   "ProcessGDBRemote::ParseFlagsFields Ignoring field \"{2}\" "
-                   "that has "
-                   "size > 64 bits, this is not supported",
-                   name->data());
-        else {
-          // A field's type may be set to the name of an enum type.
-          const FieldEnum *enum_type = nullptr;
-          if (type && !type->empty()) {
-            auto found = registers_enum_types.find(*type);
-            if (found != registers_enum_types.end()) {
-              enum_type = found->second.get();
-
-              // No enumerator can exceed the range of the field itself.
-              uint64_t max_value =
-                  RegisterFlags::Field::GetMaxValue(*start, *end);
-              for (const auto &enumerator : enum_type->GetEnumerators()) {
-                if (enumerator.m_value > max_value) {
-                  enum_type = nullptr;
-                  LLDB_LOG(
-                      log,
-                      "ProcessGDBRemote::ParseFlagsFields In enum \"{0}\" "
-                      "evalue \"{1}\" with value {2} exceeds the maximum value "
-                      "of field \"{3}\" ({4}), ignoring enum",
-                      type->data(), enumerator.m_name, enumerator.m_value,
-                      name->data(), max_value);
-                  break;
-                }
-              }
-            } else {
-              LLDB_LOG(log,
-                       "ProcessGDBRemote::ParseFlagsFields Could not find type "
-                       "\"{0}\" "
-                       "for field \"{1}\", ignoring",
-                       type->data(), name->data());
-            }
-          }
-
-          fields.push_back(
-              RegisterFlags::Field(name->str(), *start, *end, enum_type));
-        }
+      } else {
+        fields.push_back(RegisterFlags::Field(name->str(), *start, *end));
       }
     }
 
@@ -4425,14 +4272,12 @@ static std::vector<RegisterFlags::Field> ParseFlagsFields(
 
 void ParseFlags(
     XMLNode feature_node,
-    llvm::StringMap<std::unique_ptr<RegisterFlags>> &registers_flags_types,
-    const llvm::StringMap<std::unique_ptr<FieldEnum>> &registers_enum_types) {
+    llvm::StringMap<std::unique_ptr<RegisterFlags>> &registers_flags_types) {
   Log *log(GetLog(GDBRLog::Process));
 
   feature_node.ForEachChildElementWithName(
       "flags",
-      [&log, &registers_flags_types,
-       &registers_enum_types](const XMLNode &flags_node) -> bool {
+      [&log, &registers_flags_types](const XMLNode &flags_node) -> bool {
         LLDB_LOG(log, "ProcessGDBRemote::ParseFlags Found flags node \"{0}\"",
                  flags_node.GetAttributeValue("id").c_str());
 
@@ -4465,7 +4310,7 @@ void ParseFlags(
         if (id && size) {
           // Process the fields of this set of flags.
           std::vector<RegisterFlags::Field> fields =
-              ParseFlagsFields(flags_node, *size, registers_enum_types);
+              ParseFlagsFields(flags_node, *size);
           if (fields.size()) {
             // Sort so that the fields with the MSBs are first.
             std::sort(fields.rbegin(), fields.rend());
@@ -4530,19 +4375,13 @@ void ParseFlags(
 bool ParseRegisters(
     XMLNode feature_node, GdbServerTargetInfo &target_info,
     std::vector<DynamicRegisterInfo::Register> &registers,
-    llvm::StringMap<std::unique_ptr<RegisterFlags>> &registers_flags_types,
-    llvm::StringMap<std::unique_ptr<FieldEnum>> &registers_enum_types) {
+    llvm::StringMap<std::unique_ptr<RegisterFlags>> &registers_flags_types) {
   if (!feature_node)
     return false;
 
   Log *log(GetLog(GDBRLog::Process));
 
-  // Enums first because they are referenced by fields in the flags.
-  ParseEnums(feature_node, registers_enum_types);
-  for (const auto &enum_type : registers_enum_types)
-    enum_type.second->DumpToLog(log);
-
-  ParseFlags(feature_node, registers_flags_types, registers_enum_types);
+  ParseFlags(feature_node, registers_flags_types);
   for (const auto &flags : registers_flags_types)
     flags.second->DumpToLog(log);
 
@@ -4804,7 +4643,7 @@ bool ProcessGDBRemote::GetGDBServerRegisterInfoXMLAndProcess(
     if (arch_to_use.IsValid()) {
       for (auto &feature_node : feature_nodes) {
         ParseRegisters(feature_node, target_info, registers,
-                       m_registers_flags_types, m_registers_enum_types);
+                       m_registers_flags_types);
       }
 
       for (const auto &include : target_info.includes) {
@@ -4869,14 +4708,13 @@ bool ProcessGDBRemote::GetGDBServerRegisterInfo(ArchSpec &arch_to_use) {
   if (!m_gdb_comm.GetQXferFeaturesReadSupported())
     return false;
 
-  // These hold register type information for the whole of target.xml.
+  // This holds register flags information for the whole of target.xml.
   // target.xml may include further documents that
   // GetGDBServerRegisterInfoXMLAndProcess will recurse to fetch and process.
   // That's why we clear the cache here, and not in
   // GetGDBServerRegisterInfoXMLAndProcess. To prevent it being cleared on every
   // include read.
   m_registers_flags_types.clear();
-  m_registers_enum_types.clear();
   std::vector<DynamicRegisterInfo::Register> registers;
   if (GetGDBServerRegisterInfoXMLAndProcess(arch_to_use, "target.xml",
                                             registers) &&

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

@@ -484,11 +484,6 @@ class ProcessGDBRemote : public Process,
   // entries are added. Which would invalidate any pointers set in the register
   // info up to that point.
   llvm::StringMap<std::unique_ptr<RegisterFlags>> m_registers_flags_types;
-
-  // Enum types are referenced by register fields. This does not store the data
-  // directly because the map may reallocate. Pointers to these are contained
-  // within instances of RegisterFlags.
-  llvm::StringMap<std::unique_ptr<FieldEnum>> m_registers_enum_types;
 };
 
 } // namespace process_gdb_remote

lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp

@@ -43,7 +43,8 @@ CompilerType RegisterTypeBuilderClang::GetRegisterType(
       ScratchTypeSystemClang::GetForTarget(m_target);
   assert(type_system);
 
-  std::string register_type_name = "__lldb_register_fields_" + name;
+  std::string register_type_name = "__lldb_register_fields_";
+  register_type_name += name;
   // See if we have made this type before and can reuse it.
   CompilerType fields_type =
       type_system->GetTypeForIdentifier<clang::CXXRecordDecl>(
@@ -66,43 +67,8 @@ CompilerType RegisterTypeBuilderClang::GetRegisterType(
     // We assume that RegisterFlags has padded and sorted the fields
     // already.
     for (const RegisterFlags::Field &field : flags.GetFields()) {
-      CompilerType field_type = field_uint_type;
-
-      if (const FieldEnum *enum_type = field.GetEnum()) {
-        const FieldEnum::Enumerators &enumerators = enum_type->GetEnumerators();
-        if (!enumerators.empty()) {
-          std::string enum_type_name =
-              "__lldb_register_fields_enum_" + enum_type->GetID();
-
-          // Enums can be used by mutiple fields and multiple registers, so we
-          // may have built this one already.
-          CompilerType field_enum_type =
-              type_system->GetTypeForIdentifier<clang::EnumDecl>(
-                  enum_type_name);
-
-          if (field_enum_type)
-            field_type = field_enum_type;
-          else {
-            field_type = type_system->CreateEnumerationType(
-                enum_type_name, type_system->GetTranslationUnitDecl(),
-                OptionalClangModuleID(), Declaration(), field_uint_type, false);
-
-            type_system->StartTagDeclarationDefinition(field_type);
-
-            Declaration decl;
-            for (auto enumerator : enumerators) {
-              type_system->AddEnumerationValueToEnumerationType(
-                  field_type, decl, enumerator.m_name.c_str(),
-                  enumerator.m_value, byte_size * 8);
-            }
-
-            type_system->CompleteTagDeclarationDefinition(field_type);
-          }
-        }
-      }
-
       type_system->AddFieldToRecordType(fields_type, field.GetName(),
-                                        field_type, lldb::eAccessPublic,
+                                        field_uint_type, lldb::eAccessPublic,
                                         field.GetSizeInBits());
     }