Re: [PATCH 1/2] Support the new PPC476 processor -- Arch Independent

Luis Machado
2009-12-29T18:59:33Z

Hey,

On Tue, 2009-12-29 at 16:55 -0200, Luis Machado wrote:
> > > -/* Check watchpoint condition. */
> > > +/* Check watchpoint condition. We can't use value_equal because it coerces
> > > + an array to a pointer, thus comparing just the address of the array instead
> > > + of its contents. This is not what we want. */
> > > +
> > > +static int
> > > +value_equal_watchpoint (struct value *arg1, struct value *arg2)
> > > +{
> > > + struct type *type1, *type2;
> > > +
> > > + type1 = check_typedef (value_type (arg1));
> > > + type2 = check_typedef (value_type (arg2));
> > > +
> > > + return TYPE_CODE (type1) == TYPE_CODE (type2)
> > > + && TYPE_LENGTH (type1) == TYPE_LENGTH (type2)
> > > + && memcmp (value_contents (arg1), value_contents (arg2),
> > > + TYPE_LENGTH (type1)) == 0;
> > > +}
> > >
> > > static int
> > > watchpoint_check (void *p)
> > > @@ -3246,7 +3388,7 @@ watchpoint_check (void *p)
> > >
> > > fetch_watchpoint_value (b->exp, &new_val, NULL, NULL);
> > > if ((b->val != NULL) != (new_val != NULL)
> > > - || (b->val != NULL && !value_equal (b->val, new_val)))
> > > + || (b->val != NULL && !value_equal_watchpoint (b->val, new_val)))
> >
> > Can you elaborate the need for this change? It seems to change the
> > semantics of watchpoint_check, so I wonder why it is done.
> >
>
> It's done so we can detect triggers due to a range watchpoint. Since we
> don't have the data address that caused the trigger, we need to go
> through the entire range of a range watchpoint in order to tell if
> something has changed. That's why we have a specific function to check
> that (value_equal_watchpoint (b->val, new_val)).

Just ignore this bit since Thiago already dealt with this one in his
recent patch.

Luis