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

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

[Doug Evans]

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

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

[Doug Evans]

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.

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

[Paul Pluzhnikov]

On Tue, Mar 17, 2009 at 9:36 PM, Doug Evans <dje@google.com> wrote:

> I noticed sigbpt.exp is failing.

I noticed that too, but then I re-ran just that test case alone
and it succeeded, and I didn't investigate further :-(

> I think this patch is the right fix.

Looks correct to me.
Thanks for debugging this.

-- 
Paul Pluzhnikov

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

[Joel Brobecker]

> 2009-03-17  Doug Evans  <dje@google.com>
> 
> 	* printcmd.c (display_uses_solib_p): Redo loop, scan element list
> 	backwards.

I missed the fact that operator_length operatored from the last index
of the operator :-(. Thanks for fixing this. This is OK.

-- 
Joel