[Lldb-commits] [lldb] [lldb] Override default struct layout when building register types (PR #189590)
David Spickett via lldb-commits
lldb-commits at lists.llvm.org
Wed Apr 15 07:58:09 PDT 2026
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/189590
>From 2f63a30e4b657ccbfb0c449311d393ca61bc1497 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at arm.com>
Date: Fri, 27 Mar 2026 11:23:50 +0000
Subject: [PATCH 1/2] [lldb] Override default struct layout when building
register types
When printing register "flags" types (basically C bitfield structs),
we have 2 goals:
1. Extract the values (the fields) correctly.
2. Display the fields in most significant to least significant order,
to match architecture manuals.
Currently LLDB achieves these by:
* Putting the most significant field as the first member of the
struct type. Though we know it is the MSB, it will in fact be
put at bit 0 by Clang.
* To compensate for that, we reverse the order of the fields
within the register value. If the original was `[a][b][c]`,
we change that to `[c][b][a]`.
This works when the only type we have is "flags" aka a bitfield
struct. I have been trying to implement "union" types (like C unions),
and found that this method is not compatible with a "union" of
two different "flags".
Below is a union of two flags which have different layouts.
`w` is bigger than `x`, and `y` is smaller than `z`.
```
some_union
|
-> big_little: wwww_wwww_wwww_wwww_wwww_xxxx_xxxx_xxxx
-> little_big: yyyy_yyyy_yyyy_zzzz_zzzz_zzzz_zzzz_zzzz
```
We cannot modify the value using both field layouts, we must pick one.
Whichever one we pick results in us displaying the other one
incorrectly because it has a different layout.
In other words: the current method only works when there is exactly 1
field layout. For unions, this may not be true.
We need to achieve goals 1 and 2 without relying on details of
the register's type.
The first method I prototyped was to build the struct types in reverse
(so field values are correct), then print them in reverse with a Synthetic
Child Provider (so the display order is correct).
This works but it has the small downside that the underlying type
would be backwards if a user were to inspect it. This is not possible
today but eventually I want to allow register types in expressions,
and with that you could use the underlying type.
Users are very unlikely to do this, but I wondered if it was a sign
that I was pursuing a half baked solution.
This PR implements an alternative. The types are still built with the
most significant field first, so they are visually correct. Then an
ExternalASTSource is used to tell Clang to lay out the struct in an
MSB to LSB order. This means the fields will have correct values.
This method will work for unions because the only change we have to
make to the register value is an endian swap in some cases. This endian
swap does not rely on any layout information.
Here is a "real" example using WIP patches for union and vector types.
```
(lldb) register read x0
x0 = 0x1234567812345678
= {
big_little = (w = 0x123456781234, x = 0x5678)
little_big = (y = 0x1234, z = 0x567812345678)
vector = {
0x8 = (0x78, 0x56, 0x34, 0x12, 0x78, 0x56, 0x34, 0x12)
0x10 = (0x5678, 0x1234, 0x5678, 0x1234)
0x20 = (0x12345678, 0x12345678)
0x40 = (0x1234567812345678)
}
}
```
(numbers all changed to hex for ease of comparison)
You can see that `big_little` and `little_big` correctly slice the register
value. This is not possible with the original field reordering method.
This new method:
* Makes it easier to explain the whole register printing process.
* Produces a type that visually matches the register. It has the correct
order, and no padding fields.
* Does not require changes to how we print types.
* Has no user visible change.
* Unblocks the implementation of unions and vectors.
---
.../DataFormatters/DumpValueObjectOptions.h | 5 -
lldb/include/lldb/Target/RegisterFlags.h | 24 +---
lldb/source/Core/DumpRegisterValue.cpp | 23 +--
.../DataFormatters/DumpValueObjectOptions.cpp | 8 +-
.../DataFormatters/ValueObjectPrinter.cpp | 6 -
.../RegisterTypeBuilderClang.cpp | 19 ++-
.../RegisterTypeBuilderClang.h | 49 +++++++
.../TypeSystem/Clang/TypeSystemClang.cpp | 4 +
.../TypeSystem/Clang/TypeSystemClang.h | 5 +-
lldb/source/Target/RegisterFlags.cpp | 131 ++++++++----------
.../gdb_remote_client/TestXMLRegisterFlags.py | 2 +-
lldb/unittests/Target/RegisterFlagsTest.cpp | 66 +--------
12 files changed, 141 insertions(+), 201 deletions(-)
diff --git a/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h b/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
index 70166f33cfc45..4d5249dd26665 100644
--- a/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
+++ b/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
@@ -52,8 +52,6 @@ class DumpValueObjectOptions {
const DumpValueObjectOptions &, Stream &)>
DeclPrintingHelper;
- typedef std::function<bool(ConstString)> ChildPrintingDecider;
-
static const DumpValueObjectOptions DefaultOptions() {
static DumpValueObjectOptions g_default_options;
@@ -70,8 +68,6 @@ class DumpValueObjectOptions {
DumpValueObjectOptions &SetDeclPrintingHelper(DeclPrintingHelper helper);
- DumpValueObjectOptions &SetChildPrintingDecider(ChildPrintingDecider decider);
-
DumpValueObjectOptions &SetShowTypes(bool show = false);
DumpValueObjectOptions &SetShowLocation(bool show = false);
@@ -142,7 +138,6 @@ class DumpValueObjectOptions {
lldb::LanguageType m_varformat_language = lldb::eLanguageTypeUnknown;
PointerDepth m_max_ptr_depth;
DeclPrintingHelper m_decl_printing_helper;
- ChildPrintingDecider m_child_printing_decider;
PointerAsArraySettings m_pointer_as_array;
unsigned m_expand_ptr_type_flags = 0;
// The following flags commonly default to false.
diff --git a/lldb/include/lldb/Target/RegisterFlags.h b/lldb/include/lldb/Target/RegisterFlags.h
index 1250fd0330958..44356decaa6bf 100644
--- a/lldb/include/lldb/Target/RegisterFlags.h
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -86,11 +86,6 @@ class RegisterFlags {
/// 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 {
- return (register_value & GetMask()) >> m_start;
- }
-
const std::string &GetName() const { return m_name; }
unsigned GetStart() const { return m_start; }
unsigned GetEnd() const { return m_end; }
@@ -132,7 +127,7 @@ class RegisterFlags {
/// This assumes that:
/// * There is at least one field.
/// * The fields are sorted in descending order.
- /// Gaps are allowed, they will be filled with anonymous padding fields.
+ /// Gaps are allowed.
RegisterFlags(std::string id, unsigned size,
const std::vector<Field> &fields);
@@ -145,23 +140,6 @@ class RegisterFlags {
/// 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.
- // Use this when you are going to show the register using a bitfield struct
- // type. If that struct expects MSB first and you are on little endian where
- // LSB would be first, this corrects that (and vice versa for big endian).
- template <typename T> T ReverseFieldOrder(T value) const {
- T ret = 0;
- unsigned shift = 0;
- for (auto field : GetFields()) {
- ret |= field.GetValue(value) << shift;
- shift += field.GetSizeInBits();
- }
-
- return ret;
- }
-
const std::vector<Field> &GetFields() const { return m_fields; }
const std::string &GetID() const { return m_id; }
unsigned GetSize() const { return m_size; }
diff --git a/lldb/source/Core/DumpRegisterValue.cpp b/lldb/source/Core/DumpRegisterValue.cpp
index aff4d2c621d7e..37ce1496131fe 100644
--- a/lldb/source/Core/DumpRegisterValue.cpp
+++ b/lldb/source/Core/DumpRegisterValue.cpp
@@ -28,31 +28,18 @@ static void dump_type_value(lldb_private::CompilerType &fields_type, T value,
lldb_private::Stream &strm) {
lldb::ByteOrder target_order = exe_scope->CalculateProcess()->GetByteOrder();
- // For the bitfield types we generate, it is expected that the fields are
- // in what is usually a big endian order. Most significant field first.
- // This is also clang's internal ordering and the order we want to print
- // them. On a big endian host this all matches up, for a little endian
- // host we have to swap the order of the fields before display.
- if (target_order == lldb::ByteOrder::eByteOrderLittle) {
- value = reg_info.flags_type->ReverseFieldOrder(value);
- }
-
- // Then we need to match the target's endian on a byte level as well.
+ // The type will be rendered in the target's type system, so it must match
+ // its endian.
if (lldb_private::endian::InlHostByteOrder() != target_order)
value = llvm::byteswap(value);
- lldb_private::DataExtractor data_extractor{
- &value, sizeof(T), lldb_private::endian::InlHostByteOrder(), 8};
+ lldb_private::DataExtractor data_extractor{&value, sizeof(T), target_order,
+ 8};
lldb::ValueObjectSP vobj_sp = lldb_private::ValueObjectConstResult::Create(
exe_scope, fields_type, lldb_private::ConstString(), data_extractor);
lldb_private::DumpValueObjectOptions dump_options;
- lldb_private::DumpValueObjectOptions::ChildPrintingDecider decider =
- [](lldb_private::ConstString varname) {
- // Unnamed bit-fields are padding that we don't want to show.
- return varname.GetLength();
- };
- dump_options.SetChildPrintingDecider(decider).SetHideRootType(true);
+ dump_options.SetHideRootType(true);
if (llvm::Error error = vobj_sp->Dump(strm, dump_options))
strm << "error: " << toString(std::move(error));
diff --git a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp
index e1df9522256fa..3ba4a4ab3f9f5 100644
--- a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp
+++ b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp
@@ -15,7 +15,7 @@ using namespace lldb_private;
DumpValueObjectOptions::DumpValueObjectOptions()
: m_summary_sp(), m_root_valobj_name(), m_decl_printing_helper(),
- m_child_printing_decider(), m_pointer_as_array(), m_use_synthetic(true),
+ m_pointer_as_array(), m_use_synthetic(true),
m_scope_already_checked(false), m_flat_output(false), m_ignore_cap(false),
m_show_types(false), m_show_location(false), m_use_object_desc(false),
m_hide_root_type(false), m_hide_root_name(false), m_hide_name(false),
@@ -49,12 +49,6 @@ DumpValueObjectOptions::SetDeclPrintingHelper(DeclPrintingHelper helper) {
return *this;
}
-DumpValueObjectOptions &
-DumpValueObjectOptions::SetChildPrintingDecider(ChildPrintingDecider decider) {
- m_child_printing_decider = decider;
- return *this;
-}
-
DumpValueObjectOptions &DumpValueObjectOptions::SetShowTypes(bool show) {
m_show_types = show;
return *this;
diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index cb9955b611d0c..f0ceb6846ed95 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -729,9 +729,6 @@ void ValueObjectPrinter::PrintChildren(
for (size_t idx = 0; idx < num_children; ++idx) {
if (ValueObjectSP child_sp = GenerateChild(synth_valobj, idx)) {
- if (m_options.m_child_printing_decider &&
- !m_options.m_child_printing_decider(child_sp->GetName()))
- continue;
if (!any_children_printed) {
PrintChildrenPreamble(value_printed, summary_printed);
any_children_printed = true;
@@ -789,9 +786,6 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool hide_names) {
child_sp = child_sp->GetQualifiedRepresentationIfAvailable(
m_options.m_use_dynamic, m_options.m_use_synthetic);
if (child_sp) {
- if (m_options.m_child_printing_decider &&
- !m_options.m_child_printing_decider(child_sp->GetName()))
- continue;
if (idx && did_print_children)
m_stream->PutCString(", ");
did_print_children = true;
diff --git a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
index 80d5289178ed0..e809973766604 100644
--- a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
+++ b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
@@ -38,10 +38,15 @@ RegisterTypeBuilderClang::RegisterTypeBuilderClang(Target &target)
CompilerType RegisterTypeBuilderClang::GetRegisterType(
const std::string &name, const lldb_private::RegisterFlags &flags,
uint32_t byte_size) {
- lldb::TypeSystemClangSP type_system =
- ScratchTypeSystemClang::GetForTarget(m_target);
+ lldb::TypeSystemClangSP type_system = ScratchTypeSystemClang::GetForTarget(
+ m_target, ScratchTypeSystemClang::IsolatedASTKind::Registers);
assert(type_system);
+ if (!m_external_ast) {
+ m_external_ast = llvm::makeIntrusiveRefCnt<RegisterExternalASTSource>();
+ type_system->SetExternalSource(m_external_ast);
+ }
+
std::string register_type_name = "__lldb_register_fields_" + name;
// See if we have made this type before and can reuse it.
CompilerType fields_type =
@@ -60,6 +65,7 @@ CompilerType RegisterTypeBuilderClang::GetRegisterType(
nullptr, OptionalClangModuleID(), register_type_name,
llvm::to_underlying(clang::TagTypeKind::Struct), lldb::eLanguageTypeC);
type_system->StartTagDeclarationDefinition(fields_type);
+ llvm::DenseMap<const clang::FieldDecl *, uint64_t> field_offsets;
// We assume that RegisterFlags has padded and sorted the fields
// already.
@@ -104,10 +110,15 @@ CompilerType RegisterTypeBuilderClang::GetRegisterType(
}
}
- type_system->AddFieldToRecordType(fields_type, field.GetName(),
- field_type, field.GetSizeInBits());
+ clang::FieldDecl *field_decl = type_system->AddFieldToRecordType(
+ fields_type, field.GetName(), field_type, field.GetSizeInBits());
+ field_offsets.insert({field_decl, field.GetStart()});
}
+ m_external_ast->m_struct_layouts.insert(
+ {type_system->GetAsRecordDecl(fields_type),
+ RegisterExternalASTSource::LayoutInfo{byte_size, field_offsets}});
+
type_system->CompleteTagDeclarationDefinition(fields_type);
// So that the size of the type matches the size of the register.
type_system->SetIsPacked(fields_type);
diff --git a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
index 611e2e60436ec..d346f79acceb8 100644
--- a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
+++ b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
@@ -9,6 +9,8 @@
#ifndef LLDB_SOURCE_PLUGINS_REGISTERTYPEBUILDER_REGISTERTYPEBUILDERCLANG_H
#define LLDB_SOURCE_PLUGINS_REGISTERTYPEBUILDER_REGISTERTYPEBUILDERCLANG_H
+#include "clang/AST/ExternalASTSource.h"
+
#include "lldb/Target/RegisterTypeBuilder.h"
#include "lldb/Target/Target.h"
@@ -33,6 +35,53 @@ class RegisterTypeBuilderClang : public RegisterTypeBuilder {
uint32_t byte_size) override;
private:
+ /// This external AST is used to override the layout of bitfield structs
+ /// created from sets of register "flags".
+ ///
+ /// We have two goals with register display:
+ /// 1. Most significant to least significant display order, to match
+ /// architecure
+ /// manuals.
+ /// 2. Correctly extracting field values.
+ ///
+ /// Goal 1 is achieved by building the struct with the most significant field
+ /// as the first member and the least significant as the last member.
+ ///
+ /// The default bit position of those fields is that the first member is bit
+ /// 0, and the last is bit N. This is LSB to MSB, so the replacement layouts
+ /// in this external AST reverse that to be MSB to LSB. This achieves goal 2.
+ class RegisterExternalASTSource : public clang::ExternalASTSource {
+ public:
+ struct LayoutInfo {
+ uint64_t size_bytes = 0;
+ llvm::DenseMap<const clang::FieldDecl *, uint64_t> field_offsets;
+ };
+ llvm::DenseMap<const clang::RecordDecl *, LayoutInfo> m_struct_layouts;
+
+ bool layoutRecordType(
+ const clang::RecordDecl *record, uint64_t &size, uint64_t &align,
+ llvm::DenseMap<const clang::FieldDecl *, uint64_t> &field_offsets,
+ llvm::DenseMap<const clang::CXXRecordDecl *, clang::CharUnits>
+ &base_offsets,
+ llvm::DenseMap<const clang::CXXRecordDecl *, clang::CharUnits>
+ &vbase_offsets) override {
+ auto it = m_struct_layouts.find(record);
+ if (it == m_struct_layouts.end())
+ return false;
+
+ size = it->second.size_bytes * 8;
+ align = size;
+ field_offsets = it->second.field_offsets;
+ base_offsets.clear();
+ vbase_offsets.clear();
+ return true;
+ }
+ };
+
+ // This is created the first time a register type is requested, then handed
+ // to the type system. We keep a reference to it so we can add more layouts
+ // as more register types are requested.
+ llvm::IntrusiveRefCntPtr<RegisterExternalASTSource> m_external_ast;
Target &m_target;
};
} // namespace lldb_private
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 624cdeada4eb9..32e464e11ba86 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9711,6 +9711,8 @@ GetNameForIsolatedASTKind(ScratchTypeSystemClang::IsolatedASTKind kind) {
switch (kind) {
case ScratchTypeSystemClang::IsolatedASTKind::CppModules:
return "C++ modules";
+ case ScratchTypeSystemClang::IsolatedASTKind::Registers:
+ return "Registers";
}
llvm_unreachable("Unimplemented IsolatedASTKind?");
}
@@ -9802,6 +9804,8 @@ GetSpecializedASTName(ScratchTypeSystemClang::IsolatedASTKind feature) {
switch (feature) {
case ScratchTypeSystemClang::IsolatedASTKind::CppModules:
return "scratch ASTContext for C++ module types";
+ case ScratchTypeSystemClang::IsolatedASTKind::Registers:
+ return "scratch ASContext for register types";
}
llvm_unreachable("Unimplemented ASTFeature kind?");
}
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index cf7c37fe02985..f007590a473be 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1277,7 +1277,10 @@ class ScratchTypeSystemClang : public TypeSystemClang {
/// type information from a C++ module. The templates from a C++ module
/// often conflict with the templates we generate from debug information,
/// so we put these types in their own AST.
- CppModules
+ CppModules,
+ /// Register "flags" types are converted into structures but their layout
+ /// does not follow the ABI of the target.
+ Registers
};
/// Alias for requesting the default scratch TypeSystemClang in GetForTarget.
diff --git a/lldb/source/Target/RegisterFlags.cpp b/lldb/source/Target/RegisterFlags.cpp
index 976e03870ad9e..f070e8896b9ee 100644
--- a/lldb/source/Target/RegisterFlags.cpp
+++ b/lldb/source/Target/RegisterFlags.cpp
@@ -110,40 +110,7 @@ uint64_t RegisterFlags::Field::GetMask() const {
void RegisterFlags::SetFields(const std::vector<Field> &fields) {
// We expect that these are unsorted but do not overlap.
// They could fill the register but may have gaps.
- std::vector<Field> provided_fields = fields;
-
- m_fields.clear();
- m_fields.reserve(provided_fields.size());
-
- // ProcessGDBRemote should have sorted these in descending order already.
- assert(std::is_sorted(provided_fields.rbegin(), provided_fields.rend()));
-
- // Build a new list of fields that includes anonymous (empty name) fields
- // wherever there is a gap. This will simplify processing later.
- std::optional<Field> previous_field;
- unsigned register_msb = (m_size * 8) - 1;
- for (auto field : provided_fields) {
- if (previous_field) {
- unsigned padding = previous_field->PaddingDistance(field);
- if (padding) {
- // -1 to end just before the previous field.
- unsigned end = previous_field->GetStart() - 1;
- // +1 because if you want to pad 1 bit you want to start and end
- // on the same bit.
- m_fields.push_back(Field("", field.GetEnd() + 1, end));
- }
- } else {
- // This is the first field. Check that it starts at the register's MSB.
- if (field.GetEnd() != register_msb)
- m_fields.push_back(Field("", field.GetEnd() + 1, register_msb));
- }
- m_fields.push_back(field);
- previous_field = field;
- }
-
- // The last field may not extend all the way to bit 0.
- if (previous_field && previous_field->GetStart() != 0)
- m_fields.push_back(Field("", 0, previous_field->GetStart() - 1));
+ m_fields = fields;
}
RegisterFlags::RegisterFlags(std::string id, unsigned size,
@@ -185,54 +152,76 @@ static void EmitTable(std::string &out, std::array<std::string, 3> &table) {
});
}
+static void EmitField(const RegisterFlags::Field &field, uint32_t max_width,
+ uint32_t ¤t_width, std::string &table,
+ std::array<std::string, 3> &lines) {
+ StreamString position;
+ if (field.GetEnd() == field.GetStart())
+ position.Printf(" %d ", field.GetEnd());
+ else
+ position.Printf(" %d-%d ", field.GetEnd(), field.GetStart());
+
+ StreamString name;
+ name.Printf(" %s ", field.GetName().c_str());
+
+ unsigned column_width = position.GetString().size();
+ unsigned name_width = name.GetString().size();
+ if (name_width > column_width)
+ column_width = name_width;
+
+ // If the next column would overflow and we have already formatted at least
+ // one column, put out what we have and move to a new table on the next line
+ // (+1 here because we need to cap the ends with '|'). If this is the first
+ // column, just let it overflow and we'll wrap next time around. There's not
+ // much we can do with a very small terminal.
+ if (current_width && ((current_width + column_width + 1) >= max_width)) {
+ EmitTable(table, lines);
+ // Blank line between each.
+ table += "\n\n";
+
+ for (std::string &line : lines)
+ line.clear();
+ current_width = 0;
+ }
+
+ StreamString aligned_position = FormatCell(position, column_width);
+ lines[0] += aligned_position.GetString();
+ StreamString grid;
+ grid << '|' << std::string(column_width, '-');
+ lines[1] += grid.GetString();
+ StreamString aligned_name = FormatCell(name, column_width);
+ lines[2] += aligned_name.GetString();
+
+ // +1 for the left side '|'.
+ current_width += column_width + 1;
+}
+
std::string RegisterFlags::AsTable(uint32_t max_width) const {
std::string table;
// position / gridline / name
std::array<std::string, 3> lines;
uint32_t current_width = 0;
+ std::optional<RegisterFlags::Field> previous_field = std::nullopt;
for (const RegisterFlags::Field &field : m_fields) {
- StreamString position;
- if (field.GetEnd() == field.GetStart())
- position.Printf(" %d ", field.GetEnd());
- else
- position.Printf(" %d-%d ", field.GetEnd(), field.GetStart());
-
- StreamString name;
- name.Printf(" %s ", field.GetName().c_str());
-
- unsigned column_width = position.GetString().size();
- unsigned name_width = name.GetString().size();
- if (name_width > column_width)
- column_width = name_width;
-
- // If the next column would overflow and we have already formatted at least
- // one column, put out what we have and move to a new table on the next line
- // (+1 here because we need to cap the ends with '|'). If this is the first
- // column, just let it overflow and we'll wrap next time around. There's not
- // much we can do with a very small terminal.
- if (current_width && ((current_width + column_width + 1) >= max_width)) {
- EmitTable(table, lines);
- // Blank line between each.
- table += "\n\n";
-
- for (std::string &line : lines)
- line.clear();
- current_width = 0;
+ if (previous_field) {
+ // If there is a gap between this field and the last, fill it with an
+ // anonymous field.
+ if (previous_field->PaddingDistance(field))
+ EmitField(RegisterFlags::Field("", field.GetEnd() + 1,
+ previous_field->GetStart() - 1),
+ max_width, current_width, table, lines);
}
- StreamString aligned_position = FormatCell(position, column_width);
- lines[0] += aligned_position.GetString();
- StreamString grid;
- grid << '|' << std::string(column_width, '-');
- lines[1] += grid.GetString();
- StreamString aligned_name = FormatCell(name, column_width);
- lines[2] += aligned_name.GetString();
-
- // +1 for the left side '|'.
- current_width += column_width + 1;
+ EmitField(field, max_width, current_width, table, lines);
+ previous_field = field;
}
+ // If the last field did not extend to bit 0, pad down to bit 0.
+ if (previous_field && previous_field->GetStart() != 0)
+ EmitField(RegisterFlags::Field("", 0, previous_field->GetStart() - 1),
+ max_width, current_width, table, lines);
+
// If we didn't overflow and still have table to print out.
if (lines[0].size())
EmitTable(table, lines);
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
index 1d0fd00ede3f0..cfd0bd638a1f8 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -464,7 +464,7 @@ def test_flags_multiple_lines(self):
@skipIfRemote
def test_flags_child_limit(self):
# Flags print like C types so they should follow the child limit setting.
- self.runCmd("settings set target.max-children-count 3")
+ self.runCmd("settings set target.max-children-count 2")
self.setup_flags_test(
'<field name="field_0" start="0" end="0"/>'
'<field name="field_1" start="1" end="1"/>'
diff --git a/lldb/unittests/Target/RegisterFlagsTest.cpp b/lldb/unittests/Target/RegisterFlagsTest.cpp
index ecffdd0fe44e6..188a723113878 100644
--- a/lldb/unittests/Target/RegisterFlagsTest.cpp
+++ b/lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -22,24 +22,18 @@ TEST(RegisterFlagsTest, Field) {
// start == end means a 1 bit field.
ASSERT_EQ(f1.GetSizeInBits(), (unsigned)1);
ASSERT_EQ(f1.GetMask(), (uint64_t)1);
- ASSERT_EQ(f1.GetValue(0), (uint64_t)0);
- ASSERT_EQ(f1.GetValue(3), (uint64_t)1);
// End is inclusive meaning that start 0 to end 1 includes bit 1
// to make a 2 bit field.
RegisterFlags::Field f2("", 0, 1);
ASSERT_EQ(f2.GetSizeInBits(), (unsigned)2);
ASSERT_EQ(f2.GetMask(), (uint64_t)3);
- ASSERT_EQ(f2.GetValue(UINT64_MAX), (uint64_t)3);
- ASSERT_EQ(f2.GetValue(UINT64_MAX & ~(uint64_t)3), (uint64_t)0);
// If the field doesn't start at 0 we need to shift up/down
// to account for it.
RegisterFlags::Field f3("", 2, 5);
ASSERT_EQ(f3.GetSizeInBits(), (unsigned)4);
ASSERT_EQ(f3.GetMask(), (uint64_t)0x3c);
- ASSERT_EQ(f3.GetValue(UINT64_MAX), (uint64_t)0xf);
- ASSERT_EQ(f3.GetValue(UINT64_MAX & ~(uint64_t)0x3c), (uint64_t)0);
// Fields are sorted lowest starting bit first.
ASSERT_TRUE(f2 < f3);
@@ -85,66 +79,8 @@ TEST(RegisterFlagsTest, PaddingDistance) {
ASSERT_EQ(make_field(31, 31).PaddingDistance(make_field(0)), 30ULL);
}
-static void test_padding(const std::vector<RegisterFlags::Field> &fields,
- const std::vector<RegisterFlags::Field> &expected) {
- RegisterFlags rf("", 4, fields);
- EXPECT_THAT(expected, ::testing::ContainerEq(rf.GetFields()));
-}
-
-TEST(RegisterFlagsTest, RegisterFlagsPadding) {
- // When creating a set of flags we assume that:
- // * There are >= 1 fields.
- // * They are sorted in descending order.
- // * There may be gaps between each field.
-
- // Needs no padding
- auto fields =
- std::vector<RegisterFlags::Field>{make_field(16, 31), make_field(0, 15)};
- test_padding(fields, fields);
-
- // Needs padding in between the fields, single bit.
- test_padding({make_field(17, 31), make_field(0, 15)},
- {make_field(17, 31), make_field(16), make_field(0, 15)});
- // Multiple bits of padding.
- test_padding({make_field(17, 31), make_field(0, 14)},
- {make_field(17, 31), make_field(15, 16), make_field(0, 14)});
-
- // Padding before first field, single bit.
- test_padding({make_field(0, 30)}, {make_field(31), make_field(0, 30)});
- // Multiple bits.
- test_padding({make_field(0, 15)}, {make_field(16, 31), make_field(0, 15)});
-
- // Padding after last field, single bit.
- test_padding({make_field(1, 31)}, {make_field(1, 31), make_field(0)});
- // Multiple bits.
- test_padding({make_field(2, 31)}, {make_field(2, 31), make_field(0, 1)});
-
- // Fields need padding before, in between and after.
- // [31-28][field 27-24][23-22][field 21-20][19-12][field 11-8][7-0]
- test_padding({make_field(24, 27), make_field(20, 21), make_field(8, 11)},
- {make_field(28, 31), make_field(24, 27), make_field(22, 23),
- make_field(20, 21), make_field(12, 19), make_field(8, 11),
- make_field(0, 7)});
-}
-
-TEST(RegisterFieldsTest, ReverseFieldOrder) {
- // Unchanged
- RegisterFlags rf("", 4, {make_field(0, 31)});
- ASSERT_EQ(0x12345678ULL, (unsigned long long)rf.ReverseFieldOrder(0x12345678));
-
- // Swap the two halves around.
- RegisterFlags rf2("", 4, {make_field(16, 31), make_field(0, 15)});
- ASSERT_EQ(0x56781234ULL, (unsigned long long)rf2.ReverseFieldOrder(0x12345678));
-
- // Many small fields.
- RegisterFlags rf3(
- "", 4, {make_field(31), make_field(30), make_field(29), make_field(28)});
- ASSERT_EQ(0x00000005ULL, rf3.ReverseFieldOrder(0xA0000000));
-}
-
TEST(RegisterFlagsTest, AsTable) {
- // Anonymous fields are shown with an empty name cell,
- // whether they are known up front or added during construction.
+ // Anonymous fields are shown with an empty name cell.
RegisterFlags anon_field("", 4, {make_field(0, 31)});
ASSERT_EQ("| 31-0 |\n"
"|------|\n"
>From 6e13211be720a052d416abb27d0bd81f9da1f33e Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at arm.com>
Date: Wed, 15 Apr 2026 14:40:40 +0000
Subject: [PATCH 2/2] move include
---
.../Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
index d346f79acceb8..ed032ce5db38e 100644
--- a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
+++ b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
@@ -9,10 +9,9 @@
#ifndef LLDB_SOURCE_PLUGINS_REGISTERTYPEBUILDER_REGISTERTYPEBUILDERCLANG_H
#define LLDB_SOURCE_PLUGINS_REGISTERTYPEBUILDER_REGISTERTYPEBUILDERCLANG_H
-#include "clang/AST/ExternalASTSource.h"
-
#include "lldb/Target/RegisterTypeBuilder.h"
#include "lldb/Target/Target.h"
+#include "clang/AST/ExternalASTSource.h"
namespace lldb_private {
class RegisterTypeBuilderClang : public RegisterTypeBuilder {
More information about the lldb-commits
mailing list