[lldb] Convert registers values into target endian for expressions (llvm#148836)

Relates to llvm#135707

Where it was reported that reading the PC using "register read" had
different results to an expression "$pc".

This was happening because registers are treated in lldb as pure
"values" that don't really have an endian. We have to store them
somewhere on the host of course, so the endian becomes host endian.

When you want to use a register as a value in an expression you're
pretending that it's a variable in memory. In target memory. Therefore
we must convert the register value to that endian before use.

The test I have added is based on the one used for XML register flags.
Where I fake an AArch64 little endian and an s390x big endian target. I
set up the data in such a way the pc value should print the same for
both, either with register read or an expression.

I considered just adding a live process test that checks the two are the
same but with on one doing cross endian testing, I doubt it would have
ever caught this bug.

Simulating this means most of the time, little endian hosts will test
little to little and little to big. In the minority of cases with a big
endian host, they'll check the reverse. Covering all the combinations.

Author: DavidSpickett

lldb/source/Expression/Materializer.cpp

@@ -1377,29 +1377,26 @@ class EntityRegister : public Materializer::Entity {
       return;
     }
 
-    DataExtractor register_data;
-
-    if (!reg_value.GetData(register_data)) {
-      err = Status::FromErrorStringWithFormat(
-          "couldn't get the data for register %s", m_register_info.name);
-      return;
-    }
-
-    if (register_data.GetByteSize() != m_register_info.byte_size) {
+    if (reg_value.GetByteSize() != m_register_info.byte_size) {
       err = Status::FromErrorStringWithFormat(
           "data for register %s had size %llu but we expected %llu",
-          m_register_info.name, (unsigned long long)register_data.GetByteSize(),
+          m_register_info.name, (unsigned long long)reg_value.GetByteSize(),
           (unsigned long long)m_register_info.byte_size);
       return;
     }
 
-    m_register_contents = std::make_shared<DataBufferHeap>(
-        register_data.GetDataStart(), register_data.GetByteSize());
+    lldb_private::DataBufferHeap buf(reg_value.GetByteSize(), 0);
+    reg_value.GetAsMemoryData(m_register_info, buf.GetBytes(),
+                              buf.GetByteSize(), map.GetByteOrder(), err);
+    if (!err.Success())
+      return;
+
+    m_register_contents = std::make_shared<DataBufferHeap>(buf);
 
     Status write_error;
 
-    map.WriteMemory(load_addr, register_data.GetDataStart(),
-                    register_data.GetByteSize(), write_error);
+    map.WriteMemory(load_addr, buf.GetBytes(), reg_value.GetByteSize(),
+                    write_error);
 
     if (!write_error.Success()) {
       err = Status::FromErrorStringWithFormat(

lldb/test/API/commands/expression/TestRegisterExpressionEndian.py

@@ -0,0 +1,86 @@
+""" Check that registers written to memory for expression evaluation are
+    written using the target's endian not the host's.
+"""
+
+from enum import Enum
+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 Endian(Enum):
+    BIG = 0
+    LITTLE = 1
+
+
+class Responder(MockGDBServerResponder):
+    def __init__(self, doc, endian):
+        super().__init__()
+        self.target_xml = doc
+        self.endian = endian
+
+    def qXferRead(self, obj, annex, offset, length):
+        if annex == "target.xml":
+            return self.target_xml, False
+        return (None,)
+
+    def readRegister(self, regnum):
+        return "E01"
+
+    def readRegisters(self):
+        # 64 bit pc value.
+        data = ["00", "00", "00", "00", "00", "00", "12", "34"]
+        if self.endian == Endian.LITTLE:
+            data.reverse()
+        return "".join(data)
+
+
+class TestXMLRegisterFlags(GDBRemoteTestBase):
+    def do_endian_test(self, endian):
+        architecture, pc_reg_name = {
+            Endian.BIG: ("s390x", "pswa"),
+            Endian.LITTLE: ("aarch64", "pc"),
+        }[endian]
+
+        self.server.responder = Responder(
+            dedent(
+                f"""\
+            <?xml version="1.0"?>
+              <target version="1.0">
+                <architecture>{architecture}</architecture>
+                <feature>
+                  <reg name="{pc_reg_name}" bitsize="64"/>
+                </feature>
+            </target>"""
+            ),
+            endian,
+        )
+        target = self.dbg.CreateTarget("")
+        process = self.connect(target)
+        lldbutil.expect_state_changes(
+            self, self.dbg.GetListener(), process, [lldb.eStateStopped]
+        )
+
+        # If expressions convert register values into target endian, the
+        # result of register read and expr should be the same.
+        pc_value = "0x0000000000001234"
+        self.expect(
+            "register read pc",
+            substrs=[pc_value],
+        )
+        self.expect("expr --format hex -- $pc", substrs=[pc_value])
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    def test_little_endian_target(self):
+        self.do_endian_test(Endian.LITTLE)
+
+    @skipIfXmlSupportMissing
+    @skipIfRemote
+    # Unlike AArch64, we do need the backend present for this test to work.
+    @skipIfLLVMTargetMissing("SystemZ")
+    def test_big_endian_target(self):
+        self.do_endian_test(Endian.BIG)