jasonmolenda created this revision.
jasonmolenda added a reviewer: jingham.
jasonmolenda added a subscriber: lldb-commits.
jasonmolenda set the repository for this revision to rL LLVM.
Herald added a subscriber: aemerson.
When lldb is instruction stepping (or fast-stepping / instruction stepping)
over a source line, and it instruction steps into a function call, and the
function is not "interesting" to the user, lldb sets a breakpoint on the return
address and continues the process until that breakpoint is hit on the current
thread.
lldb has a fast-stepping approach to avoid stopping at every instruction in a
source line range - it only stops on instructions that can branch / return /
call functions.
So we have one extra stop when nexting over a function call. We step into the
uninteresting-function, put a breakpoint on the return address, continue, hit
the breakpoint on the return address and then we fast-step to the next branch
instruction. That was four extra gdb-remote protocol packets for every
function call in a source line.
This patch advances the stop breakpoint to the first branching instruction
after we return, or to the end of the source line.
It passes the testsuite. I'll be doing more by-hand testing in the following
week or two when I have time, but it's a straightforward change, it shouldn't
cause problems unless I've missed something. I added the new
Process::AdvanceAddressToNextBranchInstruction() method but I'm only calling it
from ThreadPlanStepOut::ThreadPlanStepOut. I thought I'd be able to unify this
new function with the code in ThreadPlanStepRange::SetNextBranchBreakpoint but
I don't think the few lines I could remove from
ThreadPlanStepRange::SetNextBranchBreakpoint would be worth the change. Jim
might disagree with that.
The one point where this would be incorrect is a command like "finish" which
displays the return value after the function exits. On an architecture where
the return values are passed in volatile registers (x86, arm), that register
may be overwritten after the function return so we must stop on the return
address.
Repository:
rL LLVM
http://reviews.llvm.org/D15708
Files:
include/lldb/Target/Process.h
include/lldb/Target/Thread.h
include/lldb/Target/ThreadPlanStepOut.h
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
source/Target/Process.cpp
source/Target/Thread.cpp
source/Target/ThreadPlanShouldStopHere.cpp
source/Target/ThreadPlanStepInstruction.cpp
source/Target/ThreadPlanStepOut.cpp
source/Target/ThreadPlanStepOverRange.cpp
Index: source/Target/ThreadPlanStepOverRange.cpp
===================================================================
--- source/Target/ThreadPlanStepOverRange.cpp
+++ source/Target/ThreadPlanStepOverRange.cpp
@@ -185,7 +185,8 @@
stop_others,
eVoteNo,
eVoteNoOpinion,
- 0);
+ 0,
+ true);
break;
}
else
Index: source/Target/ThreadPlanStepOut.cpp
===================================================================
--- source/Target/ThreadPlanStepOut.cpp
+++ source/Target/ThreadPlanStepOut.cpp
@@ -18,6 +18,7 @@
#include "lldb/Core/ValueObjectConstResult.h"
#include "lldb/Symbol/Block.h"
#include "lldb/Symbol/Function.h"
+#include "lldb/Symbol/Symbol.h"
#include "lldb/Symbol/Type.h"
#include "lldb/Target/ABI.h"
#include "lldb/Target/Process.h"
@@ -44,7 +45,8 @@
Vote stop_vote,
Vote run_vote,
uint32_t frame_idx,
- LazyBool step_out_avoids_code_without_debug_info
+ LazyBool step_out_avoids_code_without_debug_info,
+ bool private_step_out
) :
ThreadPlan (ThreadPlan::eKindStepOut, "Step out", thread, stop_vote, run_vote),
ThreadPlanShouldStopHere (this),
@@ -86,7 +88,8 @@
eVoteNoOpinion,
eVoteNoOpinion,
frame_idx - 1,
- eLazyBoolNo));
+ eLazyBoolNo,
+ private_step_out));
static_cast<ThreadPlanStepOut *>(m_step_out_to_inline_plan_sp.get())->SetShouldStopHereCallbacks(nullptr, nullptr);
m_step_out_to_inline_plan_sp->SetPrivate(true);
}
@@ -101,7 +104,35 @@
// Find the return address and set a breakpoint there:
// FIXME - can we do this more securely if we know first_insn?
- m_return_addr = return_frame_sp->GetFrameCodeAddress().GetLoadAddress(&m_thread.GetProcess()->GetTarget());
+ Address return_address (return_frame_sp->GetFrameCodeAddress());
+ if (private_step_out)
+ {
+ SymbolContext return_address_sc;
+ AddressRange range;
+ Address return_address_decr_pc = return_address;
+ if (return_address_decr_pc.GetOffset() > 0)
+ return_address_decr_pc.Slide (-1);
+
+ return_address_decr_pc.CalculateSymbolContext (&return_address_sc, lldb::eSymbolContextLineEntry | lldb::eSymbolContextFunction | lldb::eSymbolContextSymbol);
+ if (return_address_sc.line_entry.IsValid())
+ {
+ range = return_address_sc.line_entry.GetSameLineContiguousAddressRange();
+ }
+ else if (return_address_sc.function)
+ {
+ range = return_address_sc.function->GetAddressRange();
+ }
+ else if (return_address_sc.symbol && return_address_sc.symbol->GetByteSizeIsValid())
+ {
+ range.GetBaseAddress() = return_address_sc.symbol->GetAddress();
+ range.SetByteSize (return_address_sc.symbol->GetByteSize());
+ }
+ if (range.GetByteSize() > 0)
+ {
+ return_address = m_thread.GetProcess()->AdvanceAddressToNextBranchInstruction (return_address, range, nullptr);
+ }
+ }
+ m_return_addr = return_address.GetLoadAddress(&m_thread.GetProcess()->GetTarget());
if (m_return_addr == LLDB_INVALID_ADDRESS)
return;
Index: source/Target/ThreadPlanStepInstruction.cpp
===================================================================
--- source/Target/ThreadPlanStepInstruction.cpp
+++ source/Target/ThreadPlanStepInstruction.cpp
@@ -214,7 +214,8 @@
stop_others,
eVoteNo,
eVoteNoOpinion,
- 0);
+ 0,
+ false);
return false;
}
else
Index: source/Target/ThreadPlanShouldStopHere.cpp
===================================================================
--- source/Target/ThreadPlanShouldStopHere.cpp
+++ source/Target/ThreadPlanShouldStopHere.cpp
@@ -135,7 +135,8 @@
stop_others,
eVoteNo,
eVoteNoOpinion,
- frame_index);
+ frame_index,
+ true);
return return_plan_sp;
}
Index: source/Target/Thread.cpp
===================================================================
--- source/Target/Thread.cpp
+++ source/Target/Thread.cpp
@@ -1600,7 +1600,7 @@
stop_vote,
run_vote,
frame_idx,
- step_out_avoids_code_withoug_debug_info));
+ step_out_avoids_code_withoug_debug_info, false));
if (thread_plan_sp->ValidatePlan(nullptr))
{
@@ -1620,7 +1620,8 @@
bool stop_other_threads,
Vote stop_vote,
Vote run_vote,
- uint32_t frame_idx)
+ uint32_t frame_idx,
+ bool private_step_out)
{
ThreadPlanSP thread_plan_sp(new ThreadPlanStepOut (*this,
addr_context,
@@ -1629,7 +1630,7 @@
stop_vote,
run_vote,
frame_idx,
- eLazyBoolNo));
+ eLazyBoolNo, private_step_out));
ThreadPlanStepOut *new_plan = static_cast<ThreadPlanStepOut *>(thread_plan_sp.get());
new_plan->ClearShouldStopHereCallbacks();
Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -6515,3 +6515,68 @@
if (token < m_image_tokens.size())
m_image_tokens[token] = LLDB_INVALID_IMAGE_TOKEN;
}
+
+Address
+Process::AdvanceAddressToNextBranchInstruction (Address default_stop_addr, AddressRange range_bounds,
+ InstructionList *insn_list)
+{
+ Target &target = GetTarget();
+ DisassemblerSP disassembler_sp;
+
+ Address retval = default_stop_addr;
+
+ if (target.GetUseFastStepping() == false)
+ return retval;
+ if (default_stop_addr.IsValid() == false)
+ return retval;
+
+
+ if (insn_list == NULL)
+ {
+ ExecutionContext exe_ctx (this);
+ const char *plugin_name = nullptr;
+ const char *flavor = nullptr;
+ const bool prefer_file_cache = true;
+ disassembler_sp = Disassembler::DisassembleRange(target.GetArchitecture(),
+ plugin_name,
+ flavor,
+ exe_ctx,
+ range_bounds,
+ prefer_file_cache);
+ if (disassembler_sp.get())
+ insn_list = &disassembler_sp->GetInstructionList();
+ }
+
+ if (insn_list == NULL)
+ {
+ return retval;
+ }
+
+ size_t insn_offset = insn_list->GetIndexOfInstructionAtAddress(default_stop_addr);
+ if (insn_offset == UINT32_MAX)
+ {
+ return retval;
+ }
+
+ uint32_t branch_index = insn_list->GetIndexOfNextBranchInstruction (insn_offset, target);
+ if (branch_index == UINT32_MAX)
+ {
+ return retval;
+ }
+
+ if (branch_index > insn_offset)
+ {
+ Address next_branch_insn_address = insn_list->GetInstructionAtIndex (branch_index)->GetAddress();
+ if (next_branch_insn_address.IsValid() && range_bounds.ContainsFileAddress (next_branch_insn_address))
+ {
+ retval = next_branch_insn_address;
+ }
+ }
+
+ if (disassembler_sp.get())
+ {
+ disassembler_sp->GetInstructionList().Clear(); // ThreadPlanStepRange claims there is a retain cycle
+ }
+
+ return retval;
+}
Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
===================================================================
--- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
+++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
@@ -196,7 +196,8 @@
m_stop_others,
eVoteNoOpinion,
eVoteNoOpinion,
- frame_idx);
+ frame_idx,
+ false);
m_run_to_sp->SetPrivate(true);
return false;
}
Index: include/lldb/Target/ThreadPlanStepOut.h
===================================================================
--- include/lldb/Target/ThreadPlanStepOut.h
+++ include/lldb/Target/ThreadPlanStepOut.h
@@ -31,7 +31,8 @@
Vote stop_vote,
Vote run_vote,
uint32_t frame_idx,
- LazyBool step_out_avoids_code_without_debug_info);
+ LazyBool step_out_avoids_code_without_debug_info,
+ bool private_step_out);
~ThreadPlanStepOut() override;
Index: include/lldb/Target/Thread.h
===================================================================
--- include/lldb/Target/Thread.h
+++ include/lldb/Target/Thread.h
@@ -888,6 +888,16 @@
/// @param[in] run_vote
/// See standard meanings for the stop & run votes in ThreadPlan.h.
///
+ /// @param[in] private_step_out
+ /// Normally this will enqueue a plan that will put a breakpoint on the return address and continue
+ /// to there. If private_step_out is true, this is an operation not involving the user -- e.g. stepping
+ /// "next" in a source line and we instruction stepped into another function -- so instead of putting
+ /// a breakpoint on the return address, advance the breakpoint to the end of the source line that is
+ /// doing the call, or until the next flow control instruction.
+ /// If the return value from the function call is to be retrieved / displayed to the user, you must stop
+ /// on the return address. The return value may be stored in volatile registers which are overwritten
+ /// before the next branch instruction.
+ ///
/// @return
/// A shared pointer to the newly queued thread plan, or nullptr if the plan could not be queued.
//------------------------------------------------------------------
@@ -898,7 +908,8 @@
bool stop_other_threads,
Vote stop_vote, // = eVoteYes,
Vote run_vote, // = eVoteNoOpinion);
- uint32_t frame_idx);
+ uint32_t frame_idx,
+ bool private_step_out);
//------------------------------------------------------------------
/// Gets the plan used to step through the code that steps from a function
Index: include/lldb/Target/Process.h
===================================================================
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -3149,6 +3149,43 @@
void
ResetImageToken(size_t token);
+ //------------------------------------------------------------------
+ /// Find the next branch instruction to set a breakpoint on
+ ///
+ /// When instruction stepping through a source line, instead of
+ /// stepping through each instruction, we can put a breakpoint on
+ /// the next branch instruction (within the range of instructions
+ /// we are stepping through) and continue the process to there,
+ /// yielding significant performance benefits over instruction
+ /// stepping.
+ ///
+ /// @param[in] default_stop_addr
+ /// The address of the instruction where lldb would put a
+ /// breakpoint normally.
+ ///
+ /// @param[in] range_bounds
+ /// The range which the breakpoint must be contained within.
+ /// Typically a source line.
+ ///
+ /// @param[in] insn_list
+ /// A pointer to an InstructionList for the instructions of
+ /// the range_bounds AddressRange. The caller may be able
+ /// to cache the InstructionList and re-use it across multiple
+ /// invocations. If this is NULL, this method will disassemble
+ /// the range_bounds AddressRange instructions and delete them
+ /// at the end of the method.
+ ///
+ /// @return
+ /// The address of the next branch instruction, or the end of
+ /// the range provided in range_bounds. If there are any
+ /// problems with the disassembly or getting the instructions,
+ /// The original default_stop_addr will be returned.
+ //------------------------------------------------------------------
+ Address
+ AdvanceAddressToNextBranchInstruction (Address default_stop_addr,
+ AddressRange range_bounds,
+ InstructionList *insn_list);
+
protected:
void
SetState (lldb::EventSP &event_sp);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
