Re: [RFA] Fix use after free introduced by $_hit_bpnum/$_hit_locno variables.

[Simon Marchi <simark@simark.ca>]

On 11/20/22 05:44, Philippe Waroquiers via Gdb-patches wrote:
> If the commands of the bpstat bs contain commands such as step or next or
> continue, the BS and its commands are freed by execute_control_command.
> 
> So, we cannot remember the BS that was printed. Instead, remember
> the bpnum and locno.
> 
> Regtested on debian/amd64 and re-run a few tests under valgrind.
> ---
>  gdb/breakpoint.c | 62 ++++++++++++++++++++++++++++--------------------
>  1 file changed, 36 insertions(+), 26 deletions(-)
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 7f6400db624..5b691673a0e 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -4574,18 +4574,17 @@ command_line_is_silent (struct command_line *cmd)
>    return cmd && (strcmp ("silent", cmd->line) == 0);
>  }
>  
> -/* Sets the $_hit_bpnum and $_hit_locno to the bpnum and locno of bs.  */
> +/* Sets the $_hit_bpnum and $_hit_locno to bpnum and locno.
> +   A locno 0 is changed to 1 to e.g. let the user do
> +     (gdb) disable $_hit_bpnum.$_hit_locno
> +   for a single location breakpoint.  */
> +
>  static void
> -set_hit_convenience_vars (bpstat *bs)
> +set_hit_convenience_vars (int bpnum, int locno)
>  {
> -  const struct breakpoint *b = bs->breakpoint_at;
> -  if (b != nullptr)
> -    {
> -      int locno = bpstat_locno (bs);
> -      set_internalvar_integer (lookup_internalvar ("_hit_bpnum"), b->number);
> -      set_internalvar_integer (lookup_internalvar ("_hit_locno"),
> -			       (locno > 0 ? locno : 1));
> -    }
> +  set_internalvar_integer (lookup_internalvar ("_hit_bpnum"), bpnum);
> +  set_internalvar_integer (lookup_internalvar ("_hit_locno"),
> +			   (locno > 0 ? locno : 1));

I haven't had time to look at the original patch (sorry, just so many
things to do), but the naming caught my attention.  We have both "num"
and "no" as abbreviations for "number".  It looks a bit inconsistent.
Was it on purpose?  I don't so much about the variable names, but the
names exposed to the user.

But the fix LGTM:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon