]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
Fix PR12526: -location watchpoints for bitfield arguments
authorPatrick Palka <patrick@parcs.ath.cx>
Tue, 16 Sep 2014 16:40:06 +0000 (17:40 +0100)
committerPedro Alves <palves@redhat.com>
Tue, 16 Sep 2014 16:40:06 +0000 (17:40 +0100)
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.

gdb/ChangeLog
gdb/breakpoint.c
gdb/breakpoint.h
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.base/watch-bitfields.c [new file with mode: 0644]
gdb/testsuite/gdb.base/watch-bitfields.exp [new file with mode: 0644]
gdb/value.c
gdb/value.h

index 125ab92bd916ca61371fa9629ecd73ab7a50cd06..5234a506dd7e5167fada5bc09cfb5c1fbfcabb9b 100644 (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
index f990d971ebc1a93c600470985a205d0ff2ac14d0..94b55c302ff744cd840538d089206720fdbb29a2 100644 (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;
     }
 
index 8abb5eab2de29d3e6f69493e8a70d62b708c0b67..00c88029452c752bef6fe08c1473127984a14823 100644 (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.  */
index c06ba4dc5ca657f91d6fb20ecd3c90b81e019ee8..655301eca83bac517efc6e32b5356587fb8e6eca 100644 (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
diff --git a/gdb/testsuite/gdb.base/watch-bitfields.c b/gdb/testsuite/gdb.base/watch-bitfields.c
new file mode 100644 (file)
index 0000000..fb57885
--- /dev/null
@@ -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;
+}
diff --git a/gdb/testsuite/gdb.base/watch-bitfields.exp b/gdb/testsuite/gdb.base/watch-bitfields.exp
new file mode 100644 (file)
index 0000000..3f25384
--- /dev/null
@@ -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.*"
index 6620f9682a062377a4e850dd85e59b3ec555f606..fdc8858daf6642becfe1c122fe6183475ad02ee4 100644 (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,
index 4cdbf210788edaef75cf663122874d1388854021..e3603c331e8a7908d067fb4d8678de9f0281be5c 100644 (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,