Thanks for the help with this.  

The problem is that lldb would try to advance the return breakpoint in 
no_debug_caller:

(lldb) disass -b
a.out`no_debug_caller:
    0x4005dd <+0>:  55                    pushq  %rbp
    0x4005de <+1>:  48 89 e5              movq   %rsp, %rbp
    0x4005e1 <+4>:  48 83 ec 20           subq   $0x20, %rsp
    0x4005e5 <+8>:  89 7d ec              movl   %edi, -0x14(%rbp)
    0x4005e8 <+11>: 48 89 75 e0           movq   %rsi, -0x20(%rbp)
    0x4005ec <+15>: c7 45 fc 00 00 00 00  movl   $0x0, -0x4(%rbp)
    0x4005f3 <+22>: 48 8b 55 e0           movq   -0x20(%rbp), %rdx
    0x4005f7 <+26>: 8b 45 ec              movl   -0x14(%rbp), %eax
    0x4005fa <+29>: 48 89 d6              movq   %rdx, %rsi
    0x4005fd <+32>: 89 c7                 movl   %eax, %edi
    0x4005ff <+34>: e8 b0 ff ff ff        callq  0x4005b4                  ; 
no_debug_caller_intermediate
    0x400604 <+39>: 89 45 fc              movl   %eax, -0x4(%rbp)
    0x400607 <+42>: 8b 45 fc              movl   -0x4(%rbp), %eax
    0x40060a <+45>: c9                    leave  
    0x40060b <+46>: c3                    retq   

which has no source line information so ThreadPlanStepOut::ThreadPlanStepOut() 
would advance that to the last instruction of the function (retq) and stop 
there.  This is arguably not necessary or even helpful, so I could simply 
change ThreadPlanStepOut::ThreadPlanStepOut to not advance the pc unless we 
have source line information (the only time we're really doing anything to help 
the user / performance).

When we stop on the retq instruction, we expose the fact that lldb doesn't know 
what the 'leave' instruction does so the unwind information is wrong.  Instead 
of finding the caller is main(), we find the stack frame above that, 
__libc_start_main() which has some random debug info for line 321 (which is how 
that line # came up in the testsuite failure).  You can see this with the 
unwind info difference between the assembly profiling and the eh_frame 
information:

Assembly language inspection UnwindPlan:
This UnwindPlan originally sourced from assembly insn profiling
This UnwindPlan is sourced from the compiler: no.
This UnwindPlan is valid at all instruction locations: yes.
Address range of this UnwindPlan: [a.out..text + 413-0x00000000000001cc)
row[0]:    0: CFA=rsp +8 => rsp=CFA+0 rip=[CFA-8] 
row[1]:    1: CFA=rsp+16 => rbp=[CFA-16] rsp=CFA+0 rip=[CFA-8] 
row[2]:    4: CFA=rbp+16 => rbp=[CFA-16] rsp=CFA+0 rip=[CFA-8] 
row[3]:   46: CFA=rbp+16 => rbp=[CFA-16] rsp=CFA+0 rip=[CFA-8] 

eh_frame UnwindPlan:
This UnwindPlan originally sourced from eh_frame CFI
This UnwindPlan is sourced from the compiler: yes.
This UnwindPlan is valid at all instruction locations: no.
Address range of this UnwindPlan: [a.out..text + 413-0x00000000000001cc)
row[0]:    0: CFA=rsp +8 => rip=[CFA-8] 
row[1]:    1: CFA=rsp+16 => rbp=[CFA-16] rip=[CFA-8] 
row[2]:    4: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8] 
row[3]:   46: CFA=rsp +8 => rbp=[CFA-16] rip=[CFA-8] 

clang doesn't emit the LEAVE instruction which is why I didn't see this on the 
mac when I was testing there.



I have a patch to handle the LEAVE instruction but I need to head home in a bit 
- I'll check that in tomorrow after I've had a chance to look at it more 
closely.  And I'll talk to Jim about advancing the pc when we don't have line 
level information; I'm inclined to not do that. 

Thanks again - having the same binary that was failing was a big help.



> 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

Reply via email to