[Lldb-commits] [lldb] r257117 - Performance improvement: Change lldb so that it puts a breakpoint

Siva Chandra via lldb-commits lldb-commits at lists.llvm.org
Thu Jan 7 18:16:33 PST 2016


This broke TestStepNoDebug, atleast on Linux when inferior is compiled
with GCC: http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/10091

I am able to reproduce this on my machine. Let me know if you need any
help with anything.

On Thu, Jan 7, 2016 at 4:06 PM, Jason Molenda via lldb-commits
<lldb-commits at lists.llvm.org> wrote:
> Author: jmolenda
> Date: Thu Jan  7 18:06:03 2016
> New Revision: 257117
>
> URL: http://llvm.org/viewvc/llvm-project?rev=257117&view=rev
> Log:
> 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=257117&r1=257116&r2=257117&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Target/Process.h (original)
> +++ lldb/trunk/include/lldb/Target/Process.h Thu Jan  7 18:06:03 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=257117&r1=257116&r2=257117&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Target/Thread.h (original)
> +++ lldb/trunk/include/lldb/Target/Thread.h Thu Jan  7 18:06:03 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=257117&r1=257116&r2=257117&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Target/ThreadPlanStepOut.h (original)
> +++ lldb/trunk/include/lldb/Target/ThreadPlanStepOut.h Thu Jan  7 18:06:03 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=257117&r1=257116&r2=257117&view=diff
> ==============================================================================
> --- lldb/trunk/source/Target/Process.cpp (original)
> +++ lldb/trunk/source/Target/Process.cpp Thu Jan  7 18:06:03 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=257117&r1=257116&r2=257117&view=diff
> ==============================================================================
> --- lldb/trunk/source/Target/Thread.cpp (original)
> +++ lldb/trunk/source/Target/Thread.cpp Thu Jan  7 18:06:03 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=257117&r1=257116&r2=257117&view=diff
> ==============================================================================
> --- lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp (original)
> +++ lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp Thu Jan  7 18:06:03 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=257117&r1=257116&r2=257117&view=diff
> ==============================================================================
> --- lldb/trunk/source/Target/ThreadPlanStepOut.cpp (original)
> +++ lldb/trunk/source/Target/ThreadPlanStepOut.cpp Thu Jan  7 18:06:03 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,35 @@ 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 | 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);
> +            }
> +        }
> +        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=257117&r1=257116&r2=257117&view=diff
> ==============================================================================
> --- lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp (original)
> +++ lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp Thu Jan  7 18:06:03 2016
> @@ -185,7 +185,8 @@ ThreadPlanStepOverRange::ShouldStop (Eve
>                                                                               stop_others,
>                                                                               eVoteNo,
>                                                                               eVoteNoOpinion,
> -                                                                             0);
> +                                                                             0,
> +                                                                             true);
>                  break;
>              }
>              else
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


More information about the lldb-commits mailing list