[Lldb-commits] [lldb] r257117 - Performance improvement: Change lldb so that it puts a breakpoint
Jason Molenda via lldb-commits
lldb-commits at lists.llvm.org
Thu Jan 7 18:24:17 PST 2016
Thanks Siva, I'll revert the change. I have access to a linux machine but I haven't used it in a long time. Would it be easy to send me the binary (with debug info) for this test case so I can see what gcc emitted here and figure out how it went wrong? This test passes with clang on x86_64 darwin but there must be something in gcc's debug info that is confusing it.
Jason
> On Jan 7, 2016, at 6:16 PM, Siva Chandra <sivachandra at google.com> wrote:
>
> 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