This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Forbid watchpoint on a constant value
On Thursday 20 May 2010 12:29:42, Jan Kratochvil wrote:
> On Thu, 20 May 2010 07:10:26 +0200, Sergio Durigan Junior wrote:
> > I also added other constant types to that switch statement. Please take a look
> > and see if you agree.
> [...]
> > + /* The user could provide something like:
> > +
> > + `watch *0xdeadbeef + 4'
> > +
> > + In this case, we need to check the remaining elements
> > + of this expression. */
> > + case BINOP_ADD:
>
> If you have overloaded operator '+' of some class cannot this operation
> execute an inferior function via value_x_binop()? Maybe it is not
> exploitable, I am not sure.
I don't know. I will see if I can make some tests to check.
> > + case BINOP_ASSIGN:
> > + case BINOP_ASSIGN_MODIFY:
> > + case OP_FUNCALL:
> > + case OP_OBJC_MSGCALL:
> > + case OP_F77_UNDETERMINED_ARGLIST:
> > + case UNOP_PREINCREMENT:
> > + case UNOP_POSTINCREMENT:
> > + case UNOP_PREDECREMENT:
> > + case UNOP_POSTDECREMENT:
>
> This is not a `const'/`pure' function, it has some side-effect of the
> assignment. I do not thing they should be caught as constant.
Sorry, I didn't understand why they can't be considered constant in the
context of a watchpoint.
> Offtopic here: they could be rather somehow forbidden from a watchpoint
> expression, moreover if it gets evaluated as a hardware watchpoint but that is
> already broken by incorrect/naive assumptions as filed in:
> PR breakpoints/11613: hardware watchpoint missed for -O2 -g inferior
>
>
> > + case BINOP_SUBSCRIPT:
>
> This is a regression:
> ./gdb -nx -ex 'p &line' -ex 'watch $0[0]' -ex r ./gdb
> now prints:
> Cannot watch constant value $0[0].
> but it was a valid watchpoint, hit at:
> captured_main (data=0x7fffffffd1c0) at ./main.c:322
Sorry, removed.
> > + case BINOP_VAL:
> > + case BINOP_INCL:
> > + case BINOP_EXCL:
> > + case UNOP_PLUS:
> > + case UNOP_CAP:
> > + case UNOP_CHR:
> > + case UNOP_ORD:
> > + case UNOP_ABS:
> > + case UNOP_FLOAT:
> > + case UNOP_MAX:
> > + case UNOP_MIN:
> > + case UNOP_ODD:
> > + case UNOP_TRUNC:
>
> I do not see implemented evaluation of these, also their processing should
> have been probably moved to some m2-* file.
Does it mean that I have to remove them from this list?
> > + case UNOP_LOWER:
> > + case UNOP_UPPER:
> > + case UNOP_LENGTH:
> > + case UNOP_CARD:
> > + case UNOP_CHMAX:
> > + case UNOP_CHMIN:
>
> I do not see implemented evaluation of these, also their processing should
> have been probably moved to ... the already deleted Chill support files.
Likewise.
> > + case OP_LAST:
>
> For values <=0 it will change, it is not a constant.
I think I didn't understand OP_LAST, then. Sorry about that, removed.
> > + case OP_INTERNALVAR:
>
> I would guess value of some of the internal variables can change.
Is the user allowed to put a watchpoint on an internal variable?
> > + /* UNOP_IND and UNOP_ADDR are not in this list becase
> > + they can be used in expressions like:
> > +
> > + (gdb) watch *0x12345678
> > +
> > + or
> > +
> > + (gdb) watch &some_var
> > + */
>
> I do not see why UNOP_ADDR should not be listed here (but sure not a problem).
You're right.
> > + case UNOP_SIZEOF:
>
> UNOP_SIZEOF on OP_TYPE where the type is TYPE_DYNAMIC from the VLA patchset
> would be a regression; but that is not in FSF GDB so it is OK now.
Ok, thanks for letting me know. I will remove UNOP_SIZEOF.
> > + case UNOP_HIGH:
>
> If it really should be here it could be moved into m2-* but this separation is
> already not strictly followed.
Sorry, I'm afraid I didn't understand your comment.
Thanks,
--
Sergio Durigan Junior
Red Hat