[Lldb-commits] [lldb] [lldb] Slide eh_frame unwind plan if it doesn't begin at function boundary (PR #135333)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Apr 11 02:00:39 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
<details>
<summary>Changes</summary>
This is mainly useful for discontinuous functions because individual parts of the function will have separate FDE entries, which can begin many megabytes from the start of the function. However, I'm separating it out, because it turns out we already have a test case for the situation where the FDE does not begin exactly at the function boundary.
The test works mostly by accident because the FDE starts only one byte after the beginning of the function so it doesn't really matter whether one looks up the unwind row using the function or fde offset. In this patch, I beef up the test to catch this problem more reliably.
To make this work I've also needed to change a couple of places which that an unwind plan always has a row at offset zero.
---
Full diff: https://github.com/llvm/llvm-project/pull/135333.diff
6 Files Affected:
- (modified) lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp (+1-1)
- (modified) lldb/source/Symbol/DWARFCallFrameInfo.cpp (+5-1)
- (modified) lldb/source/Symbol/UnwindPlan.cpp (+1-1)
- (modified) lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s (+10-2)
- (modified) lldb/test/Shell/Unwind/eh-frame-small-fde.test (+4-3)
- (modified) lldb/unittests/Symbol/UnwindPlanTest.cpp (+6-1)
``````````diff
diff --git a/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp b/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp
index 5f0a863e936d4..f5279758a147c 100644
--- a/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp
+++ b/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp
@@ -73,7 +73,7 @@ bool UnwindAssembly_x86::AugmentUnwindPlanFromCallSite(
int wordsize = 8;
ProcessSP process_sp(thread.GetProcess());
- if (process_sp.get() == nullptr)
+ if (!process_sp || !first_row || !last_row)
return false;
wordsize = process_sp->GetTarget().GetArchitecture().GetAddressByteSize();
diff --git a/lldb/source/Symbol/DWARFCallFrameInfo.cpp b/lldb/source/Symbol/DWARFCallFrameInfo.cpp
index fb57c61d413aa..a763acb1fdf9e 100644
--- a/lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ b/lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -190,8 +190,12 @@ bool DWARFCallFrameInfo::GetUnwindPlan(const AddressRange &range,
unwind_plan.SetUnwindPlanForSignalTrap(fde->for_signal_trap ? eLazyBoolYes
: eLazyBoolNo);
unwind_plan.SetReturnAddressRegister(fde->return_addr_reg_num);
- for (UnwindPlan::Row &row : fde->rows)
+ int64_t slide =
+ fde->range.GetBaseAddress().GetFileAddress() - addr.GetFileAddress();
+ for (UnwindPlan::Row &row : fde->rows) {
+ row.SlideOffset(slide);
unwind_plan.AppendRow(std::move(row));
+ }
return true;
}
diff --git a/lldb/source/Symbol/UnwindPlan.cpp b/lldb/source/Symbol/UnwindPlan.cpp
index 94c23137e8f12..b1a96b5e26840 100644
--- a/lldb/source/Symbol/UnwindPlan.cpp
+++ b/lldb/source/Symbol/UnwindPlan.cpp
@@ -474,7 +474,7 @@ bool UnwindPlan::PlanValidAtAddress(Address addr) const {
// If the 0th Row of unwind instructions is missing, or if it doesn't provide
// a register to use to find the Canonical Frame Address, this is not a valid
// UnwindPlan.
- const Row *row0 = GetRowForFunctionOffset(0);
+ const Row *row0 = GetRowAtIndex(0);
if (!row0 ||
row0->GetCFAValue().GetValueType() == Row::FAValue::unspecified) {
Log *log = GetLog(LLDBLog::Unwind);
diff --git a/lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s b/lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s
index b08436af3f2ea..29decefad5e51 100644
--- a/lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s
+++ b/lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s
@@ -10,12 +10,20 @@ bar:
.type foo, @function
foo:
- nop # Make the FDE entry start one byte later than the actual function.
+ # Make the FDE entry start 16 bytes later than the actual function. The
+ # size is chosen such that it is larger than the size of the FDE entry.
+ # This allows us to test that we're using the correct offset for
+ # unwinding (we'll stop 21 bytes into the function, but only 5 bytes
+ # into the FDE).
+ .nops 16
.cfi_startproc
.cfi_register %rip, %r13
call bar
addl $1, %eax
- jmp *%r13 # Return
+ movq %r13, %r14
+ .cfi_register %rip, %r14
+ movq $0, %r13
+ jmp *%r14 # Return
.cfi_endproc
.size foo, .-foo
diff --git a/lldb/test/Shell/Unwind/eh-frame-small-fde.test b/lldb/test/Shell/Unwind/eh-frame-small-fde.test
index 0ece6c2a12a3e..d86d41f73f1c1 100644
--- a/lldb/test/Shell/Unwind/eh-frame-small-fde.test
+++ b/lldb/test/Shell/Unwind/eh-frame-small-fde.test
@@ -14,9 +14,10 @@ process launch
thread backtrace
# CHECK: frame #0: {{.*}}`bar
-# CHECK: frame #1: {{.*}}`foo + 6
+# CHECK: frame #1: {{.*}}`foo + 21
# CHECK: frame #2: {{.*}}`main + 20
target modules show-unwind -n foo
-# CHECK: eh_frame UnwindPlan:
-# CHECK: row[0]: 0: CFA=rsp +8 => rip=r13
+# CHECK: eh_frame UnwindPlan:
+# CHECK: row[0]: 16: CFA=rsp +8 => rip=r13
+# CHECK-NEXT: row[1]: 27: CFA=rsp +8 => rip=r14
diff --git a/lldb/unittests/Symbol/UnwindPlanTest.cpp b/lldb/unittests/Symbol/UnwindPlanTest.cpp
index 08aa5b2dd84bb..2d7ef43f83c19 100644
--- a/lldb/unittests/Symbol/UnwindPlanTest.cpp
+++ b/lldb/unittests/Symbol/UnwindPlanTest.cpp
@@ -62,15 +62,20 @@ TEST(UnwindPlan, PlanValidAtAddress) {
UnwindPlan::Row row2 = make_simple_row(10, 47);
UnwindPlan plan(eRegisterKindGeneric);
+ // When valid address range is not set, plans are valid as long as they have a
+ // row that sets the CFA.
EXPECT_FALSE(plan.PlanValidAtAddress(Address(0)));
+ EXPECT_FALSE(plan.PlanValidAtAddress(Address(10)));
plan.InsertRow(row2);
- EXPECT_FALSE(plan.PlanValidAtAddress(Address(0)));
+ EXPECT_TRUE(plan.PlanValidAtAddress(Address(0)));
+ EXPECT_TRUE(plan.PlanValidAtAddress(Address(10)));
plan.InsertRow(row1);
EXPECT_TRUE(plan.PlanValidAtAddress(Address(0)));
EXPECT_TRUE(plan.PlanValidAtAddress(Address(10)));
+ // With an address range, they're only valid within that range.
plan.SetPlanValidAddressRanges({AddressRange(0, 5), AddressRange(15, 5)});
EXPECT_TRUE(plan.PlanValidAtAddress(Address(0)));
EXPECT_FALSE(plan.PlanValidAtAddress(Address(5)));
``````````
</details>
https://github.com/llvm/llvm-project/pull/135333
More information about the lldb-commits
mailing list