[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe07a421dd587: [lldb] Show register fields using bitfield 
struct types (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

Files:
  lldb/include/lldb/Core/DumpRegisterValue.h
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
  lldb/include/lldb/Target/RegisterFlags.h
  lldb/include/lldb/Target/RegisterTypeBuilder.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/source/Core/DumpRegisterValue.cpp
  lldb/source/Core/PluginManager.cpp
  lldb/source/DataFormatters/DumpValueObjectOptions.cpp
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/RegisterTypeBuilder/CMakeLists.txt
  lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
  lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
  lldb/source/Target/Target.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
@@ -251,6 +251,12 @@
 * LLDB is now able to show the subtype of signals found in a core file. For example
   memory tagging specific segfaults such as ``SIGSEGV: sync tag check fault``.
 
+* 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
 -
 * For Darwin users that override weak symbols, note that the dynamic linker will
Index: lldb/unittests/Target/RegisterFlagsTest.cpp
===
--- lldb/unittests/Target/RegisterFlagsTest.cpp
+++ lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -121,3 +121,19 @@
 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(0x0005ULL, rf3.ReverseFieldOrder(0xA000));
+}
Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -0,0 +1,497 @@
+""" 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.
+  '', # 64 bit x0/r0
+  '', # 32 bit cpsr/fpc
+  '', # 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")
+  

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 512710.
DavidSpickett added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

Files:
  lldb/include/lldb/Core/DumpRegisterValue.h
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
  lldb/include/lldb/Target/RegisterFlags.h
  lldb/include/lldb/Target/RegisterTypeBuilder.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/source/Core/DumpRegisterValue.cpp
  lldb/source/Core/PluginManager.cpp
  lldb/source/DataFormatters/DumpValueObjectOptions.cpp
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/RegisterTypeBuilder/CMakeLists.txt
  lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
  lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
  lldb/source/Target/Target.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
@@ -246,6 +246,12 @@
 * LLDB is now able to show the subtype of signals found in a core file. For example
   memory tagging specific segfaults such as ``SIGSEGV: sync tag check fault``.
 
+* 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
 -
 * For Darwin users that override weak symbols, note that the dynamic linker will
Index: lldb/unittests/Target/RegisterFlagsTest.cpp
===
--- lldb/unittests/Target/RegisterFlagsTest.cpp
+++ lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -121,3 +121,19 @@
 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(0x0005ULL, rf3.ReverseFieldOrder(0xA000));
+}
Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -0,0 +1,497 @@
+""" 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.
+  '', # 64 bit x0/r0
+  '', # 32 bit cpsr/fpc
+  '', # 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 

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett marked 9 inline comments as done.
DavidSpickett added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 511096.
DavidSpickett added a comment.
This revision is now accepted and ready to land.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

Files:
  lldb/include/lldb/Core/DumpRegisterValue.h
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
  lldb/include/lldb/Target/RegisterFlags.h
  lldb/include/lldb/Target/RegisterTypeBuilder.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/source/Core/DumpRegisterValue.cpp
  lldb/source/Core/PluginManager.cpp
  lldb/source/DataFormatters/DumpValueObjectOptions.cpp
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/RegisterTypeBuilder/CMakeLists.txt
  lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
  lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
  lldb/source/Target/Target.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
@@ -239,6 +239,12 @@
 * LLDB is now able to show the subtype of signals found in a core file. For example
   memory tagging specific segfaults such as ``SIGSEGV: sync tag check fault``.
 
+* 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
 -
 * For Darwin users that override weak symbols, note that the dynamic linker will
Index: lldb/unittests/Target/RegisterFlagsTest.cpp
===
--- lldb/unittests/Target/RegisterFlagsTest.cpp
+++ lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -121,3 +121,19 @@
 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(0x0005ULL, rf3.ReverseFieldOrder(0xA000));
+}
Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -0,0 +1,497 @@
+""" 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 

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/source/Commands/CommandObjectRegister.cpp:229
+if (!DumpRegister(m_exe_ctx, strm, reg_ctx, reg_info,
+  /*print_flags=*/true, type_system))
   strm.Printf("%-12s = error: unavailable\n", reg_info->name);

DavidSpickett wrote:
> aprantl wrote:
> > 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?
> Yes I could do that. Though I think even `Core/DumpRegisterValue.cpp` using 
> it isn't good.
> 
> Is that right @bulbazord ?
Yes, avoiding plugin dependencies in non-plugins would be ideal.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Core/DumpRegisterValue.cpp:136-137
+  // See if we have made this type before and can reuse it.
+  CompilerType fields_type = ast->GetTypeForIdentifier(
+  ConstString(register_type_name.c_str()));
+

DavidSpickett wrote:
> aprantl wrote:
> > DavidSpickett wrote:
> > > aprantl wrote:
> > > > bulbazord wrote:
> > > > > 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`.
> > > > 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?
> > > 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.
> > Sorry I wasn't aware of the history :-)
> 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.
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/source/Commands/CommandObjectRegister.cpp:229
+if (!DumpRegister(m_exe_ctx, strm, reg_ctx, reg_info,
+  /*print_flags=*/true, type_system))
   strm.Printf("%-12s = error: unavailable\n", reg_info->name);

aprantl wrote:
> 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?
Yes I could do that. Though I think even `Core/DumpRegisterValue.cpp` using it 
isn't good.

Is that right @bulbazord ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/source/Core/DumpRegisterValue.cpp:136-137
+  // See if we have made this type before and can reuse it.
+  CompilerType fields_type = ast->GetTypeForIdentifier(
+  ConstString(register_type_name.c_str()));
+

aprantl wrote:
> DavidSpickett wrote:
> > aprantl wrote:
> > > bulbazord wrote:
> > > > 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`.
> > > 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?
> > 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.
> Sorry I wasn't aware of the history :-)
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Commands/CommandObjectRegister.cpp:229
+if (!DumpRegister(m_exe_ctx, strm, reg_ctx, reg_info,
+  /*print_flags=*/true, type_system))
   strm.Printf("%-12s = error: unavailable\n", reg_info->name);

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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Core/DumpRegisterValue.cpp:136-137
+  // See if we have made this type before and can reuse it.
+  CompilerType fields_type = ast->GetTypeForIdentifier(
+  ConstString(register_type_name.c_str()));
+

DavidSpickett wrote:
> aprantl wrote:
> > bulbazord wrote:
> > > 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`.
> > 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?
> 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.
Sorry I wasn't aware of the history :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/source/Core/DumpRegisterValue.cpp:136-137
+  // See if we have made this type before and can reuse it.
+  CompilerType fields_type = ast->GetTypeForIdentifier(
+  ConstString(register_type_name.c_str()));
+

aprantl wrote:
> bulbazord wrote:
> > 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`.
> 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?
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Core/DumpRegisterValue.cpp:136-137
+  // See if we have made this type before and can reuse it.
+  CompilerType fields_type = ast->GetTypeForIdentifier(
+  ConstString(register_type_name.c_str()));
+

bulbazord wrote:
> 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`.
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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a subscriber: aprantl.
bulbazord added a comment.

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;
>   ...
> })(
> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

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;
  ...
})(https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

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.




Comment at: lldb/include/lldb/Core/DumpRegisterValue.h:12
 
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/lldb-enumerations.h"

JDevlieghere wrote:
> 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?
+1 Let's find a way to avoid this if possible. Perhaps we can use just 
`TypeSystem`?



Comment at: lldb/source/Commands/CommandObjectRegister.cpp:212-213
   } else {
+TypeSystemClangSP type_system =
+ScratchTypeSystemClang::GetForTarget(m_exe_ctx.GetTargetRef());
+assert(type_system);

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



Comment at: lldb/source/Core/DumpRegisterValue.cpp:136-137
+  // See if we have made this type before and can reuse it.
+  CompilerType fields_type = ast->GetTypeForIdentifier(
+  ConstString(register_type_name.c_str()));
+

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`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a subscriber: bulbazord.
JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Core/DumpRegisterValue.h:12
 
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/lldb-enumerations.h"

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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

This gdb_remote_client test looks real nice with this update.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 504583.
DavidSpickett added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

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
@@ -192,6 +192,12 @@
   omit defaulted template parameters. The full template parameter list can still be
   viewed with ``expr --raw-output``/``frame var --raw-output``. (`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,3 +121,19 @@
 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(0x0005ULL, rf3.ReverseFieldOrder(0xA000));
+}
Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -0,0 +1,497 @@
+""" 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.
+  '', # 64 bit x0/r0
+  '', # 32 bit cpsr/fpc
+  '', # 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):
+ 

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 504140.
DavidSpickett added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

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
@@ -192,6 +192,12 @@
   omit defaulted template parameters. The full template parameter list can still be
   viewed with ``expr --raw-output``/``frame var --raw-output``. (`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,3 +121,19 @@
 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(0x0005ULL, rf3.ReverseFieldOrder(0xA000));
+}
Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -0,0 +1,471 @@
+""" 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.
+  '', # 64 bit x0
+  '', # 32 bit cpsr
+  '', # 64 bit pc
+])
+
+class TestXMLRegisterFlags(GDBRemoteTestBase):
+def setup_register_test(self, registers):
+self.server.responder = MultiDocResponder(
+  # This *must* begin with the opening tag, leading whitespace is not allowed.
+  {'target.xml' : dedent("""\
+
+  
+aarch64
+
+  {}
+
+""").format(registers)})
+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_flags_test(self, flags):
+  

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

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

Fixed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

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 = 0x
   = {
   AHP = 0
   DN = 0
  <...>
 }


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 504109.
DavidSpickett added a comment.

- 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

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
@@ -192,6 +192,12 @@
   omit defaulted template parameters. The full template parameter list can still be
   viewed with ``expr --raw-output``/``frame var --raw-output``. (`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,3 +121,19 @@
 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(0x0005ULL, rf3.ReverseFieldOrder(0xA000));
+}
Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -0,0 +1,514 @@
+""" 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("""\
+  
+
+  aarch64
+  
+{}
+  
+  """.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([
+  '', # 64 bit x0
+  '', # 32 bit cpsr
+  '', # 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([
+  '', # 64 bit x0
+  '', # 32 bit cpsr
+  '', # 64 bit pc
+])
+
+class TestAArch64XMLRegisterFlags(GDBRemoteTestBase):
+def setup_register_test(self, registers):
+target = 

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 503379.
DavidSpickett added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

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 `_)
 
+* 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
@@ -123,3 +123,18 @@
 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(0x0005ULL, rf3.ReverseFieldOrder(0xA000));
+}
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("""\
+
+  
+aarch64
+
+  {}
+
+""".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([
+  '', # 64 bit x0
+  '', # 32 bit cpsr
+  '', # 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([
+  '', # 64 bit x0
+  '', # 32 bit cpsr
+  '', # 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 = 

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 503377.
DavidSpickett added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

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 `_)
 
+* 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(0x0005ULL, rf3.ReverseFieldOrder(0xA000));
+}
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("""\
+
+  
+aarch64
+
+  {}
+
+""".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([
+  '', # 64 bit x0
+  '', # 32 bit cpsr
+  '', # 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([
+  '', # 64 bit x0
+  '', # 32 bit cpsr
+  '', # 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():
+  

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

[David Spickett via Phabricator via lldb-commits]

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 `_)
 
+* 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