[Lldb-commits] [lldb] [lldb] Make GetRowForFunctionOffset compatible with discontinuous functions (PR #133250)
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Thu Mar 27 06:41:19 PDT 2025
https://github.com/labath created https://github.com/llvm/llvm-project/pull/133250
The function had special handling for -1, but that is incompatible with functions whose entry point is not the first address. Use std::nullopt instead.
>From b857aa3cb6ced263d0c89d0195ce2f5bcac50d94 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Thu, 27 Mar 2025 14:36:10 +0100
Subject: [PATCH] [lldb] Make GetRowForFunctionOffset compatible with
discontinuous functions
The function had special handling for -1, but that is incompatible with
functions whose entry point is not the first address. Use std::nullopt
instead.
---
lldb/include/lldb/Symbol/UnwindPlan.h | 11 ++++----
.../lldb/Target/RegisterContextUnwind.h | 21 ++++++++--------
lldb/source/Symbol/UnwindPlan.cpp | 7 +++---
lldb/source/Target/RegisterContextUnwind.cpp | 25 ++++++++++---------
4 files changed, 33 insertions(+), 31 deletions(-)
diff --git a/lldb/include/lldb/Symbol/UnwindPlan.h b/lldb/include/lldb/Symbol/UnwindPlan.h
index ddc54a9fdc8ae..d4c83c128e39f 100644
--- a/lldb/include/lldb/Symbol/UnwindPlan.h
+++ b/lldb/include/lldb/Symbol/UnwindPlan.h
@@ -450,11 +450,12 @@ class UnwindPlan {
void InsertRow(Row row, bool replace_existing = false);
// Returns a pointer to the best row for the given offset into the function's
- // instructions. If offset is -1 it indicates that the function start is
- // unknown - the final row in the UnwindPlan is returned. In practice, the
- // UnwindPlan for a function with no known start address will be the
- // architectural default UnwindPlan which will only have one row.
- const UnwindPlan::Row *GetRowForFunctionOffset(int offset) const;
+ // instructions. If offset is std::nullopt it indicates that the function
+ // start is unknown - the final row in the UnwindPlan is returned. In
+ // practice, the UnwindPlan for a function with no known start address will be
+ // the architectural default UnwindPlan which will only have one row.
+ const UnwindPlan::Row *
+ GetRowForFunctionOffset(std::optional<int> offset) const;
lldb::RegisterKind GetRegisterKind() const { return m_register_kind; }
diff --git a/lldb/include/lldb/Target/RegisterContextUnwind.h b/lldb/include/lldb/Target/RegisterContextUnwind.h
index 6cd918fedc003..c4ae29e657bfb 100644
--- a/lldb/include/lldb/Target/RegisterContextUnwind.h
+++ b/lldb/include/lldb/Target/RegisterContextUnwind.h
@@ -228,18 +228,17 @@ class RegisterContextUnwind : public lldb_private::RegisterContext {
lldb_private::Address m_start_pc;
lldb_private::Address m_current_pc;
- int m_current_offset; // how far into the function we've executed; -1 if
- // unknown
- // 0 if no instructions have been executed yet.
-
- // 0 if no instructions have been executed yet.
- // On architectures where the return address on the stack points
- // to the instruction after the CALL, this value will have 1
- // subtracted from it. Else a function that ends in a CALL will
- // have an offset pointing into the next function's address range.
+ /// How far into the function we've executed. 0 if no instructions have been
+ /// executed yet, std::nullopt if unknown.
+ std::optional<int> m_current_offset;
+
+ // How far into the function we've executed. 0 if no instructions have been
+ // executed yet, std::nullopt if unknown. On architectures where the return
+ // address on the stack points to the instruction after the CALL, this value
+ // will have 1 subtracted from it. Otherwise, a function that ends in a CALL
+ // will have an offset pointing into the next function's address range.
// m_current_pc has the actual address of the "current" pc.
- int m_current_offset_backed_up_one; // how far into the function we've
- // executed; -1 if unknown
+ std::optional<int> m_current_offset_backed_up_one;
bool m_behaves_like_zeroth_frame; // this frame behaves like frame zero
diff --git a/lldb/source/Symbol/UnwindPlan.cpp b/lldb/source/Symbol/UnwindPlan.cpp
index 92daf29630ab6..718dd117930ac 100644
--- a/lldb/source/Symbol/UnwindPlan.cpp
+++ b/lldb/source/Symbol/UnwindPlan.cpp
@@ -417,9 +417,10 @@ void UnwindPlan::InsertRow(Row row, bool replace_existing) {
}
}
-const UnwindPlan::Row *UnwindPlan::GetRowForFunctionOffset(int offset) const {
- auto it = offset == -1 ? m_row_list.end()
- : llvm::upper_bound(m_row_list, offset, RowLess());
+const UnwindPlan::Row *
+UnwindPlan::GetRowForFunctionOffset(std::optional<int> offset) const {
+ auto it = offset ? llvm::upper_bound(m_row_list, *offset, RowLess())
+ : m_row_list.end();
if (it == m_row_list.begin())
return nullptr;
// upper_bound returns the row strictly greater than our desired offset, which
diff --git a/lldb/source/Target/RegisterContextUnwind.cpp b/lldb/source/Target/RegisterContextUnwind.cpp
index a035c57fbfc1c..cb3d7ee479890 100644
--- a/lldb/source/Target/RegisterContextUnwind.cpp
+++ b/lldb/source/Target/RegisterContextUnwind.cpp
@@ -94,8 +94,9 @@ bool RegisterContextUnwind::IsUnwindPlanValidForCurrentPC(
return true;
}
- // if m_current_offset <= 0, we've got nothing else to try
- if (m_current_offset <= 0)
+ // If don't have an offset or we're at the start of the function, we've got
+ // nothing else to try.
+ if (!m_current_offset || m_current_offset == 0)
return false;
// check pc - 1 to see if it's valid
@@ -198,8 +199,8 @@ void RegisterContextUnwind::InitializeZerothFrame() {
m_current_offset_backed_up_one = m_current_offset;
} else {
m_start_pc = m_current_pc;
- m_current_offset = -1;
- m_current_offset_backed_up_one = -1;
+ m_current_offset = std::nullopt;
+ m_current_offset_backed_up_one = std::nullopt;
}
// We've set m_frame_type and m_sym_ctx before these calls.
@@ -437,8 +438,8 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
m_frame_type = eNormalFrame;
}
m_all_registers_available = false;
- m_current_offset = -1;
- m_current_offset_backed_up_one = -1;
+ m_current_offset = std::nullopt;
+ m_current_offset_backed_up_one = std::nullopt;
RegisterKind row_register_kind = m_full_unwind_plan_sp->GetRegisterKind();
if (const UnwindPlan::Row *row =
m_full_unwind_plan_sp->GetRowForFunctionOffset(0)) {
@@ -569,16 +570,16 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
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 &&
- m_current_offset_backed_up_one > 0) {
- m_current_offset_backed_up_one--;
+ m_current_offset_backed_up_one != 0) {
+ --*m_current_offset_backed_up_one;
if (m_sym_ctx_valid) {
m_current_pc.SetLoadAddress(pc - 1, &process->GetTarget());
}
}
} else {
m_start_pc = m_current_pc;
- m_current_offset = -1;
- m_current_offset_backed_up_one = -1;
+ m_current_offset = std::nullopt;
+ m_current_offset_backed_up_one = std::nullopt;
}
if (IsTrapHandlerSymbol(process, m_sym_ctx)) {
@@ -746,7 +747,7 @@ bool RegisterContextUnwind::BehavesLikeZerothFrame() const {
// 2. m_sym_ctx should already be filled in, and
// 3. m_current_pc should have the current pc value for this frame
// 4. m_current_offset_backed_up_one should have the current byte offset into
-// the function, maybe backed up by 1, -1 if unknown
+// the function, maybe backed up by 1, std::nullopt if unknown
UnwindPlanSP RegisterContextUnwind::GetFastUnwindPlanForFrame() {
UnwindPlanSP unwind_plan_sp;
@@ -790,7 +791,7 @@ UnwindPlanSP RegisterContextUnwind::GetFastUnwindPlanForFrame() {
// 2. m_sym_ctx should already be filled in, and
// 3. m_current_pc should have the current pc value for this frame
// 4. m_current_offset_backed_up_one should have the current byte offset into
-// the function, maybe backed up by 1, -1 if unknown
+// the function, maybe backed up by 1, std::nullopt if unknown
UnwindPlanSP RegisterContextUnwind::GetFullUnwindPlanForFrame() {
UnwindPlanSP unwind_plan_sp;
More information about the lldb-commits
mailing list