-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lldb/aarch64] Add STR/LDR instructions for FP register to Emulator #168187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb/aarch64] Add STR/LDR instructions for FP register to Emulator #168187
Conversation
A function prologue can begin with a pre-index STR instruction for a floating-point register. To construct an unwind plan from assembly correctly, the instruction emulator must support such instructions.
|
@llvm/pr-subscribers-lldb Author: Igor Kudrin (igorkudrin) ChangesA function prologue can begin with a pre-index STR instruction for a floating-point register. To construct an unwind plan from assembly correctly, the instruction emulator must support such instructions. Full diff: https://github.com/llvm/llvm-project/pull/168187.diff 2 Files Affected:
diff --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
index a8901beda3970..7d3e72ccdc7dc 100644
--- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
+++ b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
@@ -346,6 +346,16 @@ EmulateInstructionARM64::GetOpcodeForInstruction(const uint32_t opcode) {
&EmulateInstructionARM64::EmulateLDRSTRImm<AddrMode_OFF>,
"LDR <Xt>, [<Xn|SP>{, #<pimm>}]"},
+ {0x3f200c00, 0x3c000400, No_VFP,
+ &EmulateInstructionARM64::EmulateLDRSTRImm<AddrMode_POST>,
+ "LDR|STR <Bt|Ht|St|Dt|Qt>, [<Xn|SP>], #<simm>"},
+ {0x3f200c00, 0x3c000c00, No_VFP,
+ &EmulateInstructionARM64::EmulateLDRSTRImm<AddrMode_PRE>,
+ "LDR|STR <Bt|Ht|St|Dt|Qt>, [<Xn|SP>, #<simm>]!"},
+ {0x3f000000, 0x3d000000, No_VFP,
+ &EmulateInstructionARM64::EmulateLDRSTRImm<AddrMode_OFF>,
+ "LDR|STR <Bt|Ht|St|Dt|Qt>, [<Xn|SP>{, #<pimm>}]"},
+
{0xfc000000, 0x14000000, No_VFP, &EmulateInstructionARM64::EmulateB,
"B <label>"},
{0xff000010, 0x54000000, No_VFP, &EmulateInstructionARM64::EmulateBcond,
@@ -930,9 +940,27 @@ template <EmulateInstructionARM64::AddrMode a_mode>
bool EmulateInstructionARM64::EmulateLDRSTRImm(const uint32_t opcode) {
uint32_t size = Bits32(opcode, 31, 30);
uint32_t opc = Bits32(opcode, 23, 22);
+ uint32_t vr = Bit32(opcode, 26);
uint32_t n = Bits32(opcode, 9, 5);
uint32_t t = Bits32(opcode, 4, 0);
+ MemOp memop;
+ if (vr) {
+ if (Bit32(opc, 1) == 1)
+ size += 4;
+ if (size > 4)
+ return false;
+ memop = Bit32(opc, 0) == 1 ? MemOp_LOAD : MemOp_STORE;
+ } else {
+ if (Bit32(opc, 1) == 0) {
+ memop = Bit32(opc, 0) == 1 ? MemOp_LOAD : MemOp_STORE;
+ } else {
+ memop = MemOp_LOAD;
+ if (size == 2 && Bit32(opc, 0) == 1)
+ return false;
+ }
+ }
+
bool wback;
bool postindex;
uint64_t offset;
@@ -955,16 +983,6 @@ bool EmulateInstructionARM64::EmulateLDRSTRImm(const uint32_t opcode) {
break;
}
- MemOp memop;
-
- if (Bit32(opc, 1) == 0) {
- memop = Bit32(opc, 0) == 1 ? MemOp_LOAD : MemOp_STORE;
- } else {
- memop = MemOp_LOAD;
- if (size == 2 && Bit32(opc, 0) == 1)
- return false;
- }
-
Status error;
bool success = false;
uint64_t address;
@@ -989,7 +1007,8 @@ bool EmulateInstructionARM64::EmulateLDRSTRImm(const uint32_t opcode) {
return false;
std::optional<RegisterInfo> reg_info_Rt =
- GetRegisterInfo(eRegisterKindLLDB, gpr_x0_arm64 + t);
+ vr ? GetRegisterInfo(eRegisterKindLLDB, fpu_d0_arm64 + t)
+ : GetRegisterInfo(eRegisterKindLLDB, gpr_x0_arm64 + t);
if (!reg_info_Rt)
return false;
diff --git a/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp b/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
index eaf23fd72d6d1..5d15e9621002b 100644
--- a/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
+++ b/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
@@ -856,3 +856,109 @@ TEST_F(TestArm64InstEmulation, TestCFAResetToSP) {
EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
}
+
+TEST_F(TestArm64InstEmulation, TestPrologueStartsWithStrD8) {
+ ArchSpec arch("aarch64");
+ std::unique_ptr<UnwindAssemblyInstEmulation> engine(
+ static_cast<UnwindAssemblyInstEmulation *>(
+ UnwindAssemblyInstEmulation::CreateInstance(arch)));
+ ASSERT_NE(nullptr, engine);
+
+ const UnwindPlan::Row *row;
+ AddressRange sample_range;
+ UnwindPlan unwind_plan(eRegisterKindLLDB);
+ UnwindPlan::Row::AbstractRegisterLocation regloc;
+
+ // The sample function is built with 'clang --target aarch64 -O1':
+ //
+ // int bar(float x);
+ // int foo(float x) {
+ // return bar(x) + bar(x);
+ // }
+ //
+ // The function uses one floating point register and spills it with
+ // 'str d8, [sp, #-0x20]!'.
+
+ uint8_t data[] = {
+ // prologue
+ 0xe8, 0x0f, 0x1e, 0xfc, // 0: fc1e0fe8 str d8, [sp, #-0x20]!
+ 0xfd, 0xfb, 0x00, 0xa9, // 4: a900fbfd stp x29, x30, [sp, #0x8]
+ 0xf3, 0x0f, 0x00, 0xf9, // 8: f9000ff3 str x19, [sp, #0x18]
+ 0xfd, 0x23, 0x00, 0x91, // 12: 910023fd add x29, sp, #0x8
+
+ // epilogue
+ 0xfd, 0xfb, 0x40, 0xa9, // 16: a940fbfd ldp x29, x30, [sp, #0x8]
+ 0xf3, 0x0f, 0x40, 0xf9, // 20: f9400ff3 ldr x19, [sp, #0x18]
+ 0xe8, 0x07, 0x42, 0xfc, // 24: fc4207e8 ldr d8, [sp], #0x20
+ 0xc0, 0x03, 0x5f, 0xd6, // 28: d65f03c0 ret
+ };
+
+ // UnwindPlan we expect:
+ // 0: CFA=sp +0 =>
+ // 4: CFA=sp+32 => d8=[CFA-32]
+ // 8: CFA=sp+32 => fp=[CFA-24] lr=[CFA-16] d8=[CFA-32]
+ // 12: CFA=sp+32 => x19=[CFA-8] fp=[CFA-24] lr=[CFA-16] d8=[CFA-32]
+ // 16: CFA=fp+24 => x19=[CFA-8] fp=[CFA-24] lr=[CFA-16] d8=[CFA-32]
+ // 20: CFA=sp+32 => x19=[CFA-8] fp=<same> lr=<same> d8=[CFA-32]
+ // 24: CFA=sp+32 => x19=<same> fp=<same> lr=<same> d8=[CFA-32]
+ // 28: CFA=sp +0 => x19=<same> fp=<same> lr=<same> d8=<same>
+
+ sample_range = AddressRange(0x1000, sizeof(data));
+
+ EXPECT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(
+ sample_range, data, sizeof(data), unwind_plan));
+
+ // 4: CFA=sp+32 => d8=[CFA-32]
+ row = unwind_plan.GetRowForFunctionOffset(4);
+ EXPECT_EQ(4, row->GetOffset());
+ EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
+ EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
+ EXPECT_EQ(32, row->GetCFAValue().GetOffset());
+
+ EXPECT_TRUE(row->GetRegisterInfo(fpu_d8_arm64, regloc));
+ EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
+ EXPECT_EQ(-32, regloc.GetOffset());
+
+ // 16: CFA=fp+24 => x19=[CFA-8] fp=[CFA-24] lr=[CFA-16] d8=[CFA-32]
+ row = unwind_plan.GetRowForFunctionOffset(16);
+ EXPECT_EQ(16, row->GetOffset());
+ EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_fp_arm64);
+ EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
+ EXPECT_EQ(24, row->GetCFAValue().GetOffset());
+
+ EXPECT_TRUE(row->GetRegisterInfo(gpr_x19_arm64, regloc));
+ EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
+ EXPECT_EQ(-8, regloc.GetOffset());
+
+ EXPECT_TRUE(row->GetRegisterInfo(gpr_fp_arm64, regloc));
+ EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
+ EXPECT_EQ(-24, regloc.GetOffset());
+
+ EXPECT_TRUE(row->GetRegisterInfo(gpr_lr_arm64, regloc));
+ EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
+ EXPECT_EQ(-16, regloc.GetOffset());
+
+ EXPECT_TRUE(row->GetRegisterInfo(fpu_d8_arm64, regloc));
+ EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
+ EXPECT_EQ(-32, regloc.GetOffset());
+
+ // 28: CFA=sp +0 => x19=<same> fp=<same> lr=<same> d8=<same>
+ row = unwind_plan.GetRowForFunctionOffset(28);
+ EXPECT_EQ(28, row->GetOffset());
+ EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
+ EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
+ EXPECT_EQ(0, row->GetCFAValue().GetOffset());
+
+ if (row->GetRegisterInfo(gpr_x19_arm64, regloc)) {
+ EXPECT_TRUE(regloc.IsSame());
+ }
+ if (row->GetRegisterInfo(gpr_fp_arm64, regloc)) {
+ EXPECT_TRUE(regloc.IsSame());
+ }
+ if (row->GetRegisterInfo(gpr_lr_arm64, regloc)) {
+ EXPECT_TRUE(regloc.IsSame());
+ }
+ if (row->GetRegisterInfo(fpu_d8_arm64, regloc)) {
+ EXPECT_TRUE(regloc.IsSame());
+ }
+}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
The code formatting suggestion looks strange because it degrades readability. |
You can put a |
DavidSpickett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM just needs a comment for a magic number.
| MemOp memop; | ||
| if (vr) { | ||
| if (Bit32(opc, 1) == 1) | ||
| size += 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took me a while to realise what this was doing (not because you were being cryptic, because instruction encodings aren't always straightforward). I'm looking at one of the instructions, "C7.2.208 LDR (immediate, SIMD&FP)" in the Arm manual.
The sizes when opc is 01 are - 0 =1 byte, 1 = 2 bytes, 2 = 4 bytes, 3 = 8 bytes. Size 0 and opc 11 means 128-bit, but we can think of that as size being 4 (though it is only a 2 bit field in reality).
Which means register size (in bits) = 2 ^ (3+ size). Makes sense since the minimum is 8 bit.
Please add a comment to explain the 4. Maybe:
// When opc == 01, size can be 0 (1 byte) to 3 (8 bytes). 128-bit registers are encoded using opc == 11, and size 0, but we can handle this as size 4, to continue the pattern:
// register size (bits) = 2 ^ (3 + size)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the code and added comments. Are they clearer now?
Thanks! It appears that after #146256, |
🐧 Linux x64 Test Results
|
A function prologue can begin with a pre-index STR instruction for a floating-point register. To construct an unwind plan from assembly correctly, the instruction emulator must support such instructions.