Skip to content

[lldb] Parse and display register field enums #95768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions lldb/include/lldb/Target/RegisterFlags.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,15 @@ 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 @@ -44,6 +49,8 @@ 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: 6 additions & 1 deletion lldb/source/Core/DumpRegisterInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ 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: 180 additions & 18 deletions lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4179,21 +4179,134 @@ struct GdbServerTargetInfo {
RegisterSetMap reg_set_map;
};

static std::vector<RegisterFlags::Field> ParseFlagsFields(XMLNode flags_node,
unsigned size) {
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know the density of the keys here? Might be a good candidate for llvm::IndexedMap?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory it's an enum so you'd start from 0 then 1, 2,3,4, etc. but this is hardware, so it's not going to be as regular as software enums.

The other problem with IndexedMap is that you've got to have a function uint64_t -> size_t to make the index, so for a 32 bit lldb the naive version ends up clipping off the top half of values. Are we likely to get a 64 bit enumerator value? No, but it's simpler to handle it than decide on another arbitrary cut off point.

If we ignore that issue, there is the issue of order of insertion. I'm assuming you iterate an IndexedMap using the indexes, but if the values of the enumerator are used for the indexes we may lose the original order.

Maybe if that requirement didn't come across clearly enough, I need to expand the comment with an example?

Or maybe I have the wrong end of the stick entirely.

Copy link
Collaborator Author

@DavidSpickett DavidSpickett Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've expanded the comment anyway.

I don't think anyone on the GDB side ever considered that order might matter, because I don't think they have a register info equivalent. You can print the type of the register but that's more of a C type, where enumerators being sorted isn't unexpected.

(where "matter" is defined as "I thought of some niche use case that would be nice to have" :) )

Gray code values is another use case for this. 00, 01, 11, 10 - you might list it that way and wouldn't want lldb to sort it into normal numerical order.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair, was mostly asking to get an idea of what one can expect the data to look like. std::map is a fine choice here :)


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) {
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](const XMLNode
&field_node) {
flags_node.ForEachChildElementWithName("field", [&fields, max_start_bit, &log,
&registers_enum_types](
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, max_start_bit,
field_node.ForEachAttribute([&name, &start, &end, &type, 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 @@ -4240,8 +4353,7 @@ static std::vector<RegisterFlags::Field> ParseFlagsFields(XMLNode flags_node,
attr_value.data());
}
} else if (attr_name == "type") {
// Type is a known attribute but we do not currently use it and it is
// not required.
type = attr_value;
} else {
LLDB_LOG(
log,
Expand All @@ -4254,14 +4366,55 @@ static std::vector<RegisterFlags::Field> ParseFlagsFields(XMLNode flags_node,
});

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 {
fields.push_back(RegisterFlags::Field(name->str(), *start, *end));
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));
}
}
}

Expand All @@ -4272,12 +4425,14 @@ static std::vector<RegisterFlags::Field> ParseFlagsFields(XMLNode flags_node,

void ParseFlags(
XMLNode feature_node,
llvm::StringMap<std::unique_ptr<RegisterFlags>> &registers_flags_types) {
llvm::StringMap<std::unique_ptr<RegisterFlags>> &registers_flags_types,
const llvm::StringMap<std::unique_ptr<FieldEnum>> &registers_enum_types) {
Log *log(GetLog(GDBRLog::Process));

feature_node.ForEachChildElementWithName(
"flags",
[&log, &registers_flags_types](const XMLNode &flags_node) -> bool {
[&log, &registers_flags_types,
&registers_enum_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 @@ -4310,7 +4465,7 @@ void ParseFlags(
if (id && size) {
// Process the fields of this set of flags.
std::vector<RegisterFlags::Field> fields =
ParseFlagsFields(flags_node, *size);
ParseFlagsFields(flags_node, *size, registers_enum_types);
if (fields.size()) {
// Sort so that the fields with the MSBs are first.
std::sort(fields.rbegin(), fields.rend());
Expand Down Expand Up @@ -4375,13 +4530,19 @@ 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<RegisterFlags>> &registers_flags_types,
llvm::StringMap<std::unique_ptr<FieldEnum>> &registers_enum_types) {
if (!feature_node)
return false;

Log *log(GetLog(GDBRLog::Process));

ParseFlags(feature_node, registers_flags_types);
// 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);
for (const auto &flags : registers_flags_types)
flags.second->log(log);

Expand Down Expand Up @@ -4643,7 +4804,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_flags_types, m_registers_enum_types);
}

for (const auto &include : target_info.includes) {
Expand Down Expand Up @@ -4708,13 +4869,14 @@ bool ProcessGDBRemote::GetGDBServerRegisterInfo(ArchSpec &arch_to_use) {
if (!m_gdb_comm.GetQXferFeaturesReadSupported())
return false;

// This holds register flags information for the whole of target.xml.
// These hold register type 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: 5 additions & 0 deletions lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,11 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ CompilerType RegisterTypeBuilderClang::GetRegisterType(
ScratchTypeSystemClang::GetForTarget(m_target);
assert(type_system);

std::string register_type_name = "__lldb_register_fields_";
register_type_name += name;
std::string register_type_name = "__lldb_register_fields_" + name;
// See if we have made this type before and can reuse it.
CompilerType fields_type =
type_system->GetTypeForIdentifier<clang::CXXRecordDecl>(
Expand All @@ -67,8 +66,43 @@ 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_uint_type, lldb::eAccessPublic,
field_type, lldb::eAccessPublic,
field.GetSizeInBits());
}

Expand Down
Loading
Loading