Skip to content

Commit b3396c5

Browse files
[lldb] Account for registers being host endian when casting values (llvm#150011)
Fixes llvm#135707 Follow up to llvm#148836 which fixed some of this issue but not all of it. Our Value/ValueObject system does not store the endian directly in the values. Instead it assumes that the endian of the result of a cast can be assumed to be the target's endian, or the host but only as a fallback. It assumes the place it is copying from is also that endian. This breaks down when you have register values. These are always host endian and continue to be when cast. Casting them to big endian when on a little endian host breaks certain calls like GetValueAsUnsigned. To fix this, check the context of the value. If it has a register context, always treat it as host endian and make the result host endian. I had an alternative where I passed an "is_register" flag into all calls to this, but it felt like a layering violation and changed many more lines. This solution isn't much more robust, but it works for all the test cases I know of. Perhaps you can create a register value without a RegisterInfo backing it, but I don't know of a way myself. For testing, I had to add a minimal program file for each arch so that there is a type system to support the casting. This is generated from YAML since we only need the machine and endian to be set.
1 parent 4d4966d commit b3396c5

File tree

2 files changed

+53
-9
lines changed

2 files changed

+53
-9
lines changed

lldb/source/Core/Value.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -488,9 +488,11 @@ Status Value::GetValueAsData(ExecutionContext *exe_ctx, DataExtractor &data,
488488
address = m_value.ULongLong(LLDB_INVALID_ADDRESS);
489489
address_type = eAddressTypeHost;
490490
if (exe_ctx) {
491-
Target *target = exe_ctx->GetTargetPtr();
492-
if (target) {
493-
data.SetByteOrder(target->GetArchitecture().GetByteOrder());
491+
if (Target *target = exe_ctx->GetTargetPtr()) {
492+
// Registers are always stored in host endian.
493+
data.SetByteOrder(m_context_type == ContextType::RegisterInfo
494+
? endian::InlHostByteOrder()
495+
: target->GetArchitecture().GetByteOrder());
494496
data.SetAddressByteSize(target->GetArchitecture().GetAddressByteSize());
495497
break;
496498
}

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

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,15 @@ def readRegisters(self):
4040

4141
class TestXMLRegisterFlags(GDBRemoteTestBase):
4242
def do_endian_test(self, endian):
43-
architecture, pc_reg_name = {
44-
Endian.BIG: ("s390x", "pswa"),
45-
Endian.LITTLE: ("aarch64", "pc"),
43+
architecture, pc_reg_name, yaml_file, data, machine = {
44+
Endian.BIG: ("s390x", "pswa", "s390x.yaml", "ELFDATA2MSB", "EM_S390"),
45+
Endian.LITTLE: (
46+
"aarch64",
47+
"pc",
48+
"aarch64.yaml",
49+
"ELFDATA2LSB",
50+
"EM_AARCH64",
51+
),
4652
}[endian]
4753

4854
self.server.responder = Responder(
@@ -58,29 +64,65 @@ def do_endian_test(self, endian):
5864
),
5965
endian,
6066
)
61-
target = self.dbg.CreateTarget("")
67+
68+
# We need to have a program file, so that we have a full type system,
69+
# so that we can do the casts later.
70+
obj_path = self.getBuildArtifact("main.o")
71+
yaml_path = self.getBuildArtifact(yaml_file)
72+
with open(yaml_path, "w") as f:
73+
f.write(
74+
dedent(
75+
f"""\
76+
--- !ELF
77+
FileHeader:
78+
Class: ELFCLASS64
79+
Data: {data}
80+
Type: ET_REL
81+
Machine: {machine}
82+
...
83+
"""
84+
)
85+
)
86+
self.yaml2obj(yaml_path, obj_path)
87+
target = self.dbg.CreateTarget(obj_path)
88+
6289
process = self.connect(target)
6390
lldbutil.expect_state_changes(
6491
self, self.dbg.GetListener(), process, [lldb.eStateStopped]
6592
)
6693

6794
# If expressions convert register values into target endian, the
68-
# result of register read and expr should be the same.
95+
# result of register read, expr and casts should be the same.
6996
pc_value = "0x0000000000001234"
7097
self.expect(
7198
"register read pc",
7299
substrs=[pc_value],
73100
)
74101
self.expect("expr --format hex -- $pc", substrs=[pc_value])
75102

103+
pc = (
104+
process.thread[0]
105+
.frame[0]
106+
.GetRegisters()
107+
.GetValueAtIndex(0)
108+
.GetChildMemberWithName("pc")
109+
)
110+
ull = target.FindTypes("unsigned long long").GetTypeAtIndex(0)
111+
pc_ull = pc.Cast(ull)
112+
113+
self.assertEqual(pc.GetValue(), pc_ull.GetValue())
114+
self.assertEqual(pc.GetValueAsAddress(), pc_ull.GetValueAsAddress())
115+
self.assertEqual(pc.GetValueAsSigned(), pc_ull.GetValueAsSigned())
116+
self.assertEqual(pc.GetValueAsUnsigned(), pc_ull.GetValueAsUnsigned())
117+
76118
@skipIfXmlSupportMissing
77119
@skipIfRemote
120+
@skipIfLLVMTargetMissing("AArch64")
78121
def test_little_endian_target(self):
79122
self.do_endian_test(Endian.LITTLE)
80123

81124
@skipIfXmlSupportMissing
82125
@skipIfRemote
83-
# Unlike AArch64, we do need the backend present for this test to work.
84126
@skipIfLLVMTargetMissing("SystemZ")
85127
def test_big_endian_target(self):
86128
self.do_endian_test(Endian.BIG)

0 commit comments

Comments
 (0)