OpenE2K
/
binutils-gdb
12
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix PR12526: -location watchpoints for bitfield arguments 

PR 12526 reports that -location watchpoints against bitfield arguments
trigger false positives when bits around the bitfield, but not the
bitfield itself, are modified.

This happens because -location watchpoints naturally operate at the
byte level, not at the bit level.  When the address of a bitfield
lvalue is taken, information about the bitfield (i.e. its offset and
size) is lost in the process.

This information must first be retained throughout the lifetime of the
-location watchpoint.  This patch achieves this by adding two new
fields to the watchpoint struct: val_bitpos and val_bitsize.  These
fields are set when a watchpoint is first defined in watch_command_1.
They are both equal to zero if the watchpoint is not a -location
watchpoint or if the argument is not a bitfield.

Then these bitfield parameters are used inside update_watchpoint and
watchpoint_check to extract the actual value of the bitfield from the
watchpoint address, with the help of a local helper function
extract_bitfield_from_watchpoint_value.

Finally when creating a HW breakpoint pointing to a bitfield, we
optimize the address and length of the breakpoint.  By skipping over
the bytes that don't cover the bitfield, this step reduces the
frequency at which a read watchpoint for the bitfield is triggered.
It also reduces the number of times a false-positive call to
check_watchpoint is triggered for a write watchpoint.

gdb/
	PR breakpoints/12526
	* breakpoint.h (struct watchpoint): New fields val_bitpos and
	val_bitsize.
	* breakpoint.c (watch_command_1): Use these fields to retain
	bitfield information.
	(extract_bitfield_from_watchpoint_value): New function.
	(watchpoint_check): Use it.
	(update_watchpoint): Use it.  Optimize the address and length of a
	HW watchpoint pointing to a bitfield.
	* value.h (unpack_value_bitfield): New prototype.
	* value.c (unpack_value_bitfield): Make extern.

gdb/testsuite/
	PR breakpoints/12526
	* gdb.base/watch-bitfields.exp: New file.
	* gdb.base/watch-bitfields.c: New file.
This commit is contained in:
Patrick Palka 2014-09-16 17:40:06 +01:00 committed by Pedro Alves
parent d3d3c6db1a
commit bb9d5f81c3
8 changed files with 214 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
gdb/ChangeLog
View File

@ -1,3 +1,17 @@
2014-09-16 Patrick Palka <patrick@parcs.ath.cx>
PR breakpoints/12526
* breakpoint.h (struct watchpoint): New fields val_bitpos and
val_bitsize.
* breakpoint.c (watch_command_1): Use these fields to retain
bitfield information.
(extract_bitfield_from_watchpoint_value): New function.
(watchpoint_check): Use it.
(update_watchpoint): Use it. Optimize the address and length of a
HW watchpoint pointing to a bitfield.
* value.h (unpack_value_bitfield): New prototype.
* value.c (unpack_value_bitfield): Make extern.
2014-09-16 Samuel Thibault <samuel.thibault@ens-lyon.org>
* config/i386/i386gnu.mh (NATDEPFILES): Add x86-nat.o and

74
gdb/breakpoint.c
View File

@ -1703,6 +1703,29 @@ watchpoint_del_at_next_stop (struct watchpoint *w)
b->disposition = disp_del_at_next_stop;
}
/* Extract a bitfield value from value VAL using the bit parameters contained in
watchpoint W. */
static struct value *
extract_bitfield_from_watchpoint_value (struct watchpoint *w, struct value *val)
{
struct value *bit_val;
if (val == NULL)
return NULL;
bit_val = allocate_value (value_type (val));
unpack_value_bitfield (bit_val,
w->val_bitpos,
w->val_bitsize,
value_contents_for_printing (val),
value_offset (val),
val);
return bit_val;
}
/* Assuming that B is a watchpoint:
- Reparse watchpoint expression, if REPARSE is non-zero
- Evaluate expression and store the result in B->val
@ -1877,6 +1900,12 @@ update_watchpoint (struct watchpoint *b, int reparse)
watchpoints. */
if (!b->val_valid && !is_masked_watchpoint (&b->base))
{
if (b->val_bitsize != 0)
{
v = extract_bitfield_from_watchpoint_value (b, v);
if (v != NULL)
release_value (v);
}
b->val = v;
b->val_valid = 1;
}
@ -1906,8 +1935,31 @@ update_watchpoint (struct watchpoint *b, int reparse)
CORE_ADDR addr;
int type;
struct bp_location *loc, **tmp;
int bitpos = 0, bitsize = 0;
if (value_bitsize (v) != 0)
{
/* Extract the bit parameters out from the bitfield
sub-expression. */
bitpos = value_bitpos (v);
bitsize = value_bitsize (v);
}
else if (v == result && b->val_bitsize != 0)
{
/* If VAL_BITSIZE != 0 then RESULT is actually a bitfield
lvalue whose bit parameters are saved in the fields
VAL_BITPOS and VAL_BITSIZE. */
bitpos = b->val_bitpos;
bitsize = b->val_bitsize;
}
addr = value_address (v);
if (bitsize != 0)
{
/* Skip the bytes that don't contain the bitfield. */
addr += bitpos / 8;
}
type = hw_write;
if (b->base.type == bp_read_watchpoint)
type = hw_read;
@ -1922,7 +1974,15 @@ update_watchpoint (struct watchpoint *b, int reparse)
loc->pspace = frame_pspace;
loc->address = addr;
loc->length = TYPE_LENGTH (value_type (v));
if (bitsize != 0)
{
/* Just cover the bytes that make up the bitfield. */
loc->length = ((bitpos % 8) + bitsize + 7) / 8;
}
else
loc->length = TYPE_LENGTH (value_type (v));
loc->watchpoint_type = type;
}
}
@ -5039,6 +5099,9 @@ watchpoint_check (void *p)
mark = value_mark ();
fetch_subexp_value (b->exp, &pc, &new_val, NULL, NULL, 0);
if (b->val_bitsize != 0)
new_val = extract_bitfield_from_watchpoint_value (b, new_val);
/* We use value_equal_contents instead of value_equal because
the latter coerces an array to a pointer, thus comparing just
the address of the array instead of its contents. This is
@ -11203,6 +11266,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
struct expression *exp;
const struct block *exp_valid_block = NULL, *cond_exp_valid_block = NULL;
struct value *val, *mark, *result;
int saved_bitpos = 0, saved_bitsize = 0;
struct frame_info *frame;
const char *exp_start = NULL;
const char *exp_end = NULL;
@ -11336,6 +11400,12 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
mark = value_mark ();
fetch_subexp_value (exp, &pc, &val, &result, NULL, just_location);
if (val != NULL && just_location)
{
saved_bitpos = value_bitpos (val);
saved_bitsize = value_bitsize (val);
}
if (just_location)
{
int ret;
@ -11471,6 +11541,8 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
else
{
w->val = val;
w->val_bitpos = saved_bitpos;
w->val_bitsize = saved_bitsize;
w->val_valid = 1;
}

5
gdb/breakpoint.h
View File

@ -779,6 +779,11 @@ struct watchpoint
then an error occurred reading the value. */
int val_valid;
/* When watching the location of a bitfield, contains the offset and size of
the bitfield. Otherwise contains 0. */
int val_bitpos;
int val_bitsize;
/* Holds the frame address which identifies the frame this
watchpoint should be evaluated in, or `null' if the watchpoint
should be evaluated on the outermost frame. */

6
gdb/testsuite/ChangeLog
View File

@ -1,3 +1,9 @@
2014-09-16 Patrick Palka <patrick@parcs.ath.cx>
PR breakpoints/12526
* gdb.base/watch-bitfields.exp: New file.
* gdb.base/watch-bitfields.c: New file.
2014-09-16 Pedro Alves <palves@redhat.com>
* gdb.base/watchpoint-stops-at-right-insn.exp (test): Compare

54
gdb/testsuite/gdb.base/watch-bitfields.c Normal file
View File

@ -0,0 +1,54 @@
/* This testcase is part of GDB, the GNU debugger.
Copyright 2014 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
struct foo
{
unsigned long a:1;
unsigned char b:2;
unsigned long c:3;
char d:4;
int e:5;
char f:6;
int g:7;
long h:8;
} q = { 0 };
int
main (void)
{
q.a = 1;
q.b = 2;
q.c = 3;
q.d = 4;
q.e = 5;
q.f = 6;
q.g = -7;
q.h = -8;
q.a--;
q.h--;
q.c--;
q.b--;
q.e--;
q.d--;
q.c--;
q.f--;
q.g--;
q.h--;
return 0;
}

56
gdb/testsuite/gdb.base/watch-bitfields.exp Normal file
View File

@ -0,0 +1,56 @@
# Copyright 2014 Free Software Foundation, Inc.
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# This file is part of the gdb testsuite
standard_testfile
if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
return -1
}
if {![runto_main]} {
return -1
}
# Continue inferior execution, expecting the watchpoint EXPR to be triggered
# having old value OLD and new value NEW.
proc expect_watchpoint { expr old new } {
set expr_re [string_to_regexp $expr]
gdb_test "print $expr" "\\$\\d+ = $old\\s"
gdb_test "cont" "$expr_re\\s.*Old value = $old\\s+New value = $new\\s.*"
gdb_test "print $expr" "\\$\\d+ = $new\\s"
}
# Check that -location watchpoints against bitfields trigger properly.
gdb_test "watch -l q.a"
gdb_test "watch -l q.e"
expect_watchpoint "q.a" 0 1
expect_watchpoint "q.e" 0 5
expect_watchpoint "q.a" 1 0
expect_watchpoint "q.e" 5 4
gdb_test "cont" ".*exited normally.*"
# Check that regular watchpoints against expressions involving bitfields
# trigger properly.
runto_main
gdb_test "watch q.d + q.f + q.g"
expect_watchpoint "q.d + q.f + q.g" 0 4
expect_watchpoint "q.d + q.f + q.g" 4 10
expect_watchpoint "q.d + q.f + q.g" 10 3
expect_watchpoint "q.d + q.f + q.g" 3 2
expect_watchpoint "q.d + q.f + q.g" 2 1
expect_watchpoint "q.d + q.f + q.g" 1 0
gdb_test "cont" ".*exited normally.*"

2
gdb/value.c
View File

@ -3231,7 +3231,7 @@ unpack_field_as_long (struct type *type, const gdb_byte *valaddr, int fieldno)
are unavailable/optimized out, DEST_VAL is correspondingly
marked unavailable/optimized out. */
static void
void
unpack_value_bitfield (struct value *dest_val,
int bitpos, int bitsize,
const gdb_byte *valaddr, int embedded_offset,

5
gdb/value.h
View File

@ -613,6 +613,11 @@ extern int unpack_value_field_as_long (struct type *type, const gdb_byte *valadd
int embedded_offset, int fieldno,
const struct value *val, LONGEST *result);
extern void unpack_value_bitfield (struct value *dest_val,
int bitpos, int bitsize,
const gdb_byte *valaddr, int embedded_offset,
const struct value *val);
extern struct value *value_field_bitfield (struct type *type, int fieldno,
const gdb_byte *valaddr,
int embedded_offset,