DavidSpickett created this revision.
Herald added subscribers: ChuanqiXu, kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added projects: LLDB, LLVM.
Herald added subscribers: llvm-commits, lldb-commits.

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
      cpsr = 0x60001000
   (N = 0, Z = 1, C = 1, V = 0, TCO = 0, DIT = 0, UAO = 0,
    PAN = 0, SS = 0, IL = 0, SSBS = 1, 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.

Cross endian setups are very unlikely to be tested by buildbots
but I wanted to be sure this worked on s390x (our only supported BE
target).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145580

Files:
  lldb/include/lldb/Core/DumpRegisterValue.h
  lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
  lldb/include/lldb/Target/RegisterFlags.h
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/source/Core/DumpRegisterValue.cpp
  lldb/source/DataFormatters/DumpValueObjectOptions.cpp
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
  lldb/unittests/Target/RegisterFlagsTest.cpp
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===================================================================
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -188,6 +188,12 @@
   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
 ---------------------
 
Index: lldb/unittests/Target/RegisterFlagsTest.cpp
===================================================================
--- lldb/unittests/Target/RegisterFlagsTest.cpp
+++ lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -121,4 +121,20 @@
                {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)});
-}
\ No newline at end of file
+}
+
+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));
+}
Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -0,0 +1,479 @@
+""" Check that register fields found in target XML are properly processed."""
+
+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 MyResponder(MockGDBServerResponder):
+    def __init__(self, registers):
+      super().__init__()
+      # This *must* begin with the opening tag, leading whitespace is not allowed.
+      self.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 qXferRead(self, obj, annex, offset, length):
+        if annex == "target.xml":
+            return self.target_xml, False
+        else:
+            return None,
+
+    def readRegister(self, regnum):
+        return "E01"
+
+    def readRegisters(self):
+        # 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.
+        return ''.join([
+          '77777777EEEEEEEE', # 64 bit x0
+          '7777EEEE', # 32 bit cpsr
+          '0000000000000000', # 64 bit pc
+        ])
+
+class MyMultiDocResponder(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([
+          '77777777EEEEEEEE', # 64 bit x0
+          '7777EEEE', # 32 bit cpsr
+          '0000000000000000', # 64 bit pc
+        ])
+
+class TestAArch64XMLRegisterFlags(GDBRemoteTestBase):
+    def setup_register_test(self, registers):
+      target = self.createTarget("basic_eh_frame-aarch64.yaml")
+      self.server.responder = MyResponder(registers)
+
+      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_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
+    @skipIfLLVMTargetMissing("AArch64")
+    def test_no_flags(self):
+        self.setup_flags_test("")
+        self.expect("register read cpsr", substrs=["= 0xeeee7777"])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    def test_value_byte_order(self):
+      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
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    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
+    @skipIfLLVMTargetMissing("AArch64")
+    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)"])
+
+    def setup_multidoc_test(self, docs):
+        target = self.createTarget("basic_eh_frame-aarch64.yaml")
+        self.server.responder = MyMultiDocResponder(docs)
+
+        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])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    @skipIfLLVMTargetMissing("AArch64")
+    def test_xml_includes(self):
+        target = self.createTarget("basic_eh_frame-aarch64.yaml")
+
+        # 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="aarch64-core.xml"/>
+               </target>"""),
+            'aarch64-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
+    @skipIfLLVMTargetMissing("AArch64")
+    def test_xml_includes_multiple(self):
+        target = self.createTarget("basic_eh_frame-aarch64.yaml")
+
+        self.setup_multidoc_test({
+            'target.xml' : dedent("""\
+               <?xml version="1.0"?>
+               <target version="1.0">
+                 <architecture>aarch64</architecture>
+                 <xi:include href="aarch64-core.xml"/>
+                 <xi:include href="aarch64-core-2.xml"/>
+               </target>"""),
+            'aarch64-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>"""),
+            'aarch64-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
+    @skipIfLLVMTargetMissing("AArch64")
+    def test_xml_includes_flags_redefined(self):
+        target = self.createTarget("basic_eh_frame-aarch64.yaml")
+
+        self.setup_multidoc_test({
+            'target.xml' : dedent("""\
+               <?xml version="1.0"?>
+               <target version="1.0">
+                 <architecture>aarch64</architecture>
+                 <xi:include href="aarch64-core.xml"/>
+                 <xi:include href="aarch64-core-2.xml"/>
+               </target>"""),
+            # Treating xi:include as a textual include, my_flags is first defined
+            # in aarch64-core.xml. The second definition in aarch64-core-2.xml
+            # is ignored.
+            'aarch64-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.
+            'aarch64-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)"])
\ No newline at end of file
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===================================================================
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -687,6 +687,9 @@
 
     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;
@@ -735,14 +738,19 @@
   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) {
Index: lldb/source/DataFormatters/DumpValueObjectOptions.cpp
===================================================================
--- lldb/source/DataFormatters/DumpValueObjectOptions.cpp
+++ lldb/source/DataFormatters/DumpValueObjectOptions.cpp
@@ -16,7 +16,8 @@
 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),
@@ -50,6 +51,12 @@
   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;
Index: lldb/source/Core/DumpRegisterValue.cpp
===================================================================
--- lldb/source/Core/DumpRegisterValue.cpp
+++ lldb/source/Core/DumpRegisterValue.cpp
@@ -6,21 +6,69 @@
 //
 //===----------------------------------------------------------------------===//
 
-#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)
+      .SetHideName(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;
@@ -76,4 +124,58 @@
                     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;
+  fields_stream.PutCString("\n");
+
+  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);
+  }
+
+  // Remove the trailing newline, the command will add one itself.
+  s->PutCString(fields_stream.GetString().drop_back());
 }
Index: lldb/source/Commands/CommandObjectRegister.cpp
===================================================================
--- lldb/source/Commands/CommandObjectRegister.cpp
+++ lldb/source/Commands/CommandObjectRegister.cpp
@@ -84,7 +84,8 @@
   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;
 
@@ -95,7 +96,8 @@
         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();
@@ -142,7 +144,8 @@
         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;
@@ -206,6 +209,10 @@
         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
@@ -218,7 +225,8 @@
           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",
Index: lldb/include/lldb/Target/RegisterFlags.h
===================================================================
--- lldb/include/lldb/Target/RegisterFlags.h
+++ lldb/include/lldb/Target/RegisterFlags.h
@@ -76,6 +76,23 @@
   RegisterFlags(const std::string &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; }
Index: lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
===================================================================
--- lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
+++ lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
@@ -53,6 +53,8 @@
                              const DumpValueObjectOptions &, Stream &)>
       DeclPrintingHelper;
 
+  typedef std::function<bool(ConstString)> ChildPrintingDecider;
+
   static const DumpValueObjectOptions DefaultOptions() {
     static DumpValueObjectOptions g_default_options;
 
@@ -70,6 +72,8 @@
 
   DumpValueObjectOptions &SetDeclPrintingHelper(DeclPrintingHelper helper);
 
+  DumpValueObjectOptions &SetChildPrintingDecider(ChildPrintingDecider decider);
+
   DumpValueObjectOptions &SetShowTypes(bool show = false);
 
   DumpValueObjectOptions &SetShowLocation(bool show = false);
@@ -134,6 +138,7 @@
   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;
Index: lldb/include/lldb/Core/DumpRegisterValue.h
===================================================================
--- lldb/include/lldb/Core/DumpRegisterValue.h
+++ lldb/include/lldb/Core/DumpRegisterValue.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_CORE_DUMPREGISTERVALUE_H
 #define LLDB_CORE_DUMPREGISTERVALUE_H
 
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/lldb-enumerations.h"
 #include <cstdint>
 
@@ -21,11 +22,14 @@
 
 // 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
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to