[Lldb-commits] [lldb] r257138 - Revert r257117 "Performance improvement: Change lldb so that it

Jason Molenda via lldb-commits lldb-commits at lists.llvm.org
Thu Jan 7 18:26:05 PST 2016


Author: jmolenda
Date: Thu Jan  7 20:26:03 2016
New Revision: 257138

URL: http://llvm.org/viewvc/llvm-project?rev=257138&view=rev
Log:
Revert r257117 "Performance improvement: Change lldb so that it
puts a breakpoint" it is causing a regression in the TestStepNoDebug
test case on ubuntu 14.04 with gcc 4.9.2.  Thanks for the email
Siva.  I'll recommit when I've figured out the regression.


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=257138&r1=257137&r2=257138&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Thu Jan  7 20:26:03 2016
@@ -3149,34 +3149,6 @@ 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=257138&r1=257137&r2=257138&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Thread.h (original)
+++ lldb/trunk/include/lldb/Target/Thread.h Thu Jan  7 20:26:03 2016
@@ -888,16 +888,6 @@ 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.
     //------------------------------------------------------------------
@@ -908,8 +898,7 @@ public:
                                            bool stop_other_threads,
                                            Vote stop_vote, // = eVoteYes,
                                            Vote run_vote, // = eVoteNoOpinion);
-                                           uint32_t frame_idx,
-                                           bool continue_to_next_branch = false);
+                                           uint32_t frame_idx);
 
     //------------------------------------------------------------------
     /// 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=257138&r1=257137&r2=257138&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/ThreadPlanStepOut.h (original)
+++ lldb/trunk/include/lldb/Target/ThreadPlanStepOut.h Thu Jan  7 20:26:03 2016
@@ -31,8 +31,7 @@ public:
                        Vote stop_vote,
                        Vote run_vote,
                        uint32_t frame_idx,
-                       LazyBool step_out_avoids_code_without_debug_info,
-                       bool continue_to_next_branch = false);
+                       LazyBool step_out_avoids_code_without_debug_info);
 
     ~ThreadPlanStepOut() override;
 

Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=257138&r1=257137&r2=257138&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Thu Jan  7 20:26:03 2016
@@ -6515,65 +6515,3 @@ 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=257138&r1=257137&r2=257138&view=diff
==============================================================================
--- lldb/trunk/source/Target/Thread.cpp (original)
+++ lldb/trunk/source/Target/Thread.cpp Thu Jan  7 20:26: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_without_debug_info)
+                                  LazyBool step_out_avoids_code_withoug_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_without_debug_info));
+                                                        step_out_avoids_code_withoug_debug_info));
     
     if (thread_plan_sp->ValidatePlan(nullptr))
     {
@@ -1620,8 +1620,7 @@ Thread::QueueThreadPlanForStepOutNoShoul
                                               bool stop_other_threads,
                                               Vote stop_vote,
                                               Vote run_vote,
-                                              uint32_t frame_idx,
-                                              bool continue_to_next_branch)
+                                              uint32_t frame_idx)
 {
     ThreadPlanSP thread_plan_sp(new ThreadPlanStepOut (*this,
                                                         addr_context, 
@@ -1630,8 +1629,7 @@ Thread::QueueThreadPlanForStepOutNoShoul
                                                         stop_vote, 
                                                         run_vote, 
                                                         frame_idx,
-                                                        eLazyBoolNo,
-                                                        continue_to_next_branch));
+                                                        eLazyBoolNo));
 
     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=257138&r1=257137&r2=257138&view=diff
==============================================================================
--- lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp (original)
+++ lldb/trunk/source/Target/ThreadPlanShouldStopHere.cpp Thu Jan  7 20:26:03 2016
@@ -135,8 +135,7 @@ ThreadPlanShouldStopHere::DefaultStepFro
                                                                                          stop_others,
                                                                                          eVoteNo,
                                                                                          eVoteNoOpinion,
-                                                                                         frame_index,
-                                                                                         true);
+                                                                                         frame_index);
     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=257138&r1=257137&r2=257138&view=diff
==============================================================================
--- lldb/trunk/source/Target/ThreadPlanStepOut.cpp (original)
+++ lldb/trunk/source/Target/ThreadPlanStepOut.cpp Thu Jan  7 20:26:03 2016
@@ -18,7 +18,6 @@
 #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"
@@ -45,8 +44,7 @@ ThreadPlanStepOut::ThreadPlanStepOut
     Vote stop_vote,
     Vote run_vote,
     uint32_t frame_idx,
-    LazyBool step_out_avoids_code_without_debug_info,
-    bool continue_to_next_branch
+    LazyBool step_out_avoids_code_without_debug_info
 ) :
     ThreadPlan (ThreadPlan::eKindStepOut, "Step out", thread, stop_vote, run_vote),
     ThreadPlanShouldStopHere (this),
@@ -88,8 +86,7 @@ ThreadPlanStepOut::ThreadPlanStepOut
                                                                      eVoteNoOpinion,
                                                                      eVoteNoOpinion,
                                                                      frame_idx - 1,
-                                                                     eLazyBoolNo,
-                                                                     continue_to_next_branch));
+                                                                     eLazyBoolNo));
             static_cast<ThreadPlanStepOut *>(m_step_out_to_inline_plan_sp.get())->SetShouldStopHereCallbacks(nullptr, nullptr);
             m_step_out_to_inline_plan_sp->SetPrivate(true);
         }
@@ -104,35 +101,7 @@ ThreadPlanStepOut::ThreadPlanStepOut
         // Find the return address and set a breakpoint there:
         // FIXME - can we do this more securely if we know first_insn?
 
-        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());
+        m_return_addr = return_frame_sp->GetFrameCodeAddress().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=257138&r1=257137&r2=257138&view=diff
==============================================================================
--- lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp (original)
+++ lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp Thu Jan  7 20:26:03 2016
@@ -185,8 +185,7 @@ ThreadPlanStepOverRange::ShouldStop (Eve
                                                                              stop_others,
                                                                              eVoteNo,
                                                                              eVoteNoOpinion,
-                                                                             0,
-                                                                             true);
+                                                                             0);
                 break;
             }
             else




More information about the lldb-commits mailing list