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