[lldb] Show register fields using bitfield struct types

[DavidSpickett]

Summary

This change uses the information from target.xml sent by
the GDB stub to produce C types that we can use to print
register fields.

lldb-server *does not* produce this information yet. This will
only work with GDB stubs that do. gdbserver or qemu
are 2 I know of. Testing is added that uses a mocked lldb-server.

(lldb) register read cpsr x0 fpcr fpsr x1
    cpsr = 0x60001000
         = (N = 0, Z = 1, C = 1, V = 0, TCO = 0, DIT = 0, UAO = 0, PAN = 0, SS = 0, IL = 0, SSBS = 1, BTYPE = 0, D = 0, A = 0, I = 0, F = 0, nRW = 0, EL = 0, SP = 0)

Only "register read" will display fields, and only when
we are not printing a register block.

For example, cpsr is a 32 bit register. Using the target's scratch type
system we construct a type:

struct __attribute__((__packed__)) cpsr {
  uint32_t N : 1;
  uint32_t Z : 1;
  ...
  uint32_t EL : 2;
  uint32_t SP : 1;
};

If this register had unallocated bits in it, those would
have been filled in by RegisterFlags as anonymous fields.
A new option "SetChildPrintingDecider" is added so we
can disable printing those.

Important things about this type:

• It is packed so that sizeof(struct cpsr) == sizeof(the real register). (this will hold for all flags types we create)
• Each field has the same storage type, which is the same as the type of the raw register value. This prevents fields being spilt over into more storage units, as is allowed by most ABIs.
• Each bitfield size matches that of its register field.
• The most significant field is first.

The last point is required because the most significant bit (MSB)
being on the left/top of a print out matches what you'd expect to
see in an architecture manual. In addition, having lldb print a
different field order on big/little endian hosts is not acceptable.

As a consequence, if the target is little endian we have to
reverse the order of the fields in the value. The value of each field
remains the same. For example 0b01 doesn't become 0b10, it just shifts
up or down.

This is needed because clang's type system assumes that for a struct
like the one above, the least significant bit (LSB) will be first
for a little endian target. We need the MSB to be first.

Finally, if lldb's host is a different endian to the target we have
to byte swap the host endian value to match the endian of the target's
typesystem.

Host Endian	Target Endian	Field Order Swap	Byte Order Swap
Little	Little	Yes	No
Big	Little	Yes	Yes
Little	Big	No	Yes
Big	Big	No	No

Testing was done as follows:

• Little -> Little
  • LE AArch64 native debug.
• Big -> Little
  • s390x lldb running under QEMU, connected to LE AArch64 target.
• Little -> Big
  • LE AArch64 lldb connected to QEMU's GDB stub, which is running an s390x program.
• Big -> Big
  • s390x lldb running under QEMU, connected to another QEMU's GDB stub, which is running an s390x program.

As we are not allowed to link core code to plugins directly,
I have added a new plugin RegisterTypeBuilder. There is one implementation
of this, RegisterTypeBuilderClang, which uses TypeSystemClang to build
the CompilerType from the register fields.

[DavidSpickett]

Rebase

[DavidSpickett]

Rebase

[jasonmolenda]

This all looks good to me. the phab says there's a missing newline at the end of TestXMLRegisterFlags.py.

Given that we can construct fake debug sessions with a gdb_remote_client test, do you think it would be worth having a test that's says it's a big-endian target, and confirming that we byte swap when the test is on a little-endian host? I see you're using a .yaml shell binary for the session, but I wrote a simple client test a while ago, TestStopPCs.py, where I don't even bother with that.

On the other hand, I noticed you marked these as skipIfLLVMTargetMissing("AArch64") - is this because of the clang types? We're not disassembling or jitting anything for the target, as long as lldb recognizes the triple/ArchSpec, for registers it seems like it should be fine? If this skip is really needed, then I don't know how many people would exercise an s390x test even if it was added - I don't personally build with that target normally.

[jasonmolenda]

I'll mark it as accepted, I don't know if Jim or Pavel have any comments. I think this is good.

[DavidSpickett]

• Rebase
• Add a test to show the register fields respect the child limit setting.
• Make sure alignment is correct both when printed as one line and multiple.
• Correct indentation in test file so it's all 4 space tabs.

[DavidSpickett]

The last update changes the format slightly, though for the better I think.

(lldb) register read cpsr x0 fpcr fpsr x1
    cpsr = 0x60001000
         = (N = 0, Z = 1, C = 1, V = 0, TCO = 0, DIT = 0, UAO = 0, PAN = 0, SS = 0, IL = 0, SSBS = 1, BTYPE = 0, D = 0, A = 0, I = 0, F = 0, nRW = 0, EL = 0, SP = 0)

As we don't print the root type name we get this orphaned =, so I've aligned that with the usual value line above. I think it's clear that it's a second view on the register not a new name (and removing the = is fiddly).

If the fields end up on multiple lines, we handle that too:

(lldb) register read fpcr
    fpcr = 0x00000000
         = {
             AHP = 0
             DN = 0
<...>
           }

[DavidSpickett]

This all looks good to me. the phab says there's a missing newline at the end of TestXMLRegisterFlags.py.

Fixed.

[DavidSpickett]

Remove need for aarch64 yaml file in tests. Refactor the responders.

[DavidSpickett]

I'm trying to get an s390x test going in the same fashion but figuring it out is tricky.

Also, I realise that all this XML substitution with strings is very brittle. I want to replace that with Python's xml.etree but will do that later in another patch.

[DavidSpickett]

Add an s390x test for big endian byte ordering. Turns out you can fake AArch64
without the backend, but not s390x.

Luckily, s390x is a default backend. So the LE host -> BE target part will be
tested widely. BE to LE, the AArch64 little endian byte order test will cover,
assuming there is an s390x builder out there somewhere.

[DavidSpickett]

If this skip is really needed, then I don't know how many people would exercise an s390x test even if it was added - I don't personally build with that target normally.

This is now resolved. No skip needed for AArch64, but for whatever reason, it is needed for s390x.

[jasonmolenda]

This gdb_remote_client test looks real nice with this update.

[DavidSpickett]

Also, I realise that all this XML substitution with strings is very brittle. I want to replace that with Python's xml.etree but will do that later in another patch.

Having tried this, it gives you nice indentation but it's not that much different to the raw text. Except that you can't see exactly what's going in unless you turn on logging. It is useful to have example of the format here at a glance and I don't see there being too much churn in this file so I'm leaving it as is.

[DavidSpickett]

The shakiest aspect here is probably my use of the scratch type system. It works but I'm not 100% that there isn't a better choice.

These are the current steps:

• Make the type name. I've prepended __lldb_ as I saw elsewhere. So register cpsr would have a type __llb_register_fields_cpsr.
• If we already have a type, use it.
• If we don't, make one and use that.

So I'm assuming that the scratch type system exists for the life of the debug session. It can be reset and we would regenerate the type
as needed.

Another assumption is that we won't get a new batch of register info mid session. We would just keep using the existing type if we had
already made one. Seems fairly safe to me.

The type name might change in future to be more general e.g. __lldb_register_type_cpsr as and when we support the union and struct
elements that can also be in the XML. It's an internal detail, so this is fine.

[JDevlieghere]

lldb/include/lldb/Core/DumpRegisterValue.h

12

Core components (not just the Core libraries but basically everything that's not a plugin) shouldn't depend on plugins. There's still places in LLDB where this is the case but @bulbazord has been hard at work to clean those up. This would introduce a new one. Can we avoid this?

[bulbazord]

I'm supportive of this idea but I would like to find a way to do it without introducing dependencies on plugins in non-plugin code if possible.

lldb/include/lldb/Core/DumpRegisterValue.h

12

+1 Let's find a way to avoid this if possible. Perhaps we can use just TypeSystem?

lldb/source/Commands/CommandObjectRegister.cpp

212–213

Instead of being specific to clang, it would be better if we could use Target::GetScratchTypeSystemForLanguage(eLanguageTypeC) or something to this effect.

lldb/source/Core/DumpRegisterValue.cpp

136–137

This seems highly specific to C++... Let's try to find another way to do this, ideally with TypeSystem instead of TypeSystemClang and clang::CXXRecordDecl.

[DavidSpickett]

On the subject of not using TypeSystemClang, looking around it seems like TypeSystem is purely used to look up basic types, never to make new ones. When new types are made the code asks the TypeSystem for its TypeSystemClang and then uses that to do it.

Is that a limitation of TypeSystem or just that it's missing methods? I also looked for ways to build types without the type system then add them to it, but didn't find any. Do we have anyone who knows more about this? (I sure don't)

I could move the code into an existing plugin like Platform. Problems with that:

• Platform implementations are plugins, the base class is not so I have to have 3 copies of the method.
• When using a core file, the platform is always the host. Might be able to work around that by passing the core file's triple in each time.

Or put the code into a new plugin but then I've just got the same dependency problem with more steps.

There is code for dealing with user expressions so I could make the types out of strings and evaluate something like:

*reinterpret_cast(
  __attribute__((packed)) struct __lldb_register_fields_cpsr {
    uint32_t z;
    ...
  })(<fake address of register value)

It does feel like something that code wasn't intended to do.

[bulbazord]

In D145580#4209621, @DavidSpickett wrote:

On the subject of not using TypeSystemClang, looking around it seems like TypeSystem is purely used to look up basic types, never to make new ones. When new types are made the code asks the TypeSystem for its TypeSystemClang and then uses that to do it.

Is that a limitation of TypeSystem or just that it's missing methods? I also looked for ways to build types without the type system then add them to it, but didn't find any. Do we have anyone who knows more about this? (I sure don't)

I think @aprantl might know more about TypeSystems and what their intended use/capabilities are. It's entirely possible that TypeSystem is just not as flexible as it could be.

I could move the code into an existing plugin like Platform. Problems with that:

• Platform implementations are plugins, the base class is not so I have to have 3 copies of the method.
• When using a core file, the platform is always the host. Might be able to work around that by passing the core file's triple in each time.

Or put the code into a new plugin but then I've just got the same dependency problem with more steps.

There is code for dealing with user expressions so I could make the types out of strings and evaluate something like:

*reinterpret_cast(
  __attribute__((packed)) struct __lldb_register_fields_cpsr {
    uint32_t z;
    ...
  })(<fake address of register value)

It does feel like something that code wasn't intended to do.

Right, these solutions might not be as nice. I've been working to stop using plugins in non-plugin code for a few years now so I'd like to prevent us from adding new uses where possible. I don't want to block progress though! Let's see if @aprantl can help us out.

[aprantl]

lldb/source/Core/DumpRegisterValue.cpp

136–137

Are we saving a lot of code by going through the clang typesystem here, or would walking the bits and formatting the directly be roughly the same amount of code?

[DavidSpickett]

lldb/source/Core/DumpRegisterValue.cpp

136–137

Funny you should mention that.

I was initially doing that, but in the RFC it was suggested I use the existing printers for C types (https://discourse.llvm.org/t/rfc-showing-register-fields-in-lldb/64676/2). There's also a hint that this would make assigning to specific fields more easy (though that is a very far off goal).

This is what the first version looked like:

(lldb) register read cpsr
    cpsr = 0x60001000
| N | Z | C | V | TCO | DIT | UAO | PAN | SS | IL | SSBS | BTYPE | D | A | I | F | nRW | EL | SP |
| 0 | 1 | 1 | 0 |  0  |  0  |  0  |  0  | 0  | 0  |  1   |   0   | 0 | 0 | 0 | 0 |  0  | 0  | 0  |

Of course it would be refined based on line length and field size etc etc. A lot of the things the type printers already do, which is why using them looked like it would save some effort.

Some of that extra formatting code will get written anyway because I want to eventually add a register command that will show you the register layout. So table formatting is needed either way. That would produce something like:

(lldb) register info cpsr
| 31 | 30 | ...
| N  |  Z | ...

So overall no there's nothing preventing me from walking the bits. Assuming you already have code to format text tables nicely, it would be about the same amount of code.

I need to check if we have any generic table formatting code, or could extract some from somewhere.

[aprantl]

lldb/source/Core/DumpRegisterValue.cpp

136–137

Sorry I wasn't aware of the history :-)

[aprantl]

lldb/source/Commands/CommandObjectRegister.cpp

229

Do we need to pass this in explicitly or could the implementation do the Target::GetScratchTypeSystemForLanguage::GetForTarget(m_exe_ctx.GetTargetRef())? That would avoid introducing a plugin dependence at least in this file?

[DavidSpickett]

lldb/source/Core/DumpRegisterValue.cpp

136–137

Do you have an idea of how much work it would be to make TypeSystem do the same things TypeSystemClang does? Maybe more to the point, would I be trying to make TypeSystem do something it just isn't designed to do.

[DavidSpickett]

lldb/source/Commands/CommandObjectRegister.cpp

229

Yes I could do that. Though I think even Core/DumpRegisterValue.cpp using it isn't good.

Is that right @bulbazord ?

[aprantl]

lldb/source/Core/DumpRegisterValue.cpp

136–137

Fundamentally, creating new types is something so tightly coupled to a specific type system that it may not be a good idea to expose this through the generic TypeSystem interface.

We could design a generic interface that looks like

CompilerType ty = CreateRecordType({{"field1", CompilerType1}, {"field2", ...}})

but here you explicitly want to control packing and bitfields, and so you will end up with something that most likely only TypeSystemClang can fulfill anyway.

So the better solution may be along the lines of a register formatting plugin that depends on TypeSystemClang.

[bulbazord]

lldb/source/Commands/CommandObjectRegister.cpp

229

Yes, avoiding plugin dependencies in non-plugins would be ideal.

[DavidSpickett]

So the better solution may be along the lines of a register formatting plugin that depends on TypeSystemClang.

Based on discussion here and on another patch, I'm going to try Adrian's suggestion.

[DavidSpickett]

Move the type generation into a new plugin type, RegisterTypeBuilder.

This means we're still resuing types if you read the same register
multiple times, but we are recreating the plugin each time. So that
could be improved if people think it's worth it.

The code path is now:

• register read calls dumpregister
• which calls GetRegisterType on Target
• Target gets the plugin and asks it to make the type

The way it does all that is exactly the same as before, but now
we aren't linking directly to a plugin to do it.

Also I am assuming that RegisterTypeBuilderClang is the only implementation
and is always present (a singleton plugin sort of). Which seems fine but I
don't know if that's a good way to treat plugins in general.

[DavidSpickett]

@bulbazord Please take a look and see if I've done the plugin/core code split correctly.

[bulbazord]

In D145580#4245911, @DavidSpickett wrote:

@bulbazord Please take a look and see if I've done the plugin/core code split correctly.

This looks fine. Thanks for being flexible and working with us to find something that maintains the split even if the end result feels more complex.

[DavidSpickett]

Rebase

lldb/include/lldb/Core/DumpRegisterValue.h

 //===-- DumpRegisterValue.h -------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_CORE_DUMPREGISTERVALUE_H
 #define LLDB_CORE_DUMPREGISTERVALUE_H
 
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/lldb-enumerations.h"
 #include <cstdint>
 
 namespace lldb_private {
 
 class ExecutionContextScope;
 class RegisterValue;
 struct RegisterInfo;
 class Stream;
 
 // The default value of 0 for reg_name_right_align_at means no alignment at
 // all.
+// Set print_flags to true to print register fields if they are available.
 void DumpRegisterValue(const RegisterValue &reg_val, Stream *s,
                        const RegisterInfo *reg_info, bool prefix_with_name,
                        bool prefix_with_alt_name, lldb::Format format,
                        uint32_t reg_name_right_align_at = 0,
-                       ExecutionContextScope *exe_scope = nullptr);
+                       ExecutionContextScope *exe_scope = nullptr,
+                       bool print_flags = false,
+                       lldb::TypeSystemClangSP ast = nullptr);
 
 } // namespace lldb_private
 
 #endif // LLDB_CORE_DUMPREGISTERVALUE_H

lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h

 //===-- DumpValueObjectOptions.h --------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
struct PointerAsArraySettings {
 
     operator bool() { return m_element_count > 0; }
   };
 
   typedef std::function<bool(ConstString, ConstString,
                              const DumpValueObjectOptions &, Stream &)>
       DeclPrintingHelper;
 
+  typedef std::function<bool(ConstString)> ChildPrintingDecider;
+
   static const DumpValueObjectOptions DefaultOptions() {
     static DumpValueObjectOptions g_default_options;
 
     return g_default_options;
   }
 
   DumpValueObjectOptions();
 
   DumpValueObjectOptions(ValueObject &valobj);
 
   DumpValueObjectOptions &
   SetMaximumPointerDepth(PointerDepth depth = {PointerDepth::Mode::Never, 0});
 
   DumpValueObjectOptions &SetMaximumDepth(uint32_t depth, bool is_default);
 
   DumpValueObjectOptions &SetDeclPrintingHelper(DeclPrintingHelper helper);
 
+  DumpValueObjectOptions &SetChildPrintingDecider(ChildPrintingDecider decider);
+
   DumpValueObjectOptions &SetShowTypes(bool show = false);
 
   DumpValueObjectOptions &SetShowLocation(bool show = false);
 
   DumpValueObjectOptions &SetUseObjectiveC(bool use = false);
 
   DumpValueObjectOptions &SetShowSummary(bool show = true);
 
public:
   lldb::DynamicValueType m_use_dynamic = lldb::eNoDynamicValues;
   uint32_t m_omit_summary_depth = 0;
   lldb::Format m_format = lldb::eFormatDefault;
   lldb::TypeSummaryImplSP m_summary_sp;
   std::string m_root_valobj_name;
   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;
   bool m_use_synthetic : 1;
   bool m_scope_already_checked : 1;
   bool m_flat_output : 1;
   bool m_ignore_cap : 1;
   bool m_show_types : 1;
   bool m_show_location : 1;
   bool m_use_objc : 1;

lldb/include/lldb/Target/RegisterFlags.h

 //===-- RegisterFlags.h -----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
public:
 
   // 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.
   RegisterFlags(llvm::StringRef id, unsigned size,
                 const std::vector<Field> &fields);
 
+  // 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; }
   void log(Log *log) const;
 
 private:
   std::string m_id;
   // Size in bytes
   unsigned m_size;
   std::vector<Field> m_fields;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_TARGET_REGISTERFLAGS_H

lldb/source/Commands/CommandObjectRegister.cpp

 //===-- CommandObjectRegister.cpp -----------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
HandleArgumentCompletion(CompletionRequest &request,
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eRegisterCompletion,
         request, nullptr);
   }
 
   Options *GetOptions() override { return &m_option_group; }
 
   bool DumpRegister(const ExecutionContext &exe_ctx, Stream &strm,
-                    RegisterContext *reg_ctx, const RegisterInfo *reg_info) {
+                    RegisterContext *reg_ctx, const RegisterInfo *reg_info,
+                    bool print_flags, TypeSystemClangSP ast) {
     if (reg_info) {
       RegisterValue reg_value;
 
       if (reg_ctx->ReadRegister(reg_info, reg_value)) {
         strm.Indent();
 
         bool prefix_with_altname = (bool)m_command_options.alternate_name;
         bool prefix_with_name = !prefix_with_altname;
         DumpRegisterValue(reg_value, &strm, reg_info, prefix_with_name,
                           prefix_with_altname, m_format_options.GetFormat(), 8,
-                          exe_ctx.GetBestExecutionContextScope());
+                          exe_ctx.GetBestExecutionContextScope(), print_flags,
+                          ast);
         if ((reg_info->encoding == eEncodingUint) ||
             (reg_info->encoding == eEncodingSint)) {
           Process *process = exe_ctx.GetProcessPtr();
           if (process && reg_info->byte_size == process->GetAddressByteSize()) {
             addr_t reg_addr = reg_value.GetAsUInt64(LLDB_INVALID_ADDRESS);
             if (reg_addr != LLDB_INVALID_ADDRESS) {
               Address so_reg_addr;
               if (exe_ctx.GetTargetRef()
if (reg_set) {
       const size_t num_registers = reg_set->num_registers;
       for (size_t reg_idx = 0; reg_idx < num_registers; ++reg_idx) {
         const uint32_t reg = reg_set->registers[reg_idx];
         const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoAtIndex(reg);
         // Skip the dumping of derived register if primitive_only is true.
         if (primitive_only && reg_info && reg_info->value_regs)
           continue;
 
-        if (DumpRegister(exe_ctx, strm, reg_ctx, reg_info))
+        if (DumpRegister(exe_ctx, strm, reg_ctx, reg_info,
+                         /*print_flags=*/false, nullptr))
           ++available_count;
         else
           ++unavailable_count;
       }
       strm.IndentLess();
       if (unavailable_count) {
         strm.Indent();
         strm.Printf("%u registers were unavailable.\n", unavailable_count);
bool DoExecute(Args &command, CommandReturnObject &result) override {
     } else {
       if (m_command_options.dump_all_sets) {
         result.AppendError("the --all option can't be used when registers "
                            "names are supplied as arguments\n");
       } else if (m_command_options.set_indexes.GetSize() > 0) {
         result.AppendError("the --set <set> option can't be used when "
                            "registers names are supplied as arguments\n");
       } else {
+        TypeSystemClangSP type_system =
+            ScratchTypeSystemClang::GetForTarget(m_exe_ctx.GetTargetRef());
+        assert(type_system);
+
         for (auto &entry : command) {
           // in most LLDB commands we accept $rbx as the name for register RBX
           // - and here we would reject it and non-existant. we should be more
           // consistent towards the user and allow them to say reg read $rbx -
           // internally, however, we should be strict and not allow ourselves
           // to call our registers $rbx in our own API
           auto arg_str = entry.ref();
           arg_str.consume_front("$");
 
           reg_info = reg_ctx->GetRegisterInfoByName(arg_str);
 
           if (reg_info) {
-            if (!DumpRegister(m_exe_ctx, strm, reg_ctx, reg_info))
+            if (!DumpRegister(m_exe_ctx, strm, reg_ctx, reg_info,
+                              /*print_flags=*/true, type_system))
               strm.Printf("%-12s = error: unavailable\n", reg_info->name);
           } else {
             result.AppendErrorWithFormat("Invalid register name '%s'.\n",
                                          arg_str.str().c_str());
           }
         }
       }
     }

lldb/source/Core/DumpRegisterValue.cpp

 //===-- DumpRegisterValue.cpp ---------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "lldb/Core/DumpRegisterValue.h"
+#include "clang/AST/DeclCXX.h"
+
 #include "lldb/Core/DumpDataExtractor.h"
+#include "lldb/Core/DumpRegisterValue.h"
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Core/ValueObjectConstResult.h"
+#include "lldb/DataFormatters/DumpValueObjectOptions.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/Endian.h"
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/lldb-private-types.h"
 
 using namespace lldb;
 
+static uint32_t swap_value(uint32_t v) { return __builtin_bswap32(v); }
+static uint64_t swap_value(uint64_t v) { return __builtin_bswap64(v); }
+
+template <typename T>
+static void dump_type_value(lldb_private::CompilerType &fields_type, T value,
+                            lldb_private::ExecutionContextScope *exe_scope,
+                            const lldb_private::RegisterInfo &reg_info,
+                            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.
+  if (lldb_private::endian::InlHostByteOrder() != target_order)
+    value = swap_value(value);
+
+  lldb_private::DataExtractor data_extractor{
+      &value, sizeof(T), lldb_private::endian::InlHostByteOrder(), 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);
+
+  vobj_sp->Dump(strm, dump_options);
+}
+
 void lldb_private::DumpRegisterValue(const RegisterValue &reg_val, Stream *s,
                                      const RegisterInfo *reg_info,
                                      bool prefix_with_name,
                                      bool prefix_with_alt_name, Format format,
                                      uint32_t reg_name_right_align_at,
-                                     ExecutionContextScope *exe_scope) {
+                                     ExecutionContextScope *exe_scope,
+                                     bool print_flags, TypeSystemClangSP ast) {
   DataExtractor data;
   if (!reg_val.GetData(data))
     return;
 
   bool name_printed = false;
   // For simplicity, alignment of the register name printing applies only in
   // the most common case where:
   //
DumpDataExtractor(data, s,
                     format,               // Format to use when dumping
                     reg_info->byte_size,  // item_byte_size
                     1,                    // item_count
                     UINT32_MAX,           // num_per_line
                     LLDB_INVALID_ADDRESS, // base_addr
                     0,                    // item_bit_size
                     0,                    // item_bit_offset
                     exe_scope);
+
+  if (!print_flags || !reg_info->flags_type || !exe_scope ||
+      (reg_info->byte_size != 4 && reg_info->byte_size != 8))
+    return;
+
+  // Use a new stream so we can remove a trailing newline later.
+  StreamString fields_stream;
+
+  std::string register_type_name = "__lldb_register_fields_";
+  register_type_name += reg_info->name;
+  // See if we have made this type before and can reuse it.
+  CompilerType fields_type = ast->GetTypeForIdentifier<clang::CXXRecordDecl>(
+      ConstString(register_type_name.c_str()));
+
+  if (!fields_type) {
+    // In most ABI, a change of field type means a change in storage unit.
+    // We want it all in one unit, so we use a field type the same as the
+    // register's size.
+    CompilerType field_uint_type = ast->GetBuiltinTypeForEncodingAndBitSize(
+        eEncodingUint, reg_info->byte_size * 8);
+
+    fields_type = ast->CreateRecordType(
+        nullptr, OptionalClangModuleID(), lldb::eAccessPublic, reg_info->name,
+        clang::TTK_Struct, lldb::eLanguageTypeC);
+    ast->StartTagDeclarationDefinition(fields_type);
+
+    // We assume that RegisterFlags has padded and sorted the fields
+    // already.
+    for (const RegisterFlags::Field &field :
+         reg_info->flags_type->GetFields()) {
+      ast->AddFieldToRecordType(fields_type, field.GetName(), field_uint_type,
+                                lldb::eAccessPublic, field.GetSizeInBits());
+    }
+
+    ast->CompleteTagDeclarationDefinition(fields_type);
+    // So that the size of the type matches the size of the register.
+    ast->SetIsPacked(fields_type);
+
+    // This should be true if RegisterFlags padded correctly.
+    assert(*fields_type.GetByteSize(nullptr) ==
+           reg_info->flags_type->GetSize());
+  }
+
+  if (reg_info->byte_size == 4) {
+    dump_type_value(fields_type, reg_val.GetAsUInt32(), exe_scope, *reg_info,
+                    fields_stream);
+  } else {
+    dump_type_value(fields_type, reg_val.GetAsUInt64(), exe_scope, *reg_info,
+                    fields_stream);
+  }
+
+  // Registers are indented like:
+  // (lldb) register read foo
+  //     foo = 0x12345678
+  // So we need to indent to match that.
+
+  // First drop the extra newline that the value printer added. The register
+  // command will add one itself.
+  llvm::StringRef fields_str = fields_stream.GetString().drop_back();
+
+  // End the line that contains "    foo = 0x12345678".
+  s->EOL();
+
+  // Then split the value lines and indent each one.
+  bool first = true;
+  while (fields_str.size()) {
+    std::pair<llvm::StringRef, llvm::StringRef> split = fields_str.split('\n');
+    fields_str = split.second;
+    // Indent as far as the register name did.
+    s->Printf(fmt.c_str(), "");
+
+    // Lines after the first won't have " = " so compensate for that.
+    if (!first)
+      (*s) << "   ";
+    first = false;
+
+    (*s) << split.first;
+
+    // On the last line we don't want a newline because the command will add
+    // one too.
+    if (fields_str.size())
+      s->EOL();
+  }
 }

lldb/source/DataFormatters/DumpValueObjectOptions.cpp

 //===-- DumpValueObjectOptions.cpp ----------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/DataFormatters/DumpValueObjectOptions.h"
 
 #include "lldb/Core/ValueObject.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 DumpValueObjectOptions::DumpValueObjectOptions()
     : m_summary_sp(), m_root_valobj_name(),
       m_max_ptr_depth(PointerDepth{PointerDepth::Mode::Default, 0}),
-      m_decl_printing_helper(), m_pointer_as_array(), m_use_synthetic(true),
+      m_decl_printing_helper(), m_child_printing_decider(),
+      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_objc(false),
       m_hide_root_type(false), m_hide_name(false), m_hide_value(false),
       m_run_validator(false), m_use_type_display_name(true),
       m_allow_oneliner_mode(true), m_hide_pointer_value(false),
       m_reveal_empty_aggregates(true) {}
 
 DumpValueObjectOptions::DumpValueObjectOptions(ValueObject &valobj)

 }
 
 DumpValueObjectOptions &
 DumpValueObjectOptions::SetDeclPrintingHelper(DeclPrintingHelper helper) {
   m_decl_printing_helper = 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;
 }
 
 DumpValueObjectOptions &DumpValueObjectOptions::SetShowLocation(bool show) {
   m_show_location = show;
   return *this;

lldb/source/DataFormatters/ValueObjectPrinter.cpp

 //===-- ValueObjectPrinter.cpp --------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
void ValueObjectPrinter::PrintChildren(
 
   bool print_dotdotdot = false;
   size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
   if (num_children) {
     bool any_children_printed = false;
 
     for (size_t idx = 0; idx < num_children; ++idx) {
       if (ValueObjectSP child_sp = GenerateChild(synth_m_valobj, idx)) {
+        if (m_options.m_child_printing_decider &&
+            !m_options.m_child_printing_decider(child_sp->GetName()))
+          continue;
         if (!any_children_printed) {
           PrintChildrenPreamble();
           any_children_printed = true;
         }
         PrintChild(child_sp, curr_ptr_depth);
       }
     }
 
bool ValueObjectPrinter::PrintChildrenOneLiner(bool hide_names) {
   ValueObject *synth_m_valobj = GetValueObjectForChildrenGeneration();
 
   bool print_dotdotdot = false;
   size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
 
   if (num_children) {
     m_stream->PutChar('(');
 
+    bool did_print_children = false;
     for (uint32_t idx = 0; idx < num_children; ++idx) {
       lldb::ValueObjectSP child_sp(synth_m_valobj->GetChildAtIndex(idx, true));
       if (child_sp)
         child_sp = child_sp->GetQualifiedRepresentationIfAvailable(
             m_options.m_use_dynamic, m_options.m_use_synthetic);
       if (child_sp) {
-        if (idx)
+        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;
         if (!hide_names) {
           const char *name = child_sp.get()->GetName().AsCString();
           if (name && *name) {
             m_stream->PutCString(name);
             m_stream->PutCString(" = ");
           }
         }
         child_sp->DumpPrintableRepresentation(

lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py

+""" Check that register fields found in target XML are properly processed.
+
+These tests make XML out of string substitution. This can lead to some strange
+failures. Check that the final XML is valid and each child is indented more than
+the parent tag.
+"""
+
+from textwrap import dedent
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+class MultiDocResponder(MockGDBServerResponder):
+    # docs is a dictionary of filename -> file content.
+    def __init__(self, docs):
+        super().__init__()
+        self.docs = docs
+
+    def qXferRead(self, obj, annex, offset, length):
+        try:
+            return self.docs[annex], False
+        except KeyError:
+            return None,
+
+    def readRegister(self, regnum):
+        return "E01"
+
+    def readRegisters(self):
+        return ''.join([
+          # Data for all registers requested by the tests below.
+          # 0x7 and 0xE are used because their lsb and msb are opposites, which
+          # is needed for a byte order test.
+          '77777777EEEEEEEE', # 64 bit x0/r0
+          '7777EEEE', # 32 bit cpsr/fpc
+          '0000000000000000', # 64 bit pc/pswa
+        ])
+
+class TestXMLRegisterFlags(GDBRemoteTestBase):
+    def setup_multidoc_test(self, docs):
+        self.server.responder = MultiDocResponder(docs)
+        target = self.dbg.CreateTarget('')
+
+        if self.TraceOn():
+            self.runCmd("log enable gdb-remote packets process")
+            self.addTearDownHook(
+                lambda: self.runCmd("log disable gdb-remote packets process"))
+
+        process = self.connect(target)
+        lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+                                      [lldb.eStateStopped])
+
+    def setup_register_test(self, registers):
+        self.setup_multidoc_test(
+          # This *must* begin with the opening tag, leading whitespace is not allowed.
+          {'target.xml' : dedent("""\
+            <?xml version="1.0"?>
+              <target version="1.0">
+                <architecture>aarch64</architecture>
+                <feature name="org.gnu.gdb.aarch64.core">
+                  {}
+                </feature>
+            </target>""").format(registers)})
+
+    def setup_flags_test(self, flags):
+        # pc is required here though we don't look at it in the tests.
+        # x0 is only used by some tests but always including it keeps the data ordering
+        # the same throughout.
+        self.setup_register_test("""\
+          <flags id="cpsr_flags" size="4">
+            {}
+          </flags>
+          <reg name="pc" bitsize="64"/>
+          <reg name="x0" regnum="0" bitsize="64" type="x0_flags"/>
+          <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"/>""".format(
+            flags))
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_no_flags(self):
+        self.setup_flags_test("")
+        self.expect("register read cpsr", substrs=["= 0xeeee7777"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_single_field_pad_msb(self):
+        self.setup_flags_test("""<field name="SP" start="0" end="0"/>""")
+        # Pads from 31 to 1.
+        self.expect("register read cpsr", substrs=["(SP = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_single_field_pad_lsb(self):
+        self.setup_flags_test("""<field name="SP" start="31" end="31"/>""")
+        self.expect("register read cpsr", substrs=["(SP = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_multiple_fields_sorted(self):
+        self.setup_flags_test("""<field name="SP" start="0" end="0"/>
+                                 <field name="EL" start="1" end="2"/>""")
+
+        # Fields should be sorted with MSB on the left.
+        self.expect("register read cpsr", substrs=["(EL = 3, SP = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_ignore_invalid_start_end(self):
+        self.setup_flags_test(
+          # Is valid so is used.
+          '<field name="EL" start="2" end="3"/>'
+          # Start/end cannot be negative, ignored.
+          '<field name="SP" start="-1" end="2"/>'
+          '<field name="SP2" start="1" end="-5"/>'
+          # Start is not <= end, ignored.
+          '<field name="ABC" start="12" end="10"/>'
+          # Start cannot be >= (size of register in bits)
+          '<field name="?" start="32" end="29"/>'
+          # End cannot be >= (size of register in bits)
+          '<field name="DEF" start="30" end="35"/>')
+
+        self.expect("register read cpsr", substrs=["(EL = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_field_overlap(self):
+        self.setup_flags_test(
+          '<field name="?" start="10" end="12"/>'
+          # A overlaps B
+          '<field name="A" start="0" end="3"/>'
+          '<field name="B" start="0" end="0"/>')
+
+        # Ignore the whole flags set, it is unlikely to be valid.
+        self.expect("register read cpsr", substrs=["("], matching=False)
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_field_required_attributes(self):
+        # Fields must have a name, start and end. Any without are ignored.
+        self.setup_flags_test(
+          # Missing name
+          '<field start="0" end="0"/>'
+          # Missing start
+          '<field name="A" end="0"/>'
+          # Missing end
+          '<field name="B" start="0"/>'
+          # Valid
+          '<field name="C" start="0" end="0"/>')
+
+        self.expect("register read cpsr", substrs=["(C = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_little_endian_target_order(self):
+        # We are using little endian AArch64 here.
+        self.setup_register_test("""\
+           <flags id="cpsr_flags" size="4">
+             <field name="lsb" start="0" end="0"/>
+             <field name="msb" start="31" end="31"/>
+           </flags>
+           <flags id="x0_flags" size="8">
+             <field name="lsb" start="0" end="0"/>
+             <field name="msb" start="63" end="63"/>
+           </flags>
+           <reg name="pc" bitsize="64"/>
+           <reg name="x0" regnum="0" bitsize="64" type="x0_flags"/>
+           <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"/>""")
+
+        # If lldb used the wrong byte ordering for the value for printing fields,
+        # these field values would flip. Since the top and bottom bits of 0x7 and 0xE
+        # are different.
+        self.expect("register read cpsr x0", substrs=[
+          "    cpsr = 0xeeee7777\n"
+          "         = (msb = 1, lsb = 1)\n"
+          "      x0 = 0xeeeeeeee77777777\n"
+          "         = (msb = 1, lsb = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    # Unlike AArch64, we do need the backend present for this test to work.
+    @skipIfLLVMTargetMissing("SystemZ")
+    def test_big_endian_target_order(self):
+        # s390x/SystemZ is big endian.
+        self.setup_multidoc_test({
+            'target.xml' : dedent("""\
+              <?xml version="1.0"?>
+              <target version="1.0">
+                <architecture>s390x</architecture>
+                <feature name="org.gnu.gdb.s390x.core">
+                  <flags id="r0_flags" size="8">
+                    <field name="lsb" start="0" end="0"/>
+                    <field name="msb" start="63" end="63"/>
+                  </flags>
+                  <flags id="fpc_flags" size="4">
+                    <field name="lsb" start="0" end="0"/>
+                    <field name="msb" start="31" end="31"/>
+                  </flags>
+                  <reg name="r0" bitsize="64" type="r0_flags"/>
+                  <reg name="fpc" bitsize="32" type="fpc_flags"/>
+                  <reg name="pswa" bitsize="64"/>
+                </feature>
+              </target>""")})
+
+        # If we did not swap correctly, these fields would show as 1s when run on
+        # a little endian host.
+        self.expect("register read r0 fpc", substrs=[
+          "      r0 = 0x77777777eeeeeeee\n"
+          "         = (msb = 0, lsb = 0)\n"
+          "     fpc = 0x7777eeee\n"
+          "         = (msb = 0, lsb = 0)\n"
+        ])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_many_flag_sets(self):
+        self.setup_register_test("""\
+          <flags id="cpsr_flags" size="4">
+            <field name="correct" start="0" end="0"/>
+          </flags>
+          <flags id="cpsr_flags_alt" size="4">
+            <field name="incorrect" start="0" end="0"/>
+          </flags>
+          <flags id="x0_flags" size="8">
+            <field name="foo" start="0" end="0"/>
+          </flags>
+          <reg name="pc" bitsize="64"/>
+          <reg name="x0" regnum="0" bitsize="64" type="x0_flags"/>
+          <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"/>""")
+
+        self.expect("register read cpsr x0", substrs=[
+          "    cpsr = 0xeeee7777\n"
+          "         = (correct = 1)\n"
+          "      x0 = 0xeeeeeeee77777777\n"
+          "         = (foo = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_repeated_flag_set(self):
+        # The second definition of "cpsr_flags" should be ignored.
+        # This is because we assign the types to registers as we go. If we allowed
+        # the later flag set, it would destroy the first definition, making the
+        # pointer to the flags invalid.
+        self.setup_register_test("""\
+          <flags id="cpsr_flags" size="4">
+            <field name="correct" start="0" end="0"/>
+          </flags>
+          <flags id="cpsr_flags" size="4">
+            <field name="incorrect" start="0" end="0"/>
+          </flags>
+          <reg name="pc" bitsize="64"/>
+          <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"/>""")
+
+        self.expect("register read cpsr", substrs=["(correct = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_missing_flags(self):
+        self.setup_register_test("""\
+          <reg name="pc" bitsize="64"/>
+          <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"/>""")
+
+        # Register prints with default formatting only if we can't find the
+        # flags type.
+        self.expect("register read cpsr", substrs=["cpsr = 0xeeee7777"])
+        self.expect("register read cpsr", substrs=["("], matching=False)
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_flags_invalid_size(self):
+        # We're not using the size for anything yet so just check that we handle
+        # it not being a positive integer.
+        self.setup_register_test("""\
+          <flags id="cpsr_flags" size="???">
+            <field name="A" start="0" end="0"/>
+          </flags>
+          <flags id="cpsr_flags" size="-1">
+            <field name="B" start="0" end="0"/>
+          </flags>
+          <flags id="cpsr_flags" size="4">
+            <field name="C" start="0" end="0"/>
+          </flags>
+          <reg name="pc" bitsize="64"/>
+          <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"/>""")
+
+        # Only the final set has a valid size, use that.
+        self.expect("register read cpsr", substrs=["(C = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_flags_unknown_attribute(self):
+        # Unknown attributes on flags or field are ignored.
+        self.setup_register_test("""\
+          <flags id="cpsr_flags" size="4" stuff="abcd">
+            <field name="A" start="0" abcd="???" end="0"/>
+          </flags>
+          <reg name="pc" bitsize="64"/>
+          <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"/>""")
+
+        self.expect("register read cpsr", substrs=["(A = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_flags_requried_attributes(self):
+        # flags must have an id and size so the flags with "C" is the only valid one
+        # here.
+        self.setup_register_test("""\
+          <flags size="4">
+            <field name="A" start="0" end="0"/>
+          </flags>
+          <flags id="cpsr_flags">
+            <field name="B" start="0" end="0"/>
+          </flags>
+          <flags id="cpsr_flags" size="4">
+            <field name="C" start="0" end="0"/>
+          </flags>
+          <reg name="pc" bitsize="64"/>
+          <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"/>""")
+
+        self.expect("register read cpsr", substrs=["(C = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_flags_set_even_if_format_set(self):
+        # lldb also sends "format". If that is set, we should still read the
+        # flags type.
+        self.setup_register_test("""\
+          <flags id="cpsr_flags" size="4">
+            <field name="B" start="0" end="0"/>
+          </flags>
+          <reg name="pc" bitsize="64"/>
+          <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"
+            format="example"/>""")
+
+        self.expect("register read cpsr", substrs=["(B = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_flags_set_even_if_encoding_set(self):
+        # lldb also sends "encoding". If that is set, we should still read the
+        # flags type.
+        self.setup_register_test("""\
+          <flags id="cpsr_flags" size="4">
+            <field name="B" start="0" end="0"/>
+          </flags>
+          <reg name="pc" bitsize="64"/>
+          <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"
+            encoding="example"/>""")
+
+        self.expect("register read cpsr", substrs=["(B = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_flags_set_even_if_encoding_and_format_set(self):
+        # As above but both encoding and format are set.
+        self.setup_register_test("""\
+          <flags id="cpsr_flags" size="4">
+            <field name="B" start="0" end="0"/>
+          </flags>
+          <reg name="pc" bitsize="64"/>
+          <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"
+            encoding="example" format="example"/>""")
+
+        self.expect("register read cpsr", substrs=["(B = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_flags_multiple_lines(self):
+        # Since we use C types they follow lldb's usual decisions as to whether
+        # to print them on one line or many. Long field names will usually mean
+        # many lines.
+        self.setup_flags_test(
+          '<field name="this_is_a_long_field_0" start="0" end="0"/>'
+          '<field name="this_is_a_long_field_1" start="1" end="1"/>'
+          '<field name="this_is_a_long_field_2" start="2" end="2"/>'
+          '<field name="this_is_a_long_field_3" start="3" end="3"/>')
+
+        self.expect("register read cpsr", substrs=[
+          "    cpsr = 0xeeee7777\n"
+          "         = {\n"
+          "             this_is_a_long_field_3 = 0\n"
+          "             this_is_a_long_field_2 = 1\n"
+          "             this_is_a_long_field_1 = 1\n"
+          "             this_is_a_long_field_0 = 1\n"
+          "           }"])
+
+    @skipIfXmlSupportMissing
+    @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.setup_flags_test(
+          '<field name="field_0" start="0" end="0"/>'
+          '<field name="field_1" start="1" end="1"/>'
+          '<field name="field_2" start="2" end="2"/>')
+
+        self.expect("register read cpsr", substrs=["= (field_2 = 1, field_1 = 1, ...)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_xml_includes(self):
+        # Certain targets e.g. s390x QEMU split their defintions over multiple
+        # files that are included into target.xml.
+        self.setup_multidoc_test({
+            # The formatting is very specific here. lldb doesn't like leading
+            # spaces, and nested tags must be indented more than their parent.
+            'target.xml' : dedent("""\
+               <?xml version="1.0"?>
+               <target version="1.0">
+                 <architecture>aarch64</architecture>
+                 <xi:include href="core.xml"/>
+               </target>"""),
+            'core.xml' : dedent("""\
+                <?xml version="1.0"?>
+                <feature name="org.gnu.gdb.aarch64.core">
+                  <flags id="cpsr_flags" size="4">
+                    <field name="B" start="0" end="0"/>
+                  </flags>
+                  <reg name="pc" bitsize="64"/>
+                  <reg name="x0" regnum="0" bitsize="64" type="x0_flags"/>
+                  <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"/>
+                </feature>
+            """),
+        })
+
+        self.expect("register read cpsr", substrs=["(B = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_xml_includes_multiple(self):
+        self.setup_multidoc_test({
+            'target.xml' : dedent("""\
+               <?xml version="1.0"?>
+               <target version="1.0">
+                 <architecture>aarch64</architecture>
+                 <xi:include href="core.xml"/>
+                 <xi:include href="core-2.xml"/>
+               </target>"""),
+            'core.xml' : dedent("""\
+                <?xml version="1.0"?>
+                <feature name="org.gnu.gdb.aarch64.core">
+                  <flags id="x0_flags" size="4">
+                    <field name="B" start="0" end="0"/>
+                  </flags>
+                  <reg name="pc" bitsize="64"/>
+                  <reg name="x0" regnum="0" bitsize="64" type="x0_flags"/>
+                </feature>"""),
+            'core-2.xml' : dedent("""\
+                <?xml version="1.0"?>
+                <feature name="org.gnu.gdb.aarch64.core">
+                  <flags id="cpsr_flags" size="4">
+                    <field name="C" start="0" end="0"/>
+                  </flags>
+                  <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"/>
+                </feature>
+            """),
+        })
+
+        self.expect("register read x0 cpsr", substrs=["(B = 1)", "(C = 1)"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_xml_includes_flags_redefined(self):
+        self.setup_multidoc_test({
+            'target.xml' : dedent("""\
+               <?xml version="1.0"?>
+               <target version="1.0">
+                 <architecture>aarch64</architecture>
+                 <xi:include href="core.xml"/>
+                 <xi:include href="core-2.xml"/>
+               </target>"""),
+            # Treating xi:include as a textual include, my_flags is first defined
+            # in core.xml. The second definition in core-2.xml
+            # is ignored.
+            'core.xml' : dedent("""\
+                <?xml version="1.0"?>
+                <feature name="org.gnu.gdb.aarch64.core">
+                  <flags id="my_flags" size="4">
+                    <field name="correct" start="0" end="0"/>
+                  </flags>
+                  <reg name="pc" bitsize="64"/>
+                  <reg name="x0" regnum="0" bitsize="64" type="my_flags"/>
+                </feature>"""),
+            # The my_flags here is ignored, so cpsr will use the my_flags from above.
+            'core-2.xml' : dedent("""\
+                <?xml version="1.0"?>
+                <feature name="org.gnu.gdb.aarch64.core">
+                  <flags id="my_flags" size="4">
+                    <field name="incorrect" start="0" end="0"/>
+                  </flags>
+                  <reg name="cpsr" regnum="33" bitsize="32" type="my_flags"/>
+                </feature>
+            """),
+        })
+
+        self.expect("register read x0", substrs=["(correct = 1)"])
+        self.expect("register read cpsr", substrs=["(correct = 1)"])

lldb/unittests/Target/RegisterFlagsTest.cpp

 //===-- RegisterFlagsTest.cpp ---------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
TEST(RegisterFlagsTest, RegisterFlagsPadding) {
 
   // 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, rf.ReverseFieldOrder(0x12345678));
+
+  // Swap the two halves around.
+  RegisterFlags rf2("", 4, {make_field(16, 31), make_field(0, 15)});
+  ASSERT_EQ(0x56781234ULL, rf2.ReverseFieldOrder(0x12345678));
+
+  // Many small fields.
+  RegisterFlags rf3("", 4,
+                    {make_field(31, 31), make_field(30, 30), make_field(29, 29),
+                     make_field(28, 28)});
+  ASSERT_EQ(0x00000005ULL, rf3.ReverseFieldOrder(0xA0000000));
+}

llvm/docs/ReleaseNotes.rst

 
 Changes to LLDB
 ---------------------------------
 
 * In the results of commands such as ``expr`` and ``frame var``, type summaries will now
   omit defaulted template parameters. The full template parameter list can still be
   viewed with ``expr --raw-output``/``frame var --raw-output``. (`D141828 <https://reviews.llvm.org/D141828>`_)
 
+* LLDB can now display register fields if they are described in target XML sent
+  by a debug server such as ``gdbserver`` (``lldb-server`` does not currently produce
+  this information). Fields are only printed when reading named registers, for
+  example ``register read cpsr``. They are not shown when reading a register set,
+  ``register read -s 0``.
+
 Changes to Sanitizers
 ---------------------
 
 Other Changes
 -------------
 
 External Open Source Projects Using LLVM 15
 ===========================================