[RFC][PATCH 0/6] Step program considering the source column information

[RFC][PATCH 0/6] Step program considering the source column information

[Hannes Domani]

Basically, this implements the new commands nextc (nc) and stepc (sc),
which allow to step through the program until an instruction is reached,
that belongs to another source column (according to the debug information).

The current column location is visualized in the frame info, and in the TUI.

Also, the column information is added to 'maint info line-table' and the
python line table interface.

Since the frame info output is different, this certainly breaks the
testsuite, so this column indicator needs a parameter to disable, but I
wasn't concerned about this yet.

With the example code from PR25913, it looks like this:

(gdb) start
Temporary breakpoint 2 at 0x40162d: file gdb-25911.c, line 4.
Starting program: C:\src\tests\gdb-25911.exe

Temporary breakpoint 2, main () at gdb-25911.c:4
4         int a = 4;
              ^
(gdb) nc
6         a = 5; a = 6; a = 7;
            ^
(gdb) nc
6         a = 5; a = 6; a = 7;
                   ^
(gdb) nc
6         a = 5; a = 6; a = 7;
                          ^
(gdb) nc
8         return 0;


What do you think of this so far?


[PATCH 1/6] Add column information of dwarf to the symbol information
[PATCH 2/6] Implement nextc and stepc commands (PR gdb/25913)
[PATCH 3/6] Add column information to maint info line-table
[PATCH 4/6] Add LineTableEntry.column to python line table interface
[PATCH 5/6] Show column of current execution point in frame info (PR
[PATCH 6/6] Show column of current execution point in TUI

[PATCH 1/6] Add column information of dwarf to the symbol information

[Hannes Domani]

---
 gdb/buildsym-legacy.c |  2 +-
 gdb/buildsym.c        |  7 ++++---
 gdb/buildsym.h        |  2 +-
 gdb/dwarf2/read.c     | 38 ++++++++++++++++++++++++++++++--------
 gdb/frame.c           |  1 +
 gdb/symtab.c          |  3 +++
 gdb/symtab.h          | 11 +++++++++++
 7 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/gdb/buildsym-legacy.c b/gdb/buildsym-legacy.c
index d9c27ebc95..0a4f78fe23 100644
--- a/gdb/buildsym-legacy.c
+++ b/gdb/buildsym-legacy.c
@@ -254,7 +254,7 @@ record_line (struct subfile *subfile, int line, CORE_ADDR pc)
   gdb_assert (buildsym_compunit != nullptr);
   /* Assume every line entry is a statement start, that is a good place to
      put a breakpoint for that line number.  */
-  buildsym_compunit->record_line (subfile, line, pc, true);
+  buildsym_compunit->record_line (subfile, line, pc, true, 0);
 }
 
 /* Start a new symtab for a new source file in OBJFILE.  Called, for example,
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index b9bcc33080..fa39266269 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -661,12 +661,12 @@ buildsym_compunit::pop_subfile ()
   return name;
 }
 \f
-/* Add a linetable entry for line number LINE and address PC to the
-   line vector for SUBFILE.  */
+/* Add a linetable entry for line number LINE, address PC and column
+   number COLUMN to the line vector for SUBFILE.  */
 
 void
 buildsym_compunit::record_line (struct subfile *subfile, int line,
-				CORE_ADDR pc, bool is_stmt)
+				CORE_ADDR pc, bool is_stmt, int column)
 {
   struct linetable_entry *e;
 
@@ -719,6 +719,7 @@ buildsym_compunit::record_line (struct subfile *subfile, int line,
   e->line = line;
   e->is_stmt = is_stmt ? 1 : 0;
   e->pc = pc;
+  e->column = column;
 }
 
 \f
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index c768a4c2da..f4dafb6ba3 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -188,7 +188,7 @@ struct buildsym_compunit
   const char *pop_subfile ();
 
   void record_line (struct subfile *subfile, int line, CORE_ADDR pc,
-		    bool is_stmt);
+		    bool is_stmt, int column);
 
   struct compunit_symtab *get_compunit_symtab ()
   {
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index f9d2f255e4..4441c3d390 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -19829,6 +19829,12 @@ class lnp_state_machine
     advance_line (line_delta);
   }
 
+  /* Handle DW_LNS_set_column.  */
+  void handle_set_column (unsigned int column)
+  {
+    m_column = column;
+  }
+
   /* Handle DW_LNS_set_file.  */
   void handle_set_file (file_name_index file);
 
@@ -19890,6 +19896,7 @@ class lnp_state_machine
   /* The line table index of the current file.  */
   file_name_index m_file = 1;
   unsigned int m_line = 1;
+  unsigned int m_column = 0;
 
   /* These are initialized in the constructor.  */
 
@@ -20027,13 +20034,14 @@ dwarf_record_line_p (struct dwarf2_cu *cu,
   return 0;
 }
 
-/* Use the CU's builder to record line number LINE beginning at
-   address ADDRESS in the line table of subfile SUBFILE.  */
+/* Use the CU's builder to record line number LINE and column
+   number COLUMN beginning at address ADDRESS in the line table
+   of subfile SUBFILE.  */
 
 static void
 dwarf_record_line_1 (struct gdbarch *gdbarch, struct subfile *subfile,
 		     unsigned int line, CORE_ADDR address, bool is_stmt,
-		     struct dwarf2_cu *cu)
+		     struct dwarf2_cu *cu, unsigned int column)
 {
   CORE_ADDR addr = gdbarch_addr_bits_remove (gdbarch, address);
 
@@ -20046,7 +20054,7 @@ dwarf_record_line_1 (struct gdbarch *gdbarch, struct subfile *subfile,
     }
 
   if (cu != nullptr)
-    cu->get_builder ()->record_line (subfile, line, addr, is_stmt);
+    cu->get_builder ()->record_line (subfile, line, addr, is_stmt, column);
 }
 
 /* Subroutine of dwarf_decode_lines_1 to simplify it.
@@ -20069,7 +20077,7 @@ dwarf_finish_line (struct gdbarch *gdbarch, struct subfile *subfile,
 			  paddress (gdbarch, address));
     }
 
-  dwarf_record_line_1 (gdbarch, subfile, 0, address, true, cu);
+  dwarf_record_line_1 (gdbarch, subfile, 0, address, true, cu, 0);
 }
 
 void
@@ -20117,7 +20125,8 @@ lnp_state_machine::record_line (bool end_sequence)
 		  dwarf_record_line_1 (m_gdbarch,
 				       builder->get_current_subfile (),
 				       m_line, m_address, is_stmt,
-				       m_currently_recording_lines ? m_cu : nullptr);
+				       m_currently_recording_lines ? m_cu : nullptr,
+				       m_column);
 		}
 	      m_last_subfile = m_cu->get_builder ()->get_current_subfile ();
 	      m_last_line = m_line;
@@ -20334,8 +20343,13 @@ dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
 	      }
 	      break;
 	    case DW_LNS_set_column:
-	      (void) read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
-	      line_ptr += bytes_read;
+	      {
+		unsigned int column
+		  = read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+		line_ptr += bytes_read;
+
+		state_machine.handle_set_column (column);
+	      }
 	      break;
 	    case DW_LNS_negate_stmt:
 	      state_machine.handle_negate_stmt ();
@@ -20668,6 +20682,14 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	  SYMBOL_LINE (sym) = DW_UNSND (attr);
 	}
 
+      attr = dwarf2_attr (die,
+			  inlined_func ? DW_AT_call_column : DW_AT_decl_column,
+			  cu);
+      if (attr != nullptr)
+	{
+	  SYMBOL_COLUMN (sym) = DW_UNSND (attr);
+	}
+
       attr = dwarf2_attr (die,
 			  inlined_func ? DW_AT_call_file : DW_AT_decl_file,
 			  cu);
diff --git a/gdb/frame.c b/gdb/frame.c
index ff27b9f00e..72062c9f34 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2538,6 +2538,7 @@ find_frame_sal (frame_info *frame)
 	{
 	  sal.symtab = symbol_symtab (sym);
 	  sal.line = SYMBOL_LINE (sym);
+	  sal.column = SYMBOL_COLUMN (sym);
 	}
       else
 	/* If the symbol does not have a location, we don't know where
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 7d4857e323..999c0af8dc 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3325,6 +3325,7 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
       val.is_stmt = best->is_stmt;
       val.symtab = best_symtab;
       val.line = best->line;
+      val.column = best->column;
       val.pc = best->pc;
       if (best_end && (!alt || best_end < alt->pc))
 	val.end = best_end;
@@ -3901,6 +3902,7 @@ skip_prologue_sal (struct symtab_and_line *sal)
   sal->section = section;
   sal->symtab = start_sal.symtab;
   sal->line = start_sal.line;
+  sal->column = start_sal.column;
   sal->end = start_sal.end;
 
   /* Check if we are now inside an inlined function.  If we can,
@@ -3919,6 +3921,7 @@ skip_prologue_sal (struct symtab_and_line *sal)
       && SYMBOL_LINE (BLOCK_FUNCTION (function_block)) != 0)
     {
       sal->line = SYMBOL_LINE (BLOCK_FUNCTION (function_block));
+      sal->column = SYMBOL_COLUMN (BLOCK_FUNCTION (function_block));
       sal->symtab = symbol_symtab (BLOCK_FUNCTION (function_block));
     }
 }
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 764c567a90..c936c858e6 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1191,6 +1191,10 @@ struct symbol : public general_symbol_info, public allocate_on_obstack
 
   unsigned short line = 0;
 
+  /* Column number of this symbol's definition.  */
+
+  unsigned short column = 0;
+
   /* An arbitrary data pointer, allowing symbol readers to record
      additional information on a per-symbol basis.  Note that this data
      must be allocated using the same obstack as the symbol itself.  */
@@ -1237,6 +1241,7 @@ extern const struct symbol_impl *symbol_impls;
   (((symbol)->subclass) == SYMBOL_TEMPLATE)
 #define SYMBOL_TYPE(symbol)		(symbol)->type
 #define SYMBOL_LINE(symbol)		(symbol)->line
+#define SYMBOL_COLUMN(symbol)		(symbol)->column
 #define SYMBOL_COMPUTED_OPS(symbol)	(SYMBOL_IMPL (symbol).ops_computed)
 #define SYMBOL_BLOCK_OPS(symbol)	(SYMBOL_IMPL (symbol).ops_block)
 #define SYMBOL_REGISTER_OPS(symbol)	(SYMBOL_IMPL (symbol).ops_register)
@@ -1307,6 +1312,9 @@ struct linetable_entry
   /* The line number for this entry.  */
   int line;
 
+  /* The column number for this entry.  */
+  int column;
+
   /* True if this PC is a good location to place a breakpoint for LINE.  */
   unsigned is_stmt : 1;
 
@@ -1887,6 +1895,9 @@ struct symtab_and_line
      0 is never a valid line number; it is used to indicate that line number
      information is not available.  */
   int line = 0;
+  /* Column number.  0 is never a valid column number; it is used to
+     indicate that column number information is not available.  */
+  int column = 0;
 
   CORE_ADDR pc = 0;
   CORE_ADDR end = 0;
-- 
2.26.2

[PATCH 2/6][PR gdb/25913] Implement nextc and stepc commands

[Hannes Domani]

---
 gdb/gdbthread.h |  1 +
 gdb/infcmd.c    | 59 +++++++++++++++++++++++++++++++++++++++----------
 gdb/infrun.c    | 18 ++++++++++-----
 gdb/infrun.h    |  3 ++-
 4 files changed, 63 insertions(+), 18 deletions(-)

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 717a2ad08c..461e221409 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -332,6 +332,7 @@ class thread_info : public refcounted_object
   thread_suspend_state suspend;
 
   int current_line = 0;
+  int current_column = 0;
   struct symtab *current_symtab = NULL;
 
   /* Internal stepping state.  */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 8b01f45828..a2069c67fc 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -61,7 +61,7 @@
 
 static void until_next_command (int);
 
-static void step_1 (int, int, const char *);
+static void step_1 (int, int, int, const char *);
 
 #define ERROR_NO_INFERIOR \
    if (!target_has_execution) error (_("The program is not being run."));
@@ -917,7 +917,7 @@ continue_command (const char *args, int from_tty)
 /* Record in TP the starting point of a "step" or "next" command.  */
 
 static void
-set_step_frame (thread_info *tp)
+set_step_frame (thread_info *tp, bool step_column)
 {
   /* This can be removed once this function no longer implicitly relies on the
      inferior_ptid value.  */
@@ -926,7 +926,7 @@ set_step_frame (thread_info *tp)
   frame_info *frame = get_current_frame ();
 
   symtab_and_line sal = find_frame_sal (frame);
-  set_step_info (tp, frame, sal);
+  set_step_info (tp, frame, sal, step_column);
 
   CORE_ADDR pc = get_frame_pc (frame);
   tp->control.step_start_function = find_pc_function (pc);
@@ -937,7 +937,7 @@ set_step_frame (thread_info *tp)
 static void
 step_command (const char *count_string, int from_tty)
 {
-  step_1 (0, 0, count_string);
+  step_1 (0, 0, 0, count_string);
 }
 
 /* Likewise, but skip over subroutine calls as if single instructions.  */
@@ -945,7 +945,7 @@ step_command (const char *count_string, int from_tty)
 static void
 next_command (const char *count_string, int from_tty)
 {
-  step_1 (1, 0, count_string);
+  step_1 (1, 0, 0, count_string);
 }
 
 /* Likewise, but step only one instruction.  */
@@ -953,13 +953,27 @@ next_command (const char *count_string, int from_tty)
 static void
 stepi_command (const char *count_string, int from_tty)
 {
-  step_1 (0, 1, count_string);
+  step_1 (0, 1, 0, count_string);
 }
 
 static void
 nexti_command (const char *count_string, int from_tty)
 {
-  step_1 (1, 1, count_string);
+  step_1 (1, 1, 0, count_string);
+}
+
+/* Likewise, but step until instruction of another column is reached.  */
+
+static void
+stepc_command (const char *count_string, int from_tty)
+{
+  step_1 (0, 0, 1, count_string);
+}
+
+static void
+nextc_command (const char *count_string, int from_tty)
+{
+  step_1 (1, 0, 1, count_string);
 }
 
 /* Data for the FSM that manages the step/next/stepi/nexti
@@ -976,6 +990,9 @@ struct step_command_fsm : public thread_fsm
   /* If true, this is a stepi/nexti, otherwise a step/step.  */
   int single_inst;
 
+  /* If true, this is a stepc/nextc, otherwise a step/next.  */
+  int step_column;
+
   explicit step_command_fsm (struct interp *cmd_interp)
     : thread_fsm (cmd_interp)
   {
@@ -992,10 +1009,12 @@ struct step_command_fsm : public thread_fsm
 static void
 step_command_fsm_prepare (struct step_command_fsm *sm,
 			  int skip_subroutines, int single_inst,
+			  int step_column,
 			  int count, struct thread_info *thread)
 {
   sm->skip_subroutines = skip_subroutines;
   sm->single_inst = single_inst;
+  sm->step_column = step_column;
   sm->count = count;
 
   /* Leave the si command alone.  */
@@ -1008,7 +1027,8 @@ step_command_fsm_prepare (struct step_command_fsm *sm,
 static int prepare_one_step (thread_info *, struct step_command_fsm *sm);
 
 static void
-step_1 (int skip_subroutines, int single_inst, const char *count_string)
+step_1 (int skip_subroutines, int single_inst, int step_column,
+	const char *count_string)
 {
   int count;
   int async_exec;
@@ -1037,7 +1057,7 @@ step_1 (int skip_subroutines, int single_inst, const char *count_string)
   thr->thread_fsm = step_sm;
 
   step_command_fsm_prepare (step_sm, skip_subroutines,
-			    single_inst, count, thr);
+			    single_inst, step_column, count, thr);
 
   /* Do only one step for now, before returning control to the event
      loop.  Let the continuation figure out how many other steps we
@@ -1115,7 +1135,7 @@ prepare_one_step (thread_info *tp, struct step_command_fsm *sm)
     {
       struct frame_info *frame = get_current_frame ();
 
-      set_step_frame (tp);
+      set_step_frame (tp, sm->step_column);
 
       if (!sm->single_inst)
 	{
@@ -1489,7 +1509,7 @@ until_next_command (int from_tty)
   struct until_next_fsm *sm;
 
   clear_proceed_status (0);
-  set_step_frame (tp);
+  set_step_frame (tp, false);
 
   frame = get_current_frame ();
 
@@ -1946,7 +1966,7 @@ finish_command (const char *arg, int from_tty)
 	 called by that frame.  We don't use the magic "1" value for
 	 step_range_end, because then infrun will think this is nexti,
 	 and not step over the rest of this inlined function call.  */
-      set_step_info (tp, frame, {});
+      set_step_info (tp, frame, {}, false);
       tp->control.step_range_start = get_frame_pc (frame);
       tp->control.step_range_end = tp->control.step_range_start;
       tp->control.step_over_calls = STEP_OVER_ALL;
@@ -3348,6 +3368,21 @@ Argument N means step N times (or till program stops for another \
 reason)."));
   add_com_alias ("s", "step", class_run, 1);
 
+  add_com ("nextc", class_run, nextc_command, _("\
+Step program, proceeding through subroutine calls.\n\
+Usage: next [N]\n\
+Unlike \"stepc\", if the current source line and column calls\n\
+a subroutine, this command does not enter the subroutine, but\n\
+instead steps over the call."));
+  add_com_alias ("nc", "nextc", class_run, 0);
+
+  add_com ("stepc", class_run, stepc_command, _("\
+Step program until it reaches a different source line or column.\n\
+Usage: stepc [N]\n\
+Argument N means step N times (or till program stops for another \
+reason)."));
+  add_com_alias ("sc", "stepc", class_run, 0);
+
   c = add_com ("until", class_run, until_command, _("\
 Execute until past the current line or past a LOCATION.\n\
 Execute until the program reaches a source line greater than the current\n\
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 601a2acca4..6bc4392de6 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4112,7 +4112,7 @@ fetch_inferior_event (void *client_data)
 
 void
 set_step_info (thread_info *tp, struct frame_info *frame,
-	       struct symtab_and_line sal)
+	       struct symtab_and_line sal, bool step_column)
 {
   /* This can be removed once this function no longer implicitly relies on the
      inferior_ptid value.  */
@@ -4123,6 +4123,7 @@ set_step_info (thread_info *tp, struct frame_info *frame,
 
   tp->current_symtab = sal.symtab;
   tp->current_line = sal.line;
+  tp->current_column = step_column ? sal.column : 0;
 }
 
 /* Clear context switchable stepping state.  */
@@ -7209,7 +7210,9 @@ process_event_stop_test (struct execution_control_state *ecs)
 	     first.  Otherwise stop at the call site.  */
 
 	  if (call_sal.line == ecs->event_thread->current_line
-	      && call_sal.symtab == ecs->event_thread->current_symtab)
+	      && call_sal.symtab == ecs->event_thread->current_symtab
+	      && (ecs->event_thread->current_column == 0
+		  || call_sal.column == ecs->event_thread->current_column))
 	    {
 	      step_into_inline_frame (ecs->event_thread);
 	      if (inline_frame_is_marked_for_skip (false, ecs->event_thread))
@@ -7228,7 +7231,9 @@ process_event_stop_test (struct execution_control_state *ecs)
 	     different source line.  Otherwise continue through the
 	     inlined function.  */
 	  if (call_sal.line == ecs->event_thread->current_line
-	      && call_sal.symtab == ecs->event_thread->current_symtab)
+	      && call_sal.symtab == ecs->event_thread->current_symtab
+	      && (ecs->event_thread->current_column == 0
+		  || call_sal.column == ecs->event_thread->current_column))
 	    keep_going (ecs);
 	  else
 	    end_stepping_range (ecs);
@@ -7262,7 +7267,9 @@ process_event_stop_test (struct execution_control_state *ecs)
   bool refresh_step_info = true;
   if ((ecs->event_thread->suspend.stop_pc == stop_pc_sal.pc)
       && (ecs->event_thread->current_line != stop_pc_sal.line
- 	  || ecs->event_thread->current_symtab != stop_pc_sal.symtab))
+	  || ecs->event_thread->current_symtab != stop_pc_sal.symtab
+	  || (ecs->event_thread->current_column != 0
+	      && ecs->event_thread->current_column != stop_pc_sal.column)))
     {
       if (stop_pc_sal.is_stmt)
 	{
@@ -7309,7 +7316,8 @@ process_event_stop_test (struct execution_control_state *ecs)
   ecs->event_thread->control.step_range_end = stop_pc_sal.end;
   ecs->event_thread->control.may_range_step = 1;
   if (refresh_step_info)
-    set_step_info (ecs->event_thread, frame, stop_pc_sal);
+    set_step_info (ecs->event_thread, frame, stop_pc_sal,
+		   ecs->event_thread->current_column != 0);
 
   if (debug_infrun)
      fprintf_unfiltered (gdb_stdlog, "infrun: keep going\n");
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 9808541351..28c7582996 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -154,7 +154,8 @@ extern int stepping_past_nonsteppable_watchpoint (void);
 /* Record in TP the frame and location we're currently stepping through.  */
 extern void set_step_info (thread_info *tp,
 			   struct frame_info *frame,
-			   struct symtab_and_line sal);
+			   struct symtab_and_line sal,
+			   bool step_column);
 
 /* Several print_*_reason helper functions to print why the inferior
    has stopped to the passed in UIOUT.  */
-- 
2.26.2

[PATCH 3/6] Add column information to maint info line-table

[Hannes Domani]

---
 gdb/symmisc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index ebbedef7b9..338c4012dc 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -1022,9 +1022,10 @@ maintenance_print_one_line_table (struct symtab *symtab, void *data)
       /* Leave space for 6 digits of index and line number.  After that the
 	 tables will just not format as well.  */
       struct ui_out *uiout = current_uiout;
-      ui_out_emit_table table_emitter (uiout, 4, -1, "line-table");
+      ui_out_emit_table table_emitter (uiout, 5, -1, "line-table");
       uiout->table_header (6, ui_left, "index", _("INDEX"));
       uiout->table_header (6, ui_left, "line", _("LINE"));
+      uiout->table_header (6, ui_left, "column", _("COLUMN"));
       uiout->table_header (18, ui_left, "address", _("ADDRESS"));
       uiout->table_header (1, ui_left, "is-stmt", _("IS-STMT"));
       uiout->table_body ();
@@ -1040,6 +1041,10 @@ maintenance_print_one_line_table (struct symtab *symtab, void *data)
 	    uiout->field_signed ("line", item->line);
 	  else
 	    uiout->field_string ("line", _("END"));
+	  if (item->column > 0)
+	    uiout->field_signed ("column", item->column);
+	  else
+	    uiout->field_string ("column", "");
 	  uiout->field_core_addr ("address", objfile->arch (),
 				  item->pc);
 	  uiout->field_string ("is-stmt", item->is_stmt ? "Y" : "");
-- 
2.26.2

[PATCH 4/6] Add LineTableEntry.column to python line table interface

[Hannes Domani]

---
 gdb/python/py-linetable.c | 30 ++++++++++++++++++++++++------
 gdb/symtab.c              |  5 ++++-
 gdb/symtab.h              |  3 ++-
 3 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c
index 858313bb22..f0aa736ac7 100644
--- a/gdb/python/py-linetable.c
+++ b/gdb/python/py-linetable.c
@@ -24,6 +24,8 @@ typedef struct {
   PyObject_HEAD
   /* The line table source line.  */
   int line;
+  /* The line table source column.  */
+  int column;
   /* The pc associated with the source line.  */
   CORE_ADDR pc;
 } linetable_entry_object;
@@ -99,7 +101,7 @@ symtab_to_linetable_object (PyObject *symtab)
    and an address.  */
 
 static PyObject *
-build_linetable_entry (int line, CORE_ADDR address)
+build_linetable_entry (int line, int column, CORE_ADDR address)
 {
   linetable_entry_object *obj;
 
@@ -108,6 +110,7 @@ build_linetable_entry (int line, CORE_ADDR address)
   if (obj != NULL)
     {
       obj->line = line;
+      obj->column = column;
       obj->pc = address;
     }
 
@@ -121,7 +124,8 @@ build_linetable_entry (int line, CORE_ADDR address)
    address.  */
 
 static PyObject *
-build_line_table_tuple_from_pcs (int line, const std::vector<CORE_ADDR> &pcs)
+build_line_table_tuple_from_pcs (int line, const std::vector<CORE_ADDR> &pcs,
+				 const std::vector<int> &columns)
 {
   int i;
 
@@ -136,7 +140,7 @@ build_line_table_tuple_from_pcs (int line, const std::vector<CORE_ADDR> &pcs)
   for (i = 0; i < pcs.size (); ++i)
     {
       CORE_ADDR pc = pcs[i];
-      gdbpy_ref<> obj (build_linetable_entry (line, pc));
+      gdbpy_ref<> obj (build_linetable_entry (line, columns[i], pc));
 
       if (obj == NULL)
 	return NULL;
@@ -158,6 +162,7 @@ ltpy_get_pcs_for_line (PyObject *self, PyObject *args)
   gdb_py_longest py_line;
   struct linetable_entry *best_entry = NULL;
   std::vector<CORE_ADDR> pcs;
+  std::vector<int> columns;
 
   LTPY_REQUIRE_VALID (self, symtab);
 
@@ -166,14 +171,14 @@ ltpy_get_pcs_for_line (PyObject *self, PyObject *args)
 
   try
     {
-      pcs = find_pcs_for_symtab_line (symtab, py_line, &best_entry);
+      pcs = find_pcs_for_symtab_line (symtab, py_line, &best_entry, &columns);
     }
   catch (const gdb_exception &except)
     {
       GDB_PY_HANDLE_EXCEPTION (except);
     }
 
-  return build_line_table_tuple_from_pcs (py_line, pcs);
+  return build_line_table_tuple_from_pcs (py_line, pcs, columns);
 }
 
 /* Implementation of gdb.LineTable.has_line (self, line) -> Boolean.
@@ -339,6 +344,17 @@ ltpy_entry_get_pc (PyObject *self, void *closure)
   return  gdb_py_object_from_longest (obj->pc).release ();
 }
 
+/* Implementation of gdb.LineTableEntry.column (self) -> Long.  Returns
+   a long integer associated with the line table entry.  */
+
+static PyObject *
+ltpy_entry_get_column (PyObject *self, void *closure)
+{
+  linetable_entry_object *obj = (linetable_entry_object *) self;
+
+  return gdb_py_object_from_longest (obj->column).release ();
+}
+
 /* LineTable iterator functions.  */
 
 /* Return a new line table iterator.  */
@@ -422,7 +438,7 @@ ltpy_iternext (PyObject *self)
       item = &(SYMTAB_LINETABLE (symtab)->item[iter_obj->current_index]);
     }
 
-  obj = build_linetable_entry (item->line, item->pc);
+  obj = build_linetable_entry (item->line, item->column, item->pc);
   iter_obj->current_index++;
 
   return obj;
@@ -548,6 +564,8 @@ static gdb_PyGetSetDef linetable_entry_object_getset[] = {
     "The line number in the source file.", NULL },
   { "pc", ltpy_entry_get_pc, NULL,
     "The memory address for this line number.", NULL },
+  { "column", ltpy_entry_get_column, NULL,
+    "The column number in the source file.", NULL },
   { NULL }  /* Sentinel */
 };
 
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 999c0af8dc..19a3d86f24 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3472,7 +3472,8 @@ find_line_symtab (struct symtab *sym_tab, int line,
 
 std::vector<CORE_ADDR>
 find_pcs_for_symtab_line (struct symtab *symtab, int line,
-			  struct linetable_entry **best_item)
+			  struct linetable_entry **best_item,
+			  std::vector<int> *columns)
 {
   int start = 0;
   std::vector<CORE_ADDR> result;
@@ -3500,6 +3501,8 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line,
 	}
 
       result.push_back (SYMTAB_LINETABLE (symtab)->item[idx].pc);
+      if (columns != nullptr)
+	columns->push_back (SYMTAB_LINETABLE (symtab)->item[idx].column);
       start = idx + 1;
     }
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index c936c858e6..fb1b8c9393 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -2274,7 +2274,8 @@ void iterate_over_symtabs (const char *name,
 
 
 std::vector<CORE_ADDR> find_pcs_for_symtab_line
-    (struct symtab *symtab, int line, struct linetable_entry **best_entry);
+    (struct symtab *symtab, int line, struct linetable_entry **best_entry,
+     std::vector<int> *columns = nullptr);
 
 /* Prototype for callbacks for LA_ITERATE_OVER_SYMBOLS.  The callback
    is called once per matching symbol SYM.  The callback should return
-- 
2.26.2

[PATCH 5/6][PR gdb/25911] Show column of current execution point in frame info

[Hannes Domani]

---
 gdb/source.c | 37 ++++++++++++++++++++++++++++++++++++-
 gdb/source.h |  6 ++++++
 gdb/stack.c  | 21 +++++++++++++++++----
 3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/gdb/source.c b/gdb/source.c
index b94c6af487..bf9ec71464 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1243,7 +1243,8 @@ symtab_to_filename_for_display (struct symtab *symtab)
 
 static void
 print_source_lines_base (struct symtab *s, int line, int stopline,
-			 print_source_lines_flags flags)
+			 print_source_lines_flags flags,
+			 int *column_pos = nullptr)
 {
   bool noprint = false;
   int nlines = stopline - line;
@@ -1356,8 +1357,11 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
           uiout->text (":");
         }
       xsnprintf (buf, sizeof (buf), "%d\t", new_lineno++);
+      int printed = strlen (buf);
+      printed = ((printed >> 3) + 1) << 3;
       uiout->text (buf);
 
+      int column = 0;
       while (*iter != '\0')
 	{
 	  /* Find a run of characters that can be emitted at once.
@@ -1369,6 +1373,24 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
 	      int skip_bytes;
 
 	      char c = *iter;
+	      if (c != '\033')
+		{
+		  column++;
+		  if (column_pos != nullptr && *column_pos == column)
+		    {
+		      *column_pos = printed;
+		      column_pos = nullptr;
+		    }
+
+		  if (c == '\t')
+		    printed = ((printed >> 3) + 1) << 3;
+		  else if (c >= 0 && c < 040)
+		    printed += 2;
+		  else if (c == 0177)
+		    printed += 2;
+		  else
+		    printed++;
+		}
 	      if (c == '\033' && skip_ansi_escape (iter, &skip_bytes))
 		iter += skip_bytes;
 	      else if (c >= 0 && c < 040 && c != '\t')
@@ -1408,6 +1430,11 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
 	      ++iter;
 	    }
 	}
+      if (column_pos != nullptr && *column_pos == column)
+	{
+	  *column_pos = printed;
+	  column_pos = nullptr;
+	}
       uiout->text ("\n");
     }
 
@@ -1434,6 +1461,14 @@ print_source_lines (struct symtab *s, source_lines_range line_range,
 			   line_range.stopline (), flags);
 }
 
+/* See source.h.  */
+
+void
+print_source_line_column (struct symtab *s, int line, int &column)
+{
+  print_source_lines_base (s, line, line + 1, 0, &column);
+}
+
 
 \f
 /* Print info on range of pc's in a specified line.  */
diff --git a/gdb/source.h b/gdb/source.h
index 6273a2cc18..5535964e3c 100644
--- a/gdb/source.h
+++ b/gdb/source.h
@@ -140,6 +140,12 @@ DEF_ENUM_FLAGS_TYPE (enum print_source_lines_flag, print_source_lines_flags);
 extern void print_source_lines (struct symtab *s, int line, int stopline,
 				print_source_lines_flags flags);
 
+/* Show source line with number LINE from the file of symtab S.
+   The column number COLUMN is updated to the real number of characters
+   printed until the specified column was reached.  */
+extern void print_source_line_column (struct symtab *s, int line,
+				      int &column);
+
 /* Wrap up the logic to build a line number range for passing to
    print_source_lines when using get_lines_to_list.  An instance of this
    class can be built from a single line number and a direction (forward or
diff --git a/gdb/stack.c b/gdb/stack.c
index f67a151aee..055c4bbe29 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -989,13 +989,15 @@ print_frame_info_to_print_what (const char *print_frame_info)
 		  print_frame_info);
 }
 
-/* Print the PC from FRAME, plus any flags, to UIOUT.  */
+/* Print the PC from FRAME, plus any flags, to UIOUT.
+   Returns number of characters printed.  */
 
-static void
+static int
 print_pc (struct ui_out *uiout, struct gdbarch *gdbarch, frame_info *frame,
 	  CORE_ADDR pc)
 {
   uiout->field_core_addr ("addr", gdbarch, pc);
+  int printed = 2 + (gdbarch_addr_bit (gdbarch) <= 32 ? 8 : 16);
 
   std::string flags = gdbarch_get_pc_address_flags (gdbarch, frame, pc);
   if (!flags.empty ())
@@ -1003,7 +1005,10 @@ print_pc (struct ui_out *uiout, struct gdbarch *gdbarch, frame_info *frame,
     uiout->text (" [");
     uiout->field_string ("addr_flags", flags);
     uiout->text ("]");
+    printed += 3 + flags.size ();
   }
+
+  return printed;
 }
 
 /* See stack.h.  */
@@ -1135,6 +1140,7 @@ print_frame_info (const frame_print_options &fp_opts,
       else
 	{
 	  struct value_print_options opts;
+	  int printed = 0;
 
 	  get_user_print_options (&opts);
 	  /* We used to do this earlier, but that is clearly
@@ -1147,11 +1153,18 @@ print_frame_info (const frame_print_options &fp_opts,
 	     ability to decide for themselves if it is desired.  */
 	  if (opts.addressprint && mid_statement)
 	    {
-	      print_pc (uiout, gdbarch, frame, get_frame_pc (frame));
+	      printed = print_pc (uiout, gdbarch, frame, get_frame_pc (frame));
 	      uiout->text ("\t");
+	      printed = ((printed >> 3) + 1) << 3;
 	    }
 
-	  print_source_lines (sal.symtab, sal.line, sal.line + 1, 0);
+	  int column = sal.column;
+	  print_source_line_column (sal.symtab, sal.line, column);
+	  if (column > 0)
+	    {
+	      uiout->spaces (printed + column);
+	      uiout->text ("^\n");
+	    }
 	}
 
       /* If disassemble-next-line is set to on and there is line debug
-- 
2.26.2

[PATCH 6/6] Show column of current execution point in TUI

[Hannes Domani]

---
 gdb/tui/tui-data.h      |  6 +++++-
 gdb/tui/tui-source.c    | 18 ++++++++++++------
 gdb/tui/tui-stack.c     |  2 ++
 gdb/tui/tui-stack.h     |  1 +
 gdb/tui/tui-winsource.c | 35 +++++++++++++++++++++++++++++++++--
 gdb/tui/tui-winsource.h |  9 +++++++--
 6 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/gdb/tui/tui-data.h b/gdb/tui/tui-data.h
index 2dc73b963d..1b36b5930d 100644
--- a/gdb/tui/tui-data.h
+++ b/gdb/tui/tui-data.h
@@ -148,7 +148,11 @@ struct tui_line_or_address
   enum tui_line_or_address_kind loa;
   union
     {
-      int line_no;
+      struct
+	{
+	  int line_no;
+	  int column_no;
+	};
       CORE_ADDR addr;
     } u;
 };
diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
index fd5bd7dd96..c82647c0a4 100644
--- a/gdb/tui/tui-source.c
+++ b/gdb/tui/tui-source.c
@@ -93,20 +93,25 @@ tui_source_window::set_contents (struct gdbarch *arch,
     {
       struct tui_source_element *element = &m_content[cur_line];
 
-      std::string text;
-      if (*iter != '\0')
-	text = tui_copy_source_line (&iter, cur_line_no,
-				     m_horizontal_offset,
-				     line_width, digits);
-
       /* Set whether element is the execution point
 	 and whether there is a break point on it.  */
       element->line_or_addr.loa = LOA_LINE;
       element->line_or_addr.u.line_no = cur_line_no;
+      element->line_or_addr.u.column_no = 0;
       element->is_exec_point
 	= (filename_cmp (locator->full_name.c_str (),
 			 symtab_to_fullname (s)) == 0
 	   && cur_line_no == locator->line_no);
+      element->exec_column = element->is_exec_point ? locator->column_no : 0;
+
+      std::string text;
+      int column = element->exec_column;
+      if (*iter != '\0')
+	text = tui_copy_source_line (&iter, cur_line_no,
+				     m_horizontal_offset,
+				     line_width, digits, &column);
+      if (element->is_exec_point)
+	element->line_or_addr.u.column_no = column;
 
       m_content[cur_line].line = std::move (text);
 
@@ -212,6 +217,7 @@ tui_source_window::maybe_update (struct frame_info *fi, symtab_and_line sal)
 
       l.loa = LOA_LINE;
       l.u.line_no = sal.line;
+      l.u.column_no = sal.column;
       set_is_exec_point_at (l);
     }
 }
diff --git a/gdb/tui/tui-stack.c b/gdb/tui/tui-stack.c
index f1e075a3d2..052a11ec30 100644
--- a/gdb/tui/tui-stack.c
+++ b/gdb/tui/tui-stack.c
@@ -271,12 +271,14 @@ tui_locator_window::set_locator_info (struct gdbarch *gdbarch_in,
 
   locator_changed_p |= proc_name != procname;
   locator_changed_p |= sal.line != line_no;
+  locator_changed_p |= sal.column != column_no;
   locator_changed_p |= sal.pc != addr;
   locator_changed_p |= gdbarch_in != gdbarch;
   locator_changed_p |= full_name != fullname;
 
   proc_name = procname;
   line_no = sal.line;
+  column_no = sal.column;
   addr = sal.pc;
   gdbarch = gdbarch_in;
   set_locator_fullname (fullname);
diff --git a/gdb/tui/tui-stack.h b/gdb/tui/tui-stack.h
index fde7c6dd2c..13e4f9537b 100644
--- a/gdb/tui/tui-stack.h
+++ b/gdb/tui/tui-stack.h
@@ -62,6 +62,7 @@ struct tui_locator_window : public tui_gen_win_info
   std::string full_name;
   std::string proc_name;
   int line_no = 0;
+  int column_no = 0;
   CORE_ADDR addr = 0;
   /* Architecture associated with code at this location.  */
   struct gdbarch *gdbarch = nullptr;
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index db0add9968..24bacbbb18 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -66,7 +66,7 @@ tui_display_main ()
 
 std::string
 tui_copy_source_line (const char **ptr, int line_no, int first_col,
-		      int line_width, int ndigits)
+		      int line_width, int ndigits, int *exec_column)
 {
   const char *lineptr = *ptr;
 
@@ -87,6 +87,7 @@ tui_copy_source_line (const char **ptr, int line_no, int first_col,
     }
 
   int column = 0;
+  int raw_column = 0;
   char c;
   do
     {
@@ -105,6 +106,7 @@ tui_copy_source_line (const char **ptr, int line_no, int first_col,
 
       ++lineptr;
       ++column;
+      ++raw_column;
 
       auto process_tab = [&] ()
 	{
@@ -127,6 +129,12 @@ tui_copy_source_line (const char **ptr, int line_no, int first_col,
 	  continue;
 	}
 
+      if (exec_column != nullptr && *exec_column == raw_column)
+	{
+	  *exec_column = result.size ();
+	  exec_column = nullptr;
+	}
+
       if (c == '\n' || c == '\r' || c == '\0')
 	{
 	  /* Nothing.  */
@@ -152,6 +160,9 @@ tui_copy_source_line (const char **ptr, int line_no, int first_col,
     ++lineptr;
   *ptr = lineptr;
 
+  if (exec_column != nullptr)
+    *exec_column = 0;
+
   return result;
 }
 
@@ -261,7 +272,20 @@ tui_source_window_base::show_source_line (int lineno)
     tui_set_reverse_mode (handle.get (), true);
 
   wmove (handle.get (), lineno, TUI_EXECINFO_SIZE);
-  tui_puts (line->line.c_str (), handle.get ());
+  if (line->is_exec_point && line->line_or_addr.u.column_no > 0
+      && line->line_or_addr.u.column_no < line->line.size ())
+    {
+      /* Mark the column by disabling the reverse mode for the
+	 corresponding character.  */
+      int column = line->line_or_addr.u.column_no;
+      tui_puts (line->line.substr (0, column).c_str (), handle.get ());
+      tui_set_reverse_mode (handle.get (), false);
+      tui_puts (line->line.substr (column, 1).c_str (), handle.get ());
+      tui_set_reverse_mode (handle.get (), true);
+      tui_puts (line->line.substr (column + 1).c_str (), handle.get ());
+    }
+  else
+    tui_puts (line->line.c_str (), handle.get ());
   if (line->is_exec_point)
     tui_set_reverse_mode (handle.get (), false);
 
@@ -411,8 +435,15 @@ tui_source_window_base::set_is_exec_point_at (struct tui_line_or_address l)
         {
           changed = true;
           m_content[i].is_exec_point = new_state;
+          m_content[i].exec_column = l.loa == LOA_LINE ? l.u.column_no : 0;
           show_source_line (i + 1);
         }
+      else if (new_state && m_content[i].is_exec_point && l.loa == LOA_LINE
+	       && m_content[i].exec_column != l.u.column_no)
+	{
+          changed = true;
+          m_content[i].exec_column = l.u.column_no;
+	}
       i++;
     }
   if (changed)
diff --git a/gdb/tui/tui-winsource.h b/gdb/tui/tui-winsource.h
index 501dd31ccf..8810ba1dd6 100644
--- a/gdb/tui/tui-winsource.h
+++ b/gdb/tui/tui-winsource.h
@@ -58,6 +58,7 @@ struct tui_source_element
     : line (std::move (other.line)),
       line_or_addr (other.line_or_addr),
       is_exec_point (other.is_exec_point),
+      exec_column (other.exec_column),
       break_mode (other.break_mode)
   {
   }
@@ -65,6 +66,7 @@ struct tui_source_element
   std::string line;
   struct tui_line_or_address line_or_addr;
   bool is_exec_point = false;
+  int exec_column = 0;
   tui_bp_flags break_mode = 0;
 };
 
@@ -250,11 +252,14 @@ extern void tui_update_source_windows_with_line (struct symtab_and_line sal);
    digits are used; otherwise the line number is formatted with 6
    digits and the text is aligned to the next tab stop.  Returns a
    string holding the desired text.  PTR is updated to point to the
-   start of the next line.  */
+   start of the next line.
+   If EXEC_COLUMN is supplied, it is updated to the position of the
+   specified colum in the returned string.  */
 
 extern std::string tui_copy_source_line (const char **ptr,
 					 int line_no, int first_col,
-					 int line_width, int ndigits);
+					 int line_width, int ndigits,
+					 int *exec_column = nullptr);
 
 /* Constant definitions. */
 #define SCROLL_THRESHOLD 2	/* Threshold for lazy scroll.  */
-- 
2.26.2

Re: [RFC][PATCH 0/6] Step program considering the source column information

[Pedro Alves]

Hi,

Cool!  A few years back, Antoine Tremblay prototyped this
as well, but it only worked with Clang, because back then
GCC didn't output column information.  I complained about it
to Jakub Jelinek, who in his usual awesomeness quickly implemented
GCC support.  :-)  Quite nice that we're finally getting closer to
actually making use of it in GDB.

You can still find Antoine's prototype here:

 https://github.com/hexa00/binutils-gdb/commits/statement-stepping

It would likely be interesting if you could take a look and compare
approaches.  You should be able to borrow any code you find useful, since
he was working for Ericsson at the time, and had a copyright assignment.

On the name of the new commands -- for a user, thinking in terms
"step until next column" doesn't make sense IMO.  What the user really wants
is "step until next statement".  Column information in the debug
info enables this, but it's an implementation detail.
In Antoine's version, he thus had "nexts" instead of "nextc", which
sounds better to me:

  add_com ("nexts", class_run, nexts_command, _("\
Step program by statement, proceeding through subroutine calls.\n\
Usage: nexts [N]\n\
Unlike \"steps\", if the current source line calls a subroutine,\n\
this command does not enter the subroutine, but instead steps over\n\
the call, in effect treating it as a single statement."));
  add_com_alias ("ns", "nexts", class_run, 1);

Thanks,
Pedro Alves

Re: [RFC][PATCH 0/6] Step program considering the source column information

[Hannes Domani]

 Am Samstag, 16. Mai 2020, 20:45:30 MESZ hat Pedro Alves <palves@redhat.com> Folgendes geschrieben:

> Hi,
>
> Cool!  A few years back, Antoine Tremblay prototyped this
> as well, but it only worked with Clang, because back then
> GCC didn't output column information.  I complained about it
> to Jakub Jelinek, who in his usual awesomeness quickly implemented
> GCC support.  :-)  Quite nice that we're finally getting closer to
> actually making use of it in GDB.
>
> You can still find Antoine's prototype here:
>
> https://github.com/hexa00/binutils-gdb/commits/statement-stepping

I didn't know about that, interesting.
Is there some reason why this wasn't included in gdb back then?


> It would likely be interesting if you could take a look and compare
> approaches.  You should be able to borrow any code you find useful, since
> he was working for Ericsson at the time, and had a copyright assignment.

I just checked it out for a bit.

The actual implementation in process_event_stop_test() is a lot simpler,
I will have to try it out and compare the result, maybe that means mine
is unnecessary complicated.

And the calculation of the column indicator position is also more complicated
in mine, but that's because I had to account for the escape sequences of
the syntax highlighting.


> On the name of the new commands -- for a user, thinking in terms
> "step until next column" doesn't make sense IMO.  What the user really wants
> is "step until next statement".  Column information in the debug
> info enables this, but it's an implementation detail.
> In Antoine's version, he thus had "nexts" instead of "nextc", which
> sounds better to me:
>
>   add_com ("nexts", class_run, nexts_command, _("\
> Step program by statement, proceeding through subroutine calls.\n\
> Usage: nexts [N]\n\
> Unlike \"steps\", if the current source line calls a subroutine,\n\
> this command does not enter the subroutine, but instead steps over\n\
> the call, in effect treating it as a single statement."));
>   add_com_alias ("ns", "nexts", class_run, 1);

Yes, he has nexts (ns) and steps (ss), I don't really mind either way.
I used nextc/stepc because that was suggested in PR25913.


Hannes

Re: [PATCH 1/6] Add column information of dwarf to the symbol information

[Tom Tromey]

Hannes>  void
Hannes>  buildsym_compunit::record_line (struct subfile *subfile, int line,
Hannes> -				CORE_ADDR pc, bool is_stmt)
Hannes> +				CORE_ADDR pc, bool is_stmt, int column)

I think putting the column parameter next to the line parameter would be
more idiomatic.

Tom

Re: [PATCH 5/6][PR gdb/25911] Show column of current execution point in frame info

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:

Hannes>  gdb/source.c | 37 ++++++++++++++++++++++++++++++++++++-
Hannes>  gdb/source.h |  6 ++++++
Hannes>  gdb/stack.c  | 21 +++++++++++++++++----
Hannes>  3 files changed, 59 insertions(+), 5 deletions(-)

I think it would be great if the Python frame filter API and output were
also updated to allow for column numbers.

Tom

Re: [RFC][PATCH 0/6] Step program considering the source column information

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:

Hannes> (gdb) nc
Hannes> 6         a = 5; a = 6; a = 7;
Hannes>             ^

It looks like maybe the caret is misplaced?

Hannes> What do you think of this so far?

I think it is fantastic.

Tom

Re: [RFC][PATCH 0/6] Step program considering the source column information

[Hannes Domani]

Am Montag, 18. Mai 2020, 18:21:34 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Hannes> (gdb) nc
> Hannes> 6        a = 5; a = 6; a = 7;
> Hannes>            ^
>
> It looks like maybe the caret is misplaced?

The caret should always be below the equal sign for this example, according
to the debug information (except the initialization, there it's below 'a').


Hannes

Re: [PATCH 5/6][PR gdb/25911] Show column of current execution point in frame info

[Hannes Domani]

 Am Montag, 18. Mai 2020, 18:20:32 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Hannes>  gdb/source.c | 37 ++++++++++++++++++++++++++++++++++++-
> Hannes>  gdb/source.h |  6 ++++++
> Hannes>  gdb/stack.c  | 21 +++++++++++++++++----
> Hannes>  3 files changed, 59 insertions(+), 5 deletions(-)
>
> I think it would be great if the Python frame filter API and output were
> also updated to allow for column numbers.

I'm not very familiar with the python stuff, but I will use this as an
opportunity to educate myself further with it.

After a quick search, I'm guessing you mean the FrameDecorator module,
and to add FrameDecorator.column similar to FrameDecorator.line.
Is this what you meant?


Hannes

Re: [PATCH 1/6] Add column information of dwarf to the symbol information

[Luis Machado]

On 5/18/20 1:17 PM, Tom Tromey wrote:
> Hannes>  void
> Hannes>  buildsym_compunit::record_line (struct subfile *subfile, int line,
> Hannes> -				CORE_ADDR pc, bool is_stmt)
> Hannes> +				CORE_ADDR pc, bool is_stmt, int column)
> 
> I think putting the column parameter next to the line parameter would be
> more idiomatic.
> 
> Tom
> 

Should we avoid passing line/column separately and, instead, pass down a 
struct containing both? They're part of the same high level information 
(the position in the source file). That would be cleaner IMO.

Re: [RFC][PATCH 0/6] Step program considering the source column information

[Luis Machado]

On 5/16/20 2:26 PM, Hannes Domani via Gdb-patches wrote:
> Basically, this implements the new commands nextc (nc) and stepc (sc),
> which allow to step through the program until an instruction is reached,
> that belongs to another source column (according to the debug information).
> 
> The current column location is visualized in the frame info, and in the TUI.
> 
> Also, the column information is added to 'maint info line-table' and the
> python line table interface.
> 
> Since the frame info output is different, this certainly breaks the
> testsuite, so this column indicator needs a parameter to disable, but I
> wasn't concerned about this yet.
> 
> With the example code from PR25913, it looks like this:
> 
> (gdb) start
> Temporary breakpoint 2 at 0x40162d: file gdb-25911.c, line 4.
> Starting program: C:\src\tests\gdb-25911.exe
> 
> Temporary breakpoint 2, main () at gdb-25911.c:4
> 4         int a = 4;
>                ^
> (gdb) nc
> 6         a = 5; a = 6; a = 7;
>              ^
> (gdb) nc
> 6         a = 5; a = 6; a = 7;
>                     ^
> (gdb) nc
> 6         a = 5; a = 6; a = 7;
>                            ^
> (gdb) nc
> 8         return 0;
> 
> 
> What do you think of this so far?
> 
> 
> [PATCH 1/6] Add column information of dwarf to the symbol information
> [PATCH 2/6] Implement nextc and stepc commands (PR gdb/25913)
> [PATCH 3/6] Add column information to maint info line-table
> [PATCH 4/6] Add LineTableEntry.column to python line table interface
> [PATCH 5/6] Show column of current execution point in frame info (PR
> [PATCH 6/6] Show column of current execution point in TUI
> 

Thanks for working on this, it is really useful.

I'm guessing this series would probably handle the following situation 
very well:

https://sourceware.org/bugzilla/show_bug.cgi?id=21221

Re: [PATCH 5/6][PR gdb/25911] Show column of current execution point in frame info

[Tom Tromey]

>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:

Hannes> After a quick search, I'm guessing you mean the FrameDecorator module,
Hannes> and to add FrameDecorator.column similar to FrameDecorator.line.
Hannes> Is this what you meant?

Yes, thanks.

Tom

Re: [RFC][PATCH 0/6] Step program considering the source column information

[Hannes Domani]

 Am Dienstag, 19. Mai 2020, 14:27:44 MESZ hat Luis Machado <luis.machado@linaro.org> Folgendes geschrieben:

> Thanks for working on this, it is really useful.
>
> I'm guessing this series would probably handle the following situation
> very well:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=21221

According to the debug info, the i initialization is on line 5, column 23.
And the rest of the for-loop is on line 5, column 5.

With the new column-based next-command, it goes back and forth between
them (but you have to wait a while for the inner loop to finish).

Debug session:

(gdb) start
Temporary breakpoint 1 at 0x40162d: file gdb-21221.c, line 5, column 23.
Starting program: C:\src\tests\gdb-21221.exe

Temporary breakpoint 1, main () at gdb-21221.c:5
5           for (unsigned int i = 0U; i < 0xFFFFFU; i++)
                              ^
(gdb) nc
5           for (unsigned int i = 0U; i < 0xFFFFFU; i++)
            ^
(gdb) nc
5           for (unsigned int i = 0U; i < 0xFFFFFU; i++)
                              ^
(gdb) nc
5           for (unsigned int i = 0U; i < 0xFFFFFU; i++)
            ^
(gdb) l
1       int main (void)
2       {
3         while (1)
4         {
5           for (unsigned int i = 0U; i < 0xFFFFFU; i++)
6           {
7             ;
8           }
9         }
10      }
(gdb) disas
Dump of assembler code for function main:
   0x0000000000401620 <+0>:     push   %rbp
   0x0000000000401621 <+1>:     mov    %rsp,%rbp
   0x0000000000401624 <+4>:     sub    $0x30,%rsp
   0x0000000000401628 <+8>:     callq  0x401700 <__main>
   0x000000000040162d <+13>:    movl   $0x0,-0x4(%rbp)
   0x0000000000401634 <+20>:    jmp    0x40163a <main+26>
   0x0000000000401636 <+22>:    addl   $0x1,-0x4(%rbp)
   0x000000000040163a <+26>:    cmpl   $0xffffe,-0x4(%rbp)
   0x0000000000401641 <+33>:    jbe    0x401636 <main+22>
   0x0000000000401643 <+35>:    jmp    0x40162d <main+13>
End of assembler dump.
(gdb) maint info line-table gdb-21221.c
objfile: C:\src\tests\gdb-21221.exe ((struct objfile *) 0x11f4e670)
compunit_symtab: ((struct compunit_symtab *) 0x11f71890)
symtab: C:\src\tests\gdb-21221.c ((struct symtab *) 0x11f71910)
linetable: ((struct linetable *) 0x1204fde0):
INDEX  LINE   COLUMN ADDRESS            IS-STMT
0      2      1      0x0000000000401620 Y
1      2      1      0x0000000000401628 Y
2      5      23     0x000000000040162d Y
3      5      5      0x0000000000401634 Y
4      END           0x0000000000401645 Y

Re: [PATCH 4/6] Add LineTableEntry.column to python line table interface

[Tom de Vries]

On 16-05-2020 19:26, Hannes Domani via Gdb-patches wrote:
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index c936c858e6..fb1b8c9393 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -2274,7 +2274,8 @@ void iterate_over_symtabs (const char *name,
>  
>  
>  std::vector<CORE_ADDR> find_pcs_for_symtab_line
> -    (struct symtab *symtab, int line, struct linetable_entry **best_entry);
> +    (struct symtab *symtab, int line, struct linetable_entry **best_entry,
> +     std::vector<int> *columns = nullptr);
>  
>  /* Prototype for callbacks for LA_ITERATE_OVER_SYMBOLS.  The callback
>     is called once per matching symbol SYM.  The callback should return
> 

Hi,

I just tried to build this, and ran into:
...
In file included from /data/gdb_versions/devel/src/gdb/gdb_curses.h:55:0,
                 from /data/gdb_versions/devel/src/gdb/tui/tui-data.h:26,
                 from /data/gdb_versions/devel/src/gdb/tui/tui-command.h:25,
                 from /data/gdb_versions/devel/src/gdb/tui/tui.c:26:
/data/gdb_versions/devel/src/gdb/symtab.h:2276:24: error: expected ','
or '...' before '(' token
      std::vector<int> *columns = nullptr);
                        ^
make[1]: *** [Makefile:1610: tui/tui.o] Error 1
...

The "columns" string is defined as a macro by /usr/include/ncurses/term.h:
...
#define columns                        CUR Numbers[0]
...
so after preprocessing we have:
...
std::vector<CORE_ADDR> find_pcs_for_symtab_line
    (struct symtab *symtab, int line, struct linetable_entry **best_entry,
     std::vector<int> *
# 2276 "/data/gdb_versions/devel/src/gdb/symtab.h" 3 4
                      ((TERMTYPE *)(_nc_cur_term()))-> Numbers[0]
# 2276 "/data/gdb_versions/devel/src/gdb/symtab.h"
                              = nullptr);
...

Thanks,
- Tom

Re: [PATCH 4/6] Add LineTableEntry.column to python line table interface

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> The "columns" string is defined as a macro by /usr/include/ncurses/term.h:
Tom> ...
Tom> #define columns                        CUR Numbers[0]

FWIW I think it would be fine to add a #undef for this to gdb_curses.h.

Tom

Re: [RFC][PATCH 0/6] Step program considering the source column information

[Tom de Vries]

On 16-05-2020 19:26, Hannes Domani via Gdb-patches wrote:
> Basically, this implements the new commands nextc (nc) and stepc (sc),
> which allow to step through the program until an instruction is reached,
> that belongs to another source column (according to the debug information).
> 
> The current column location is visualized in the frame info, and in the TUI.
> 
> Also, the column information is added to 'maint info line-table' and the
> python line table interface.
> 
> Since the frame info output is different, this certainly breaks the
> testsuite, so this column indicator needs a parameter to disable, but I
> wasn't concerned about this yet.
> 
> With the example code from PR25913, it looks like this:
> 
> (gdb) start
> Temporary breakpoint 2 at 0x40162d: file gdb-25911.c, line 4.
> Starting program: C:\src\tests\gdb-25911.exe
> 
> Temporary breakpoint 2, main () at gdb-25911.c:4
> 4         int a = 4;
>               ^
> (gdb) nc
> 6         a = 5; a = 6; a = 7;
>             ^
> (gdb) nc
> 6         a = 5; a = 6; a = 7;
>                    ^
> (gdb) nc
> 6         a = 5; a = 6; a = 7;
>                           ^
> (gdb) nc
> 8         return 0;
> 
> 
> What do you think of this so far?
> 

Very nice :)

As I've just learned by looking at this (
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95360 ) PR, interesting
code in relation to this patch series is dwarf_record_line_p.

The function implements filtering of line-table elements, and has as
related test-case gdb.dwarf2/dw2-single-line-discriminators.exp.

If we disable the filtering (by returning 1 at the end instead of 0), we
get for test-case gdb.dwarf2/dw2-single-line-discriminators.exp:
...
Temporary breakpoint 2, main () at
gdb.dwarf2/dw2-single-line-discriminators.c:26
26        x = 0;
(gdb) n
28        for (i = 0; i < 10; ++i) continue; /* stepi line */
(gdb) si
28        for (i = 0; i < 10; ++i) continue; /* stepi line */
...

That is: no progress is visible after the stepi.  If we re-enable the
filtering, we have:
...
Temporary breakpoint 1, main () at
gdb.dwarf2/dw2-single-line-discriminators.c:26
26        x = 0;
(gdb) n
28        for (i = 0; i < 10; ++i) continue; /* stepi line */
(gdb) si
0x00000000004004cd      28        for (i = 0; i < 10; ++i) continue; /*
stepi line */
...

So, progress is visible.

If we now use your patch series, and with the filtering enabled, we have:
...
Temporary breakpoint 1, main () at
gdb.dwarf2/dw2-single-line-discriminators.c:26
26        x = 0;
          ^
(gdb) n
28        for (i = 0; i < 10; ++i) continue; /* stepi line */
               ^
(gdb) si
0x00000000004004cd      28        for (i = 0; i < 10; ++i) continue; /*
stepi line */
                                       ^
(gdb)
0x00000000004004d1      28        for (i = 0; i < 10; ++i) continue; /*
stepi line */
                                       ^
(gdb)
0x00000000004004d3      28        for (i = 0; i < 10; ++i) continue; /*
stepi line */
                                       ^
(gdb)
0x00000000004004d5      28        for (i = 0; i < 10; ++i) continue; /*
stepi line */
                                       ^
...

So, we see progress in the insn addresses, but the column does not progress.

OTOH, if we disable filtering:
...
Temporary breakpoint 1, main () at
gdb.dwarf2/dw2-single-line-discriminators.c:26
26        x = 0;
          ^
(gdb) n
28        for (i = 0; i < 10; ++i) continue; /* stepi line */
               ^
(gdb) si
28        for (i = 0; i < 10; ++i) continue; /* stepi line */
               ^
(gdb) si
0x00000000004004d1      28        for (i = 0; i < 10; ++i) continue; /*
stepi line */
                                       ^
(gdb) si
28        for (i = 0; i < 10; ++i) continue; /* stepi line */
                                   ^
(gdb) si
28        for (i = 0; i < 10; ++i) continue; /* stepi line */
                              ^
(gdb) si
0x00000000004004d8      28        for (i = 0; i < 10; ++i) continue; /*
stepi line */
                                                      ^
...
we don't see progress after the first stepi, but eventually we do see
progress in the column.

Looking at the line-info:
...
  [0x00000147]  Set column to 8
  [0x00000149]  Special opcode 161: advance Address by 11 to 0x4004c6
and Line by 2 to 28
  [0x0000014a]  Extended opcode 4: set Discriminator to 4
  [0x0000014e]  Special opcode 103: advance Address by 7 to 0x4004cd and
Line by 0 to 28
...
perhaps we we should collapse entries that have the same column, so
something like this in dwarf_record_line_p:
...
  if (line != last_line)
    return 1;
  if (colum != last_column)
    return 1;
  return 0;
...

Thanks,
- Tom

Re: [RFC][PATCH 0/6] Step program considering the source column information

[Hannes Domani]

 Am Mittwoch, 27. Mai 2020, 17:33:37 MESZ hat Tom de Vries <tdevries@suse.de> Folgendes geschrieben:

> On 16-05-2020 19:26, Hannes Domani via Gdb-patches wrote:
> > Basically, this implements the new commands nextc (nc) and stepc (sc),
> > which allow to step through the program until an instruction is reached,
> > that belongs to another source column (according to the debug information).
> >
> > The current column location is visualized in the frame info, and in the TUI.
> >
> > Also, the column information is added to 'maint info line-table' and the
> > python line table interface.
> >
> > Since the frame info output is different, this certainly breaks the
> > testsuite, so this column indicator needs a parameter to disable, but I
> > wasn't concerned about this yet.
> >
> > With the example code from PR25913, it looks like this:
> >
> > (gdb) start
> > Temporary breakpoint 2 at 0x40162d: file gdb-25911.c, line 4.
> > Starting program: C:\src\tests\gdb-25911.exe
> >
> > Temporary breakpoint 2, main () at gdb-25911.c:4
> > 4        int a = 4;
> >              ^
> > (gdb) nc
> > 6        a = 5; a = 6; a = 7;
> >            ^
> > (gdb) nc
> > 6        a = 5; a = 6; a = 7;
> >                    ^
> > (gdb) nc
> > 6        a = 5; a = 6; a = 7;
> >                          ^
> > (gdb) nc
> > 8        return 0;
> >
> >
> > What do you think of this so far?
> >
>
> Very nice :)
>
> As I've just learned by looking at this (
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95360 ) PR, interesting
> code in relation to this patch series is dwarf_record_line_p.
>
> The function implements filtering of line-table elements, and has as
> related test-case gdb.dwarf2/dw2-single-line-discriminators.exp.
>
> If we disable the filtering (by returning 1 at the end instead of 0), we
> get for test-case gdb.dwarf2/dw2-single-line-discriminators.exp:
> ...
> Temporary breakpoint 2, main () at
> gdb.dwarf2/dw2-single-line-discriminators.c:26
> 26        x = 0;
> (gdb) n
> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
> (gdb) si
> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
> ...
>
> That is: no progress is visible after the stepi.  If we re-enable the
> filtering, we have:
> ...
> Temporary breakpoint 1, main () at
> gdb.dwarf2/dw2-single-line-discriminators.c:26
> 26        x = 0;
> (gdb) n
> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
> (gdb) si
> 0x00000000004004cd      28        for (i = 0; i < 10; ++i) continue; /*
> stepi line */
> ...
>
> So, progress is visible.
>
> If we now use your patch series, and with the filtering enabled, we have:
> ...
> Temporary breakpoint 1, main () at
> gdb.dwarf2/dw2-single-line-discriminators.c:26
> 26        x = 0;
>           ^
> (gdb) n
> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>               ^
> (gdb) si
> 0x00000000004004cd      28        for (i = 0; i < 10; ++i) continue; /*
> stepi line */
>                                       ^
> (gdb)
> 0x00000000004004d1      28        for (i = 0; i < 10; ++i) continue; /*
> stepi line */
>                                       ^
> (gdb)
> 0x00000000004004d3      28        for (i = 0; i < 10; ++i) continue; /*
> stepi line */
>                                       ^
> (gdb)
> 0x00000000004004d5      28        for (i = 0; i < 10; ++i) continue; /*
> stepi line */
>                                       ^
> ...
>
> So, we see progress in the insn addresses, but the column does not progress.
>
> OTOH, if we disable filtering:
> ...
> Temporary breakpoint 1, main () at
> gdb.dwarf2/dw2-single-line-discriminators.c:26
> 26        x = 0;
>           ^
> (gdb) n
> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>               ^
> (gdb) si
> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>               ^
> (gdb) si
> 0x00000000004004d1      28        for (i = 0; i < 10; ++i) continue; /*
> stepi line */
>                                       ^
> (gdb) si
> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>                                   ^
> (gdb) si
> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>                               ^
> (gdb) si
> 0x00000000004004d8      28        for (i = 0; i < 10; ++i) continue; /*
> stepi line */
>                                                       ^
> ...
> we don't see progress after the first stepi, but eventually we do see
> progress in the column.
>
> Looking at the line-info:
> ...
>   [0x00000147]  Set column to 8
>   [0x00000149]  Special opcode 161: advance Address by 11 to 0x4004c6
> and Line by 2 to 28
>   [0x0000014a]  Extended opcode 4: set Discriminator to 4
>   [0x0000014e]  Special opcode 103: advance Address by 7 to 0x4004cd and
> Line by 0 to 28
> ...
> perhaps we we should collapse entries that have the same column, so
> something like this in dwarf_record_line_p:
> ...
>   if (line != last_line)
>     return 1;
>   if (colum != last_column)
>     return 1;
>
>   return 0;
>
> ...

Interesting.
I'm just wondering, if dwarf_record_line_p merges all entries of the same line,
why does my column-based stepping even work at all?

Is it because of line_has_non_zero_discriminator?
If yes, what is the discriminator?


Hannes

Re: [RFC][PATCH 0/6] Step program considering the source column information

[Tom de Vries]

On 27-05-2020 18:04, Hannes Domani via Gdb-patches wrote:
>  Am Mittwoch, 27. Mai 2020, 17:33:37 MESZ hat Tom de Vries <tdevries@suse.de> Folgendes geschrieben:
> 
>> On 16-05-2020 19:26, Hannes Domani via Gdb-patches wrote:
>>> Basically, this implements the new commands nextc (nc) and stepc (sc),
>>> which allow to step through the program until an instruction is reached,
>>> that belongs to another source column (according to the debug information).
>>>
>>> The current column location is visualized in the frame info, and in the TUI.
>>>
>>> Also, the column information is added to 'maint info line-table' and the
>>> python line table interface.
>>>
>>> Since the frame info output is different, this certainly breaks the
>>> testsuite, so this column indicator needs a parameter to disable, but I
>>> wasn't concerned about this yet.
>>>
>>> With the example code from PR25913, it looks like this:
>>>
>>> (gdb) start
>>> Temporary breakpoint 2 at 0x40162d: file gdb-25911.c, line 4.
>>> Starting program: C:\src\tests\gdb-25911.exe
>>>
>>> Temporary breakpoint 2, main () at gdb-25911.c:4
>>> 4        int a = 4;
>>>               ^
>>> (gdb) nc
>>> 6        a = 5; a = 6; a = 7;
>>>             ^
>>> (gdb) nc
>>> 6        a = 5; a = 6; a = 7;
>>>                     ^
>>> (gdb) nc
>>> 6        a = 5; a = 6; a = 7;
>>>                           ^
>>> (gdb) nc
>>> 8        return 0;
>>>
>>>
>>> What do you think of this so far?
>>>
>>
>> Very nice :)
>>
>> As I've just learned by looking at this (
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95360 ) PR, interesting
>> code in relation to this patch series is dwarf_record_line_p.
>>
>> The function implements filtering of line-table elements, and has as
>> related test-case gdb.dwarf2/dw2-single-line-discriminators.exp.
>>
>> If we disable the filtering (by returning 1 at the end instead of 0), we
>> get for test-case gdb.dwarf2/dw2-single-line-discriminators.exp:
>> ...
>> Temporary breakpoint 2, main () at
>> gdb.dwarf2/dw2-single-line-discriminators.c:26
>> 26        x = 0;
>> (gdb) n
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>> (gdb) si
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>> ...
>>
>> That is: no progress is visible after the stepi.  If we re-enable the
>> filtering, we have:
>> ...
>> Temporary breakpoint 1, main () at
>> gdb.dwarf2/dw2-single-line-discriminators.c:26
>> 26        x = 0;
>> (gdb) n
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>> (gdb) si
>> 0x00000000004004cd      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>> ...
>>
>> So, progress is visible.
>>
>> If we now use your patch series, and with the filtering enabled, we have:
>> ...
>> Temporary breakpoint 1, main () at
>> gdb.dwarf2/dw2-single-line-discriminators.c:26
>> 26        x = 0;
>>            ^
>> (gdb) n
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>>                ^
>> (gdb) si
>> 0x00000000004004cd      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>>                                        ^
>> (gdb)
>> 0x00000000004004d1      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>>                                        ^
>> (gdb)
>> 0x00000000004004d3      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>>                                        ^
>> (gdb)
>> 0x00000000004004d5      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>>                                        ^
>> ...
>>
>> So, we see progress in the insn addresses, but the column does not progress.
>>
>> OTOH, if we disable filtering:
>> ...
>> Temporary breakpoint 1, main () at
>> gdb.dwarf2/dw2-single-line-discriminators.c:26
>> 26        x = 0;
>>            ^
>> (gdb) n
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>>                ^
>> (gdb) si
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>>                ^
>> (gdb) si
>> 0x00000000004004d1      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>>                                        ^
>> (gdb) si
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>>                                    ^
>> (gdb) si
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>>                                ^
>> (gdb) si
>> 0x00000000004004d8      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>>                                                        ^
>> ...
>> we don't see progress after the first stepi, but eventually we do see
>> progress in the column.
>>
>> Looking at the line-info:
>> ...
>>    [0x00000147]  Set column to 8
>>    [0x00000149]  Special opcode 161: advance Address by 11 to 0x4004c6
>> and Line by 2 to 28
>>    [0x0000014a]  Extended opcode 4: set Discriminator to 4
>>    [0x0000014e]  Special opcode 103: advance Address by 7 to 0x4004cd and
>> Line by 0 to 28
>> ...
>> perhaps we we should collapse entries that have the same column, so
>> something like this in dwarf_record_line_p:
>> ...
>>    if (line != last_line)
>>      return 1;
>>    if (colum != last_column)
>>      return 1;
>>
>>    return 0;
>>
>> ...
> 

Sorry for the late reply, I didn't notice this earlier (not on To/CC
list for some reason).

> Interesting.
> I'm just wondering, if dwarf_record_line_p merges all entries of the same line,
> why does my column-based stepping even work at all?
> 
> Is it because of line_has_non_zero_discriminator?

Yes. ( See also https://sourceware.org/bugzilla/show_bug.cgi?id=17276 ).

> If yes, what is the discriminator?

DWARF 4 standard, 6.2.2 State Machine Registers :
...
An unsigned integer identifying the block to which the current
instruction belongs. Discriminator values are assigned arbitrarily by
the DWARF producer and serve to distinguish among multiple blocks that
may all be associated with the same source file, line, and column. Where
only one lock exists for a given source position, the discriminator
value should be zero.
...

Thanks,
- Tom