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 <sivachan...@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@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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
