This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFC] Fix verification of changed values for big values.


Hi,

This is the fix that SÃrgio mentioned on the thread about our progress
on the BookE debug registers support.

Right now, GDB calls value_equal when comparing the old and new values
of a watchpoint. IMO this is not correct, since that function will call
coerce_array and effectively just compare the addresses of arrays being
watched.

This patch introduces a new value comparison function which works in the
mentioned case, and a testcase which fails without the patch and passes
with it. Ok to commit?
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


gdb/
	* breakpoint.c (value_equal_watchpoint): New function.
	(watchpoint_check): Use value_equal_watchpoint instead
	of value_equal.

gdb/testsuite/
	* gdb.base/watchpoint.exp (test_watchpoint_in_big_blob): New function.
	(top level): Call test_watchpoint_in_big_blob.
	* gdb.base/watchpoint.c (buf): Change size to value too big for hardware
	watchpoints.
	(func3): Write to buf.

Index: gdb.git/gdb/breakpoint.c
===================================================================
--- gdb.git.orig/gdb/breakpoint.c	2009-05-31 17:05:44.000000000 -0300
+++ gdb.git/gdb/breakpoint.c	2009-05-31 17:19:29.000000000 -0300
@@ -2717,7 +2717,23 @@ watchpoints_triggered (struct target_wai
 #define BP_TEMPFLAG 1
 #define BP_HARDWAREFLAG 2
 
-/* 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)
@@ -2785,7 +2801,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)))
 	{
 	  if (new_val != NULL)
 	    {
Index: gdb.git/gdb/testsuite/gdb.base/watchpoint.c
===================================================================
--- gdb.git.orig/gdb/testsuite/gdb.base/watchpoint.c	2009-05-31 17:04:24.000000000 -0300
+++ gdb.git/gdb/testsuite/gdb.base/watchpoint.c	2009-05-31 17:16:35.000000000 -0300
@@ -30,7 +30,7 @@ int ival2 = -1;
 int ival3 = -1;
 int ival4 = -1;
 int ival5 = -1;
-char buf[10];
+char buf[30];
 struct foo
 {
   int val;
@@ -95,6 +95,7 @@ func3 ()
   x = 1;				/* second x assignment */
   y = 1;
   y = 2;
+  buf[26] = 3;
 }
 
 int
Index: gdb.git/gdb/testsuite/gdb.base/watchpoint.exp
===================================================================
--- gdb.git.orig/gdb/testsuite/gdb.base/watchpoint.exp	2009-05-31 17:04:24.000000000 -0300
+++ gdb.git/gdb/testsuite/gdb.base/watchpoint.exp	2009-05-31 17:17:04.000000000 -0300
@@ -678,6 +678,24 @@ proc test_inaccessible_watchpoint {} {
     }
 }
     
+proc test_watchpoint_in_big_blob {} {
+    global gdb_prompt
+
+    gdb_test "watch buf" ".*atchpoint \[0-9\]+: buf"
+    send_gdb "cont\n"
+    gdb_expect {
+	-re "Continuing.*\[Ww\]atchpoint.*buf.*Old value = .*$gdb_prompt $" {
+	    pass "watchpoint on buf hit"
+	}
+	-re "Continuing.*$gdb_prompt $" {
+	    fail "watchpoint on buf hit"
+	}
+	-re ".*$gdb_prompt $" { fail "watchpoint on buf hit" ; return }
+	timeout { fail "watchpoint on buf hit (timeout)" ; return }
+	eof { fail "watchpoint on buf hit (eof)" ; return }
+    }
+}
+
 # Start with a fresh gdb.
 
 gdb_exit
@@ -842,6 +860,8 @@ if [initialize] then {
     }
 
     test_watchpoint_and_breakpoint
+
+    test_watchpoint_in_big_blob
 }
 
 # Restore old timeout



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]