Re: [PATCHv2] Fix an undefined behavior in record_line

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

On 3/13/20 12:55 PM, Bernd Edlinger wrote:
> Additionally do not completely remove symbols
> at the same PC than the end marker, instead
> make them non-is-stmt breakpoints.
> 
> Also fix the condition when the line table need to be resized,
> that was wasting one element.
> 
> 2020-03-10  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 	* buildsym.c (record_line): Fix ub and preserve lines at eof.
> ---
>  gdb/buildsym.c | 28 +++++++++++-----------------
>  1 file changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
> index 7155db3..960a36c 100644
> --- a/gdb/buildsym.c
> +++ b/gdb/buildsym.c
> @@ -695,7 +695,7 @@ struct blockvector *
>  	}
>      }
>  
> -  if (subfile->line_vector->nitems + 1 >= subfile->line_vector_length)
> +  if (subfile->line_vector->nitems >= subfile->line_vector_length)
>      {
>        subfile->line_vector_length *= 2;
>        subfile->line_vector = (struct linetable *)
> @@ -705,27 +705,21 @@ struct blockvector *
>  		      * sizeof (struct linetable_entry))));
>      }
>  
> -  /* Normally, we treat lines as unsorted.  But the end of sequence
> -     marker is special.  We sort line markers at the same PC by line
> -     number, so end of sequence markers (which have line == 0) appear
> -     first.  This is right if the marker ends the previous function,
> -     and there is no padding before the next function.  But it is
> -     wrong if the previous line was empty and we are now marking a
> -     switch to a different subfile.  We must leave the end of sequence
> -     marker at the end of this group of lines, not sort the empty line
> -     to after the marker.  The easiest way to accomplish this is to
> -     delete any empty lines from our table, if they are followed by
> -     end of sequence markers.  All we lose is the ability to set
> -     breakpoints at some lines which contain no instructions
> -     anyway.  */
> +  /* The end of sequence marker is special.  We need to reset the
> +     is_stmt flag on previous lines at the same PC, otherwise these
> +     lines may cause problems.  All we lose is the ability to set
> +     breakpoints at some lines which contain no instructions anyway.  */
>    if (line == 0 && subfile->line_vector->nitems > 0)
>      {
> -      e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
> -      while (subfile->line_vector->nitems > 0 && e->pc == pc)
> +      e = subfile->line_vector->item + subfile->line_vector->nitems;
> +      do
>  	{
>  	  e--;
> -	  subfile->line_vector->nitems--;
> +	  if (e->pc != pc || e->line == 0)
> +	    break;
> +	  e->is_stmt = 0;
>  	}
> +      while (e > subfile->line_vector->item);
>      }
>  
>    e = subfile->line_vector->item + subfile->line_vectoms++;
> 

Hi everyone,

I'd say this should be a no-brainer, fixind undefined
behavior, and not removing data when not necessary.

Is it OK for trunk?


Thanks
Bernd.