Mark the current column when displaying the context of the current breakpoint on the terminal.

[tfiala]

Summary

Here's a fun little idea plus a preliminary patch implementing it.

When I'm debugging programs I often wonder what exactly will happen when I step-in. This is particularly a problem with code that has lots of control flow happening on a single line, such as a C++ for loop with iterators, a function call with a lambda or block definition, or plain old nested function calls.

For example, let's say I'm stopped inside lldb in SourceManager.cpp:93 — I would really like to know which function I'll be stepping into next. Will the next "step" take me into get(), new, File(), or reset()?

   90  	    // If file_sp is no good or it points to a non-existent file, reset it.
   91  	    if (!file_sp || !file_sp->GetFileSpec().Exists())
   92  	    {
-> 93  	        file_sp.reset (new File (file_spec, target_sp.get()));
   94  	
   95  	        if (debugger_sp)
   96  	            debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
(lldb) step

Of course a debugger cannot predict the future, but what it can do is tell me exactly where I am stopped now!
Compilers like clang already include column information in the debug info by default. The attached patch makes use of this by adding an underline attribute to the character on the current column to indicate the exact breakpoint on the current line (here simulated with a caret):

   90      // If file_sp is no good or it points to a non-existent file, reset it.
   91      if (!file_sp || !file_sp->GetFileSpec().Exists())
   92      {
-> 93          file_sp.reset (new File (file_spec, target_sp.get()));
                                  ^
   94  
   95          if (debugger_sp)
   96              debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
(lldb) step

With this markup I may now assume that "step" will take me into the File() constructor. Just what I wanted to know.

This is of course just scratching the surface of what we could do with column information, but probably a good starting point. Having a more fine-grained visualization, for example, it might be interesting to have "next" take me to the next "is_stmt" in the line table instead of always to the next line, and so on.

Let me know what you think!

[jingham]

This looks fine. Can you add a setting in the "stop-line" vein that will turn on and off the column listing. Seems odd to control all the other stuff, but not this.

[aprantl]

I was also wondering if there was a preferred way to detect whether LLDB is connected to a dumb terminal. I noticed that Stream has an eANSI flag, but it seemed to be always zero.

[aprantl]

Added a use-column-info setting.

[jingham]

I wouldn't call the setting "use-column-info". It would be better to call it "stop-print-column-info" or something that makes it clear this is just for printing. If we get better column info & start using it for stepping, etc, we might want to provide a way to turn that off too, but independently from this stop-info printing.

[clayborg]

"display-column-info"?

[jingham]

All the other controls for printing the stop info have "stop-*". It would be clearer to keep doing this, I think.

Jim

[aprantl]

Rename the property to "stop-show-column" and query the stream whether ansi escape sequences are allowed.

[jingham]

Looks fine to me.

[aprantl]

I have one last question:
While trying to add a testcase I noticed that DisplaySourceLinesWithLineNumbers is still exported into Python with the old signature. What is the correct way to update the Python bindings?

[tfiala]

I'm going to help Adrian land this patch.

I'm looking into this now.

[tfiala]

In D20835#446040, @aprantl wrote:

I have one last question:
While trying to add a testcase I noticed that DisplaySourceLinesWithLineNumbers is still exported into Python with the old signature. What is the correct way to update the Python bindings?

On the LLVM.org side, the bindings are usually updated fine if all the swig input files are updated (so the .i files I think would be relevant here).

[tfiala]

This will need another change.

Existing SBAPI methods cannot have their signatures modified per the SBAPI contract. We'll need to add an entirely new method on SBSourceManager with the new signature.

I'll just add newer version of DisplaySourceLinesWithLineNumbers() with a slightly modified name, maybe:
DisplaySourceLinesWithLineNumbersAndColumnInfo(...).

[aprantl]

I'm abandon this review so tfiala can claim it.

[tfiala]

Grabbing it.

I had to get that DarwinLog support wrapped up (it was in process for something like a month). Now that I wrapped that up, I'll get back to this.

[tfiala]

Reclaiming. I'll be working on this now.

[tfiala]

This will unfortunately get a little messy due to the code reformatting. My first patch up for it will be to get the existing impl refreshed for the code reformatting.

[tfiala]

Updated patch, tweaked and verified it is working, rebased against r281243.

I'd like to add some unittests for this, so this isn't ready quite yet.

[tfiala]

This change does the following:

• Removes storing the ANSI escape support flag on the stream and directly consults the Debugger::GetUseColor() method. We should be able to change this during a debug session (via settings set use-color {true|false}) and have it take effect immediately.
• Adds a test case for the use-color case. (The previous change only had a test for the no-color case).

This is ready for a proper review.

[tfiala]

Minor tweak:

• Shut off color usage in the shared object unloading test. One of the screen-scraped results it is checking for is not set up to handle the ansi underline sequence on the column where the process is stopped.

[jingham]

See inlined comments. Otherwise this is great.

include/lldb/Core/SourceManager.h

34–35

A target always has a debugger, so given a target you can always get to the debugger. If you have another use case where you need to make this File, but only have a DebuggerSP, can you make a constructor that takes a FileSpec & a DebuggerSP, but not a target? That seems clearer to me.

source/Commands/CommandObjectSource.cpp

1155–1158

I think I will occasionally want to see the source listing w/o the column stop info (e.g. to cut & paste it somewhere) so it might be convenient to have a command option that overrides the setting. It's always annoying to have to go change & reset a setting to do this, plus then I can't make aliases for the different behaviors.

[tfiala]

In D20835#541901, @jingham wrote:

See inlined comments. Otherwise this is great.

Great, I'll fix those up. I like the command-line option idea.

[tfiala]

Greg had a few pieces of feedback as well that we just discussed:

• Change the on/off switch for column marking to support the following states:
  • terminal-code-only (i.e. only do terminal code highlighting, not the text caret)
  • terminal-code + text caret
  • text caret only
  • off

• Add a pre and post terminal-code-format format string option for the marker. This allows the user to modify what kind of highlighting the user can do. It will default to the way I'm doing it now, which is ansi underline.

[tfiala]

packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py

163 ↗

(On Diff #71262)

Upon further reflection, there is a logically more appropriate approach. The better one is to explicitly turn off column marking and not count on the fact that the column marker happens on the line below the stop match text. (This test initially failed because the stop text that is scraped was getting confused by the newly-added ansi escape sequences around the first character).

[tfiala]

Coming back to this today.

[tfiala]

This change addresses Jim's and Greg's suggestions.

• stop-show-column now is a quad-state:
  • ansi-or-caret (default): Shows ANSI if color is enabled; otherwise, uses the text-based caret.
  • ansi-only: ANSI highlighting is used if color is enabled; otherwise, no column marking.
  • caret-only: text-based caret on the following line will always be used when showing stop column.
  • none: no column marking is done.

• stop-show-column-decorator-pre: this is a format entity (with tab completion for ${ansi.*} codes). It defaults to the start code for underlining (${ansi.underline}), the default column stop marker display style when using ANSI highlighting.
• stop-show-column-decorator-post: this is the format entity for disabling whatever was enabled in the "-pre" setting. This defaults to ${ansi.normal}. This would need to be changed if color codes were used for background/foreground of anything other than inverse.

• other minor odds and ends, like changing the constructor of SourceManager::File to either take a Target or a Debugger, but not both.

[tfiala]

source/Core/Disassembler.cpp

292–294

I will leave this for a potential follow-up patch at another time. At that point, I would suggest enabling stop line and column marking on the "source list" command, which currently doesn't mark up a displayed line that coincides with any currently-stopped thread.

I'll strip out this TODO on the final check-in.

[clayborg]

A few naming requests.

source/Core/Debugger.cpp

151

Make this "ansi" instead of "ansi-only"?

154

Make this "caret" instead?

199

Make this "stop-show-column-ansi-prefix"?

204

Make this "stop-show-column-ansi-suffix"?

[tfiala]

Updates for Greg's latest comments.

[clayborg]

Looks good.

[tfiala]

(I'll close this out as soon as Jim accepts).

[tfiala]

Thanks, Jim!

include/lldb/API/SBSourceManager.h

     size_t
     DisplaySourceLinesWithLineNumbers (const lldb::SBFileSpec &file,
                                        uint32_t line,
+                                       uint32_t column,
                                        uint32_t context_before,
                                        uint32_t context_after,
                                        const char* current_line_cstr,

include/lldb/Core/Debugger.h

     
     bool
     SetUseColor (bool use_color);
-    
+    bool
+    GetStopShowColumn () const;
+
     uint32_t
     GetStopSourceLineCount (bool before) const;
-    
     StopDisassemblyType
     GetStopDisassemblyDisplay () const;
     

include/lldb/Core/SourceManager.h

 
         size_t
         DisplaySourceLines (uint32_t line,
-                            uint32_t context_before,
-                            uint32_t context_after,
+                            uint32_t column,
                             Stream *s);
         void
         FindLinesMatchingRegex (RegularExpression& regex, 

     size_t
     DisplaySourceLinesWithLineNumbers(const FileSpec &file,
                                       uint32_t line,
+                                      uint32_t column,
                                       uint32_t context_before,
                                       uint32_t context_after,
                                       const char* current_line_cstr,

     DisplaySourceLinesWithLineNumbersUsingLastFile(uint32_t start_line,
                                                    uint32_t count,
                                                    uint32_t curr_line,
+                                                   uint32_t curr_column,
                                                    const char* current_line_cstr,
                                                    Stream *s,
                                                    const SymbolContextList *bp_locs = nullptr);

packages/Python/lldbsuite/test/settings/TestSettings.py

                                  "stop-disassembly-display",
                                  "stop-line-count-after",
                                  "stop-line-count-before",
+                                 "stop-show-column",
                                  "term-width",
                                  "thread-format",
                                  "use-external-editor",

scripts/interface/SBSourceManager.i

         stream = lldb.SBStream()
         source_mgr.DisplaySourceLinesWithLineNumbers(filespec,
                                                      self.line,
+                                                     self.column,
                                                      2, # context before
                                                      2, # context after
                                                      '=>', # prefix for current line

     size_t
     DisplaySourceLinesWithLineNumbers (const lldb::SBFileSpec &file,
                                        uint32_t line,
+                                       uint32_t column,
                                        uint32_t context_before,
                                        uint32_t context_after,
                                        const char* current_line_cstr,

scripts/interface/SBStream.i

         stream = lldb.SBStream()
         source_mgr.DisplaySourceLinesWithLineNumbers(filespec,
                                                      self.line,
+                                                     self.column,
                                                      2, # context before
                                                      2, # context after
                                                      '=>', # prefix for current line

source/API/SBSourceManager.cpp

         size_t
         DisplaySourceLinesWithLineNumbers (const lldb_private::FileSpec &file,
                                            uint32_t line,
+                                           uint32_t column,
                                            uint32_t context_before,
                                            uint32_t context_after,
                                            const char *current_line_cstr,

             {
                 return target_sp->GetSourceManager().DisplaySourceLinesWithLineNumbers (file,
                                                                                         line,
+                                                                                        column,
                                                                                         context_before,
                                                                                         context_after,
                                                                                         current_line_cstr,

                 {
                     return debugger_sp->GetSourceManager().DisplaySourceLinesWithLineNumbers (file,
                                                                                               line,
+                                                                                              column,
                                                                                               context_before,
                                                                                               context_after,
                                                                                               current_line_cstr,

 (
     const SBFileSpec &file,
     uint32_t line,
+    uint32_t column,
     uint32_t context_before,
     uint32_t context_after,
     const char *current_line_cstr,

 
     return m_opaque_ap->DisplaySourceLinesWithLineNumbers (file.ref(),
                                                            line,
+                                                           column,
                                                            context_before,
                                                            context_after,
                                                            current_line_cstr,

source/Commands/CommandObjectSource.cpp

             return target->GetSourceManager().DisplaySourceLinesWithLineNumbers (start_file,
                                                                                  line_no,
                                                                                  0,
+                                                                                 0,
                                                                                  m_options.num_lines,
                                                                                  "",
                                                                                  &result.GetOutputStream(),

 
                     target->GetSourceManager().DisplaySourceLinesWithLineNumbers (sc.comp_unit,
                                                                                   sc.line_entry.line,
+                                                                                  m_interpreter.GetDebugger().GetStopShowColumn () ? sc.line_entry.column : 0,
                                                                                   lines_to_back_up,
                                                                                   m_options.num_lines - lines_to_back_up,
                                                                                   "->",

                             m_options.start_line,   // Line to display
                             m_options.num_lines,    // Lines after line to
                             UINT32_MAX,             // Don't mark "line"
+                            0,                      // Don't mark "column"
                             "",                     // Don't mark "line"
                             &result.GetOutputStream(),
                             GetBreakpointLocations ()))

                 target->GetSourceManager().DisplaySourceLinesWithLineNumbers (sc.line_entry.file ? sc.line_entry.file : sc.comp_unit,
                                                                               m_options.start_line,
                                                                               0,
+                                                                              0,
                                                                               m_options.num_lines,
                                                                               "",
                                                                               &result.GetOutputStream(),

source/Core/Debugger.cpp

 {   "stop-disassembly-display", OptionValue::eTypeEnum        , true, Debugger::eStopDisassemblyTypeNoDebugInfo, nullptr, g_show_disassembly_enum_values, "Control when to display disassembly when displaying a stopped context." },
 {   "stop-line-count-after",    OptionValue::eTypeSInt64      , true, 3    , nullptr, nullptr, "The number of sources lines to display that come after the current source line when displaying a stopped context." },
 {   "stop-line-count-before",   OptionValue::eTypeSInt64      , true, 3    , nullptr, nullptr, "The number of sources lines to display that come before the current source line when displaying a stopped context." },
+{   "stop-show-column",         OptionValue::eTypeBoolean     , true, true , nullptr, nullptr, "If true, LLDB will use the column information from the debug info to mark the current position when displaying a stopped context." },
 {   "term-width",               OptionValue::eTypeSInt64      , true, 80   , nullptr, nullptr, "The maximum number of columns to use for displaying text." },
 {   "thread-format",            OptionValue::eTypeFormatEntity, true, 0    , DEFAULT_THREAD_FORMAT, nullptr, "The default thread format string to use when displaying thread information." },
 {   "use-external-editor",      OptionValue::eTypeBoolean     , true, false, nullptr, nullptr, "Whether to use an external editor or not." },

     ePropertyStopDisassemblyDisplay,
     ePropertyStopLineCountAfter,
     ePropertyStopLineCountBefore,
+    ePropertyStopShowColumn,
     ePropertyTerminalWidth,
     ePropertyThreadFormat,
     ePropertyUseExternalEditor,

     return ret;
 }
 
+bool
+Debugger::GetStopShowColumn () const
+{
+    const uint32_t idx = ePropertyStopShowColumn;
+    return m_collection_sp->GetPropertyAtIndexAsBoolean(nullptr, idx, g_properties[idx].default_uint_value != 0);
+}
+
 uint32_t
 Debugger::GetStopSourceLineCount (bool before) const
 {

source/Core/Disassembler.cpp

                                 {
                                     source_manager.DisplaySourceLinesWithLineNumbers (sc.line_entry.file,
                                                                                       sc.line_entry.line,
+                                                                                      sc.line_entry.column,
                                                                                       num_mixed_context_lines,
                                                                                       num_mixed_context_lines,
                                                                                       ((inst_is_at_pc && (options & eOptionMarkPCSourceLine)) ? "->" : ""),

source/Core/SourceManager.cpp

 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/AnsiTerminal.h"
 
 using namespace lldb;
 using namespace lldb_private;

 SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile (uint32_t start_line,
                                                                uint32_t count,
                                                                uint32_t curr_line,
+                                                               uint32_t curr_column,
                                                                const char* current_line_cstr,
                                                                Stream *s,
                                                                const SymbolContextList *bp_locs)

                                       prefix,
                                       line == curr_line ? current_line_cstr : "",
                                       line);
-            size_t this_line_size = m_last_file_sp->DisplaySourceLines (line, 0, 0, s);
+            size_t this_line_size =
+                m_last_file_sp->DisplaySourceLines(line, line == curr_line ? curr_column : 0, s);
             if (this_line_size == 0)
             {
                 m_last_line = UINT32_MAX;

 (
     const FileSpec &file_spec,
     uint32_t line,
+    uint32_t column,
     uint32_t context_before,
     uint32_t context_after,
     const char* current_line_cstr,

             m_last_line = 0;
         m_last_file_sp = file_sp;
     }
-    return DisplaySourceLinesWithLineNumbersUsingLastFile (start_line, count, line, current_line_cstr, s, bp_locs);
+    return DisplaySourceLinesWithLineNumbersUsingLastFile (start_line, count, line, column, current_line_cstr, s, bp_locs);
 }
 
 size_t

         else
             m_last_line = 1;
         
-        return DisplaySourceLinesWithLineNumbersUsingLastFile (m_last_line, m_last_count, UINT32_MAX, "", s, bp_locs);
+        return DisplaySourceLinesWithLineNumbersUsingLastFile (m_last_line, m_last_count, UINT32_MAX, UINT32_MAX, "", s, bp_locs);
     }
     return 0;
 }

 }
 
 size_t
-SourceManager::File::DisplaySourceLines (uint32_t line, uint32_t context_before, uint32_t context_after, Stream *s)
+SourceManager::File::DisplaySourceLines (uint32_t line, uint32_t column, Stream *s)
 {
     // Sanity check m_data_sp before proceeding.
     if (!m_data_sp)
         return 0;
 
-    const uint32_t start_line = line <= context_before ? 1 : line - context_before;
+    const uint32_t start_line = line <= 0 ? 1 : line;
     const uint32_t start_line_offset = GetLineOffset (start_line);
     if (start_line_offset != UINT32_MAX)
     {
-        const uint32_t end_line = line + context_after;
+        const uint32_t end_line = line;
         uint32_t end_line_offset = GetLineOffset (end_line + 1);
         if (end_line_offset == UINT32_MAX)
             end_line_offset = m_data_sp->GetByteSize();

         {
             size_t count = end_line_offset - start_line_offset;
             const uint8_t *cstr = m_data_sp->GetBytes() + start_line_offset;
-            bytes_written = s->Write(cstr, count);
+            if (column && column < count)
+            {
+                // Mark the current column with an underline.
+                std::string ctl;
+                bytes_written = s->Write(cstr, column-1);
+                ctl = lldb_utility::ansi::FormatAnsiTerminalCodes("${ansi.underline}", s->GetFlags().Test(Stream::eANSIColor));
+                bytes_written += s->Write(ctl.c_str(), ctl.length());
+                bytes_written += s->Write(cstr+column-1, 1);
+                ctl = lldb_utility::ansi::FormatAnsiTerminalCodes("${ansi.normal}", s->GetFlags().Test(Stream::eANSIColor));
+                bytes_written += s->Write(ctl.c_str(), ctl.length());
+                bytes_written += s->Write(cstr+column, count-column);
+            } else
+              bytes_written = s->Write(cstr, count);
+
             if (!is_newline_char(cstr[count-1]))
                 bytes_written += s->EOL();
         }

source/Core/StreamAsynchronousIO.cpp

 using namespace lldb;
 using namespace lldb_private;
 
-
-StreamAsynchronousIO::StreamAsynchronousIO (Debugger &debugger, bool for_stdout) :
-    Stream (0, 4, eByteOrderBig),
-    m_debugger (debugger),
-    m_data (),
-    m_for_stdout (for_stdout)
+StreamAsynchronousIO::StreamAsynchronousIO(Debugger &debugger, bool for_stdout)
+    : Stream(debugger.GetUseColor() ? eANSIColor : 0, 4, eByteOrderBig),
+      m_debugger(debugger),
+      m_data(),
+      m_for_stdout(for_stdout)
 {
 }
 

source/Target/StackFrame.cpp

                 {
                     size_t num_lines = target->GetSourceManager().DisplaySourceLinesWithLineNumbers (m_sc.line_entry.file,
                                                                                       m_sc.line_entry.line,
+                                                                                      m_sc.line_entry.column,
                                                                                       source_lines_before,
                                                                                       source_lines_after,
                                                                                       "->",