[RFC][PATCH 7/6] Implement column breakpoints

[RFC][PATCH 7/6] Implement column breakpoints

[Hannes Domani]

This is a late addition to my 'column' patch series.

I think it's straightforward, you just add the column number after the line
number, separated by a colon:

(gdb) break gdb-25911.c:6:10
Breakpoint 1 at 0x40163b: file gdb-25911.c, line 6, column 12.
(gdb) r
Starting program: C:\src\tests\gdb-25911.exe

Breakpoint 1, main () at gdb-25911.c:6
6         a = 5; a = 6; a = 7;
                   ^
(gdb) info b
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x000000000040163b in main at gdb-25911.c:6:12
        breakpoint already hit 1 time
(gdb) l
1       int
2       main (void)
3       {
4         int a = 4;
5
6         a = 5; a = 6; a = 7;
7
8         return 0;
9       }

As seen here, if there is no exact match at the specified line:column
location, it uses the next location of the same line at a higher column
(if there is none, it continues to the next line, ignoring the column).
---
 gdb/breakpoint.c          | 14 ++++++++++-
 gdb/breakpoint.h          |  4 +++
 gdb/linespec.c            | 53 +++++++++++++++++++++++++++++----------
 gdb/location.c            | 22 ++++++++++++++++
 gdb/location.h            |  3 +++
 gdb/python/py-linetable.c | 14 +++++------
 gdb/symtab.c              | 44 +++++++++++++++++++-------------
 gdb/symtab.h              |  7 +++---
 8 files changed, 118 insertions(+), 43 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 480f095876..ae9467e595 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5836,6 +5836,12 @@ print_breakpoint_location (struct breakpoint *b,
 	uiout->field_string ("fullname", symtab_to_fullname (loc->symtab));
       
       uiout->field_signed ("line", loc->line_number);
+
+      if (loc->column_number != 0)
+	{
+	  uiout->text (":");
+	  uiout->field_signed ("column", loc->column_number);
+	}
     }
   else if (loc)
     {
@@ -8461,6 +8467,7 @@ momentary_breakpoint_from_master (struct breakpoint *orig,
   copy->loc->pspace = orig->loc->pspace;
   copy->loc->probe = orig->loc->probe;
   copy->loc->line_number = orig->loc->line_number;
+  copy->loc->column_number = orig->loc->column_number;
   copy->loc->symtab = orig->loc->symtab;
   copy->loc->enabled = loc_enabled;
   copy->frame_id = orig->frame_id;
@@ -8552,6 +8559,7 @@ add_location_to_breakpoint (struct breakpoint *b,
   loc->section = sal->section;
   loc->gdbarch = loc_gdbarch;
   loc->line_number = sal->line;
+  loc->column_number = sal->column;
   loc->symtab = sal->symtab;
   loc->symbol = sal->symbol;
   loc->msymbol = sal->msymbol;
@@ -11334,6 +11342,7 @@ clear_command (const char *arg, int from_tty)
 		      && sal_fullname != NULL
 		      && sal.pspace == loc->pspace
 		      && loc->line_number == sal.line
+		      && loc->column_number == sal.column
 		      && filename_cmp (symtab_to_fullname (loc->symtab),
 				       sal_fullname) == 0)
 		    line_match = 1;
@@ -12077,10 +12086,13 @@ say_where (struct breakpoint *b)
 	    {
 	      const char *filename
 		= symtab_to_filename_for_display (b->loc->symtab);
-	      printf_filtered (": file %ps, line %d.",
+	      printf_filtered (": file %ps, line %d",
 			       styled_string (file_name_style.style (),
 					      filename),
 			       b->loc->line_number);
+	      if (b->loc->column_number != 0)
+		printf_filtered (", column %d", b->loc->column_number);
+	      printf_filtered (".");
 	    }
 	  else
 	    /* This is not ideal, but each location may have a
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 347aeb75f3..2457f54966 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -485,6 +485,10 @@ class bp_location
 
   int line_number = 0;
 
+  /* Column number which was used to place this location.  */
+
+  int column_number = 0;
+
   /* Symtab which was used to place this location.  This is used
      to find the corresponding source file name.  */
 
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 1e70cdb0dc..7c1385c407 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -401,7 +401,7 @@ static std::vector<symtab *>
 static std::vector<symtab_and_line> decode_digits_ordinary
   (struct linespec_state *self,
    linespec_p ls,
-   int line,
+   int line, int column,
    linetable_entry **best_entry);
 
 static std::vector<symtab_and_line> decode_digits_list_mode
@@ -1793,6 +1793,20 @@ linespec_parse_basic (linespec_parser *parser)
       /* Get the next token.  */
       token = linespec_lexer_consume_token (parser);
 
+      /* If there is an additional colon, followed by another number,
+	 it's the optional column field.  */
+      if (token.type == LSTOKEN_COLON)
+	{
+	  token = linespec_lexer_consume_token (parser);
+	  if (token.type != LSTOKEN_NUMBER)
+	    unexpected_linespec_error (parser);
+
+	  name = copy_token_string (token);
+	  PARSER_EXPLICIT (parser)->column = atoi (name.get ());
+
+	  token = linespec_lexer_consume_token (parser);
+	}
+
       /* If the next token is a comma, stop parsing and return.  */
       if (token.type == LSTOKEN_COMMA)
 	{
@@ -2109,6 +2123,7 @@ create_sals_line_offset (struct linespec_state *self,
 
   symtab_and_line val;
   val.line = ls->explicit_loc.line_offset.offset;
+  val.column = ls->explicit_loc.column;
   switch (ls->explicit_loc.line_offset.sign)
     {
     case LINE_OFFSET_PLUS:
@@ -2140,11 +2155,17 @@ create_sals_line_offset (struct linespec_state *self,
       int i, j;
 
       std::vector<symtab_and_line> intermediate_results
-	= decode_digits_ordinary (self, ls, val.line, &best_entry);
+	= decode_digits_ordinary (self, ls, val.line, val.column, &best_entry);
       if (intermediate_results.empty () && best_entry != NULL)
-	intermediate_results = decode_digits_ordinary (self, ls,
-						       best_entry->line,
-						       &best_entry);
+	{
+	  int best_column = 0;
+	  if (val.column != 0 && val.line == best_entry->line)
+	    best_column = best_entry->column;
+	  intermediate_results = decode_digits_ordinary (self, ls,
+							 best_entry->line,
+							 best_column,
+							 &best_entry);
+	}
 
       /* For optimized code, the compiler can scatter one source line
 	 across disjoint ranges of PC values, even when no duplicate
@@ -2373,7 +2394,8 @@ convert_explicit_location_to_linespec (struct linespec_state *self,
 				       const char *function_name,
 				       symbol_name_match_type fname_match_type,
 				       const char *label_name,
-				       struct line_offset line_offset)
+				       struct line_offset line_offset,
+				       int column)
 {
   std::vector<block_symbol> symbols;
   std::vector<block_symbol> *labels;
@@ -2434,6 +2456,9 @@ convert_explicit_location_to_linespec (struct linespec_state *self,
 
   if (line_offset.sign != LINE_OFFSET_UNKNOWN)
     result->explicit_loc.line_offset = line_offset;
+
+  if (column != 0)
+    result->explicit_loc.column = column;
 }
 
 /* Convert the explicit location EXPLICIT_LOC into SaLs.  */
@@ -2448,7 +2473,8 @@ convert_explicit_location_to_sals (struct linespec_state *self,
 					 explicit_loc->function_name,
 					 explicit_loc->func_name_match_type,
 					 explicit_loc->label_name,
-					 explicit_loc->line_offset);
+					 explicit_loc->line_offset,
+					 explicit_loc->column);
   return convert_linespec_to_sals (self, result);
 }
 
@@ -2928,7 +2954,7 @@ linespec_complete_label (completion_tracker &tracker,
 					     source_filename,
 					     function_name,
 					     func_name_match_type,
-					     NULL, unknown_offset);
+					     NULL, unknown_offset, 0);
     }
   catch (const gdb_exception_error &ex)
     {
@@ -4111,28 +4137,29 @@ decode_digits_list_mode (struct linespec_state *self,
 static std::vector<symtab_and_line>
 decode_digits_ordinary (struct linespec_state *self,
 			linespec_p ls,
-			int line,
+			int line, int column,
 			struct linetable_entry **best_entry)
 {
   std::vector<symtab_and_line> sals;
   for (const auto &elt : *ls->file_symtabs)
     {
-      std::vector<CORE_ADDR> pcs;
+      std::vector<std::pair<CORE_ADDR, int>> pcs;
 
       /* The logic above should ensure this.  */
       gdb_assert (elt != NULL);
 
       set_current_program_space (SYMTAB_PSPACE (elt));
 
-      pcs = find_pcs_for_symtab_line (elt, line, best_entry);
-      for (CORE_ADDR pc : pcs)
+      pcs = find_pcs_for_symtab_line (elt, line, column, best_entry);
+      for (const auto &pc : pcs)
 	{
 	  symtab_and_line sal;
 	  sal.pspace = SYMTAB_PSPACE (elt);
 	  sal.symtab = elt;
 	  sal.line = line;
+	  sal.column = pc.second;
 	  sal.explicit_line = true;
-	  sal.pc = pc;
+	  sal.pc = pc.first;
 	  sals.push_back (std::move (sal));
 	}
     }
diff --git a/gdb/location.c b/gdb/location.c
index faa3752561..8b73abe5bf 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -202,6 +202,9 @@ new_explicit_location (const struct explicit_location *explicit_loc)
 
       if (explicit_loc->line_offset.sign != LINE_OFFSET_UNKNOWN)
 	EL_EXPLICIT (&tmp)->line_offset = explicit_loc->line_offset;
+
+      if (explicit_loc->column != 0)
+	EL_EXPLICIT (&tmp)->column = explicit_loc->column;
     }
 
   return copy_event_location (&tmp);
@@ -280,6 +283,14 @@ explicit_to_string_internal (int as_linespec,
 		   : (explicit_loc->line_offset.sign
 		      == LINE_OFFSET_PLUS ? "+" : "-")),
 		  explicit_loc->line_offset.offset);
+
+      if (explicit_loc->column != 0)
+	{
+	  buf.putc (space);
+	  if (!as_linespec)
+	    buf.puts ("-column ");
+	  buf.printf ("%d", explicit_loc->column);
+	}
     }
 
   return xstrdup (buf.c_str ());
@@ -341,6 +352,7 @@ copy_event_location (const struct event_location *src)
 	EL_EXPLICIT (dst)->label_name = xstrdup (EL_EXPLICIT (src)->label_name);
 
       EL_EXPLICIT (dst)->line_offset = EL_EXPLICIT (src)->line_offset;
+      EL_EXPLICIT (dst)->column = EL_EXPLICIT (src)->column;
       break;
 
 
@@ -825,6 +837,16 @@ string_to_explicit_location (const char **argp,
 	      continue;
 	    }
 	}
+      else if (strncmp (opt.get (), "-column", len) == 0)
+	{
+	  set_oarg (explicit_location_lex_one (argp, language, NULL));
+	  *argp = skip_spaces (*argp);
+	  if (have_oarg)
+	    {
+	      EL_EXPLICIT (location)->column = atoi (oarg.get ());
+	      continue;
+	    }
+	}
       else if (strncmp (opt.get (), "-label", len) == 0)
 	{
 	  set_oarg (explicit_location_lex_one (argp, language, completion_info));
diff --git a/gdb/location.h b/gdb/location.h
index 6d74e1eeb5..a5cd95dc14 100644
--- a/gdb/location.h
+++ b/gdb/location.h
@@ -102,6 +102,9 @@ struct explicit_location
      identified by the above fields or the current symtab
      if the other fields are NULL.  */
   struct line_offset line_offset;
+
+  /* The column of the line specified by line_offset.  */
+  int column;
 };
 
 /* Return the type of the given event location.  */
diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c
index f0aa736ac7..55199af21d 100644
--- a/gdb/python/py-linetable.c
+++ b/gdb/python/py-linetable.c
@@ -124,8 +124,8 @@ build_linetable_entry (int line, int column, CORE_ADDR address)
    address.  */
 
 static PyObject *
-build_line_table_tuple_from_pcs (int line, const std::vector<CORE_ADDR> &pcs,
-				 const std::vector<int> &columns)
+build_line_table_tuple_from_pcs (int line,
+				 const std::vector<std::pair<CORE_ADDR, int>> &pcs)
 {
   int i;
 
@@ -139,8 +139,8 @@ 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, columns[i], pc));
+      CORE_ADDR pc = pcs[i].first;
+      gdbpy_ref<> obj (build_linetable_entry (line, pcs[i].second, pc));
 
       if (obj == NULL)
 	return NULL;
@@ -161,7 +161,7 @@ ltpy_get_pcs_for_line (PyObject *self, PyObject *args)
   struct symtab *symtab;
   gdb_py_longest py_line;
   struct linetable_entry *best_entry = NULL;
-  std::vector<CORE_ADDR> pcs;
+  std::vector<std::pair<CORE_ADDR, int>> pcs;
   std::vector<int> columns;
 
   LTPY_REQUIRE_VALID (self, symtab);
@@ -171,14 +171,14 @@ ltpy_get_pcs_for_line (PyObject *self, PyObject *args)
 
   try
     {
-      pcs = find_pcs_for_symtab_line (symtab, py_line, &best_entry, &columns);
+      pcs = find_pcs_for_symtab_line (symtab, py_line, 0, &best_entry);
     }
   catch (const gdb_exception &except)
     {
       GDB_PY_HANDLE_EXCEPTION (except);
     }
 
-  return build_line_table_tuple_from_pcs (py_line, pcs, columns);
+  return build_line_table_tuple_from_pcs (py_line, pcs);
 }
 
 /* Implementation of gdb.LineTable.has_line (self, line) -> Boolean.
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 19a3d86f24..3896f57da0 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -76,7 +76,7 @@
 
 static void rbreak_command (const char *, int);
 
-static int find_line_common (struct linetable *, int, int *, int);
+static int find_line_common (struct linetable *, int, int, int *, int);
 
 static struct block_symbol
   lookup_symbol_aux (const char *name,
@@ -3390,7 +3390,7 @@ find_line_symtab (struct symtab *sym_tab, int line,
   /* First try looking it up in the given symtab.  */
   best_linetable = SYMTAB_LINETABLE (sym_tab);
   best_symtab = sym_tab;
-  best_index = find_line_common (best_linetable, line, &exact, 0);
+  best_index = find_line_common (best_linetable, line, 0, &exact, 0);
   if (best_index < 0 || !exact)
     {
       /* Didn't find an exact match.  So we better keep looking for
@@ -3432,7 +3432,7 @@ find_line_symtab (struct symtab *sym_tab, int line,
 				    symtab_to_fullname (s)) != 0)
 		    continue;	
 		  l = SYMTAB_LINETABLE (s);
-		  ind = find_line_common (l, line, &exact, 0);
+		  ind = find_line_common (l, line, 0, &exact, 0);
 		  if (ind >= 0)
 		    {
 		      if (exact)
@@ -3466,17 +3466,16 @@ find_line_symtab (struct symtab *sym_tab, int line,
   return best_symtab;
 }
 
-/* Given SYMTAB, returns all the PCs function in the symtab that
-   exactly match LINE.  Returns an empty vector if there are no exact
-   matches, but updates BEST_ITEM in this case.  */
+/* Given SYMTAB, returns all the PCs and columns in the symtab that
+   exactly match LINE (and COLUMN, if specified).  Returns an empty vector
+   if there are no exact matches, but updates BEST_ITEM in this case.  */
 
-std::vector<CORE_ADDR>
-find_pcs_for_symtab_line (struct symtab *symtab, int line,
-			  struct linetable_entry **best_item,
-			  std::vector<int> *columns)
+std::vector<std::pair<CORE_ADDR, int>>
+find_pcs_for_symtab_line (struct symtab *symtab, int line, int column,
+			  struct linetable_entry **best_item)
 {
   int start = 0;
-  std::vector<CORE_ADDR> result;
+  std::vector<std::pair<CORE_ADDR, int>> result;
 
   /* First, collect all the PCs that are at this line.  */
   while (1)
@@ -3484,8 +3483,8 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line,
       int was_exact;
       int idx;
 
-      idx = find_line_common (SYMTAB_LINETABLE (symtab), line, &was_exact,
-			      start);
+      idx = find_line_common (SYMTAB_LINETABLE (symtab), line, column,
+			      &was_exact, start);
       if (idx < 0)
 	break;
 
@@ -3500,9 +3499,8 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line,
 	  break;
 	}
 
-      result.push_back (SYMTAB_LINETABLE (symtab)->item[idx].pc);
-      if (columns != nullptr)
-	columns->push_back (SYMTAB_LINETABLE (symtab)->item[idx].column);
+      result.push_back ({SYMTAB_LINETABLE (symtab)->item[idx].pc,
+			 SYMTAB_LINETABLE (symtab)->item[idx].column});
       start = idx + 1;
     }
 
@@ -3582,7 +3580,7 @@ find_line_pc_range (struct symtab_and_line sal, CORE_ADDR *startptr,
    Set *EXACT_MATCH nonzero if the value returned is an exact match.  */
 
 static int
-find_line_common (struct linetable *l, int lineno,
+find_line_common (struct linetable *l, int lineno, int columnno,
 		  int *exact_match, int start)
 {
   int i;
@@ -3594,6 +3592,7 @@ find_line_common (struct linetable *l, int lineno,
 
   int best_index = -1;
   int best = 0;
+  int bestcol = 0;
 
   *exact_match = 0;
 
@@ -3611,7 +3610,8 @@ find_line_common (struct linetable *l, int lineno,
       if (!item->is_stmt)
 	continue;
 
-      if (item->line == lineno)
+      if (item->line == lineno
+	  && (columnno == 0 || item->column == columnno))
 	{
 	  /* Return the first (lowest address) entry which matches.  */
 	  *exact_match = 1;
@@ -3623,6 +3623,14 @@ find_line_common (struct linetable *l, int lineno,
 	  best = item->line;
 	  best_index = i;
 	}
+      else if (columnno != 0 && item->line == lineno
+	       && item->column > columnno
+	       && (best == 0 || item->line < best || item->column < bestcol))
+	{
+	  best = item->line;
+	  bestcol = item->column;
+	  best_index = i;
+	}
     }
 
   /* If we got here, we didn't get an exact match.  */
diff --git a/gdb/symtab.h b/gdb/symtab.h
index fb1b8c9393..03c9440a93 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -2272,10 +2272,9 @@ bool iterate_over_some_symtabs (const char *name,
 void iterate_over_symtabs (const char *name,
 			   gdb::function_view<bool (symtab *)> callback);
 
-
-std::vector<CORE_ADDR> find_pcs_for_symtab_line
-    (struct symtab *symtab, int line, struct linetable_entry **best_entry,
-     std::vector<int> *columns = nullptr);
+std::vector<std::pair<CORE_ADDR, int>> find_pcs_for_symtab_line
+    (struct symtab *symtab, int line, int column,
+     struct linetable_entry **best_entry);
 
 /* Prototype for callbacks for LA_ITERATE_OVER_SYMBOLS.  The callback
    is called once per matching symbol SYM.  The callback should return
-- 
2.26.2

Re: [RFC][PATCH 7/6] Implement column breakpoints

[Tom Tromey]

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

Hannes> I think it's straightforward, you just add the column number after the line
Hannes> number, separated by a colon:

Seems like a good idea to me.

Hannes> +  /* Column number which was used to place this location.  */
Hannes> +
Hannes> +  int column_number = 0;

It would be good to mention the special treatment of 0 in this comment.

thanks,
Tom