This is the mail archive of the gdb-patches@sourceware.cygnus.com 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]

Re: Problems with hardware watchpoint on ia32.



Okay, to put my money where my mouth is, I'm posting patches to
correct two problems related to hardware breakpoints and watchpoints.
These were all found in the DJGPP (a.k.a. go32) version of GDB, but I
firmly believe that they can be seen on any x86 platform, at least in
native debugging, and possibly on other platforms as well.

Each one of the two messages, this and the next, handles a specific
problem.

Problem no.1: GDB cannot watch bit fields with hardware watchpoints.

Example:

$ cat bf.c

int main (int argc, char *argv[])
{
  struct foo {
    int iv;
    double dv;
    unsigned flag1:1;
    unsigned flag2:2;
    int jv;
  } foo_var, *foo_ptr;

  foo_var.flag1 = 1;
  foo_var.flag2 = foo_var.flag1 + 1;
  return 0;
}

$ gcc -g -o bf bf.c
$ gdb bf
(gdb) b main
Breakpoint 1 at 0x1573: file bf.c, line 11.
(gdb) r
Starting program: g:/gdbsnap/gdb-0222/gdb/bf

Breakpoint 1, main (argc=1, argv=0x8c6fc) at bf.c:11
11	foo_var.flag1 = 1;
(gdb) watch foo_var.flag1
Watchpoint 2: foo_var.flag1

Here are the patches, after which you can set hardware watchpoints on
bit fields.  You will notice that these patches also augment the
lazy-value trick suggested by Jim Blandy as a means to watch struct
members; this is because that trick doesn't work for bit fields, and
because I regard it generally fragile as far as watchpoints are
considered (we depend on the assumption that GDB doesn't evaluate
parent structures, but nothing in GDB's code suggests that this
assumption will hold).


2000-03-08  Eli Zaretskii  <eliz@is.elta.co.il>

	* breakpoint.c (insert_breakpoints, remove_breakpoint)
	(bpstat_stop_status, can_use_hardware_watchpoint): Don't insert,
	remove, or check status of hardware watchpoints for entire structs
	and arrays unless the user explicitly asked to watch that struct
	or array.  
	(insert_breakpoints): Try to insert watchpoints for all the values
	on the value chain, even if some of them fail to insert.

	* values.c (value_primitive_field): Set the offset in struct value
	we return when the field is a packed bitfield.


--- gdb/breakpoint.c~1	Wed Mar  8 18:16:00 2000
+++ gdb/breakpoint.c	Wed Mar  8 19:20:28 2000
@@ -974,24 +974,39 @@ insert_breakpoints ()
 		if (VALUE_LVAL (v) == lval_memory
 		    && ! VALUE_LAZY (v))
 		  {
-		    CORE_ADDR addr;
-		    int len, type;
+		    struct type *vtype = check_typedef (VALUE_TYPE (v));
 
-		    addr = VALUE_ADDRESS (v) + VALUE_OFFSET (v);
-		    len = TYPE_LENGTH (VALUE_TYPE (v));
-		    type   = hw_write;
-		    if (b->type == bp_read_watchpoint)
-		      type = hw_read;
-		    else if (b->type == bp_access_watchpoint)
-		      type = hw_access;
-
-		    val = target_insert_watchpoint (addr, len, type);
-		    if (val == -1)
+		    /* We only watch structs and arrays if user asked
+		       for it explicitly, never if they just happen to
+		       appear in the middle of some value chain.  */
+		    if (v == b->val_chain
+			|| (TYPE_CODE (vtype) != TYPE_CODE_STRUCT
+			    && TYPE_CODE (vtype) != TYPE_CODE_ARRAY))
 		      {
-			b->inserted = 0;
-			break;
+			CORE_ADDR addr;
+			int len, type;
+
+			addr = VALUE_ADDRESS (v) + VALUE_OFFSET (v);
+			len = TYPE_LENGTH (VALUE_TYPE (v));
+			type   = hw_write;
+			if (b->type == bp_read_watchpoint)
+			  type = hw_read;
+			else if (b->type == bp_access_watchpoint)
+			  type = hw_access;
+
+			val = target_insert_watchpoint (addr, len, type);
+			if (val == -1)
+			  {
+			    /* Don't exit the loop, try to insert
+			       every value on the value chain.  That's
+			       because we will be removing all the
+			       watches below, and removing a
+			       watchpoint we didn't insert could have
+			       adverse effects.  */
+			    b->inserted = 0;
+			  }
+			val = 0;
 		      }
-		    val = 0;
 		  }
 	      }
 	    /* Failure to insert a watchpoint on any memory value in the
@@ -1327,21 +1342,28 @@ remove_breakpoint (b, is)
 	  if (VALUE_LVAL (v) == lval_memory
 	      && ! VALUE_LAZY (v))
 	    {
-	      CORE_ADDR addr;
-	      int len, type;
+	      struct type *vtype = check_typedef (VALUE_TYPE (v));
 
-	      addr = VALUE_ADDRESS (v) + VALUE_OFFSET (v);
-	      len = TYPE_LENGTH (VALUE_TYPE (v));
-	      type   = hw_write;
-	      if (b->type == bp_read_watchpoint)
-		type = hw_read;
-	      else if (b->type == bp_access_watchpoint)
-		type = hw_access;
-
-	      val = target_remove_watchpoint (addr, len, type);
-	      if (val == -1)
-		b->inserted = 1;
-	      val = 0;
+	      if (v == b->val_chain
+		  || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT
+		      && TYPE_CODE (vtype) != TYPE_CODE_ARRAY))
+		{
+		  CORE_ADDR addr;
+		  int len, type;
+
+		  addr = VALUE_ADDRESS (v) + VALUE_OFFSET (v);
+		  len = TYPE_LENGTH (VALUE_TYPE (v));
+		  type   = hw_write;
+		  if (b->type == bp_read_watchpoint)
+		    type = hw_read;
+		  else if (b->type == bp_access_watchpoint)
+		    type = hw_access;
+
+		  val = target_remove_watchpoint (addr, len, type);
+		  if (val == -1)
+		    b->inserted = 1;
+		  val = 0;
+		}
 	    }
 	}
       /* Failure to remove any of the hardware watchpoints comes here.  */
@@ -2571,14 +2593,21 @@ bpstat_stop_status (pc, not_a_breakpoint
 	    if (VALUE_LVAL (v) == lval_memory
 		&& ! VALUE_LAZY (v))
 	      {
-		CORE_ADDR vaddr;
+		struct type *vtype = check_typedef (VALUE_TYPE (v));
 
-		vaddr = VALUE_ADDRESS (v) + VALUE_OFFSET (v);
-		/* Exact match not required.  Within range is sufficient.  
-		 */
-		if (addr >= vaddr &&
-		    addr < vaddr + TYPE_LENGTH (VALUE_TYPE (v)))
-		  found = 1;
+		if (v == b->val_chain
+		    || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT
+			&& TYPE_CODE (vtype) != TYPE_CODE_ARRAY))
+		  {
+		    CORE_ADDR vaddr;
+
+		    vaddr = VALUE_ADDRESS (v) + VALUE_OFFSET (v);
+		    /* Exact match not required.  Within range is
+                       sufficient.  */
+		    if (addr >= vaddr &&
+			addr < vaddr + TYPE_LENGTH (VALUE_TYPE (v)))
+		      found = 1;
+		  }
 	      }
 	  }
 	if (found)
@@ -5515,6 +5544,7 @@ can_use_hardware_watchpoint (v)
      struct value *v;
 {
   int found_memory_cnt = 0;
+  struct value *head = v;
 
   /* Did the user specifically forbid us to use hardware watchpoints? */
   if (!can_use_hw_watchpoints)
@@ -5552,13 +5582,23 @@ can_use_hardware_watchpoint (v)
 	    {
 	      /* Ahh, memory we actually used!  Check if we can cover
                  it with hardware watchpoints.  */
-	      CORE_ADDR vaddr = VALUE_ADDRESS (v) + VALUE_OFFSET (v);
-	      int       len   = TYPE_LENGTH (VALUE_TYPE (v));
+	      struct type *vtype = check_typedef (VALUE_TYPE (v));
 
-	      if (!TARGET_REGION_OK_FOR_HW_WATCHPOINT (vaddr, len))
-		return 0;
-	      else
-		found_memory_cnt++;
+	      /* We only watch structs and arrays if user asked for it
+		 explicitly, never if they just happen to appear in a
+		 middle of some value chain.  */
+	      if (v == head
+		  || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT
+		      && TYPE_CODE (vtype) != TYPE_CODE_ARRAY))
+		{
+		  CORE_ADDR vaddr = VALUE_ADDRESS (v) + VALUE_OFFSET (v);
+		  int       len   = TYPE_LENGTH (VALUE_TYPE (v));
+
+		  if (!TARGET_REGION_OK_FOR_HW_WATCHPOINT (vaddr, len))
+		    return 0;
+		  else
+		    found_memory_cnt++;
+		}
 	    }
 	}
       else if (v->lval != not_lval && v->modifiable == 0)
--- gdb/values.c~0	Thu Sep  9 02:20:18 1999
+++ gdb/values.c	Wed Mar  8 18:53:48 2000
@@ -802,6 +802,8 @@ value_primitive_field (arg1, offset, fie
 						    fieldno));
       VALUE_BITPOS (v) = TYPE_FIELD_BITPOS (arg_type, fieldno) % 8;
       VALUE_BITSIZE (v) = TYPE_FIELD_BITSIZE (arg_type, fieldno);
+      VALUE_OFFSET (v) = VALUE_OFFSET (arg1) + offset
+	+ TYPE_FIELD_BITPOS (arg_type, fieldno) / 8;
     }
   else if (fieldno < TYPE_N_BASECLASSES (arg_type))
     {

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