[Lldb-commits] [lldb] df552ed - Update the CFA to use $sp when $fp is restored on arm64

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2022-05-04T14:54:17-07:00
New Revision: df552edb08c4a1f7600b97444f6315a63c998c59

URL: https://github.com/llvm/llvm-project/commit/df552edb08c4a1f7600b97444f6315a63c998c59
DIFF: https://github.com/llvm/llvm-project/commit/df552edb08c4a1f7600b97444f6315a63c998c59.diff

LOG: Update the CFA to use $sp when $fp is restored on arm64

In UnwindAssemblyInstEmulation we correctly recognize when a LDP
restores the fp & lr in an epilogue, and mark them as having the
caller's contents now, but we don't update the CFA register rule
at that point to indicate that the CFA is now calculated in terms
of $sp.  This doesn't impact the backtrace because the register
contents are all <same> now, but it can confuse the stepper when
the StackID changes mid-epilogue.

Differential Revision: https://reviews.llvm.org/D124492
rdar://92064415

Added: 
    

Modified: 
    lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
    lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
index e9b652bcfb368..ac5e316eecb01 100644
--- a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -614,6 +614,26 @@ bool UnwindAssemblyInstEmulation::WriteRegister(
           m_curr_row->SetRegisterLocationToSame(reg_num,
                                                 false /*must_replace*/);
           m_curr_row_modified = true;
+
+          // FP has been restored to its original value, we are back
+          // to using SP to calculate the CFA.
+          if (m_fp_is_cfa) {
+            m_fp_is_cfa = false;
+            RegisterInfo sp_reg_info;
+            lldb::RegisterKind sp_reg_kind = eRegisterKindGeneric;
+            uint32_t sp_reg_num = LLDB_REGNUM_GENERIC_SP;
+            m_inst_emulator_up->GetRegisterInfo(sp_reg_kind, sp_reg_num,
+                                                sp_reg_info);
+            RegisterValue sp_reg_val;
+            if (GetRegisterValue(sp_reg_info, sp_reg_val)) {
+              m_cfa_reg_info = sp_reg_info;
+              const uint32_t cfa_reg_num =
+                  sp_reg_info.kinds[m_unwind_plan_ptr->GetRegisterKind()];
+              assert(cfa_reg_num != LLDB_INVALID_REGNUM);
+              m_curr_row->GetCFAValue().SetIsRegisterPlusOffset(
+                  cfa_reg_num, m_initial_sp - sp_reg_val.GetAsUInt64());
+            }
+          }
         }
         break;
       case EmulateInstruction::eInfoTypeISA:

diff  --git a/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp b/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
index 6c8bfa1a57f6d..80abeb8fae9e5 100644
--- a/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
+++ b/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
@@ -778,3 +778,65 @@ TEST_F(TestArm64InstEmulation, TestCFARegisterTrackedAcrossJumps) {
   EXPECT_EQ(32, row_sp->GetCFAValue().GetOffset());
 }
 
+TEST_F(TestArm64InstEmulation, TestCFAResetToSP) {
+  ArchSpec arch("arm64-apple-ios15");
+  std::unique_ptr<UnwindAssemblyInstEmulation> engine(
+      static_cast<UnwindAssemblyInstEmulation *>(
+          UnwindAssemblyInstEmulation::CreateInstance(arch)));
+  ASSERT_NE(nullptr, engine);
+
+  UnwindPlan::RowSP row_sp;
+  AddressRange sample_range;
+  UnwindPlan unwind_plan(eRegisterKindLLDB);
+  UnwindPlan::Row::RegisterLocation regloc;
+
+  // The called_from_nodebug() from TestStepNoDebug.py
+  // Most of the previous unit tests have $sp being set as
+  // $fp plus an offset, and the unwinder recognizes that
+  // as a CFA change.  This codegen overwrites $fp and we
+  // need to know that CFA is now in terms of $sp.
+  uint8_t data[] = {
+      // prologue
+      0xff, 0x83, 0x00, 0xd1, //  0: 0xd10083ff sub sp, sp, #0x20
+      0xfd, 0x7b, 0x01, 0xa9, //  4: 0xa9017bfd stp x29, x30, [sp, #0x10]
+      0xfd, 0x43, 0x00, 0x91, //  8: 0x910043fd add x29, sp, #0x10
+
+      // epilogue
+      0xfd, 0x7b, 0x41, 0xa9, // 12: 0xa9417bfd ldp x29, x30, [sp, #0x10]
+      0xff, 0x83, 0x00, 0x91, // 16: 0x910083ff add sp, sp, #0x20
+      0xc0, 0x03, 0x5f, 0xd6, // 20: 0xd65f03c0 ret
+  };
+
+  // UnwindPlan we expect:
+  // row[0]:    0: CFA=sp +0 =>
+  // row[1]:    4: CFA=sp+32 =>
+  // row[2]:    8: CFA=sp+32 => fp=[CFA-16] lr=[CFA-8]
+  // row[3]:   12: CFA=fp+16 => fp=[CFA-16] lr=[CFA-8]
+  // row[4]:   16: CFA=sp+32 => x0= <same> fp= <same> lr= <same>
+  // row[5]:   20: CFA=sp +0 => x0= <same> fp= <same> lr= <same>
+
+  // The specific issue we're testing for is after the
+  // ldp x29, x30, [sp, #0x10]
+  // when $fp and $lr have been restored to the original values,
+  // the CFA is now set in terms of the stack pointer.  If it is
+  // left as being in terms of the frame pointer, $fp now has the
+  // caller function's $fp value and our StackID will be wrong etc.
+
+  sample_range = AddressRange(0x1000, sizeof(data));
+
+  EXPECT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(
+      sample_range, data, sizeof(data), unwind_plan));
+
+  // Confirm CFA before epilogue instructions is in terms of $fp
+  row_sp = unwind_plan.GetRowForFunctionOffset(12);
+  EXPECT_EQ(12ull, row_sp->GetOffset());
+  EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == gpr_fp_arm64);
+  EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
+
+  // Confirm that after restoring $fp to caller's value, CFA is now in
+  // terms of $sp
+  row_sp = unwind_plan.GetRowForFunctionOffset(16);
+  EXPECT_EQ(16ull, row_sp->GetOffset());
+  EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
+  EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
+}