[Lldb-commits] [lldb] r257210 - Re-apply r257117 (reverted in r257138 temporarily),

Jason Molenda via lldb-commits lldb-commits at lists.llvm.org
Fri Jan 8 13:40:13 PST 2016


Author: jmolenda
Date: Fri Jan  8 15:40:11 2016
New Revision: 257210

URL: http://llvm.org/viewvc/llvm-project?rev=257210&view=rev
Log:
Re-apply r257117 (reverted in r257138 temporarily),
with the one change that ThreadPlanStepOut::ThreadPlanStepOut
will now only advance the return address breakpoint to
the end of a source line, if we have source line debug information.
It will not advance to the end of a Symbol if we lack source line
information.  This, or the recognition of the LEAVE instruction
in r257209, would have fixed the regression that Siva was seeing.
Both were good changes, so I've made both.

Original commit message:

Performance improvement: Change lldb so that it puts a breakpoint
on the first branch instruction after a function return (or the end
of a source line), instead of a breakpoint on the return address,
to skip an extra stop & start of the inferior process.

I changed Process::AdvanceAddressToNextBranchInstruction to not
take an optional InstructionList argument - no callers are providing
a cached InstructionList today, and if this function was going to
do that, the right thing to do would be to fill out / use a
DisassemblerSP which is a disassembler with the InstructionList for
this address range.


http://reviews.llvm.org/D15708
<rdar://problem/23309838> 

Modified:
    lldb/trunk/include/lldb/Target/Process.h
    lldb/trunk/include/lldb/Target/Thread.h
    lldb/trunk/include/lldb/Target/ThreadPlanStepOut.h
    lldb/trunk/source/Target/Process.cpp
    lldb/trunk/source/Target/Thread.cpp
    lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp
    lldb/trunk/source/Target/ThreadPlanStepOut.cpp
    lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp

Modified: lldb/trunk/include/lldb/Target/Process.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=257210&r1=257209&r2=257210&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Fri Jan  8 15:40:11 2016
@@ -3149,6 +3149,34 @@ public:
     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.
+    ///
+    /// @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);
+
 protected:
     void
     SetState (lldb::EventSP &event_sp);

Modified: lldb/trunk/include/lldb/Target/Thread.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Thread.h?rev=257210&r1=257209&r2=257210&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Thread.h (original)
+++ lldb/trunk/include/lldb/Target/Thread.h Fri Jan  8 15:40:11 2016
@@ -888,6 +888,16 @@ public:
     /// @param[in] run_vote
     ///    See standard meanings for the stop & run votes in ThreadPlan.h.
     ///
+    /// @param[in] continue_to_next_branch
+    ///    Normally this will enqueue a plan that will put a breakpoint on the return address and continue
+    ///    to there.  If continue_to_next_branch 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 @@ public:
                                            bool stop_other_threads,
                                            Vote stop_vote, // = eVoteYes,
                                            Vote run_vote, // = eVoteNoOpinion);
-                                           uint32_t frame_idx);
+                                           uint32_t frame_idx,
+                                           bool continue_to_next_branch = false);
 
     //------------------------------------------------------------------
     /// Gets the plan used to step through the code that steps from a function

Modified: lldb/trunk/include/lldb/Target/ThreadPlanStepOut.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/ThreadPlanStepOut.h?rev=257210&r1=257209&r2=257210&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/ThreadPlanStepOut.h (original)
+++ lldb/trunk/include/lldb/Target/ThreadPlanStepOut.h Fri Jan  8 15:40:11 2016
@@ -31,7 +31,8 @@ public:
                        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 continue_to_next_branch = false);
 
     ~ThreadPlanStepOut() override;
 

Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=257210&r1=257209&r2=257210&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Fri Jan  8 15:40:11 2016
@@ -6515,3 +6515,65 @@ Process::ResetImageToken(size_t token)
     if (token < m_image_tokens.size())
         m_image_tokens[token] = LLDB_INVALID_IMAGE_TOKEN;
 }
+
+Address
+Process::AdvanceAddressToNextBranchInstruction (Address default_stop_addr, AddressRange range_bounds)
+{
+    Target &target = GetTarget();
+    DisassemblerSP disassembler_sp;
+    InstructionList *insn_list = NULL;
+
+    Address retval = default_stop_addr;
+
+    if (target.GetUseFastStepping() == false)
+        return retval;
+    if (default_stop_addr.IsValid() == false)
+        return retval;
+
+    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())
+    {
+        // FIXME: The DisassemblerLLVMC has a reference cycle and won't go away if it has any active instructions.
+        disassembler_sp->GetInstructionList().Clear();
+    }
+
+    return retval;
+}

Modified: lldb/trunk/source/Target/Thread.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Thread.cpp?rev=257210&r1=257209&r2=257210&view=diff
==============================================================================
--- lldb/trunk/source/Target/Thread.cpp (original)
+++ lldb/trunk/source/Target/Thread.cpp Fri Jan  8 15:40:11 2016
@@ -1591,7 +1591,7 @@ Thread::QueueThreadPlanForStepOut(bool a
                                   Vote stop_vote,
                                   Vote run_vote,
                                   uint32_t frame_idx,
-                                  LazyBool step_out_avoids_code_withoug_debug_info)
+                                  LazyBool step_out_avoids_code_without_debug_info)
 {
     ThreadPlanSP thread_plan_sp (new ThreadPlanStepOut (*this, 
                                                         addr_context, 
@@ -1600,7 +1600,7 @@ Thread::QueueThreadPlanForStepOut(bool a
                                                         stop_vote, 
                                                         run_vote, 
                                                         frame_idx,
-                                                        step_out_avoids_code_withoug_debug_info));
+                                                        step_out_avoids_code_without_debug_info));
     
     if (thread_plan_sp->ValidatePlan(nullptr))
     {
@@ -1620,7 +1620,8 @@ Thread::QueueThreadPlanForStepOutNoShoul
                                               bool stop_other_threads,
                                               Vote stop_vote,
                                               Vote run_vote,
-                                              uint32_t frame_idx)
+                                              uint32_t frame_idx,
+                                              bool continue_to_next_branch)
 {
     ThreadPlanSP thread_plan_sp(new ThreadPlanStepOut (*this,
                                                         addr_context, 
@@ -1629,7 +1630,8 @@ Thread::QueueThreadPlanForStepOutNoShoul
                                                         stop_vote, 
                                                         run_vote, 
                                                         frame_idx,
-                                                        eLazyBoolNo));
+                                                        eLazyBoolNo,
+                                                        continue_to_next_branch));
 
     ThreadPlanStepOut *new_plan = static_cast<ThreadPlanStepOut *>(thread_plan_sp.get());
     new_plan->ClearShouldStopHereCallbacks();

Modified: lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp?rev=257210&r1=257209&r2=257210&view=diff
==============================================================================
--- lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp (original)
+++ lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp Fri Jan  8 15:40:11 2016
@@ -135,7 +135,8 @@ ThreadPlanShouldStopHere::DefaultStepFro
                                                                                          stop_others,
                                                                                          eVoteNo,
                                                                                          eVoteNoOpinion,
-                                                                                         frame_index);
+                                                                                         frame_index,
+                                                                                         true);
     return return_plan_sp;
 }
 

Modified: lldb/trunk/source/Target/ThreadPlanStepOut.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ThreadPlanStepOut.cpp?rev=257210&r1=257209&r2=257210&view=diff
==============================================================================
--- lldb/trunk/source/Target/ThreadPlanStepOut.cpp (original)
+++ lldb/trunk/source/Target/ThreadPlanStepOut.cpp Fri Jan  8 15:40:11 2016
@@ -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 @@ ThreadPlanStepOut::ThreadPlanStepOut
     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 continue_to_next_branch
 ) :
     ThreadPlan (ThreadPlan::eKindStepOut, "Step out", thread, stop_vote, run_vote),
     ThreadPlanShouldStopHere (this),
@@ -86,7 +88,8 @@ ThreadPlanStepOut::ThreadPlanStepOut
                                                                      eVoteNoOpinion,
                                                                      eVoteNoOpinion,
                                                                      frame_idx - 1,
-                                                                     eLazyBoolNo));
+                                                                     eLazyBoolNo,
+                                                                     continue_to_next_branch));
             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,27 @@ ThreadPlanStepOut::ThreadPlanStepOut
         // 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 (continue_to_next_branch)
+        {
+            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);
+            if (return_address_sc.line_entry.IsValid())
+            {
+                range = return_address_sc.line_entry.GetSameLineContiguousAddressRange();
+                if (range.GetByteSize() > 0)
+                {
+                    return_address = m_thread.GetProcess()->AdvanceAddressToNextBranchInstruction (return_address, 
+                                                                                                   range);
+                }
+            }
+        }
+        m_return_addr = return_address.GetLoadAddress(&m_thread.GetProcess()->GetTarget());
         
         if (m_return_addr == LLDB_INVALID_ADDRESS)
             return;

Modified: lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp?rev=257210&r1=257209&r2=257210&view=diff
==============================================================================
--- lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp (original)
+++ lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp Fri Jan  8 15:40:11 2016
@@ -185,7 +185,8 @@ ThreadPlanStepOverRange::ShouldStop (Eve
                                                                              stop_others,
                                                                              eVoteNo,
                                                                              eVoteNoOpinion,
-                                                                             0);
+                                                                             0,
+                                                                             true);
                 break;
             }
             else




More information about the lldb-commits mailing list