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

On Fri,  7 Aug 2026 12:46:37 +0200
Alexandra Hájková <[email protected]> wrote:

[...]
> +/* Print captured source lines to stdout, marking the breakpoint line with '>'.  */

Nit: the captured source lines are actually printed via gdb_printf / uiout.
You might just say "Print captured source lines, marking the breakpoint..."

> +
> +static void
> +breakpoint_source_print (const breakpoint_source *src)

[...]

> +/* 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 ();
> +      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 ());
> +  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;
> +  expanded = location_spec_to_sals (new_locspec.get (), filter_pspace, &found);

This assignment (above) to the reference variable 'expanded'...

> +  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;

...won't actually keep the original locations intact as the message
states.  In this case, if it's not found, the list of locations returned
will be the empty vector and it'll replace the perfectly good list which
should be preserved when an error occurs.

I recommend doing the initial assignment to a local variable and then
making the assignment to 'expanded' after this error handling block.

> +    }
> +  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));
> +}
> +
>  /* The default re_set method, for typical hardware or software
>     breakpoints.  Reevaluate the breakpoint and recreate its
>     locations.  */
[...]