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

[Andrew Burgess <aburgess@redhat.com>]

Alexandra Hájková <ahajkova@redhat.com> 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 <eliz@gnu.org>

Hi Alexandra,

Thanks for your continued work on this feature.  I think this might be
the last round of feedback.  There's a few really minor issues, there
are a couple that really need addressing before this can be merged, but
it's looking really close now.

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 10c182067f9..a111b3c6ea8 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,25 @@
>  
>  *** Changes since GDB 18
>  
> +* 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'.
> +
> +* New commands
> +
> +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

You should place a blank line before the 'maintenance info
source-tracking-context BPNUM' line.

> +  Displays the captured source context that GDB uses to track and
> +  adjust a breakpoint when source changes.
> +
>  *** Changes in GDB 18
>  
>  * Support for the Common Trace Format (CTF) has been removed.  GDB now
> @@ -143,6 +162,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.
> +
>  save history FILENAME
>    Save the command history to the given file.
>

This is a duplicate and should be deleted.



> +/* 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)

Both of these arguments can be 'const breakpoint_source *' as neither
are updated.

But if you're changing this then it would be nice to update these
arguments to 'const breakpoint_source &' -- references rather than
pointers.  This removes the possibility of either being NULL.  You'll
need to update the function body to replace '->' with '.', and the
calling line becomes:

  int new_bp_line = sliding_window_match (*bp_source.get (), tmp_source);

But that would be neater.



> +/* 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.  */

Nit: 'setup' -> 'set up'.

> +  gdb_assert (breakpoint_locspec_suitable_for_tracking (locspec.get ()));
> +
> +  std::string line;
> +  scoped_restore 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 ();
> +      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;

This call to breakpoint_source_capture could, in theory, fail, though I
don't know why it ever would.  But still, you should handle this case
like you do all the other breakpoint_source_capture calls:

  if (!breakpoint_source_is_tracked (b->bp_source.get ()))
    {
      warning (_("unable to refresh source tracking for breakpoint %d, "
                 "source tracking disabled"),
               number);
      b->bp_source.reset ();
    }

> +    }
> +
> +  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;
> +    }
> +
> +  explicit_location_spec *explicit_loc = as_explicit_location_spec (locspec.get ());
> +  location_spec_up new_locspec = explicit_loc->clone ();
> +  explicit_location_spec *new_explicit = as_explicit_location_spec (new_locspec.get ());
> +  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;
> +  std::vector<symtab_and_line> new_expanded = location_spec_to_sals (new_locspec.get (),
> +								     filter_pspace, &found);
> +  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 (!breakpoint_source_is_tracked (bp_source.get ()))
> +    warning (_("failed to capture breakpoint source context, "
> +	       "disabling source tracking for breakpoint %d"),
> +	     number);

In other places where you disable source tracking you reset bp_source,
but not here.  I think this should be made consistent.


> diff --git a/gdb/testsuite/gdb.base/source-tracking-inline.exp b/gdb/testsuite/gdb.base/source-tracking-inline.exp
> new file mode 100644
> index 00000000000..96849dd1bb6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/source-tracking-inline.exp
> @@ -0,0 +1,80 @@
> +# 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/>.
> +
> +# Test of the source tracking breakpoint feature when trying to place
> +# a breakpoint on inline functions.  As the breakpoint resolves to
> +# multiple locations we don't currently source track these
> +# breakpoints.
> +
> +standard_testfile -1.c -2.c
> +
> +set build_srcfile [standard_output_file ${testfile}.c]
> +
> +remote_exec build "cp $srcdir/$subdir/$srcfile $build_srcfile"
> +if { [prepare_for_testing "failed to prepare" $testfile $build_srcfile] } {
> +    return
> +}
> +
> +if {![runto_main]} {
> +    return
> +}
> +
> +gdb_test_no_output "set breakpoint source-tracking enabled on" \
> +    "enable source tracking breakpoints"
> +
> +set lineno [gdb_get_line_number "Breakpoint here." $build_srcfile]
> +
> +# The breakpoint is on an inline function called from two places,
> +# so it resolves to 2 locations.
> +gdb_test "break ${testfile}.c:$lineno" \
> +    "^Breakpoint $decimal at $hex: ${testfile}.c:$lineno\\. \\(2 locations\\)"
> +
> +# Check that we are tracking the expected number of lines.
> +#
> +# With the multi-location fix, this breakpoint should NOT be tracked
> +# because it has 2 locations (inline function called from two places).
> +# The output should show <MULTIPLE> but NOT "source-tracking enabled".
> +gdb_test_multiple "info breakpoints" \
> +  "multi-location breakpoint not tracked" {
> +    -re -wrap "source-tracking enabled.*" {
> +      fail "$gdb_test_name (tracking incorrectly enabled)"
> +    }
> +    -re -wrap "<MULTIPLE>.*" {
> +      pass $gdb_test_name
> +    }
> +}
> +
> +sleep 1
> +remote_exec build "cp $srcdir/$subdir/$srcfile2 $build_srcfile"
> +if { [build_executable "failed to build" $testfile $build_srcfile] } {
> +    return
> +}

The two files:

  gdb/testsuite/gdb.base/source-tracking-inline-1.c
  gdb/testsuite/gdb.base/source-tracking-inline-2.c

are identical, so there's no reason to ship these two different files.
The point of this particular test is to ensure that we don't start
tracking a multi-location breakpoint, so you cannot change the source
too much, but given the nature of the test, it might be nice if there
was some actual change.

Thanks,
Andrew