jasonmolenda updated this revision to Diff 327930.
jasonmolenda added a comment.
Rewrite patch to expose "get pc for symbolication" APIs instead of "behaves
like zeroth frame" APIs so that we don't have different parts of lldb
decrementing the pc for symbolication. Doing a final round of tests on
different targets etc but I believe this is done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97644/new/
https://reviews.llvm.org/D97644
Files:
lldb/include/lldb/Target/LanguageRuntime.h
lldb/include/lldb/Target/RegisterContext.h
lldb/include/lldb/Target/RegisterContextUnwind.h
lldb/include/lldb/Target/StackFrame.h
lldb/source/Target/LanguageRuntime.cpp
lldb/source/Target/RegisterContext.cpp
lldb/source/Target/RegisterContextUnwind.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/UnwindLLDB.cpp
Index: lldb/source/Target/UnwindLLDB.cpp
===================================================================
--- lldb/source/Target/UnwindLLDB.cpp
+++ lldb/source/Target/UnwindLLDB.cpp
@@ -420,6 +420,8 @@
// too behaves like the zeroth frame (i.e. the pc might not
// be pointing just past a call in it)
behaves_like_zeroth_frame = true;
+ } else if (m_frames[idx]->reg_ctx_lldb_sp->BehavesLikeZerothFrame()) {
+ behaves_like_zeroth_frame = true;
} else {
behaves_like_zeroth_frame = false;
}
Index: lldb/source/Target/StackFrameList.cpp
===================================================================
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -522,27 +522,10 @@
SymbolContext unwind_sc = unwind_frame_sp->GetSymbolContext(
eSymbolContextBlock | eSymbolContextFunction);
Block *unwind_block = unwind_sc.block;
+ TargetSP target_sp = m_thread.CalculateTarget();
if (unwind_block) {
- Address curr_frame_address(unwind_frame_sp->GetFrameCodeAddress());
- TargetSP target_sp = m_thread.CalculateTarget();
- // Be sure to adjust the frame address to match the address that was
- // used to lookup the symbol context above. If we are in the first
- // concrete frame, then we lookup using the current address, else we
- // decrement the address by one to get the correct location.
- if (idx > 0) {
- if (curr_frame_address.GetOffset() == 0) {
- // If curr_frame_address points to the first address in a section
- // then after adjustment it will point to an other section. In that
- // case resolve the address again to the correct section plus
- // offset form.
- addr_t load_addr = curr_frame_address.GetOpcodeLoadAddress(
- target_sp.get(), AddressClass::eCode);
- curr_frame_address.SetOpcodeLoadAddress(
- load_addr - 1, target_sp.get(), AddressClass::eCode);
- } else {
- curr_frame_address.Slide(-1);
- }
- }
+ Address curr_frame_address(
+ unwind_frame_sp->GetFrameCodeAddressForSymbolication());
SymbolContext next_frame_sc;
Address next_frame_address;
Index: lldb/source/Target/StackFrame.cpp
===================================================================
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -213,6 +213,31 @@
return m_frame_code_addr;
}
+Address StackFrame::GetFrameCodeAddressForSymbolication() {
+ Address lookup_addr(GetFrameCodeAddress());
+ if (!lookup_addr.IsValid())
+ return lookup_addr;
+ if (m_behaves_like_zeroth_frame)
+ return lookup_addr;
+
+ addr_t offset = lookup_addr.GetOffset();
+ if (offset > 0) {
+ lookup_addr.SetOffset(offset - 1);
+ } else {
+ // lookup_addr is the start of a section. We need do the math on the
+ // actual load address and re-compute the section. We're working with
+ // a 'noreturn' function at the end of a section.
+ TargetSP target_sp = CalculateTarget();
+ if (target_sp) {
+ addr_t addr_minus_one = lookup_addr.GetOpcodeLoadAddress(
+ target_sp.get(), AddressClass::eCode) -
+ 1;
+ lookup_addr.SetOpcodeLoadAddress(addr_minus_one, target_sp.get());
+ }
+ }
+ return lookup_addr;
+}
+
bool StackFrame::ChangePC(addr_t pc) {
std::lock_guard<std::recursive_mutex> guard(m_mutex);
// We can't change the pc value of a history stack frame - it is immutable.
@@ -288,30 +313,7 @@
// If this is not frame zero, then we need to subtract 1 from the PC value
// when doing address lookups since the PC will be on the instruction
// following the function call instruction...
-
- Address lookup_addr(GetFrameCodeAddress());
- if (!m_behaves_like_zeroth_frame && lookup_addr.IsValid()) {
- addr_t offset = lookup_addr.GetOffset();
- if (offset > 0) {
- lookup_addr.SetOffset(offset - 1);
-
- } else {
- // lookup_addr is the start of a section. We need do the math on the
- // actual load address and re-compute the section. We're working with
- // a 'noreturn' function at the end of a section.
- ThreadSP thread_sp(GetThread());
- if (thread_sp) {
- TargetSP target_sp(thread_sp->CalculateTarget());
- if (target_sp) {
- addr_t addr_minus_one =
- lookup_addr.GetLoadAddress(target_sp.get()) - 1;
- lookup_addr.SetLoadAddress(addr_minus_one, target_sp.get());
- } else {
- lookup_addr.SetOffset(offset - 1);
- }
- }
- }
- }
+ Address lookup_addr(GetFrameCodeAddressForSymbolication());
if (m_sc.module_sp) {
// We have something in our stack frame symbol context, lets check if we
Index: lldb/source/Target/RegisterContextUnwind.cpp
===================================================================
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -58,10 +58,11 @@
m_fast_unwind_plan_sp(), m_full_unwind_plan_sp(),
m_fallback_unwind_plan_sp(), m_all_registers_available(false),
m_frame_type(-1), m_cfa(LLDB_INVALID_ADDRESS),
- m_afa(LLDB_INVALID_ADDRESS), m_start_pc(),
- m_current_pc(), m_current_offset(0), m_current_offset_backed_up_one(0),
- m_sym_ctx(sym_ctx), m_sym_ctx_valid(false), m_frame_number(frame_number),
- m_registers(), m_parent_unwind(unwind_lldb) {
+ m_afa(LLDB_INVALID_ADDRESS), m_start_pc(), m_current_pc(),
+ m_current_offset(0), m_current_offset_backed_up_one(0),
+ m_behaves_like_zeroth_frame(false), m_sym_ctx(sym_ctx),
+ m_sym_ctx_valid(false), m_frame_number(frame_number), m_registers(),
+ m_parent_unwind(unwind_lldb) {
m_sym_ctx.Clear(false);
m_sym_ctx_valid = false;
@@ -140,6 +141,12 @@
if (abi)
current_pc = abi->FixCodeAddress(current_pc);
+ UnwindPlanSP lang_runtime_plan_sp = LanguageRuntime::GetRuntimeUnwindPlan(
+ m_thread, this, m_behaves_like_zeroth_frame);
+ if (lang_runtime_plan_sp.get()) {
+ UnwindLogMsg("This is an async frame");
+ }
+
// Initialize m_current_pc, an Address object, based on current_pc, an
// addr_t.
m_current_pc.SetLoadAddress(current_pc, &process->GetTarget());
@@ -203,6 +210,38 @@
UnwindPlan::RowSP active_row;
lldb::RegisterKind row_register_kind = eRegisterKindGeneric;
+
+ // If we have LanguageRuntime UnwindPlan for this unwind, use those
+ // rules to find the caller frame instead of the function's normal
+ // UnwindPlans. The full unwind plan for this frame will be
+ // the LanguageRuntime-provided unwind plan, and there will not be a
+ // fast unwind plan.
+ if (lang_runtime_plan_sp.get()) {
+ active_row =
+ lang_runtime_plan_sp->GetRowForFunctionOffset(m_current_offset);
+ row_register_kind = lang_runtime_plan_sp->GetRegisterKind();
+ if (!ReadFrameAddress(row_register_kind, active_row->GetCFAValue(),
+ m_cfa)) {
+ UnwindLogMsg("Cannot set cfa");
+ } else {
+ m_full_unwind_plan_sp = lang_runtime_plan_sp;
+ if (log) {
+ StreamString active_row_strm;
+ active_row->Dump(active_row_strm, lang_runtime_plan_sp.get(), &m_thread,
+ m_start_pc.GetLoadAddress(exe_ctx.GetTargetPtr()));
+ UnwindLogMsg("async active row: %s", active_row_strm.GetData());
+ }
+ UnwindLogMsg("m_cfa = 0x%" PRIx64 " m_afa = 0x%" PRIx64, m_cfa, m_afa);
+ UnwindLogMsg(
+ "initialized async frame current pc is 0x%" PRIx64
+ " cfa is 0x%" PRIx64 " afa is 0x%" PRIx64,
+ (uint64_t)m_current_pc.GetLoadAddress(exe_ctx.GetTargetPtr()),
+ (uint64_t)m_cfa, (uint64_t)m_afa);
+
+ return;
+ }
+ }
+
if (m_full_unwind_plan_sp &&
m_full_unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
active_row =
@@ -294,8 +333,8 @@
// A LanguageRuntime may provide an UnwindPlan that is used in this
// stack trace base on the RegisterContext contents, intsead
// of the normal UnwindPlans we would use for the return-pc.
- UnwindPlanSP lang_runtime_plan_sp =
- LanguageRuntime::GetRuntimeUnwindPlan(m_thread, this);
+ UnwindPlanSP lang_runtime_plan_sp = LanguageRuntime::GetRuntimeUnwindPlan(
+ m_thread, this, m_behaves_like_zeroth_frame);
if (lang_runtime_plan_sp.get()) {
UnwindLogMsg("This is an async frame");
}
@@ -483,6 +522,8 @@
// so do not decrement and recompute if the symbol we already found is a trap
// handler.
decr_pc_and_recompute_addr_range = false;
+ } else if (m_behaves_like_zeroth_frame) {
+ decr_pc_and_recompute_addr_range = false;
} else {
// Decrement to find the function containing the call.
decr_pc_and_recompute_addr_range = true;
@@ -675,6 +716,14 @@
bool RegisterContextUnwind::IsFrameZero() const { return m_frame_number == 0; }
+bool RegisterContextUnwind::BehavesLikeZerothFrame() const {
+ if (m_frame_number == 0)
+ return true;
+ if (m_behaves_like_zeroth_frame)
+ return true;
+ return false;
+}
+
// Find a fast unwind plan for this frame, if possible.
//
// On entry to this method,
@@ -745,10 +794,9 @@
"unable to get architectural default UnwindPlan from ABI plugin");
}
- bool behaves_like_zeroth_frame = false;
if (IsFrameZero() || GetNextFrame()->m_frame_type == eTrapHandlerFrame ||
GetNextFrame()->m_frame_type == eDebuggerFrame) {
- behaves_like_zeroth_frame = true;
+ m_behaves_like_zeroth_frame = true;
// If this frame behaves like a 0th frame (currently executing or
// interrupted asynchronously), all registers can be retrieved.
m_all_registers_available = true;
@@ -764,7 +812,7 @@
if ((!m_sym_ctx_valid ||
(m_sym_ctx.function == nullptr && m_sym_ctx.symbol == nullptr)) &&
- behaves_like_zeroth_frame && m_current_pc.IsValid()) {
+ m_behaves_like_zeroth_frame && m_current_pc.IsValid()) {
uint32_t permissions;
addr_t current_pc_addr =
m_current_pc.GetLoadAddress(exe_ctx.GetTargetPtr());
@@ -892,7 +940,7 @@
// Typically the NonCallSite UnwindPlan is the unwind created by inspecting
// the assembly language instructions
- if (behaves_like_zeroth_frame && process) {
+ if (m_behaves_like_zeroth_frame && process) {
unwind_plan_sp = func_unwinders_sp->GetUnwindPlanAtNonCallSite(
process->GetTarget(), m_thread);
if (unwind_plan_sp && unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
Index: lldb/source/Target/RegisterContext.cpp
===================================================================
--- lldb/source/Target/RegisterContext.cpp
+++ lldb/source/Target/RegisterContext.cpp
@@ -150,6 +150,22 @@
return success;
}
+bool RegisterContext::GetPCForSymbolication(Address &address) {
+ addr_t pc = GetPC(LLDB_INVALID_ADDRESS);
+ if (pc == LLDB_INVALID_ADDRESS)
+ return false;
+ TargetSP target_sp = m_thread.CalculateTarget();
+ if (!target_sp.get())
+ return false;
+
+ if (!BehavesLikeZerothFrame())
+ pc--;
+ const bool allow_section_end = true;
+ address.SetOpcodeLoadAddress(pc, target_sp.get(), AddressClass::eCode,
+ allow_section_end);
+ return true;
+}
+
bool RegisterContext::SetPC(Address addr) {
TargetSP target_sp = m_thread.CalculateTarget();
Target *target = target_sp.get();
Index: lldb/source/Target/LanguageRuntime.cpp
===================================================================
--- lldb/source/Target/LanguageRuntime.cpp
+++ lldb/source/Target/LanguageRuntime.cpp
@@ -259,14 +259,16 @@
return exc_breakpt_sp;
}
-UnwindPlanSP LanguageRuntime::GetRuntimeUnwindPlan(Thread &thread,
- RegisterContext *regctx) {
+UnwindPlanSP
+LanguageRuntime::GetRuntimeUnwindPlan(Thread &thread, RegisterContext *regctx,
+ bool &behaves_like_zeroth_frame) {
ProcessSP process_sp = thread.GetProcess();
if (!process_sp.get())
return UnwindPlanSP();
for (const lldb::LanguageType lang_type : Language::GetSupportedLanguages()) {
if (LanguageRuntime *runtime = process_sp->GetLanguageRuntime(lang_type)) {
- UnwindPlanSP plan_sp = runtime->GetRuntimeUnwindPlan(process_sp, regctx);
+ UnwindPlanSP plan_sp = runtime->GetRuntimeUnwindPlan(
+ process_sp, regctx, behaves_like_zeroth_frame);
if (plan_sp.get())
return plan_sp;
}
Index: lldb/include/lldb/Target/StackFrame.h
===================================================================
--- lldb/include/lldb/Target/StackFrame.h
+++ lldb/include/lldb/Target/StackFrame.h
@@ -134,6 +134,24 @@
/// The Address object set to the current PC value.
const Address &GetFrameCodeAddress();
+ /// Get the current code Address suitable for symbolication,
+ /// may not be the same as GetFrameCodeAddress().
+ ///
+ /// For a frame in the middle of the stack, the return-pc is the
+ /// current code address, but for symbolication purposes the
+ /// return address after a noreturn call may point to the next
+ /// function, a DWARF location list entry that is a completely
+ /// different code path, or the wrong source line.
+ ///
+ /// The address returned should be used for symbolication (source line,
+ /// block, function, DWARF location entry selection) but should NOT
+ /// be shown to the user. It may not point to an actual instruction
+ /// boundary.
+ ///
+ /// \return
+ /// The Address object set to the current PC value.
+ Address GetFrameCodeAddressForSymbolication();
+
/// Change the pc value for a given thread.
///
/// Change the current pc value for the frame on this thread.
Index: lldb/include/lldb/Target/RegisterContextUnwind.h
===================================================================
--- lldb/include/lldb/Target/RegisterContextUnwind.h
+++ lldb/include/lldb/Target/RegisterContextUnwind.h
@@ -67,6 +67,11 @@
bool ReadPC(lldb::addr_t &start_pc);
+ // Indicates whether this frame *behaves* like frame zero -- the currently
+ // executing frame -- or not. This can be true in the middle of the stack
+ // above asynchronous trap handlers (sigtramp) for instance.
+ bool BehavesLikeZerothFrame() const override;
+
private:
enum FrameType {
eNormalFrame,
@@ -228,14 +233,16 @@
// unknown
// 0 if no instructions have been executed yet.
- int m_current_offset_backed_up_one; // how far into the function we've
- // executed; -1 if unknown
// 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.
// 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
+
+ bool m_behaves_like_zeroth_frame; // this frame behaves like frame zero
lldb_private::SymbolContext &m_sym_ctx;
bool m_sym_ctx_valid; // if ResolveSymbolContextForAddress fails, don't try to
Index: lldb/include/lldb/Target/RegisterContext.h
===================================================================
--- lldb/include/lldb/Target/RegisterContext.h
+++ lldb/include/lldb/Target/RegisterContext.h
@@ -148,6 +148,31 @@
uint64_t GetPC(uint64_t fail_value = LLDB_INVALID_ADDRESS);
+ /// Get an address suitable for symbolication.
+ /// When symbolicating -- computing line, block, function --
+ /// for a function in the middle of the stack, using the return
+ /// address can lead to unexpected results for the user.
+ /// A function that ends in a tail-call may have another function
+ /// as the "return" address, but it will never actually return.
+ /// Or a noreturn call in the middle of a function is the end of
+ /// a block of instructions, and a DWARF location list entry for
+ /// the return address may be a very different code path with
+ /// incorrect results when printing variables for this frame.
+ ///
+ /// At a source line view, the user expects the current-line indictation
+ /// to point to the function call they're under, not the next source line.
+ ///
+ /// The return address (GetPC()) should always be shown to the user,
+ /// but when computing context, keeping within the bounds of the
+ /// call instruction is what the user expects to see.
+ ///
+ /// \param [out] address
+ /// An Address object that will be filled in, if a PC can be retrieved.
+ ///
+ /// \return
+ /// Returns true if the Address param was filled in.
+ bool GetPCForSymbolication(Address &address);
+
bool SetPC(uint64_t pc);
bool SetPC(Address addr);
@@ -196,6 +221,19 @@
void SetStopID(uint32_t stop_id) { m_stop_id = stop_id; }
protected:
+ /// Indicates that this frame is currently executing code,
+ /// that the PC value is not a return-pc but an actual executing
+ /// instruction. Some places in lldb will treat a return-pc
+ /// value differently than the currently-executing-pc value,
+ /// and this method can indicate if that should be done.
+ /// The base class implementation only uses the frame index,
+ /// but subclasses may have additional information that they
+ /// can use to detect frames in this state, for instance a
+ /// frame above a trap handler (sigtramp etc)..
+ virtual bool BehavesLikeZerothFrame() const {
+ return m_concrete_frame_idx == 0;
+ }
+
// Classes that inherit from RegisterContext can see and modify these
Thread &m_thread; // The thread that this register context belongs to.
uint32_t m_concrete_frame_idx; // The concrete frame index for this register
Index: lldb/include/lldb/Target/LanguageRuntime.h
===================================================================
--- lldb/include/lldb/Target/LanguageRuntime.h
+++ lldb/include/lldb/Target/LanguageRuntime.h
@@ -183,9 +183,29 @@
/// asynchronous calls they made, but are part of a logical backtrace that
/// we want to show the developer because that's how they think of the
/// program flow.
+ ///
+ /// \param[in] thread
+ /// The thread that the unwind is happening on.
+ ///
+ /// \param[in] regctx
+ /// The RegisterContext for the frame we need to create an UnwindPlan.
+ /// We don't yet have a StackFrame when we're selecting the UnwindPlan.
+ ///
+ /// \param[out] behaves_like_zeroth_frame
+ /// With normal ABI calls, all stack frames except the zeroth frame need
+ /// to have the return-pc value backed up by 1 for symbolication purposes.
+ /// For these LanguageRuntime unwind plans, they may not follow normal ABI
+ /// calling conventions and the return pc may need to be symbolicated
+ /// as-is.
+ ///
+ /// \return
+ /// Returns an UnwindPlan to find the caller frame if it should be used,
+ /// instead of the UnwindPlan that would normally be used for this
+ /// function.
static lldb::UnwindPlanSP
GetRuntimeUnwindPlan(lldb_private::Thread &thread,
- lldb_private::RegisterContext *regctx);
+ lldb_private::RegisterContext *regctx,
+ bool &behaves_like_zeroth_frame);
protected:
// The static GetRuntimeUnwindPlan method above is only implemented in the
@@ -193,7 +213,8 @@
// provide one of these UnwindPlans.
virtual lldb::UnwindPlanSP
GetRuntimeUnwindPlan(lldb::ProcessSP process_sp,
- lldb_private::RegisterContext *regctx) {
+ lldb_private::RegisterContext *regctx,
+ bool &behaves_like_zeroth_frame) {
return lldb::UnwindPlanSP();
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
