https://github.com/igorkudrin updated https://github.com/llvm/llvm-project/pull/151460
>From e7ae06b1388e172a83fea0340f81d437b9da3c88 Mon Sep 17 00:00:00 2001 From: Igor Kudrin <ikud...@accesssoftek.com> Date: Wed, 30 Jul 2025 23:28:59 -0700 Subject: [PATCH 1/5] [lldb] Fix auto advance PC in 'EmulateInstructionARM64' if PC >= 4G The `EmulateInstructionARM64::EvaluateInstruction()` method used `uint32_t` to store the PC value, causing addresses greater than 2^32 to be truncated. As for now, the issue does not affect the mainline because the method is never called with both `eEmulateInstructionOptionAutoAdvancePC` and `eEmulateInstructionOptionIgnoreConditions` options set. However, it can trigger on a downstream that uses software stepping. --- .../ARM64/EmulateInstructionARM64.cpp | 4 +- .../Instruction/ARM64/TestAArch64Emulator.cpp | 126 +++++++++++++++++- 2 files changed, 127 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp index 29f03fee47b0d..a8901beda3970 100644 --- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp +++ b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp @@ -404,7 +404,7 @@ bool EmulateInstructionARM64::EvaluateInstruction(uint32_t evaluate_options) { if (!success && !m_ignore_conditions) return false; - uint32_t orig_pc_value = 0; + uint64_t orig_pc_value = 0; if (auto_advance_pc) { orig_pc_value = ReadRegisterUnsigned(eRegisterKindLLDB, gpr_pc_arm64, 0, &success); @@ -418,7 +418,7 @@ bool EmulateInstructionARM64::EvaluateInstruction(uint32_t evaluate_options) { return false; if (auto_advance_pc) { - uint32_t new_pc_value = + uint64_t new_pc_value = ReadRegisterUnsigned(eRegisterKindLLDB, gpr_pc_arm64, 0, &success); if (!success) return false; diff --git a/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp b/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp index 4506c200dee3b..c021d994bb062 100644 --- a/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp +++ b/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp @@ -13,15 +13,124 @@ #include "lldb/Core/Disassembler.h" #include "lldb/Target/ExecutionContext.h" #include "lldb/Utility/ArchSpec.h" +#include "lldb/Utility/RegisterValue.h" #include "Plugins/Instruction/ARM64/EmulateInstructionARM64.h" +#include "Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h" +#include "Plugins/Process/Utility/lldb-arm64-register-enums.h" using namespace lldb; using namespace lldb_private; struct Arch64EmulatorTester : public EmulateInstructionARM64 { + RegisterInfoPOSIX_arm64::GPR gpr; + uint8_t memory[64] = {0}; + uint64_t memory_offset = 0; + Arch64EmulatorTester() - : EmulateInstructionARM64(ArchSpec("arm64-apple-ios")) {} + : EmulateInstructionARM64(ArchSpec("arm64-apple-ios")) { + memset(&gpr, 0, sizeof(gpr)); + EmulateInstruction::SetCallbacks(ReadMemoryCallback, WriteMemoryCallback, + ReadRegisterCallback, + WriteRegisterCallback); + } + + static bool ReadRegisterCallback(EmulateInstruction *instruction, void *baton, + const RegisterInfo *reg_info, + RegisterValue ®_value) { + auto *tester = static_cast<Arch64EmulatorTester *>(instruction); + uint32_t reg = reg_info->kinds[eRegisterKindLLDB]; + if (reg >= gpr_x1_arm64 && reg <= gpr_x28_arm64) { + reg_value.SetUInt64(tester->gpr.x[reg - gpr_x0_arm64]); + return true; + } + if (reg >= gpr_w1_arm64 && reg <= gpr_w28_arm64) { + reg_value.SetUInt32(tester->gpr.x[reg - gpr_w0_arm64]); + return true; + } + switch (reg) { + case gpr_x0_arm64: + reg_value.SetUInt64(0); + return true; + case gpr_w0_arm64: + reg_value.SetUInt32(0); + return true; + case gpr_fp_arm64: + reg_value.SetUInt64(tester->gpr.fp); + return true; + case gpr_lr_arm64: + reg_value.SetUInt64(tester->gpr.lr); + return true; + case gpr_sp_arm64: + reg_value.SetUInt64(tester->gpr.sp); + return true; + case gpr_pc_arm64: + reg_value.SetUInt64(tester->gpr.pc); + return true; + case gpr_cpsr_arm64: + reg_value.SetUInt64(tester->gpr.cpsr); + return true; + default: + return false; + } + } + + static bool WriteRegisterCallback(EmulateInstruction *instruction, + void *baton, const Context &context, + const RegisterInfo *reg_info, + const RegisterValue ®_value) { + auto *tester = static_cast<Arch64EmulatorTester *>(instruction); + uint32_t reg = reg_info->kinds[eRegisterKindLLDB]; + if (reg >= gpr_x1_arm64 && reg <= gpr_x28_arm64) { + tester->gpr.x[reg - gpr_x0_arm64] = reg_value.GetAsUInt64(); + return true; + } + if (reg >= gpr_w1_arm64 && reg <= gpr_w28_arm64) { + tester->gpr.x[reg - gpr_w0_arm64] = reg_value.GetAsUInt32(); + return true; + } + switch (reg) { + case gpr_fp_arm64: + tester->gpr.fp = reg_value.GetAsUInt64(); + return true; + case gpr_lr_arm64: + tester->gpr.lr = reg_value.GetAsUInt64(); + return true; + case gpr_sp_arm64: + tester->gpr.sp = reg_value.GetAsUInt64(); + return true; + case gpr_pc_arm64: + tester->gpr.pc = reg_value.GetAsUInt64(); + return true; + case gpr_cpsr_arm64: + tester->gpr.cpsr = reg_value.GetAsUInt64(); + return true; + default: + return false; + } + } + + static size_t ReadMemoryCallback(EmulateInstruction *instruction, void *baton, + const Context &context, addr_t addr, + void *dst, size_t length) { + auto *tester = static_cast<Arch64EmulatorTester *>(instruction); + assert(addr >= tester->memory_offset); + assert(addr - tester->memory_offset + length <= sizeof(tester->memory)); + if (addr >= tester->memory_offset && + addr - tester->memory_offset + length <= sizeof(tester->memory)) { + memcpy(dst, tester->memory + addr - tester->memory_offset, length); + return length; + } + return 0; + }; + + static size_t WriteMemoryCallback(EmulateInstruction *instruction, + void *baton, const Context &context, + addr_t addr, const void *dst, + size_t length) { + // TODO: implement when required + return 0; + }; static uint64_t AddWithCarry(uint32_t N, uint64_t x, uint64_t y, bool carry_in, EmulateInstructionARM64::ProcState &proc_state) { @@ -60,3 +169,18 @@ TEST_F(TestAArch64Emulator, TestOverflow) { ASSERT_EQ(pstate.V, 1ULL); ASSERT_EQ(pstate.C, 0ULL); } + +TEST_F(TestAArch64Emulator, TestAutoAdvancePC) { + Arch64EmulatorTester emu; + emu.memory_offset = 0x1234567800; + emu.gpr.pc = 0x1234567800; + emu.gpr.x[8] = 0x1234567820; + memcpy(emu.memory, "\x08\x01\x40\xb9", 4); // ldr w8, [x8] + memcpy(emu.memory + 0x20, "\x11\x22\x33\x44", 4); // 0x44332211 + ASSERT_TRUE(emu.ReadInstruction()); + ASSERT_TRUE( + emu.EvaluateInstruction(eEmulateInstructionOptionAutoAdvancePC | + eEmulateInstructionOptionIgnoreConditions)); + ASSERT_EQ(emu.gpr.pc, 0x1234567804); + ASSERT_EQ(emu.gpr.x[8], 0x44332211); +} >From 39092fde9f6e642d8ad36767f8fbdcf2c034643e Mon Sep 17 00:00:00 2001 From: Igor Kudrin <ikud...@accesssoftek.com> Date: Wed, 30 Jul 2025 23:41:19 -0700 Subject: [PATCH 2/5] fixup! formatting --- lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp b/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp index c021d994bb062..4837bd0e3620b 100644 --- a/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp +++ b/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp @@ -175,7 +175,7 @@ TEST_F(TestAArch64Emulator, TestAutoAdvancePC) { emu.memory_offset = 0x1234567800; emu.gpr.pc = 0x1234567800; emu.gpr.x[8] = 0x1234567820; - memcpy(emu.memory, "\x08\x01\x40\xb9", 4); // ldr w8, [x8] + memcpy(emu.memory, "\x08\x01\x40\xb9", 4); // ldr w8, [x8] memcpy(emu.memory + 0x20, "\x11\x22\x33\x44", 4); // 0x44332211 ASSERT_TRUE(emu.ReadInstruction()); ASSERT_TRUE( >From 536b1835f55cbd2527c3da9f0d29a1ca3a195731 Mon Sep 17 00:00:00 2001 From: Igor Kudrin <ikud...@accesssoftek.com> Date: Thu, 31 Jul 2025 09:30:15 -0700 Subject: [PATCH 3/5] fixup! x0 is a regular register --- .../Instruction/ARM64/TestAArch64Emulator.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp b/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp index 4837bd0e3620b..a31a5b01c7ece 100644 --- a/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp +++ b/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp @@ -40,21 +40,15 @@ struct Arch64EmulatorTester : public EmulateInstructionARM64 { RegisterValue ®_value) { auto *tester = static_cast<Arch64EmulatorTester *>(instruction); uint32_t reg = reg_info->kinds[eRegisterKindLLDB]; - if (reg >= gpr_x1_arm64 && reg <= gpr_x28_arm64) { + if (reg >= gpr_x0_arm64 && reg <= gpr_x28_arm64) { reg_value.SetUInt64(tester->gpr.x[reg - gpr_x0_arm64]); return true; } - if (reg >= gpr_w1_arm64 && reg <= gpr_w28_arm64) { + if (reg >= gpr_w0_arm64 && reg <= gpr_w28_arm64) { reg_value.SetUInt32(tester->gpr.x[reg - gpr_w0_arm64]); return true; } switch (reg) { - case gpr_x0_arm64: - reg_value.SetUInt64(0); - return true; - case gpr_w0_arm64: - reg_value.SetUInt32(0); - return true; case gpr_fp_arm64: reg_value.SetUInt64(tester->gpr.fp); return true; @@ -81,11 +75,11 @@ struct Arch64EmulatorTester : public EmulateInstructionARM64 { const RegisterValue ®_value) { auto *tester = static_cast<Arch64EmulatorTester *>(instruction); uint32_t reg = reg_info->kinds[eRegisterKindLLDB]; - if (reg >= gpr_x1_arm64 && reg <= gpr_x28_arm64) { + if (reg >= gpr_x0_arm64 && reg <= gpr_x28_arm64) { tester->gpr.x[reg - gpr_x0_arm64] = reg_value.GetAsUInt64(); return true; } - if (reg >= gpr_w1_arm64 && reg <= gpr_w28_arm64) { + if (reg >= gpr_w0_arm64 && reg <= gpr_w28_arm64) { tester->gpr.x[reg - gpr_w0_arm64] = reg_value.GetAsUInt32(); return true; } >From c04cfa3a163c541d2bc3d9aa784d6e7c169e8d2c Mon Sep 17 00:00:00 2001 From: Igor Kudrin <ikud...@accesssoftek.com> Date: Thu, 31 Jul 2025 09:36:38 -0700 Subject: [PATCH 4/5] fixup! add an assert() to WriteMemoryCallback() --- lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp b/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp index a31a5b01c7ece..51399392d4e89 100644 --- a/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp +++ b/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp @@ -123,6 +123,7 @@ struct Arch64EmulatorTester : public EmulateInstructionARM64 { addr_t addr, const void *dst, size_t length) { // TODO: implement when required + assert(false); return 0; }; >From 21abebcbad833b89c2de62023ceaf2a5adbeed4b Mon Sep 17 00:00:00 2001 From: Igor Kudrin <ikud...@accesssoftek.com> Date: Thu, 31 Jul 2025 09:38:15 -0700 Subject: [PATCH 5/5] fixup! make PC even bigger --- lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp b/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp index 51399392d4e89..4aaaaab1fbb6e 100644 --- a/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp +++ b/lldb/unittests/Instruction/ARM64/TestAArch64Emulator.cpp @@ -167,15 +167,15 @@ TEST_F(TestAArch64Emulator, TestOverflow) { TEST_F(TestAArch64Emulator, TestAutoAdvancePC) { Arch64EmulatorTester emu; - emu.memory_offset = 0x1234567800; - emu.gpr.pc = 0x1234567800; - emu.gpr.x[8] = 0x1234567820; + emu.memory_offset = 0x123456789abcde00; + emu.gpr.pc = 0x123456789abcde00; + emu.gpr.x[8] = 0x123456789abcde20; memcpy(emu.memory, "\x08\x01\x40\xb9", 4); // ldr w8, [x8] memcpy(emu.memory + 0x20, "\x11\x22\x33\x44", 4); // 0x44332211 ASSERT_TRUE(emu.ReadInstruction()); ASSERT_TRUE( emu.EvaluateInstruction(eEmulateInstructionOptionAutoAdvancePC | eEmulateInstructionOptionIgnoreConditions)); - ASSERT_EQ(emu.gpr.pc, 0x1234567804); + ASSERT_EQ(emu.gpr.pc, 0x123456789abcde04); ASSERT_EQ(emu.gpr.x[8], 0x44332211); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits