Re: [PATCH v10] gdb: Add source-tracking breakpoints feature

Hi Alexandra,

I think this is looking amazing now!  Thank you for continuing to work
on this feature.

I do have some more feedback, but it really is getting super minor now,
and I don't think there are any bugs that I've found, the worst I've got
is a couple of missing warnings, where GDB will stop tracking a
breakpoint but not tell the user.

Then there's some typos and style issues, but unless anyone else has
feedback, I hope V11 will be ready to merge.

Again, thanks for your amazing work on this.  I'm excited to see this
merged, and how it might develop in the future.

Alexandra Hájková <[email protected]> writes:

> When we rerun the executable after changing its source files,
> GDB would re-set all previously set breakpoints. The
> breakpoints set to the function names would remain at their initial
> locations. But the breakpoints which used filename:line notation would
> be silently shifted following the source code changes.
>
> To address this, GDB now optionally captures a small window of source
> code lines around each breakpoint set with filename:line notation,
> when it is first set. When the binary is reloaded, GDB detects the BFD
> change and tries to locate the same source context in the new file,
> and if successful, re-sets the breakpoint to the matched source code
> line.
>
> The breakpoint_source structure stores captured source code lines
> around a breakpoint location, along with a reference to the BFD
> that was current when the source was captured.
>
> When source tracking is enabled (via 'set breakpoint source-tracking
> enabled on'), GDB captures 3 lines of source context
> (BREAKPOINT_SRC_CTX_LINES) along with the current BFD when a
> breakpoint is first set.  On executable reload (detected by comparing
> BFDs), it searches within a 12-line window
> (BREAKPOINT_SRC_CTX_LINES * BREAKPOINT_SRC_SEARCH_MULTIPLIER) for
> the best match and adjusts the breakpoint location if needed.
>
> If source tracking is disabled after breakpoints have been tracked,
> all existing source tracking information is discarded and a message
> is printed.
>
> Tests added:
>   gdb.base/adjust_breakpoint.exp
>   gdb.base/adjust_breakpoint-missing-source.exp
>   gdb.base/source-tracking-inline.exp
>   gdb.base/test_source_tracking.exp
>
> adjust_breakpoint.exp covers four scenarios:
>   - adjust the breakpoint when lines are deleted
>   - adjust the breakpoint when lines are inserted
>   - the tracked line disappears entirely
>   - verify the tracking can be disabled
>
> adjust_breakpoint-missing-source.exp covers the edge case where source
> files are unavailable, verifying GDB falls back to non-tracking breakpoints.
>
> source-tracking-inline.exp covers source tracking with inline functions.
>
> test_source_tracking.exp verifies that source context is correctly captured
> when the breakpoint is on the last line of the file.
>
> Add maintenance command to print tracked source code.
> Add documentation for the new source-tracking breakpoints feature.
>
> Limitations of the current implementation:
>
> Source tracking is not enabled for pending breakpoints that become
> non-pending.  When a breakpoint is created pending (e.g. with 'set
> breakpoint pending on'), source context is not captured at creation
> time since no symtab is available yet.  When the breakpoint later
> resolves to a location, re_set_default() only updates existing tracked
> breakpoints and does not initiate tracking for newly resolved ones.
> This could be fixed in the future by initiating source tracking in
> re_set_default() when a breakpoint transitions from pending to
> non-pending.
>
> Source tracking for ranged breakpoints is not currently supported.
> Ranged breakpoints have a start and end location spec, and tracking
> both independently raises questions about whether to preserve the
> range length or track each end separately.  For now, ranged
> breakpoints will never be source-tracked.
>
> Reviewed-By: Eli Zaretskii <[email protected]>
> ---
> v10: 
> - update the comment
> - catch the result location_spec_to_sals to the temporary variable
>
>  gdb/NEWS                                      |  17 +
>  gdb/breakpoint.c                              | 487 ++++++++++++++++++
>  gdb/breakpoint.h                              |  15 +
>  gdb/doc/gdb.texinfo                           |  44 ++
>  .../gdb.base/adjust_breakpoint-2.cpp          |  39 ++
>  .../gdb.base/adjust_breakpoint-3.cpp          |  41 ++
>  .../gdb.base/adjust_breakpoint-4.cpp          |  37 ++
>  .../adjust_breakpoint-missing-source.exp      |  55 ++
>  gdb/testsuite/gdb.base/adjust_breakpoint.cpp  |  40 ++
>  gdb/testsuite/gdb.base/adjust_breakpoint.exp  | 167 ++++++
>  .../gdb.base/source-tracking-inline-1.c       |  50 ++
>  .../gdb.base/source-tracking-inline-2.c       |  49 ++
>  .../gdb.base/source-tracking-inline.exp       |  80 +++
>  gdb/testsuite/gdb.base/test_source_tracking.c |  18 +
>  .../gdb.base/test_source_tracking.exp         |  42 ++
>  15 files changed, 1181 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/adjust_breakpoint-2.cpp
>  create mode 100644 gdb/testsuite/gdb.base/adjust_breakpoint-3.cpp
>  create mode 100644 gdb/testsuite/gdb.base/adjust_breakpoint-4.cpp
>  create mode 100644 gdb/testsuite/gdb.base/adjust_breakpoint-missing-source.exp
>  create mode 100644 gdb/testsuite/gdb.base/adjust_breakpoint.cpp
>  create mode 100644 gdb/testsuite/gdb.base/adjust_breakpoint.exp
>  create mode 100644 gdb/testsuite/gdb.base/source-tracking-inline-1.c
>  create mode 100644 gdb/testsuite/gdb.base/source-tracking-inline-2.c
>  create mode 100644 gdb/testsuite/gdb.base/source-tracking-inline.exp
>  create mode 100644 gdb/testsuite/gdb.base/test_source_tracking.c
>  create mode 100644 gdb/testsuite/gdb.base/test_source_tracking.exp
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 10c182067f9..17f522bc71f 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -8,6 +8,13 @@
>  * Support for the Common Trace Format (CTF) has been removed.  GDB now
>    saves trace information exclusively in its own "tfile" format.
>  
> +* GDB now supports source-tracking breakpoints, which automatically
> +  adjust their location when source code changes between rebuilds.
> +  When enabled, file and line breakpoints capture the surrounding
> +  source code context and use it to adjust the breakpoint line if the
> +  source is modified.  Source tracking can be enabled with 'set
> +  breakpoint source-tracking enabled on'.

Now that GDB 18 has branched this needs moving into the "Changes since
GDB 18" section.

> +
>  * Support for .gdb_index sections with version less than 7 has been
>    removed.
>  
> @@ -143,6 +150,16 @@ unset local-environment
>    environment.  The local environment is used by "shell", "pipe", and
>    other commands that launch a subprocess other than an inferior.
>  
> +set breakpoint source-tracking enabled [on|off]
> +show breakpoint source-tracking enabled
> +  Enable or disable source-tracking for file and line breakpoints.
> +  When enabled, breakpoints capture surrounding source code and
> +  automatically adjust their location when the source changes between
> +  recompilations.
> +maintenance info source-tracking-context BPNUM
> +  Displays the captured source context that GDB uses to track and
> +  adjust a breakpoint when source changes.
> +

And these will need moving too.

>  save history FILENAME
>    Save the command history to the given file.
>  
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 6ca196133fd..b23e43502b3 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -69,6 +69,8 @@
>  #include "cli/cli-style.h"
>  #include "cli/cli-decode.h"
>  #include "break-cond-parse.h"
> +#include "source-cache.h"
> +#include "gdb_bfd.h"
>  
>  /* readline defines this.  */
>  #undef savestring

> @@ -582,6 +621,60 @@ show_always_inserted_mode (struct ui_file *file, int from_tty,
>  	      value);
>  }
>  
> +/* When true file and line breakpoints are created as source-tracking
> +   breakpoints.  */

Missing a comma, should be "When true, file and line ....".

> @@ -593,6 +686,176 @@ show_debug_breakpoint (struct ui_file *file, int from_tty,
>    gdb_printf (file, _("Breakpoint location debugging is %s.\n"), value);
>  }
>  
> +/* Return true if the breakpoint source is being tracked.  */
> +
> +static bool
> +breakpoint_source_is_tracked (const breakpoint_source *src)
> +{
> +  if (src == nullptr)
> +    return false;
> +  return !src->source_lines.empty () && src->bp_line > 0;
> +}
> +
> +/* Calculate the starting line number for captured source.  */
> +
> +static int
> +breakpoint_source_get_start_line (const breakpoint_source *src)
> +{
> +  if (!breakpoint_source_is_tracked (src))
> +    return 0;
> +  return src->bp_line - src->bp_line_stored;
> +}
> +
> +/* Return true if SPEC is suitable for source tracking, otherwise false.  A
> +   location spec is suitable for tracking if it is an explicit location
> +   spec, and the line offset is an absolute line number.  We also don't
> +   allow for SPEC to be function or label based.  Most of these
> +   restrictions could be lifted, but this would likely require additional
> +   work to support these changes, especially when updating the location
> +   spec.  */
> +
> +static bool
> +breakpoint_locspec_suitable_for_tracking (const location_spec *spec)
> +{
> +  if (spec->type () != EXPLICIT_LOCATION_SPEC)
> +    return false;
> +
> +  const explicit_location_spec *explicit_loc
> +    = as_explicit_location_spec (spec);
> +
> +  if (explicit_loc->function_name.get () != nullptr
> +      || explicit_loc->label_name.get () != nullptr
> +      || explicit_loc->source_filename.get () == nullptr)
> +    return false;
> +
> +  if (explicit_loc->line_offset.sign != LINE_OFFSET_NONE)
> +    return false;
> +
> +  return explicit_loc->line_offset.offset > 0;
> +}
> +
> +/* Print captured source lines, marking the breakpoint line with '>'.  */
> +
> +static void
> +breakpoint_source_print (const breakpoint_source *src)
> +{
> +  if (!breakpoint_source_is_tracked (src))
> +    return;
> +
> +  int start_line = breakpoint_source_get_start_line (src);
> +  for (std::size_t j = 0; j < src->source_lines.size (); j++)
> +    {
> +      int line_num = start_line + (int) j;
> +      char prefix;
> +      if (j == src->bp_line_stored)
> +	prefix = '>';
> +      else
> +	prefix = ' ';
> +      gdb_printf ("%c %ps %s", prefix,
> +		  styled_string (line_number_style.style (),
> +				 plongest (line_num)),
> +		  src->source_lines[j].c_str ());
> +      if (src->source_lines[j].empty ()
> +	  || src->source_lines[j].back () != '\n')
> +	gdb_putc ('\n');
> +    }
> +}
> +
> +/* Implement the "maintenance info source-tracking-context" command.  */
> +
> +static void
> +maintenance_info_source_tracking_context (const char *args, int from_tty)
> +{
> +  if (args == nullptr || *args == '\0')
> +    error (_("Breakpoint number required."));
> +
> +  /* Parse the breakpoint number.  */
> +  const char *end = args;
> +  int num = get_number_trailer (&end, 0);
> +
> +  if (num <= 0)
> +    error (_("Invalid breakpoint number '%s'."), args);
> +
> +  /* Find the breakpoint.  */
> +  breakpoint *b = nullptr;
> +  for (breakpoint &bp : all_breakpoints ())
> +    {
> +      if (bp.number == num)
> +	{
> +	  b = &bp;
> +	  break;
> +	}
> +    }
> +
> +  if (b == nullptr)
> +    error (_("No breakpoint number %d."), num);

There's a helper for this!  You can write:

  breakpoint *b = get_breakpoint (num);
  if (b == nullptr)
    error (_("No breakpoint number %d."), num);


> @@ -13181,6 +13485,154 @@ code_breakpoint::location_spec_to_sals (location_spec *locspec,
>    return sals;
>  }
>  
> +/* Search for the best match of BP_SOURCE's captured context lines within
> +   TMP_SOURCE's larger search window.  For each candidate position where
> +   the breakpoint line matches, compare all BREAKPOINT_SRC_CTX_LINES
> +   captured context lines and track the position with the highest match
> +   score.  Only accept the match if a majority of the context lines
> +   matched.
> +
> +   Returns new breakpoint line on success or -1 on failure.  */
> +
> +static int
> +sliding_window_match (breakpoint_source *bp_source,
> +		      breakpoint_source *tmp_source)
> +{
> +  /* The index into BP_SOURCE's lines where the breakpoint was placed.  */
> +  int bp_stored = (int) bp_source->bp_line_stored;
> +  int bp_size = (int) bp_source->source_lines.size ();
> +
> +  /* Now search TMP_SOURCE for the breakpoint line.  */
> +  int tmp_size = (int) tmp_source->source_lines.size ();
> +
> +  int best_score = 0;
> +  int best_pos = -1;
> +
> +  for (int i = 0; i < tmp_size; i++)
> +    {
> +      /* Fast filter: breakpoint line must match.  */
> +      if (bp_source->source_lines[bp_stored] != tmp_source->source_lines[i])
> +	continue;
> +
> +      int match = 0;
> +      for (int j = 0; j < bp_size; j++)
> +	{
> +	  int idx = i - bp_stored + j;
> +	  if (idx >= 0 && idx < tmp_size
> +	      && bp_source->source_lines[j]
> +	      == tmp_source->source_lines[idx])
> +	    match++;
> +	}
> +
> +      if (match > best_score)
> +	{
> +	  best_score = match;
> +	  best_pos = i;
> +	}
> +    }
> +
> +  /* Only accept the match if a majority of the context lines matched.  */
> +  if (best_pos >= 0 && best_score > bp_size / 2)
> +    return tmp_source->bp_line + best_pos - tmp_source->bp_line_stored;
> +
> +  /* The updated breakpoint location has not been found in TMP_SOURCE.  */
> +  return -1;
> +}
> +
> +/* See breakpoint.h.  */
> +
> +void
> +code_breakpoint::adjust_bp_for_source_tracking
> +  (program_space *filter_pspace,
> +   std::vector<symtab_and_line> &expanded)
> +{
> +  if (expanded.empty () || expanded[0].symtab == nullptr
> +      || !breakpoint_source_is_tracked (bp_source.get ()))
> +    return;
> +
> +  struct compunit_symtab &cust = expanded[0].symtab->compunit ();
> +  if (cust.objfile () == nullptr)
> +    return;
> +
> +  bfd *current_bfd = cust.objfile ()->obfd.get ();
> +  if (bp_source->source_bfd.get () == current_bfd)
> +    return;
> +
> +  /* BFD changed - executable was reloaded.  */
> +  if (expanded.size () != 1)
> +    {
> +      warning (_("Breakpoint %d now has multiple locations after reload, "
> +		 "disabling source tracking."), number);
> +      bp_source.reset ();
> +      return;
> +    }
> +
> +  /* If this fails then the location spec has changed since the
> +     breakpoint's source tracking was initially setup.  */
> +  gdb_assert (breakpoint_locspec_suitable_for_tracking (locspec.get ()));
> +
> +  std::string line;
> +  auto restore_styling = make_scoped_restore (&source_styling, false);
> +  if (!g_source_cache.get_source_lines (expanded[0].symtab,
> +					expanded[0].line,
> +					expanded[0].line, &line))
> +    {
> +      /* Source is unreadable after reload - drop tracking.  */
> +      bp_source.reset ();

Should we not issue a warning at this point:

  warning (_("could not re-capture source lines, "
             "breakpoint %d no longer source tracked"),
           number);

> +      return;
> +    }
> +
> +  if (line == bp_source->source_lines[bp_source->bp_line_stored])
> +    {
> +      /* Line unchanged - just refresh the capture with the new BFD.  */
> +      bp_source = std::make_unique<breakpoint_source>
> +	(breakpoint_source_capture (expanded, BREAKPOINT_SRC_CTX_LINES));
> +      return;
> +    }
> +
> +  breakpoint_source tmp_source
> +    = breakpoint_source_capture (expanded,
> +				 BREAKPOINT_SRC_CTX_LINES
> +				 * BREAKPOINT_SRC_SEARCH_MULTIPLIER);
> +  int new_bp_line = sliding_window_match (bp_source.get (), &tmp_source);
> +  if (new_bp_line == -1)
> +    {
> +      warning (_("Breakpoint %d source code not found "
> +		 "after reload, keeping original location."), number);
> +      bp_source.reset ();
> +      return;
> +    }
> +
> +  auto *explicit_loc = as_explicit_location_spec (locspec.get ());
> +  location_spec_up new_locspec = explicit_loc->clone ();
> +  auto *new_explicit = as_explicit_location_spec (new_locspec.get ());

I'm really not a fan of this use of `auto` and until I'm told otherwise,
I'm on a mission to try and stop these creeping into GDB.  My reasoning
is that code is written once and read many times.  These `auto` make the
code quicker to write, but harder to read as you need to go and lookup
what type this actually is.

Please replace the `auto` here with `const explicit_location_spec`.

> +  new_explicit->line_offset.offset = new_bp_line;
> +  new_explicit->line_offset.sign = LINE_OFFSET_NONE;
> +  /* Invalidate the cached display string.  */
> +  new_explicit->set_string ("");
> +
> +  int found;
> +  auto new_expanded = location_spec_to_sals (new_locspec.get (), filter_pspace, &found);

And here, `auto` should be `std::vector<symtab_and_line>` (IMHO).

> +  if (!found)
> +    {
> +      warning (_("Breakpoint %d adjusted to line %d but location could not "
> +		"be resolved; keeping original location."), number, new_bp_line);
> +      bp_source.reset ();
> +      return;
> +    }
> +  expanded = std::move (new_expanded);
> +  locspec = std::move (new_locspec);
> +  if (new_bp_line != bp_source->bp_line)
> +    {
> +      gdb_printf (_("Breakpoint %d adjusted from line %d to line %d.\n"),
> +		  number, bp_source->bp_line, new_bp_line);
> +      notify_breakpoint_modified (this);
> +    }
> +
> +  bp_source = std::make_unique<breakpoint_source>
> +    (breakpoint_source_capture (expanded, BREAKPOINT_SRC_CTX_LINES));

If this call to breakpoint_source_capture fails, and returns an empty
breakpoint_source, then this breakpoint will stop being tracked.  This
could confuse a user as the breakpoint _was_ tracked, and we _did_ just
update its location.  It's pretty unlikely that this call would fail
given all of the above, but if it did it might be worth adding a check
after this line:

  if (!breakpoint_source_is_tracked (bp_source.get ()))
    warning (_("failed to capture breakpoint source context, "
               "disabling source tracking for breakpoint %d"),
             number);

> +}
> +
>  /* The default re_set method, for typical hardware or software
>     breakpoints.  Reevaluate the breakpoint and recreate its
>     locations.  */

> diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
> index 722d75390fa..ba6189ec504 100644
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -27,6 +27,7 @@
>  #include "probe.h"
>  #include "location.h"
>  #include <vector>
> +#include <memory>

This extra include is not needed as this is already pulled in
(indirectly) via GDB's defs.h file, which is included in every file
compiled under gdb/.

>  #include "gdbsupport/array-view.h"
>  #include "gdbsupport/filtered-iterator.h"
>  #include "gdbsupport/iterator-range.h"

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 0030698dcee..7aa64054fad 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -4661,6 +4661,27 @@ program.
>  On some systems, you can set breakpoints in shared libraries before
>  the executable is run.
>  
> +@cindex source-tracking breakpoints
> +@cindex breakpoints, automatic adjustment when source changes
> +@value{GDBN} supports @dfn{source-tracking breakpoints}, which
> +automatically adjust their location when source code changes between
> +recompilations.  When enabled with @code{set breakpoint source-tracking

Use @kbd not @code here please.

> +enabled on}, breakpoints set by file and line number capture a small
> +window of surrounding source lines centered on the breakpoint line.
> +If the source file is modified and the executable is rebuilt,
> +@value{GDBN} searches a window of approximately 12 lines centered on the
> +breakpoint's original position for the best match.  For each candidate
> +line that matches the original breakpoint line, @value{GDBN} compares
> +all captured context lines at their corresponding offsets and scores the
> +match by how many lines agree.  The candidate with the highest score is
> +selected, provided a majority of the context lines match, reducing false
> +positives from short or repeated lines.  If no match meets the threshold,
> +@value{GDBN} issues a warning and keeps the breakpoint at its original
> +location, disabling source tracking for that breakpoint.  Note that
> +breakpoints set by function name or address are not affected by source
> +tracking.
> +@xref{Set Breaks}.
> +
>  @cindex watchpoints
>  @cindex data breakpoints
>  @cindex memory tracing
> @@ -42065,6 +42086,13 @@ Shared library events.
>  
>  @end table
>  
> +@kindex maint info source-tracking-context
> +@item maint info source-tracking-context @var{num}
> +For source-tracking breakpoints (@pxref{Breakpoints}), print the
> +tracked source code context for breakpoint @var{num}.  If breakpoint
> +@var{num} is not source tracked, or @var{num} is not a valid
> +breakpoint number, then the command gives an error.
> +
>  @kindex maint info btrace
>  @item maint info btrace
>  Pint information about raw branch tracing data.
> @@ -42933,6 +42961,22 @@ Control whether to show all non zero areas within a 1k block starting
>  at thread local base, when using the @samp{info w32 thread-information-block}
>  command.
>  
> +@kindex set breakpoint source-tracking enabled
> +@kindex show breakpoint source-tracking enabled
> +@item set breakpoint source-tracking enabled @r{[}on@r{|}off@r{]}
> +@itemx show breakpoint source-tracking enabled
> +Control whether to enable source-tracking for breakpoints set by file and
> +line number.  Use @code{on} to enable, @code{off} to disable.  When
> +enabled, @value{GDBN} captures a window of source lines around each
> +new file:line breakpoint and uses it to relocate the breakpoint if the
> +source is modified and the executable is rebuilt.  @xref{Breakpoints},
> +for a full description of the matching algorithm and its limitations.
> +The default is @code{off}.  Breakpoints set by function name or address
> +are not affected by this setting.
> +
> +If this setting is changed from @code{on} to @code{off}, then any
> +existing source tracking information will be discarded.

In the above use @samp instead of @code when wrapping on/off please.

> +
>  @kindex maint set target-async
>  @kindex maint show target-async
>  @item maint set target-async


> diff --git a/gdb/testsuite/gdb.base/source-tracking-inline-2.c b/gdb/testsuite/gdb.base/source-tracking-inline-2.c
> new file mode 100644
> index 00000000000..ab95a312c6a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/source-tracking-inline-2.c
> @@ -0,0 +1,49 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2026 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +volatile int global_var = 0;

There should be a blank line before the "volatile int global_var = 0;" line.

Thanks,
Andrew