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 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.
> + 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.
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
> + 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.
> + 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.
> + case OP_LAST:
For values <=0 it will change, it is not a constant.
> + case OP_INTERNALVAR:
I would guess value of some of the internal variables can change.
> + /* 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).
> + 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.
> + case UNOP_HIGH:
If it really should be here it could be moved into m2-* but this separation is
already not strictly followed.
Thanks,
Jan