jasonmolenda created this revision.
jasonmolenda added a reviewer: jingham.
jasonmolenda added a subscriber: lldb-commits.
jasonmolenda set the repository for this revision to rL LLVM.

This is a performance optimization.  

Some compilers, for instance clang, may indicate subexpressions on a source 
line by emitting multiple line table entries and using the column information 
to indicate the different expressions present in that source line.

lldb's step / next commands have a fast-stepping mode where they look at the 
instructions within the address range of a source line, and if there are no 
flow control instructions (branch, jump, call, return), it will put a 
breakpoint at the end of the source line address range or on the flow control 
instructions and continue to that breakpoint instead of instruction stepping.

When we have multiple line table entries for a source line, though, this means 
that we'll only fast-step through each sub-expression.  For instance, at -O0, 
for the input

int main (int argc, char **argv) { 
int c = argc + argc + argc + argc + argc;
return c;
}

clang may emit six line table entries for the 'int c = ...' line.  (at anything 
but -O0, this whole program probably turns into three assembly instructions).  
The patch has a few pieces.

One: A new method on LineEntry (the object from which we get a line table 
entry's address range) to get the address range for this line table entry, plus 
any entries for the same source line that are contiguous.

Two: New overloaded methods QueueThreadPlanForStepOverRange and 
QueueThreadPlanForStepInRange which take a LineEntry instead of an 
AddressRange.  There are some callers of these methods 
(SBThreadPlan::QueueThreadPlanForStepInRange, 
SBThreadPlan::QueueThreadPlanForStepOverRange) which actually do need to use a 
byte range but most of the callers should pass in a LineEntry instead of 
resolving it down to an AddressRange.

Three: Update ThreadPlanStepRange::InRange() so when it has branched to a new 
part of a line table entry, it extends that one as well.  (we may have branched 
over a different line number's code, and our old address range may not extend 
as far as it should.)

There is one additional wrinkle in that the swift compiler will emit line table 
entries with a line number of 0 to indicate that they are compiler-generated 
code, not user authored code, and the debugger should strive to skip over those 
source lines.  If LineEntry::GetSameLineContiguousAddressRange is called with a 
LineEntry of 0, it will scan forward until it finds its first non-0 LineEntry, 
and use that line number as the line number it is trying to extend.  If it is 
combining LineEntries for a concrete line number and finds a LineEntry with 
line 0, it will add that to the address range and continue to scan for more 
line table entries of the same real line number.


For the new overloaded methods in Thread, QueueThreadPlanForStepOverRange, 
QueueThreadPlanForStepInRange, maybe it would be better to call these 
QueueThreadPlanForStepInLineEntry ?  The "Range" may imply that it's accepting 
an AddressRange, or it may just mean that we're stepping in a range of code.  
I'm unable to decide on this.

Repository:
  rL LLVM

http://reviews.llvm.org/D15407

Files:
  include/lldb/Symbol/LineEntry.h
  include/lldb/Target/Thread.h
  source/API/SBThread.cpp
  source/Commands/CommandObjectThread.cpp
  source/Symbol/LineEntry.cpp
  source/Target/Thread.cpp
  source/Target/ThreadPlanStepRange.cpp

Index: source/Target/ThreadPlanStepRange.cpp
===================================================================
--- source/Target/ThreadPlanStepRange.cpp
+++ source/Target/ThreadPlanStepRange.cpp
@@ -162,7 +162,7 @@
                 if (m_addr_context.line_entry.line == new_context.line_entry.line)
                 {
                     m_addr_context = new_context;
-                    AddRange(m_addr_context.line_entry.range);
+                    AddRange(m_addr_context.line_entry.GetSameLineContiguousAddressRange());
                     ret_value = true;
                     if (log)
                     {
@@ -181,7 +181,7 @@
                 {
                     new_context.line_entry.line = m_addr_context.line_entry.line;
                     m_addr_context = new_context;
-                    AddRange(m_addr_context.line_entry.range);
+                    AddRange(m_addr_context.line_entry.GetSameLineContiguousAddressRange());
                     ret_value = true;
                     if (log)
                     {
Index: source/Target/Thread.cpp
===================================================================
--- source/Target/Thread.cpp
+++ source/Target/Thread.cpp
@@ -1535,7 +1535,18 @@
     return thread_plan_sp;
 }
 
+// Call the QueueThreadPlanForStepOverRange method which takes an address range.
 ThreadPlanSP
+Thread::QueueThreadPlanForStepOverRange(bool abort_other_plans,
+                                        const LineEntry &line_entry,
+                                        const SymbolContext &addr_context,
+                                        lldb::RunMode stop_other_threads,
+                                        LazyBool step_out_avoids_code_withoug_debug_info)
+{
+    return QueueThreadPlanForStepOverRange (abort_other_plans, line_entry.GetSameLineContiguousAddressRange(), addr_context, stop_other_threads, step_out_avoids_code_withoug_debug_info);
+}
+
+ThreadPlanSP
 Thread::QueueThreadPlanForStepInRange(bool abort_other_plans,
                                       const AddressRange &range,
                                       const SymbolContext &addr_context,
@@ -1559,7 +1570,21 @@
     return thread_plan_sp;
 }
 
+// Call the QueueThreadPlanForStepInRange method which takes an address range.
 ThreadPlanSP
+Thread::QueueThreadPlanForStepInRange(bool abort_other_plans,
+                                      const LineEntry &line_entry,
+                                      const SymbolContext &addr_context,
+                                      const char *step_in_target,
+                                      lldb::RunMode stop_other_threads,
+                                      LazyBool step_in_avoids_code_without_debug_info,
+                                      LazyBool step_out_avoids_code_without_debug_info)
+{
+    return QueueThreadPlanForStepInRange (abort_other_plans, line_entry.GetSameLineContiguousAddressRange(), addr_context, step_in_target, stop_other_threads, step_in_avoids_code_without_debug_info, step_out_avoids_code_without_debug_info);
+}
+
+
+ThreadPlanSP
 Thread::QueueThreadPlanForStepOut(bool abort_other_plans,
                                   SymbolContext *addr_context,
                                   bool first_insn,
@@ -2374,7 +2399,7 @@
         {
             SymbolContext sc(frame_sp->GetSymbolContext(eSymbolContextEverything));
             new_plan_sp = QueueThreadPlanForStepInRange (abort_other_plans,
-                                                         sc.line_entry.range,
+                                                         sc.line_entry,
                                                          sc,
                                                          NULL,
                                                          run_mode,
@@ -2420,7 +2445,7 @@
         {
             SymbolContext sc(frame_sp->GetSymbolContext(eSymbolContextEverything));
             new_plan_sp = QueueThreadPlanForStepOverRange (abort_other_plans,
-                                                           sc.line_entry.range,
+                                                           sc.line_entry,
                                                            sc,
                                                            run_mode,
                                                            step_out_avoids_code_without_debug_info);
Index: source/Symbol/LineEntry.cpp
===================================================================
--- source/Symbol/LineEntry.cpp
+++ source/Symbol/LineEntry.cpp
@@ -244,3 +244,56 @@
     return FileSpec::Compare (a.file, b.file, true);
 }
 
+AddressRange
+LineEntry::GetSameLineContiguousAddressRange () const
+{
+    // Add each LineEntry's range to complete_line_range until we find
+    // a different file / line number.
+    AddressRange complete_line_range = range;
+
+    // "line" may be 0 if this LineEntry is compiler-generated code.  In that case,
+    // the first non-0 LineEntry we find will be called our "real" line number and
+    // we will use that for all matches.
+    uint32_t line_number_to_match = line;
+
+    while (true)
+    {
+        SymbolContext next_line_sc;
+        Address range_end (complete_line_range.GetBaseAddress());
+        range_end.Slide (complete_line_range.GetByteSize());
+        range_end.CalculateSymbolContext (&next_line_sc, lldb::eSymbolContextLineEntry);
+
+        if (next_line_sc.line_entry.IsValid() == false || next_line_sc.line_entry.range.GetByteSize() == 0)
+        {
+            // no infinite looping allowed
+            break;
+        }
+        if (file == next_line_sc.line_entry.file)
+        {
+            // If we started with a line number of 0, get a non-zero line number as soon as we see one.
+            if (line_number_to_match == 0)
+            {
+                line_number_to_match = next_line_sc.line_entry.line;
+            }
+
+            // Include any line 0 contiguous entries - they indicate that this is 
+            // compiler-generated code that does not correspond to user source code.
+            if (next_line_sc.line_entry.line == 0)
+            {
+                complete_line_range.SetByteSize (complete_line_range.GetByteSize() + next_line_sc.line_entry.range.GetByteSize());
+                continue;
+            }
+
+            if (line_number_to_match == next_line_sc.line_entry.line)
+            {
+                // next_line_sc is the same file & line as this LineEntry, so extend our
+                // AddressRange by its size and continue to see if there are more LineEntries
+                // that we can combine.
+                complete_line_range.SetByteSize (complete_line_range.GetByteSize() + next_line_sc.line_entry.range.GetByteSize());
+                continue;
+            }
+        }
+        break;
+    }
+    return complete_line_range;
+}
Index: source/Commands/CommandObjectThread.cpp
===================================================================
--- source/Commands/CommandObjectThread.cpp
+++ source/Commands/CommandObjectThread.cpp
@@ -586,7 +586,7 @@
             if (frame->HasDebugInformation ())
             {
                 new_plan_sp = thread->QueueThreadPlanForStepInRange (abort_other_plans,
-                                                                frame->GetSymbolContext(eSymbolContextEverything).line_entry.range, 
+                                                                frame->GetSymbolContext(eSymbolContextEverything).line_entry,
                                                                 frame->GetSymbolContext(eSymbolContextEverything),
                                                                 m_options.m_step_in_target.c_str(),
                                                                 stop_other_threads,
@@ -609,7 +609,7 @@
 
             if (frame->HasDebugInformation())
                 new_plan_sp = thread->QueueThreadPlanForStepOverRange (abort_other_plans,
-                                                                    frame->GetSymbolContext(eSymbolContextEverything).line_entry.range, 
+                                                                    frame->GetSymbolContext(eSymbolContextEverything).line_entry,
                                                                     frame->GetSymbolContext(eSymbolContextEverything), 
                                                                     stop_other_threads,
                                                                     m_options.m_step_out_avoid_no_debug);
Index: source/API/SBThread.cpp
===================================================================
--- source/API/SBThread.cpp
+++ source/API/SBThread.cpp
@@ -747,7 +747,7 @@
                 const LazyBool avoid_no_debug = eLazyBoolCalculate;
                 SymbolContext sc(frame_sp->GetSymbolContext(eSymbolContextEverything));
                 new_plan_sp = thread->QueueThreadPlanForStepOverRange (abort_other_plans,
-                                                                    sc.line_entry.range,
+                                                                    sc.line_entry,
                                                                     sc,
                                                                     stop_other_threads,
                                                                     avoid_no_debug);
@@ -799,7 +799,7 @@
             const LazyBool step_in_avoids_code_without_debug_info = eLazyBoolCalculate;
             SymbolContext sc(frame_sp->GetSymbolContext(eSymbolContextEverything));
             new_plan_sp = thread->QueueThreadPlanForStepInRange (abort_other_plans,
-                                                              sc.line_entry.range,
+                                                              sc.line_entry,
                                                               sc,
                                                               target_name,
                                                               stop_other_threads,
Index: include/lldb/Target/Thread.h
===================================================================
--- include/lldb/Target/Thread.h
+++ include/lldb/Target/Thread.h
@@ -754,6 +754,15 @@
                                      lldb::RunMode stop_other_threads,
                                      LazyBool step_out_avoids_code_without_debug_info = eLazyBoolCalculate);
 
+    // Helper function that takes a LineEntry to step, insted of an AddressRange.  This may combine multiple
+    // LineEntries of the same source line number to step over a longer address range in a single operation.
+    virtual lldb::ThreadPlanSP
+    QueueThreadPlanForStepOverRange (bool abort_other_plans,
+                                     const LineEntry &line_entry,
+                                     const SymbolContext &addr_context,
+                                     lldb::RunMode stop_other_threads,
+                                     LazyBool step_out_avoids_code_without_debug_info = eLazyBoolCalculate);
+
     //------------------------------------------------------------------
     /// Queues the plan used to step through an address range, stepping into functions.
     ///
@@ -800,6 +809,17 @@
                                    LazyBool step_in_avoids_code_without_debug_info = eLazyBoolCalculate,
                                    LazyBool step_out_avoids_code_without_debug_info = eLazyBoolCalculate);
 
+    // Helper function that takes a LineEntry to step, insted of an AddressRange.  This may combine multiple
+    // LineEntries of the same source line number to step over a longer address range in a single operation.
+    virtual lldb::ThreadPlanSP
+    QueueThreadPlanForStepInRange (bool abort_other_plans,
+                                   const LineEntry &line_entry,
+                                   const SymbolContext &addr_context,
+                                   const char *step_in_target,
+                                   lldb::RunMode stop_other_threads,
+                                   LazyBool step_in_avoids_code_without_debug_info = eLazyBoolCalculate,
+                                   LazyBool step_out_avoids_code_without_debug_info = eLazyBoolCalculate);
+
     //------------------------------------------------------------------
     /// Queue the plan used to step out of the function at the current PC of
     /// \a thread.
Index: include/lldb/Symbol/LineEntry.h
===================================================================
--- include/lldb/Symbol/LineEntry.h
+++ include/lldb/Symbol/LineEntry.h
@@ -140,6 +140,20 @@
     static int
     Compare (const LineEntry& lhs, const LineEntry& rhs);
 
+    //------------------------------------------------------------------
+    /// Give the range for this LineEntry + any additional LineEntries for
+    /// this same source line that are contiguous.
+    ///
+    /// A compiler may emit multiple line entries for a single source line,
+    /// e.g. to indicate subexpressions at different columns.  This method
+    /// will get the AddressRange for all of the LineEntries for this source 
+    /// line that are contiguous.
+    ///
+    /// @return
+    ///     The contiguous AddressRange for this source line.
+    //------------------------------------------------------------------
+    AddressRange
+    GetSameLineContiguousAddressRange () const;
 
     //------------------------------------------------------------------
     // Member variables.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to