Skip to content

Commit

Permalink
Revert "[lldb] Parse and display register field enums" (#97258)
Browse files Browse the repository at this point in the history 
Reverts #95768 due to a test failure on macOS with
ASAN:

https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-sanitized/425/console
  • Loading branch information
@DavidSpickett
DavidSpickett authored Jul 1, 2024
1 parent 37661a1 commit d9e659c
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 673 deletions.
7 changes: 0 additions & 7 deletions lldb/include/lldb/Target/RegisterFlags.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -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;
Expand Down
7 changes: 1 addition & 6 deletions lldb/source/Core/DumpRegisterInfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
198 changes: 18 additions & 180 deletions lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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));
}
}

Expand All @@ -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());

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) &&
Expand Down
5 changes: 0 additions & 5 deletions lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 3 additions & 37 deletions lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand All @@ -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());
}

Expand Down
Loading

0 comments on commit d9e659c

Please sign in to comment.