https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/148836
Relates to https://github.com/llvm/llvm-project/issues/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. >From fc80b9c5f559eed17701ab90f1a538796993e365 Mon Sep 17 00:00:00 2001 From: David Spickett <david.spick...@linaro.org> Date: Mon, 14 Jul 2025 14:24:14 +0000 Subject: [PATCH] [lldb] Convert registers values into target endian for expressions Fixes https://github.com/llvm/llvm-project/issues/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. --- lldb/source/Expression/Materializer.cpp | 25 +++--- .../TestRegisterExpressionEndian.py | 86 +++++++++++++++++++ 2 files changed, 97 insertions(+), 14 deletions(-) create mode 100644 lldb/test/API/commands/expression/TestRegisterExpressionEndian.py diff --git a/lldb/source/Expression/Materializer.cpp b/lldb/source/Expression/Materializer.cpp index 17ea1596806d0..8fc3df1360824 100644 --- a/lldb/source/Expression/Materializer.cpp +++ b/lldb/source/Expression/Materializer.cpp @@ -1376,29 +1376,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( diff --git a/lldb/test/API/commands/expression/TestRegisterExpressionEndian.py b/lldb/test/API/commands/expression/TestRegisterExpressionEndian.py new file mode 100644 index 0000000000000..66e38df3a9696 --- /dev/null +++ b/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) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
