Author: Pavel Labath Date: 2025-03-31T11:45:11+02:00 New Revision: 9d61eaa9ecd9a46d22a8a4efc67d31b9abba3616
URL: https://github.com/llvm/llvm-project/commit/9d61eaa9ecd9a46d22a8a4efc67d31b9abba3616 DIFF: https://github.com/llvm/llvm-project/commit/9d61eaa9ecd9a46d22a8a4efc67d31b9abba3616.diff LOG: [lldb] Make GetRowForFunctionOffset compatible with discontinuous functions (#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. Added: Modified: lldb/include/lldb/Symbol/UnwindPlan.h lldb/include/lldb/Target/RegisterContextUnwind.h lldb/source/Symbol/UnwindPlan.cpp lldb/source/Target/RegisterContextUnwind.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Symbol/UnwindPlan.h b/lldb/include/lldb/Symbol/UnwindPlan.h index db9aade93b6ba..9adda27b8f928 100644 --- a/lldb/include/lldb/Symbol/UnwindPlan.h +++ b/lldb/include/lldb/Symbol/UnwindPlan.h @@ -467,11 +467,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 48089cbdecd97..f2846eb927bf8 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; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
