https://github.com/labath created
https://github.com/llvm/llvm-project/pull/137155
AddressFunctionScope was always returning the first address range of the
function (assuming it was the only one). This doesn't work for
RegisterContextUnwind (it's only caller), when the function doesn't
start at the lowest address because it throws off the 'how many bytes
"into" a function I am' computation. This patch replaces the result with
a call to (recently introduced)
SymbolContext::GetFunctionOrSymbolAddress.
Note to reviewers: This patch consists of two commits. The first one is
equivalent to #137006. The two PRs are functionally independent, but without
the other PR, unwinding would be so broken there wouldn't be anything to test.
When reviewing, I recommend viewing only the second commit in the PR.
>From bae70dbaaa0bbc06be304e99f63120dad1e93470 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pa...@labath.sk>
Date: Wed, 23 Apr 2025 17:17:45 +0200
Subject: [PATCH 1/2] [lldb] Parse DWARF CFI for discontinuous functions
This patch uses the previously build infrastructure to parse multiple
FDE entries into a single unwind plan. There is one catch though: we
parse only one FDE entry per unwind range. This is not fully correct
because lldb coalesces adjecant address ranges, which means that
something that originally looked like two separate address ranges (and
two FDE entries) may get merged into one because if the linker decides
to put the two ranges next to each other. In this case, we will ignore
the second FDE entry.
It would be more correct to try to parse another entry when the one we
found turns out to be short, but I'm not doing this (yet), because:
- this is how we've done things so far (although, monolithic functions
are unlikely to have more than one FDE entry)
- in cases where we don't have debug info or (full) symbol tables, we
can end up with "symbols" which appear to span many megabytes
(potentially, the whole module). If we tried to fill short FDE
entries, we could end up parsing the entire eh_frame section in a
single go. In a way, this would be more correct, but it would also
probably be very slow.
I haven't quite decided what to do about this case yet, though it's not
particularly likely to happen in the "production" cases as typically the
functions are split into two parts (hot/cold) instead of one part per
basic block.
---
lldb/include/lldb/Symbol/DWARFCallFrameInfo.h | 9 ++-
lldb/source/Symbol/DWARFCallFrameInfo.cpp | 64 ++++++++++---------
lldb/source/Symbol/FuncUnwinders.cpp | 21 ++----
lldb/source/Symbol/UnwindTable.cpp | 27 ++++----
lldb/source/Target/RegisterContextUnwind.cpp | 20 +++---
.../Inputs/basic-block-sections-with-dwarf.s | 54 +++++++++-------
...asic-block-sections-with-dwarf-static.test | 37 +++++++----
.../Symbol/TestDWARFCallFrameInfo.cpp | 13 ++--
8 files changed, 131 insertions(+), 114 deletions(-)
diff --git a/lldb/include/lldb/Symbol/DWARFCallFrameInfo.h
b/lldb/include/lldb/Symbol/DWARFCallFrameInfo.h
index 679f652c7f2e0..c214ed1f60919 100644
--- a/lldb/include/lldb/Symbol/DWARFCallFrameInfo.h
+++ b/lldb/include/lldb/Symbol/DWARFCallFrameInfo.h
@@ -47,12 +47,15 @@ class DWARFCallFrameInfo {
/// Return an UnwindPlan based on the call frame information encoded in the
/// FDE of this DWARFCallFrameInfo section. The returned plan will be valid
/// (at least) for the given address.
- bool GetUnwindPlan(const Address &addr, UnwindPlan &unwind_plan);
+ std::unique_ptr<UnwindPlan> GetUnwindPlan(const Address &addr);
/// Return an UnwindPlan based on the call frame information encoded in the
/// FDE of this DWARFCallFrameInfo section. The returned plan will be valid
- /// (at least) for some address in the given range.
- bool GetUnwindPlan(const AddressRange &range, UnwindPlan &unwind_plan);
+ /// (at least) for some address in the given ranges. If no unwind information
+ /// is found, nullptr is returned. \a addr represents the entry point of the
+ /// function. It corresponds to the offset zero in the returned UnwindPlan.
+ std::unique_ptr<UnwindPlan> GetUnwindPlan(llvm::ArrayRef<AddressRange>
ranges,
+ const Address &addr);
typedef RangeVector<lldb::addr_t, uint32_t> FunctionAddressAndSizeVector;
diff --git a/lldb/source/Symbol/DWARFCallFrameInfo.cpp
b/lldb/source/Symbol/DWARFCallFrameInfo.cpp
index a763acb1fdf9e..cb8aa8a26c3f1 100644
--- a/lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ b/lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -151,53 +151,57 @@ DWARFCallFrameInfo::DWARFCallFrameInfo(ObjectFile
&objfile,
SectionSP §ion_sp, Type type)
: m_objfile(objfile), m_section_sp(section_sp), m_type(type) {}
-bool DWARFCallFrameInfo::GetUnwindPlan(const Address &addr,
- UnwindPlan &unwind_plan) {
- return GetUnwindPlan(AddressRange(addr, 1), unwind_plan);
+std::unique_ptr<UnwindPlan>
+DWARFCallFrameInfo::GetUnwindPlan(const Address &addr) {
+ return GetUnwindPlan({AddressRange(addr, 1)}, addr);
}
-bool DWARFCallFrameInfo::GetUnwindPlan(const AddressRange &range,
- UnwindPlan &unwind_plan) {
+std::unique_ptr<UnwindPlan>
+DWARFCallFrameInfo::GetUnwindPlan(llvm::ArrayRef<AddressRange> ranges,
+ const Address &addr) {
FDEEntryMap::Entry fde_entry;
- Address addr = range.GetBaseAddress();
// Make sure that the Address we're searching for is the same object file as
// this DWARFCallFrameInfo, we only store File offsets in m_fde_index.
ModuleSP module_sp = addr.GetModule();
if (module_sp.get() == nullptr || module_sp->GetObjectFile() == nullptr ||
module_sp->GetObjectFile() != &m_objfile)
- return false;
+ return nullptr;
- std::optional<FDEEntryMap::Entry> entry = GetFirstFDEEntryInRange(range);
- if (!entry)
- return false;
+ std::vector<AddressRange> valid_ranges;
- std::optional<FDE> fde = ParseFDE(entry->data, addr);
- if (!fde)
- return false;
-
- unwind_plan.SetSourceName(m_type == EH ? "eh_frame CFI" : "DWARF CFI");
+ auto result = std::make_unique<UnwindPlan>(GetRegisterKind());
+ result->SetSourceName(m_type == EH ? "eh_frame CFI" : "DWARF CFI");
// In theory the debug_frame info should be valid at all call sites
// ("asynchronous unwind info" as it is sometimes called) but in practice
// gcc et al all emit call frame info for the prologue and call sites, but
// not for the epilogue or all the other locations during the function
// reliably.
- unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
- unwind_plan.SetSourcedFromCompiler(eLazyBoolYes);
- unwind_plan.SetRegisterKind(GetRegisterKind());
-
- unwind_plan.SetPlanValidAddressRanges({fde->range});
- unwind_plan.SetUnwindPlanForSignalTrap(fde->for_signal_trap ? eLazyBoolYes
- : eLazyBoolNo);
- unwind_plan.SetReturnAddressRegister(fde->return_addr_reg_num);
- int64_t slide =
- fde->range.GetBaseAddress().GetFileAddress() - addr.GetFileAddress();
- for (UnwindPlan::Row &row : fde->rows) {
- row.SlideOffset(slide);
- unwind_plan.AppendRow(std::move(row));
+ result->SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
+ result->SetSourcedFromCompiler(eLazyBoolYes);
+ result->SetUnwindPlanForSignalTrap(eLazyBoolNo);
+ for (const AddressRange &range : ranges) {
+ std::optional<FDEEntryMap::Entry> entry = GetFirstFDEEntryInRange(range);
+ if (!entry)
+ continue;
+ std::optional<FDE> fde = ParseFDE(entry->data, addr);
+ if (!fde)
+ continue;
+ int64_t slide =
+ fde->range.GetBaseAddress().GetFileAddress() - addr.GetFileAddress();
+ valid_ranges.push_back(std::move(fde->range));
+ if (fde->for_signal_trap)
+ result->SetUnwindPlanForSignalTrap(eLazyBoolYes);
+ result->SetReturnAddressRegister(fde->return_addr_reg_num);
+ for (UnwindPlan::Row &row : fde->rows) {
+ row.SlideOffset(slide);
+ result->AppendRow(std::move(row));
+ }
}
-
- return true;
+ result->SetPlanValidAddressRanges(std::move(valid_ranges));
+ if (result->GetRowCount() == 0)
+ return nullptr;
+ return result;
}
bool DWARFCallFrameInfo::GetAddressRange(Address addr, AddressRange &range) {
diff --git a/lldb/source/Symbol/FuncUnwinders.cpp
b/lldb/source/Symbol/FuncUnwinders.cpp
index 11600825e8e38..faec24cde7fdd 100644
--- a/lldb/source/Symbol/FuncUnwinders.cpp
+++ b/lldb/source/Symbol/FuncUnwinders.cpp
@@ -149,13 +149,9 @@ FuncUnwinders::GetEHFrameUnwindPlan(Target &target) {
return m_unwind_plan_eh_frame_sp;
m_tried_unwind_plan_eh_frame = true;
- if (m_range.GetBaseAddress().IsValid()) {
- DWARFCallFrameInfo *eh_frame = m_unwind_table.GetEHFrameInfo();
- if (eh_frame) {
- auto plan_sp = std::make_shared<UnwindPlan>(lldb::eRegisterKindGeneric);
- if (eh_frame->GetUnwindPlan(m_range, *plan_sp))
- m_unwind_plan_eh_frame_sp = std::move(plan_sp);
- }
+ if (m_addr.IsValid()) {
+ if (DWARFCallFrameInfo *eh_frame = m_unwind_table.GetEHFrameInfo())
+ m_unwind_plan_eh_frame_sp = eh_frame->GetUnwindPlan(m_ranges, m_addr);
}
return m_unwind_plan_eh_frame_sp;
}
@@ -167,13 +163,10 @@ FuncUnwinders::GetDebugFrameUnwindPlan(Target &target) {
return m_unwind_plan_debug_frame_sp;
m_tried_unwind_plan_debug_frame = true;
- if (m_range.GetBaseAddress().IsValid()) {
- DWARFCallFrameInfo *debug_frame = m_unwind_table.GetDebugFrameInfo();
- if (debug_frame) {
- auto plan_sp = std::make_shared<UnwindPlan>(lldb::eRegisterKindGeneric);
- if (debug_frame->GetUnwindPlan(m_range, *plan_sp))
- m_unwind_plan_debug_frame_sp = std::move(plan_sp);
- }
+ if (!m_ranges.empty()) {
+ if (DWARFCallFrameInfo *debug_frame = m_unwind_table.GetDebugFrameInfo())
+ m_unwind_plan_debug_frame_sp =
+ debug_frame->GetUnwindPlan(m_ranges, m_addr);
}
return m_unwind_plan_debug_frame_sp;
}
diff --git a/lldb/source/Symbol/UnwindTable.cpp
b/lldb/source/Symbol/UnwindTable.cpp
index 21ecd434d7212..3aca495696c84 100644
--- a/lldb/source/Symbol/UnwindTable.cpp
+++ b/lldb/source/Symbol/UnwindTable.cpp
@@ -122,6 +122,13 @@ AddressRanges UnwindTable::GetAddressRanges(const Address
&addr,
return {};
}
+static Address GetFunctionOrSymbolAddress(const Address &addr,
+ const SymbolContext &sc) {
+ if (Address result = sc.GetFunctionOrSymbolAddress(); result.IsValid())
+ return result;
+ return addr;
+}
+
FuncUnwindersSP
UnwindTable::GetFuncUnwindersContainingAddress(const Address &addr,
const SymbolContext &sc) {
@@ -131,25 +138,20 @@ UnwindTable::GetFuncUnwindersContainingAddress(const
Address &addr,
// There is an UnwindTable per object file, so we can safely use file handles
addr_t file_addr = addr.GetFileAddress();
- iterator end = m_unwinds.end();
- iterator insert_pos = end;
- if (!m_unwinds.empty()) {
- insert_pos = m_unwinds.lower_bound(file_addr);
- iterator pos = insert_pos;
- if ((pos == m_unwinds.end()) ||
- (pos != m_unwinds.begin() &&
- pos->second->GetFunctionStartAddress() != addr))
- --pos;
-
+ iterator insert_pos = m_unwinds.upper_bound(file_addr);
+ if (insert_pos != m_unwinds.begin()) {
+ auto pos = std::prev(insert_pos);
if (pos->second->ContainsAddress(addr))
return pos->second;
}
+ Address start_addr = GetFunctionOrSymbolAddress(addr, sc);
AddressRanges ranges = GetAddressRanges(addr, sc);
if (ranges.empty())
return nullptr;
- auto func_unwinder_sp = std::make_shared<FuncUnwinders>(*this, addr, ranges);
+ auto func_unwinder_sp =
+ std::make_shared<FuncUnwinders>(*this, start_addr, ranges);
for (const AddressRange &range : ranges)
m_unwinds.emplace_hint(insert_pos, range.GetBaseAddress().GetFileAddress(),
func_unwinder_sp);
@@ -164,11 +166,12 @@ FuncUnwindersSP
UnwindTable::GetUncachedFuncUnwindersContainingAddress(
const Address &addr, const SymbolContext &sc) {
Initialize();
+ Address start_addr = GetFunctionOrSymbolAddress(addr, sc);
AddressRanges ranges = GetAddressRanges(addr, sc);
if (ranges.empty())
return nullptr;
- return std::make_shared<FuncUnwinders>(*this, addr, std::move(ranges));
+ return std::make_shared<FuncUnwinders>(*this, start_addr, std::move(ranges));
}
void UnwindTable::Dump(Stream &s) {
diff --git a/lldb/source/Target/RegisterContextUnwind.cpp
b/lldb/source/Target/RegisterContextUnwind.cpp
index 3ed49e12476dd..4c760b84e45a5 100644
--- a/lldb/source/Target/RegisterContextUnwind.cpp
+++ b/lldb/source/Target/RegisterContextUnwind.cpp
@@ -868,13 +868,11 @@ RegisterContextUnwind::GetFullUnwindPlanForFrame() {
// Even with -fomit-frame-pointer, we can try eh_frame to get back on
// track.
- DWARFCallFrameInfo *eh_frame =
- pc_module_sp->GetUnwindTable().GetEHFrameInfo();
- if (eh_frame) {
- auto unwind_plan_sp =
- std::make_shared<UnwindPlan>(lldb::eRegisterKindGeneric);
- if (eh_frame->GetUnwindPlan(m_current_pc, *unwind_plan_sp))
- return unwind_plan_sp;
+ if (DWARFCallFrameInfo *eh_frame =
+ pc_module_sp->GetUnwindTable().GetEHFrameInfo()) {
+ if (std::unique_ptr<UnwindPlan> plan_up =
+ eh_frame->GetUnwindPlan(m_current_pc))
+ return plan_up;
}
ArmUnwindInfo *arm_exidx =
@@ -1345,9 +1343,9 @@ RegisterContextUnwind::SavedLocationForRegister(
// value instead of the Return Address register.
// If $pc is not available, fall back to the RA reg.
UnwindPlan::Row::AbstractRegisterLocation scratch;
- if (m_frame_type == eTrapHandlerFrame &&
- active_row->GetRegisterInfo
- (pc_regnum.GetAsKind (unwindplan_registerkind), scratch)) {
+ if (m_frame_type == eTrapHandlerFrame && active_row &&
+ active_row->GetRegisterInfo(
+ pc_regnum.GetAsKind(unwindplan_registerkind), scratch)) {
UnwindLogMsg("Providing pc register instead of rewriting to "
"RA reg because this is a trap handler and there is "
"a location for the saved pc register value.");
@@ -1377,7 +1375,7 @@ RegisterContextUnwind::SavedLocationForRegister(
}
}
- if (regnum.IsValid() &&
+ if (regnum.IsValid() && active_row &&
active_row->GetRegisterInfo(regnum.GetAsKind(unwindplan_registerkind),
unwindplan_regloc)) {
have_unwindplan_regloc = true;
diff --git a/lldb/test/Shell/Unwind/Inputs/basic-block-sections-with-dwarf.s
b/lldb/test/Shell/Unwind/Inputs/basic-block-sections-with-dwarf.s
index c405e51c227cb..ede04c88a030f 100644
--- a/lldb/test/Shell/Unwind/Inputs/basic-block-sections-with-dwarf.s
+++ b/lldb/test/Shell/Unwind/Inputs/basic-block-sections-with-dwarf.s
@@ -4,7 +4,9 @@
# int bar() { return foo(0); }
# int foo(int flag) { return flag ? bar() : baz(); }
# int main() { return foo(1); }
-# The function bar has been placed "in the middle" of foo.
+# The function bar has been placed "in the middle" of foo. The functions are
not
+# using the frame pointer register and the are deliberately adjusting the stack
+# pointer to test that we're using the correct unwind row.
.text
@@ -20,26 +22,29 @@ baz:
.type foo,@function
foo:
.cfi_startproc
- pushq %rbp
+ pushq %rbx
.cfi_def_cfa_offset 16
- .cfi_offset %rbp, -16
- movq %rsp, %rbp
- .cfi_def_cfa_register %rbp
- subq $16, %rsp
- movl %edi, -8(%rbp)
- cmpl $0, -8(%rbp)
+ .cfi_offset %rbx, -16
+ movl %edi, %ebx
+ cmpl $0, %ebx
je foo.__part.2
jmp foo.__part.1
.cfi_endproc
.Lfoo_end:
.size foo, .Lfoo_end-foo
+# NB: Deliberately inserting padding to separate the two parts of the function
+# as we're currently only parsing a single FDE entry from a (coalesced) address
+# range.
+ nop
+
foo.__part.1:
.cfi_startproc
- .cfi_def_cfa %rbp, 16
- .cfi_offset %rbp, -16
+ .cfi_def_cfa_offset 16
+ .cfi_offset %rbx, -16
+ subq $16, %rsp
+ .cfi_def_cfa_offset 32
callq bar
- movl %eax, -4(%rbp)
jmp foo.__part.3
.Lfoo.__part.1_end:
.size foo.__part.1, .Lfoo.__part.1_end-foo.__part.1
@@ -47,8 +52,6 @@ foo.__part.1:
bar:
.cfi_startproc
-# NB: Decrease the stack pointer to make the unwind info for this function
-# different from the surrounding foo function.
subq $24, %rsp
.cfi_def_cfa_offset 32
xorl %edi, %edi
@@ -62,22 +65,26 @@ bar:
foo.__part.2:
.cfi_startproc
- .cfi_def_cfa %rbp, 16
- .cfi_offset %rbp, -16
+ .cfi_def_cfa_offset 16
+ .cfi_offset %rbx, -16
+ subq $16, %rsp
+ .cfi_def_cfa_offset 32
callq baz
- movl %eax, -4(%rbp)
jmp foo.__part.3
.Lfoo.__part.2_end:
.size foo.__part.2, .Lfoo.__part.2_end-foo.__part.2
.cfi_endproc
+# NB: Deliberately inserting padding to separate the two parts of the function
+# as we're currently only parsing a single FDE entry from a (coalesced) address
+# range.
+ nop
+
foo.__part.3:
.cfi_startproc
- .cfi_def_cfa %rbp, 16
- .cfi_offset %rbp, -16
- movl -4(%rbp), %eax
- addq $16, %rsp
- popq %rbp
+ .cfi_def_cfa_offset 32
+ .cfi_offset %rbx, -16
+ addq $24, %rsp
.cfi_def_cfa %rsp, 8
retq
.Lfoo.__part.3_end:
@@ -186,9 +193,8 @@ main:
.byte 86
.asciz "foo" # DW_AT_name
.byte 4 # Abbrev [4]
DW_TAG_formal_parameter
- .byte 2 # DW_AT_location
- .byte 145
- .byte 120
+ .byte 1 # DW_AT_location
+ .byte 0x53 # DW_OP_reg3
.asciz "flag" # DW_AT_name
.long .Lint-.Lcu_begin0 # DW_AT_type
.byte 0 # End Of Children Mark
diff --git a/lldb/test/Shell/Unwind/basic-block-sections-with-dwarf-static.test
b/lldb/test/Shell/Unwind/basic-block-sections-with-dwarf-static.test
index 9f94468ceecdb..a4ed73e14de01 100644
--- a/lldb/test/Shell/Unwind/basic-block-sections-with-dwarf-static.test
+++ b/lldb/test/Shell/Unwind/basic-block-sections-with-dwarf-static.test
@@ -17,23 +17,32 @@
image show-unwind --cached true -n foo
# CHECK: UNWIND PLANS for {{.*}}`foo
#
-# CHECK: Assembly language inspection UnwindPlan:
-# CHECK-NEXT: This UnwindPlan originally sourced from assembly insn profiling
-# CHECK-NEXT: This UnwindPlan is sourced from the compiler: no.
-# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: yes.
+# CHECK: eh_frame UnwindPlan:
+# CHECK-NEXT: This UnwindPlan originally sourced from eh_frame CFI
+# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.
+# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: no.
# CHECK-NEXT: This UnwindPlan is for a trap handler function: no.
-# TODO: This address range isn't correct right now. We're just checking that
-# it's a different range from the one in the next query.
-# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text +
6-0x0000000000000046)
+# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text +
6-0x0000000000000010)[{{.*}}.text + 17-0x000000000000001c)[{{.*}}.text +
44-0x0000000000000037)[{{.*}}.text + 56-0x000000000000003d)
+# CHECK-NEXT: row[0]: 0: CFA=rsp +8 => rip=[CFA-8]
+# CHECK-NEXT: row[1]: 1: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[2]: 11: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[3]: 15: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[4]: 38: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[5]: 42: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[6]: 50: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[7]: 54: CFA=rsp +8 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-EMPTY:
image show-unwind --cached true -n bar
# CHECK: UNWIND PLANS for {{.*}}`bar
-# CHECK: Assembly language inspection UnwindPlan:
-# CHECK-NEXT: This UnwindPlan originally sourced from assembly insn profiling
-# CHECK-NEXT: This UnwindPlan is sourced from the compiler: no.
-# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: yes.
+# CHECK: eh_frame UnwindPlan:
+# CHECK-NEXT: This UnwindPlan originally sourced from eh_frame CFI
+# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.
+# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: no.
# CHECK-NEXT: This UnwindPlan is for a trap handler function: no.
-# TODO: This address range isn't correct right now. We're just checking that
-# it's a different range from the one in the previous query.
-# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text +
35-0x0000000000000033)
+# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text +
28-0x000000000000002c)
+# CHECK-NEXT: row[0]: 0: CFA=rsp +8 => rip=[CFA-8]
+# CHECK-NEXT: row[1]: 4: CFA=rsp+32 => rip=[CFA-8]
+# CHECK-NEXT: row[2]: 15: CFA=rsp +8 => rip=[CFA-8]
+# CHECK-EMPTY:
diff --git a/lldb/unittests/Symbol/TestDWARFCallFrameInfo.cpp
b/lldb/unittests/Symbol/TestDWARFCallFrameInfo.cpp
index 86a6cf0cacb14..c1dcab02227da 100644
--- a/lldb/unittests/Symbol/TestDWARFCallFrameInfo.cpp
+++ b/lldb/unittests/Symbol/TestDWARFCallFrameInfo.cpp
@@ -7,6 +7,7 @@
//
//===----------------------------------------------------------------------===//
+#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
@@ -236,12 +237,12 @@ void
DWARFCallFrameInfoTest::TestBasic(DWARFCallFrameInfo::Type type,
ConstString(symbol), eSymbolTypeAny);
ASSERT_NE(nullptr, sym);
- UnwindPlan plan(eRegisterKindGeneric);
- ASSERT_TRUE(cfi.GetUnwindPlan(sym->GetAddress(), plan));
- ASSERT_EQ(3, plan.GetRowCount());
- EXPECT_EQ(GetExpectedRow0(), *plan.GetRowAtIndex(0));
- EXPECT_EQ(GetExpectedRow1(), *plan.GetRowAtIndex(1));
- EXPECT_EQ(GetExpectedRow2(), *plan.GetRowAtIndex(2));
+ std::unique_ptr<UnwindPlan> plan_up = cfi.GetUnwindPlan(sym->GetAddress());
+ ASSERT_TRUE(plan_up);
+ ASSERT_EQ(3, plan_up->GetRowCount());
+ EXPECT_THAT(plan_up->GetRowAtIndex(0), testing::Pointee(GetExpectedRow0()));
+ EXPECT_THAT(plan_up->GetRowAtIndex(1), testing::Pointee(GetExpectedRow1()));
+ EXPECT_THAT(plan_up->GetRowAtIndex(2), testing::Pointee(GetExpectedRow2()));
}
TEST_F(DWARFCallFrameInfoTest, Basic_dwarf3) {
>From d9662724e40e8fadcd1b4f968ea81780ea25045f Mon Sep 17 00:00:00 2001
From: Pavel Labath <pa...@labath.sk>
Date: Thu, 24 Apr 2025 10:03:27 +0200
Subject: [PATCH 2/2] [lldb] Fix offset computation in RegisterContextUnwind
AddressFunctionScope was always returning the first address range of the
function (assuming it was the only one). This doesn't work for
RegisterContextUnwind (it's only caller), when the function doesn't
start at the lowest address because it throws off the 'how many bytes
"into" a function I am' computation. This patch replaces the result with
a call to (recently introduced)
SymbolContext::GetFunctionOrSymbolAddress.
---
lldb/include/lldb/Core/Address.h | 13 +---
lldb/source/Core/Address.cpp | 15 +----
lldb/source/Target/RegisterContextUnwind.cpp | 40 +++++--------
.../Inputs/basic-block-sections-with-dwarf.s | 59 ++++++++++---------
...asic-block-sections-with-dwarf-static.test | 24 ++++----
5 files changed, 65 insertions(+), 86 deletions(-)
diff --git a/lldb/include/lldb/Core/Address.h b/lldb/include/lldb/Core/Address.h
index 9b5874f8b1fbe..85b2ab7bb3cfe 100644
--- a/lldb/include/lldb/Core/Address.h
+++ b/lldb/include/lldb/Core/Address.h
@@ -371,22 +371,15 @@ class Address {
bool ResolveAddressUsingFileSections(lldb::addr_t addr,
const SectionList *sections);
- /// Resolve this address to its containing function and optionally get
- /// that function's address range.
+ /// Resolve this address to its containing function.
///
/// \param[out] sym_ctx
/// The symbol context describing the function in which this address lies
///
- /// \parm[out] addr_range_ptr
- /// Pointer to the AddressRange to fill in with the function's address
- /// range. Caller may pass null if they don't need the address range.
- ///
/// \return
- /// Returns \b false if the function/symbol could not be resolved
- /// or if the address range was requested and could not be resolved;
+ /// Returns \b false if the function/symbol could not be resolved;
/// returns \b true otherwise.
- bool ResolveFunctionScope(lldb_private::SymbolContext &sym_ctx,
- lldb_private::AddressRange *addr_range_ptr =
nullptr);
+ bool ResolveFunctionScope(lldb_private::SymbolContext &sym_ctx);
/// Set the address to represent \a load_addr.
///
diff --git a/lldb/source/Core/Address.cpp b/lldb/source/Core/Address.cpp
index 1dab874a96583..a967bf5491211 100644
--- a/lldb/source/Core/Address.cpp
+++ b/lldb/source/Core/Address.cpp
@@ -263,22 +263,11 @@ bool Address::ResolveAddressUsingFileSections(addr_t
file_addr,
return false; // Failed to resolve this address to a section offset value
}
-/// if "addr_range_ptr" is not NULL, then fill in with the address range of
the function.
-bool Address::ResolveFunctionScope(SymbolContext &sym_ctx,
- AddressRange *addr_range_ptr) {
+bool Address::ResolveFunctionScope(SymbolContext &sym_ctx) {
constexpr SymbolContextItem resolve_scope =
eSymbolContextFunction | eSymbolContextSymbol;
- if (!(CalculateSymbolContext(&sym_ctx, resolve_scope) & resolve_scope)) {
- if (addr_range_ptr)
- addr_range_ptr->Clear();
- return false;
- }
-
- if (!addr_range_ptr)
- return true;
-
- return sym_ctx.GetAddressRange(resolve_scope, 0, false, *addr_range_ptr);
+ return CalculateSymbolContext(&sym_ctx, resolve_scope) & resolve_scope;
}
ModuleSP Address::GetModule() const {
diff --git a/lldb/source/Target/RegisterContextUnwind.cpp
b/lldb/source/Target/RegisterContextUnwind.cpp
index 4c760b84e45a5..c7185962ae8ea 100644
--- a/lldb/source/Target/RegisterContextUnwind.cpp
+++ b/lldb/source/Target/RegisterContextUnwind.cpp
@@ -160,8 +160,7 @@ void RegisterContextUnwind::InitializeZerothFrame() {
UnwindLogMsg("using architectural default unwind method");
}
- AddressRange addr_range;
- m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
+ m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx);
if (m_sym_ctx.symbol) {
UnwindLogMsg("with pc value of 0x%" PRIx64 ", symbol name is '%s'",
@@ -185,15 +184,9 @@ void RegisterContextUnwind::InitializeZerothFrame() {
// If we were able to find a symbol/function, set addr_range to the bounds of
// that symbol/function. else treat the current pc value as the start_pc and
// record no offset.
- if (addr_range.GetBaseAddress().IsValid()) {
- m_start_pc = addr_range.GetBaseAddress();
- if (m_current_pc.GetSection() == m_start_pc.GetSection()) {
- m_current_offset = m_current_pc.GetOffset() - m_start_pc.GetOffset();
- } else if (m_current_pc.GetModule() == m_start_pc.GetModule()) {
- // This means that whatever symbol we kicked up isn't really correct ---
- // we should not cross section boundaries ... We really should NULL out
- // the function/symbol in this case unless there is a bad assumption here
- // due to inlined functions?
+ if (m_sym_ctx_valid) {
+ m_start_pc = m_sym_ctx.GetFunctionOrSymbolAddress();
+ if (m_current_pc.GetModule() == m_start_pc.GetModule()) {
m_current_offset =
m_current_pc.GetFileAddress() - m_start_pc.GetFileAddress();
}
@@ -498,8 +491,7 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
return;
}
- AddressRange addr_range;
- m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
+ m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx);
if (m_sym_ctx.symbol) {
UnwindLogMsg("with pc value of 0x%" PRIx64 ", symbol name is '%s'", pc,
@@ -523,9 +515,8 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
// Don't decrement if we're "above" an asynchronous event like
// sigtramp.
decr_pc_and_recompute_addr_range = false;
- } else if (!addr_range.GetBaseAddress().IsValid() ||
- addr_range.GetBaseAddress().GetSection() !=
m_current_pc.GetSection() ||
- addr_range.GetBaseAddress().GetOffset() !=
m_current_pc.GetOffset()) {
+ } else if (Address addr = m_sym_ctx.GetFunctionOrSymbolAddress();
+ addr != m_current_pc) {
// If our "current" pc isn't the start of a function, decrement the pc
// if we're up the stack.
if (m_behaves_like_zeroth_frame)
@@ -558,7 +549,7 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
Address temporary_pc;
temporary_pc.SetLoadAddress(pc - 1, &process->GetTarget());
m_sym_ctx.Clear(false);
- m_sym_ctx_valid = temporary_pc.ResolveFunctionScope(m_sym_ctx,
&addr_range);
+ m_sym_ctx_valid = temporary_pc.ResolveFunctionScope(m_sym_ctx);
UnwindLogMsg("Symbol is now %s",
GetSymbolOrFunctionName(m_sym_ctx).AsCString(""));
@@ -567,8 +558,8 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
// If we were able to find a symbol/function, set addr_range_ptr to the
// bounds of that symbol/function. else treat the current pc value as the
// start_pc and record no offset.
- if (addr_range.GetBaseAddress().IsValid()) {
- m_start_pc = addr_range.GetBaseAddress();
+ if (m_sym_ctx_valid) {
+ m_start_pc = m_sym_ctx.GetFunctionOrSymbolAddress();
m_current_offset = pc - m_start_pc.GetLoadAddress(&process->GetTarget());
m_current_offset_backed_up_one = m_current_offset;
if (decr_pc_and_recompute_addr_range &&
@@ -1939,8 +1930,7 @@ void
RegisterContextUnwind::PropagateTrapHandlerFlagFromUnwindPlan(
GetSymbolOrFunctionName(m_sym_ctx).AsCString(""));
m_current_offset_backed_up_one = m_current_offset;
- AddressRange addr_range;
- m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx,
&addr_range);
+ m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx);
UnwindLogMsg("Symbol is now %s",
GetSymbolOrFunctionName(m_sym_ctx).AsCString(""));
@@ -1949,9 +1939,11 @@ void
RegisterContextUnwind::PropagateTrapHandlerFlagFromUnwindPlan(
Process *process = exe_ctx.GetProcessPtr();
Target *target = &process->GetTarget();
- m_start_pc = addr_range.GetBaseAddress();
- m_current_offset =
- m_current_pc.GetLoadAddress(target) -
m_start_pc.GetLoadAddress(target);
+ if (m_sym_ctx_valid) {
+ m_start_pc = m_sym_ctx.GetFunctionOrSymbolAddress();
+ m_current_offset = m_current_pc.GetLoadAddress(target) -
+ m_start_pc.GetLoadAddress(target);
+ }
}
}
diff --git a/lldb/test/Shell/Unwind/Inputs/basic-block-sections-with-dwarf.s
b/lldb/test/Shell/Unwind/Inputs/basic-block-sections-with-dwarf.s
index ede04c88a030f..a7b5431a7afaf 100644
--- a/lldb/test/Shell/Unwind/Inputs/basic-block-sections-with-dwarf.s
+++ b/lldb/test/Shell/Unwind/Inputs/basic-block-sections-with-dwarf.s
@@ -19,19 +19,16 @@ baz:
.Lbaz_end:
.size baz, .Lbaz_end-baz
- .type foo,@function
-foo:
+foo.__part.3:
.cfi_startproc
- pushq %rbx
- .cfi_def_cfa_offset 16
+ .cfi_def_cfa_offset 32
.cfi_offset %rbx, -16
- movl %edi, %ebx
- cmpl $0, %ebx
- je foo.__part.2
- jmp foo.__part.1
+ addq $24, %rsp
+ .cfi_def_cfa %rsp, 8
+ retq
+.Lfoo.__part.3_end:
+ .size foo.__part.3, .Lfoo.__part.3_end-foo.__part.3
.cfi_endproc
-.Lfoo_end:
- .size foo, .Lfoo_end-foo
# NB: Deliberately inserting padding to separate the two parts of the function
# as we're currently only parsing a single FDE entry from a (coalesced) address
@@ -40,11 +37,13 @@ foo:
foo.__part.1:
.cfi_startproc
- .cfi_def_cfa_offset 16
+ .cfi_def_cfa_offset 32
.cfi_offset %rbx, -16
subq $16, %rsp
- .cfi_def_cfa_offset 32
+ .cfi_def_cfa_offset 48
callq bar
+ addq $16, %rsp
+ .cfi_def_cfa_offset 32
jmp foo.__part.3
.Lfoo.__part.1_end:
.size foo.__part.1, .Lfoo.__part.1_end-foo.__part.1
@@ -52,46 +51,50 @@ foo.__part.1:
bar:
.cfi_startproc
- subq $24, %rsp
- .cfi_def_cfa_offset 32
+ subq $88, %rsp
+ .cfi_def_cfa_offset 96
xorl %edi, %edi
callq foo
- addq $24, %rsp
+ addq $88, %rsp
.cfi_def_cfa %rsp, 8
retq
.cfi_endproc
.Lbar_end:
.size bar, .Lbar_end-bar
-foo.__part.2:
+ .type foo,@function
+foo:
.cfi_startproc
+ pushq %rbx
.cfi_def_cfa_offset 16
.cfi_offset %rbx, -16
+ movl %edi, %ebx
+ cmpl $0, %ebx
+ je foo.__part.2
subq $16, %rsp
.cfi_def_cfa_offset 32
- callq baz
- jmp foo.__part.3
-.Lfoo.__part.2_end:
- .size foo.__part.2, .Lfoo.__part.2_end-foo.__part.2
+ jmp foo.__part.1
.cfi_endproc
+.Lfoo_end:
+ .size foo, .Lfoo_end-foo
# NB: Deliberately inserting padding to separate the two parts of the function
# as we're currently only parsing a single FDE entry from a (coalesced) address
# range.
nop
-foo.__part.3:
+foo.__part.2:
.cfi_startproc
- .cfi_def_cfa_offset 32
+ .cfi_def_cfa_offset 16
.cfi_offset %rbx, -16
- addq $24, %rsp
- .cfi_def_cfa %rsp, 8
- retq
-.Lfoo.__part.3_end:
- .size foo.__part.3, .Lfoo.__part.3_end-foo.__part.3
+ subq $16, %rsp
+ .cfi_def_cfa_offset 32
+ callq baz
+ jmp foo.__part.3
+.Lfoo.__part.2_end:
+ .size foo.__part.2, .Lfoo.__part.2_end-foo.__part.2
.cfi_endproc
-
.globl main
.type main,@function
main:
diff --git a/lldb/test/Shell/Unwind/basic-block-sections-with-dwarf-static.test
b/lldb/test/Shell/Unwind/basic-block-sections-with-dwarf-static.test
index a4ed73e14de01..b83e388e79d21 100644
--- a/lldb/test/Shell/Unwind/basic-block-sections-with-dwarf-static.test
+++ b/lldb/test/Shell/Unwind/basic-block-sections-with-dwarf-static.test
@@ -22,15 +22,17 @@ image show-unwind --cached true -n foo
# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.
# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: no.
# CHECK-NEXT: This UnwindPlan is for a trap handler function: no.
-# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text +
6-0x0000000000000010)[{{.*}}.text + 17-0x000000000000001c)[{{.*}}.text +
44-0x0000000000000037)[{{.*}}.text + 56-0x000000000000003d)
-# CHECK-NEXT: row[0]: 0: CFA=rsp +8 => rip=[CFA-8]
-# CHECK-NEXT: row[1]: 1: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
-# CHECK-NEXT: row[2]: 11: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
-# CHECK-NEXT: row[3]: 15: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
-# CHECK-NEXT: row[4]: 38: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
-# CHECK-NEXT: row[5]: 42: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
-# CHECK-NEXT: row[6]: 50: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
-# CHECK-NEXT: row[7]: 54: CFA=rsp +8 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text +
6-0x000000000000000b)[{{.*}}.text + 12-0x000000000000001b)[{{.*}}.text +
43-0x0000000000000039)[{{.*}}.text + 58-0x0000000000000045)
+# CHECK-NEXT: row[0]: -37: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[1]: -33: CFA=rsp +8 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[2]: -31: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[3]: -27: CFA=rsp+48 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[4]: -18: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[5]: 0: CFA=rsp +8 => rip=[CFA-8]
+# CHECK-NEXT: row[6]: 1: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[7]: 12: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[8]: 15: CFA=rsp+16 => rbx=[CFA-16] rip=[CFA-8]
+# CHECK-NEXT: row[9]: 19: CFA=rsp+32 => rbx=[CFA-16] rip=[CFA-8]
# CHECK-EMPTY:
image show-unwind --cached true -n bar
@@ -41,8 +43,8 @@ image show-unwind --cached true -n bar
# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.
# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: no.
# CHECK-NEXT: This UnwindPlan is for a trap handler function: no.
-# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text +
28-0x000000000000002c)
+# CHECK-NEXT: Address range of this UnwindPlan: [{{.*}}.text +
27-0x000000000000002b)
# CHECK-NEXT: row[0]: 0: CFA=rsp +8 => rip=[CFA-8]
-# CHECK-NEXT: row[1]: 4: CFA=rsp+32 => rip=[CFA-8]
+# CHECK-NEXT: row[1]: 4: CFA=rsp+96 => rip=[CFA-8]
# CHECK-NEXT: row[2]: 15: CFA=rsp +8 => rip=[CFA-8]
# CHECK-EMPTY:
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
