Re: [RFA] (display_uses_solib_p): Redo loop, scan element list backwards.

[Doug Evans <dje@google.com>]

On Tue, Mar 17, 2009 at 9:36 PM, Doug Evans <dje@google.com> wrote:
> Hi.
>
> I noticed sigbpt.exp is failing.
> I was getting a "Value out of range" error inside display_uses_solib_p.
> I traced it to the expression elements array being referenced out of bounds.
>
> I think this patch is the right fix.
> The expression elements array needs to be scanned backwards
> when using operator_length: it examines the element at the _end_
> of the array.
>
> Ok to check in?
>
> 2009-03-17  Doug Evans  <dje@google.com>
>
>        * printcmd.c (display_uses_solib_p): Redo loop, scan element list
>        backwards.
>
> Index: printcmd.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/printcmd.c,v
> retrieving revision 1.147
> diff -u -p -r1.147 printcmd.c
> --- printcmd.c  9 Mar 2009 22:38:37 -0000       1.147
> +++ printcmd.c  18 Mar 2009 04:28:26 -0000
> @@ -1763,18 +1763,23 @@ static int
>  display_uses_solib_p (const struct display *d,
>                      const struct so_list *solib)
>  {
> -  int i;
> +  int endpos;
>   struct expression *const exp = d->exp;
> +  union exp_element *const elts = exp->elts;
>
>   if (d->block != NULL
>       && solib_contains_address_p (solib, d->block->startaddr))
>     return 1;
>
> -  for (i = 0; i < exp->nelts; )
> +  for (endpos = exp->nelts; endpos > 0; )
>     {
> -      int args, oplen = 0;
> -      const union exp_element *const elts = exp->elts;
> +      int i, args, oplen = 0;
>
> +      exp->language_defn->la_exp_desc->operator_length (exp, endpos,
> +                                                       &oplen, &args);
> +      gdb_assert (oplen > 0);
> +
> +      i = endpos - oplen;
>       if (elts[i].opcode == OP_VAR_VALUE)
>        {
>          const struct block *const block = elts[i + 1].block;
> @@ -1789,11 +1794,9 @@ display_uses_solib_p (const struct displ
>          if (section && section->objfile == solib->objfile)
>            return 1;
>        }
> -      exp->language_defn->la_exp_desc->operator_length (exp, i + 1,
> -                                                       &oplen, &args);
> -      gdb_assert (oplen > 0);
> -      i += oplen;
> +      endpos -= oplen;
>     }
> +
>   return 0;
>  }
>
>


btw, this was tested by singlestepping through display_uses_solib_p
with several moderately complex display expressions.
E.g.,
display/x ($pc + 123) * 321

before and after the patch is enough to see the problem.