https://github.com/labath updated
https://github.com/llvm/llvm-project/pull/127661
>From 8419178c265d64ee07fe72089421f500f4bb5452 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pa...@labath.sk>
Date: Tue, 18 Feb 2025 16:14:30 +0100
Subject: [PATCH] [lldb] s/ValidRange/ValidRanges in UnwindPlan
To be able to describe discontinuous functions, this patch changes the
UnwindPlan to accept more than one address range.
I've also squeezed in a couple improvements/modernizations, for example
using the lower_bound function instead of a linear scan.
---
lldb/include/lldb/Symbol/UnwindPlan.h | 17 ++--
.../ObjectFile/PECOFF/PECallFrameInfo.cpp | 4 +-
.../Breakpad/SymbolFileBreakpad.cpp | 12 +--
.../x86/x86AssemblyInspectionEngine.cpp | 4 +-
lldb/source/Symbol/CompactUnwindInfo.cpp | 2 +-
lldb/source/Symbol/DWARFCallFrameInfo.cpp | 2 +-
lldb/source/Symbol/UnwindPlan.cpp | 90 ++++++++-----------
lldb/unittests/Symbol/CMakeLists.txt | 1 +
lldb/unittests/Symbol/UnwindPlanTest.cpp | 76 ++++++++++++++++
.../x86/Testx86AssemblyInspectionEngine.cpp | 6 +-
10 files changed, 138 insertions(+), 76 deletions(-)
create mode 100644 lldb/unittests/Symbol/UnwindPlanTest.cpp
diff --git a/lldb/include/lldb/Symbol/UnwindPlan.h
b/lldb/include/lldb/Symbol/UnwindPlan.h
index 462c1a52b01db..e4d34b8cf7773 100644
--- a/lldb/include/lldb/Symbol/UnwindPlan.h
+++ b/lldb/include/lldb/Symbol/UnwindPlan.h
@@ -438,7 +438,7 @@ class UnwindPlan {
// Performs a deep copy of the plan, including all the rows (expensive).
UnwindPlan(const UnwindPlan &rhs)
- : m_plan_valid_address_range(rhs.m_plan_valid_address_range),
+ : m_plan_valid_ranges(rhs.m_plan_valid_ranges),
m_register_kind(rhs.m_register_kind),
m_return_addr_register(rhs.m_return_addr_register),
m_source_name(rhs.m_source_name),
@@ -492,10 +492,8 @@ class UnwindPlan {
// This UnwindPlan may not be valid at every address of the function span.
// For instance, a FastUnwindPlan will not be valid at the prologue setup
// instructions - only in the body of the function.
- void SetPlanValidAddressRange(const AddressRange &range);
-
- const AddressRange &GetAddressRange() const {
- return m_plan_valid_address_range;
+ void SetPlanValidAddressRanges(std::vector<AddressRange> ranges) {
+ m_plan_valid_ranges = std::move(ranges);
}
bool PlanValidAtAddress(Address addr);
@@ -544,11 +542,11 @@ class UnwindPlan {
m_plan_is_for_signal_trap = is_for_signal_trap;
}
- int GetRowCount() const;
+ int GetRowCount() const { return m_row_list.size(); }
void Clear() {
m_row_list.clear();
- m_plan_valid_address_range.Clear();
+ m_plan_valid_ranges.clear();
m_register_kind = lldb::eRegisterKindDWARF;
m_source_name.Clear();
m_plan_is_sourced_from_compiler = eLazyBoolCalculate;
@@ -571,9 +569,8 @@ class UnwindPlan {
}
private:
- typedef std::vector<RowSP> collection;
- collection m_row_list;
- AddressRange m_plan_valid_address_range;
+ std::vector<RowSP> m_row_list;
+ std::vector<AddressRange> m_plan_valid_ranges;
lldb::RegisterKind m_register_kind; // The RegisterKind these register
numbers
// are in terms of - will need to be
// translated to lldb native reg nums at unwind time
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/PECallFrameInfo.cpp
b/lldb/source/Plugins/ObjectFile/PECOFF/PECallFrameInfo.cpp
index 9bd48f2b9f60f..abb9cbde35711 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/PECallFrameInfo.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/PECallFrameInfo.cpp
@@ -495,9 +495,9 @@ bool PECallFrameInfo::GetUnwindPlan(const AddressRange
&range,
for (auto it = rows.rbegin(); it != rows.rend(); ++it)
unwind_plan.AppendRow(*it);
- unwind_plan.SetPlanValidAddressRange(AddressRange(
+ unwind_plan.SetPlanValidAddressRanges({AddressRange(
m_object_file.GetAddress(runtime_function->StartAddress),
- runtime_function->EndAddress - runtime_function->StartAddress));
+ runtime_function->EndAddress - runtime_function->StartAddress)});
unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
return true;
diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
index dee5a7ce2876d..1e53f5f7b9a6f 100644
--- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -656,9 +656,9 @@ SymbolFileBreakpad::ParseCFIUnwindPlan(const Bookmark
&bookmark,
plan_sp->SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
plan_sp->SetUnwindPlanForSignalTrap(eLazyBoolNo);
plan_sp->SetSourcedFromCompiler(eLazyBoolYes);
- plan_sp->SetPlanValidAddressRange(
- AddressRange(base + init_record->Address, *init_record->Size,
- m_objfile_sp->GetModule()->GetSectionList()));
+ plan_sp->SetPlanValidAddressRanges(
+ {AddressRange(base + init_record->Address, *init_record->Size,
+ m_objfile_sp->GetModule()->GetSectionList())});
auto row_sp = std::make_shared<UnwindPlan::Row>();
row_sp->SetOffset(0);
@@ -698,9 +698,9 @@ SymbolFileBreakpad::ParseWinUnwindPlan(const Bookmark
&bookmark,
plan_sp->SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
plan_sp->SetUnwindPlanForSignalTrap(eLazyBoolNo);
plan_sp->SetSourcedFromCompiler(eLazyBoolYes);
- plan_sp->SetPlanValidAddressRange(
- AddressRange(base + record->RVA, record->CodeSize,
- m_objfile_sp->GetModule()->GetSectionList()));
+ plan_sp->SetPlanValidAddressRanges(
+ {AddressRange(base + record->RVA, record->CodeSize,
+ m_objfile_sp->GetModule()->GetSectionList())});
auto row_sp = std::make_shared<UnwindPlan::Row>();
row_sp->SetOffset(0);
diff --git
a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
index 84f37ebe52492..90f40c9d1b55f 100644
--- a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
+++ b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
@@ -918,7 +918,7 @@ bool
x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
UnwindPlan::Row::AbstractRegisterLocation initial_regloc;
UnwindPlan::RowSP row(new UnwindPlan::Row);
- unwind_plan.SetPlanValidAddressRange(func_range);
+ unwind_plan.SetPlanValidAddressRanges({func_range});
unwind_plan.SetRegisterKind(eRegisterKindLLDB);
// At the start of the function, find the CFA by adding wordsize to the SP
@@ -1573,7 +1573,7 @@ bool
x86AssemblyInspectionEngine::AugmentUnwindPlanFromCallSite(
}
}
- unwind_plan.SetPlanValidAddressRange(func_range);
+ unwind_plan.SetPlanValidAddressRanges({func_range});
if (unwind_plan_updated) {
std::string unwind_plan_source(unwind_plan.GetSourceName().AsCString());
unwind_plan_source += " plus augmentation from assembly parsing";
diff --git a/lldb/source/Symbol/CompactUnwindInfo.cpp
b/lldb/source/Symbol/CompactUnwindInfo.cpp
index c9039ea51ff70..fdce79e92ae13 100644
--- a/lldb/source/Symbol/CompactUnwindInfo.cpp
+++ b/lldb/source/Symbol/CompactUnwindInfo.cpp
@@ -206,7 +206,7 @@ bool CompactUnwindInfo::GetUnwindPlan(Target &target,
Address addr,
function_info.valid_range_offset_end -
function_info.valid_range_offset_start,
sl);
- unwind_plan.SetPlanValidAddressRange(func_range);
+ unwind_plan.SetPlanValidAddressRanges({func_range});
}
}
diff --git a/lldb/source/Symbol/DWARFCallFrameInfo.cpp
b/lldb/source/Symbol/DWARFCallFrameInfo.cpp
index a743de596b8d8..19839c80659fe 100644
--- a/lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ b/lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -623,7 +623,7 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t
dwarf_offset,
uint32_t code_align = cie->code_align;
int32_t data_align = cie->data_align;
- unwind_plan.SetPlanValidAddressRange(range);
+ unwind_plan.SetPlanValidAddressRanges({range});
UnwindPlan::Row *cie_initial_row = new UnwindPlan::Row;
*cie_initial_row = cie->initial_row;
UnwindPlan::RowSP row(cie_initial_row);
diff --git a/lldb/source/Symbol/UnwindPlan.cpp
b/lldb/source/Symbol/UnwindPlan.cpp
index a3df927cddae8..b9c55c3e0ce58 100644
--- a/lldb/source/Symbol/UnwindPlan.cpp
+++ b/lldb/source/Symbol/UnwindPlan.cpp
@@ -15,6 +15,7 @@
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/DebugInfo/DIContext.h"
#include "llvm/DebugInfo/DWARF/DWARFExpression.h"
#include <optional>
@@ -397,36 +398,35 @@ void UnwindPlan::AppendRow(const UnwindPlan::RowSP
&row_sp) {
m_row_list.back() = row_sp;
}
+struct RowLess {
+ bool operator()(addr_t a, const UnwindPlan::RowSP &b) const {
+ return a < b->GetOffset();
+ }
+ bool operator()(const UnwindPlan::RowSP &a, addr_t b) const {
+ return a->GetOffset() < b;
+ }
+};
+
void UnwindPlan::InsertRow(const UnwindPlan::RowSP &row_sp,
bool replace_existing) {
- collection::iterator it = m_row_list.begin();
- while (it != m_row_list.end()) {
- RowSP row = *it;
- if (row->GetOffset() >= row_sp->GetOffset())
- break;
- it++;
- }
- if (it == m_row_list.end() || (*it)->GetOffset() != row_sp->GetOffset())
+ auto it = llvm::lower_bound(m_row_list, row_sp->GetOffset(), RowLess());
+ if (it == m_row_list.end() || it->get()->GetOffset() > row_sp->GetOffset())
m_row_list.insert(it, row_sp);
- else if (replace_existing)
- *it = row_sp;
+ else {
+ assert(it->get()->GetOffset() == row_sp->GetOffset());
+ if (replace_existing)
+ *it = row_sp;
+ }
}
const UnwindPlan::Row *UnwindPlan::GetRowForFunctionOffset(int offset) const {
- if (m_row_list.empty())
+ auto it = offset == -1 ? m_row_list.end()
+ : llvm::upper_bound(m_row_list, offset, RowLess());
+ if (it == m_row_list.begin())
return nullptr;
- if (offset == -1)
- return m_row_list.back().get();
-
- RowSP row;
- collection::const_iterator pos, end = m_row_list.end();
- for (pos = m_row_list.begin(); pos != end; ++pos) {
- if ((*pos)->GetOffset() <= static_cast<lldb::offset_t>(offset))
- row = *pos;
- else
- break;
- }
- return row.get();
+ // upper_bound returns the row strictly greater than our desired offset,
which
+ // means that the row before it is a match.
+ return std::prev(it)->get();
}
bool UnwindPlan::IsValidRowIndex(uint32_t idx) const {
@@ -445,20 +445,13 @@ const UnwindPlan::Row *UnwindPlan::GetRowAtIndex(uint32_t
idx) const {
const UnwindPlan::Row *UnwindPlan::GetLastRow() const {
if (m_row_list.empty()) {
- Log *log = GetLog(LLDBLog::Unwind);
- LLDB_LOGF(log, "UnwindPlan::GetLastRow() when rows are empty");
+ LLDB_LOG(GetLog(LLDBLog::Unwind),
+ "UnwindPlan::GetLastRow() when rows are empty");
return nullptr;
}
return m_row_list.back().get();
}
-int UnwindPlan::GetRowCount() const { return m_row_list.size(); }
-
-void UnwindPlan::SetPlanValidAddressRange(const AddressRange &range) {
- if (range.GetBaseAddress().IsValid() && range.GetByteSize() != 0)
- m_plan_valid_address_range = range;
-}
-
bool UnwindPlan::PlanValidAtAddress(Address addr) {
// If this UnwindPlan has no rows, it is an invalid UnwindPlan.
if (GetRowCount() == 0) {
@@ -482,9 +475,9 @@ bool UnwindPlan::PlanValidAtAddress(Address addr) {
// 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.
- if (GetRowAtIndex(0) == nullptr ||
- GetRowAtIndex(0)->GetCFAValue().GetValueType() ==
- Row::FAValue::unspecified) {
+ const Row *row0 = GetRowForFunctionOffset(0);
+ if (!row0 ||
+ row0->GetCFAValue().GetValueType() == Row::FAValue::unspecified) {
Log *log = GetLog(LLDBLog::Unwind);
if (log) {
StreamString s;
@@ -503,17 +496,15 @@ bool UnwindPlan::PlanValidAtAddress(Address addr) {
return false;
}
- if (!m_plan_valid_address_range.GetBaseAddress().IsValid() ||
- m_plan_valid_address_range.GetByteSize() == 0)
+ if (m_plan_valid_ranges.empty())
return true;
if (!addr.IsValid())
return true;
- if (m_plan_valid_address_range.ContainsFileAddress(addr))
- return true;
-
- return false;
+ return llvm::any_of(m_plan_valid_ranges, [&](const AddressRange &range) {
+ return range.ContainsFileAddress(addr);
+ });
}
void UnwindPlan::Dump(Stream &s, Thread *thread, lldb::addr_t base_addr) const
{
@@ -570,20 +561,17 @@ void UnwindPlan::Dump(Stream &s, Thread *thread,
lldb::addr_t base_addr) const {
s.Printf("not specified.\n");
break;
}
- if (m_plan_valid_address_range.GetBaseAddress().IsValid() &&
- m_plan_valid_address_range.GetByteSize() > 0) {
+ if (!m_plan_valid_ranges.empty()) {
s.PutCString("Address range of this UnwindPlan: ");
TargetSP target_sp(thread->CalculateTarget());
- m_plan_valid_address_range.Dump(&s, target_sp.get(),
- Address::DumpStyleSectionNameOffset);
+ for (const AddressRange &range : m_plan_valid_ranges)
+ range.Dump(&s, target_sp.get(), Address::DumpStyleSectionNameOffset);
s.EOL();
}
- collection::const_iterator pos, begin = m_row_list.begin(),
- end = m_row_list.end();
- for (pos = begin; pos != end; ++pos) {
- s.Printf("row[%u]: ", (uint32_t)std::distance(begin, pos));
- (*pos)->Dump(s, this, thread, base_addr);
- s.Printf("\n");
+ for (const auto &[index, row_sp] : llvm::enumerate(m_row_list)) {
+ s.Format("row[{0}]: ", index);
+ row_sp->Dump(s, this, thread, base_addr);
+ s << "\n";
}
}
diff --git a/lldb/unittests/Symbol/CMakeLists.txt
b/lldb/unittests/Symbol/CMakeLists.txt
index ab5cecd101833..5664c21adbe3f 100644
--- a/lldb/unittests/Symbol/CMakeLists.txt
+++ b/lldb/unittests/Symbol/CMakeLists.txt
@@ -12,6 +12,7 @@ add_lldb_unittest(SymbolTests
TestDWARFCallFrameInfo.cpp
TestType.cpp
TestLineEntry.cpp
+ UnwindPlanTest.cpp
LINK_LIBS
lldbCore
diff --git a/lldb/unittests/Symbol/UnwindPlanTest.cpp
b/lldb/unittests/Symbol/UnwindPlanTest.cpp
new file mode 100644
index 0000000000000..afbd9ab6ac38d
--- /dev/null
+++ b/lldb/unittests/Symbol/UnwindPlanTest.cpp
@@ -0,0 +1,76 @@
+//===-- UnwindPlanTest.cpp
------------------------------------------------===//
+//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Symbol/UnwindPlan.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace lldb;
+
+static UnwindPlan::RowSP make_simple_row(addr_t offset, uint64_t cfa_value) {
+ UnwindPlan::RowSP row_sp = std::make_shared<UnwindPlan::Row>();
+ row_sp->SetOffset(offset);
+ row_sp->GetCFAValue().SetIsConstant(cfa_value);
+ return row_sp;
+}
+
+TEST(UnwindPlan, InsertRow) {
+ UnwindPlan::RowSP row1_sp = make_simple_row(0, 42);
+ UnwindPlan::RowSP row2_sp = make_simple_row(0, 47);
+
+ UnwindPlan plan(eRegisterKindGeneric);
+ plan.InsertRow(row1_sp);
+ EXPECT_THAT(plan.GetRowForFunctionOffset(0), testing::Pointee(*row1_sp));
+
+ plan.InsertRow(row2_sp, /*replace_existing=*/false);
+ EXPECT_THAT(plan.GetRowForFunctionOffset(0), testing::Pointee(*row1_sp));
+
+ plan.InsertRow(row2_sp, /*replace_existing=*/true);
+ EXPECT_THAT(plan.GetRowForFunctionOffset(0), testing::Pointee(*row2_sp));
+}
+
+TEST(UnwindPlan, GetRowForFunctionOffset) {
+ UnwindPlan::RowSP row1_sp = make_simple_row(10, 42);
+ UnwindPlan::RowSP row2_sp = make_simple_row(20, 47);
+
+ UnwindPlan plan(eRegisterKindGeneric);
+ plan.InsertRow(row1_sp);
+ plan.InsertRow(row2_sp);
+
+ EXPECT_THAT(plan.GetRowForFunctionOffset(0), nullptr);
+ EXPECT_THAT(plan.GetRowForFunctionOffset(9), nullptr);
+ EXPECT_THAT(plan.GetRowForFunctionOffset(10), testing::Pointee(*row1_sp));
+ EXPECT_THAT(plan.GetRowForFunctionOffset(19), testing::Pointee(*row1_sp));
+ EXPECT_THAT(plan.GetRowForFunctionOffset(20), testing::Pointee(*row2_sp));
+ EXPECT_THAT(plan.GetRowForFunctionOffset(99), testing::Pointee(*row2_sp));
+}
+
+TEST(UnwindPlan, PlanValidAtAddress) {
+ UnwindPlan::RowSP row1_sp = make_simple_row(0, 42);
+ UnwindPlan::RowSP row2_sp = make_simple_row(10, 47);
+
+ UnwindPlan plan(eRegisterKindGeneric);
+ EXPECT_FALSE(plan.PlanValidAtAddress(Address(0)));
+
+ plan.InsertRow(row2_sp);
+ EXPECT_FALSE(plan.PlanValidAtAddress(Address(0)));
+
+ plan.InsertRow(row1_sp);
+ EXPECT_TRUE(plan.PlanValidAtAddress(Address(0)));
+ EXPECT_TRUE(plan.PlanValidAtAddress(Address(10)));
+
+ plan.SetPlanValidAddressRanges({AddressRange(0, 5), AddressRange(15, 5)});
+ EXPECT_TRUE(plan.PlanValidAtAddress(Address(0)));
+ EXPECT_FALSE(plan.PlanValidAtAddress(Address(5)));
+ EXPECT_FALSE(plan.PlanValidAtAddress(Address(10)));
+ EXPECT_TRUE(plan.PlanValidAtAddress(Address(15)));
+ EXPECT_FALSE(plan.PlanValidAtAddress(Address(20)));
+ EXPECT_FALSE(plan.PlanValidAtAddress(Address(25)));
+}
diff --git
a/lldb/unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
b/lldb/unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
index 3600360fad229..c55e13fce6935 100644
--- a/lldb/unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
+++ b/lldb/unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp
@@ -2248,7 +2248,7 @@ TEST_F(Testx86AssemblyInspectionEngine,
TestSpArithx86_64Augmented) {
sample_range = AddressRange(0x1000, sizeof(data));
unwind_plan.SetSourceName("unit testing hand-created unwind plan");
- unwind_plan.SetPlanValidAddressRange(sample_range);
+ unwind_plan.SetPlanValidAddressRanges({sample_range});
unwind_plan.SetRegisterKind(eRegisterKindLLDB);
row_sp = std::make_shared<UnwindPlan::Row>();
@@ -2335,7 +2335,7 @@ TEST_F(Testx86AssemblyInspectionEngine,
TestSimplex86_64Augmented) {
sample_range = AddressRange(0x1000, sizeof(data));
unwind_plan.SetSourceName("unit testing hand-created unwind plan");
- unwind_plan.SetPlanValidAddressRange(sample_range);
+ unwind_plan.SetPlanValidAddressRanges({sample_range});
unwind_plan.SetRegisterKind(eRegisterKindLLDB);
row_sp = std::make_shared<UnwindPlan::Row>();
@@ -2413,7 +2413,7 @@ TEST_F(Testx86AssemblyInspectionEngine,
TestSimplei386ugmented) {
sample_range = AddressRange(0x1000, sizeof(data));
unwind_plan.SetSourceName("unit testing hand-created unwind plan");
- unwind_plan.SetPlanValidAddressRange(sample_range);
+ unwind_plan.SetPlanValidAddressRanges({sample_range});
unwind_plan.SetRegisterKind(eRegisterKindLLDB);
row_sp = std::make_shared<UnwindPlan::Row>();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
