[RFA/RFC 2] Remove hardware break and watchpoints at program exit.

[RFA/RFC 2] Remove hardware break and watchpoints at program exit.

[Pierre Muller]

[-- Attachment #1: Type: text/plain, Size: 2491 bytes --]

At 10:20 09/01/2002 , vous avez écrit:

>On Wed, 9 Jan 2002, Pierre Muller wrote:
>
> > The patch simply calls i386_cleanup_dregs
> > in breakpoint_init_inferior if context is inf_exited.


  I finally change completely my patch to 
another (more correct, in my opinion) strategy.

   In breakpoint_init_inferior
I added code that conditionally removes hardware watch and breakpoint 
if the context is inf_exited, i.e. at exit of the debugged program.

  This leads to a much cleaner removal of the watchpoints.
I still kept the call to a generic hardware removal function
and testing the i386 case, I could see that even though the dr_mirror
array is zeored out, the dr_control_mirror and
dr_status_mirror still aren't set to zero.
For dr_control_register, this is due to an error in the I386_DR_DISABLE
because that macro only resets the active bit, but not the size and type bits 
associated to that debug register.

I suspect that before this patch,
the hardware watchpoint code that was in s390-nat.c
didn't reset the area_base to nil when a program was exited.
Could DJ Barrow please test this and see if the patch below does
fix this.
(I can't test this as I ave no acces to s390 machine,
but it appears to me that this might cause certain 
watchpoints to still show up after removal, if
the debuggee was run several times).
After the patch, probably no HARDWARE_REGISTER_RESET 
function is needed anymore, but I didn't inspect the code entirely.

I have a little doubt about the default value of the
HARDWARE_REGISTER_RESET macro.
What should be used as a no-op default ?


2002-01-09  Pierre Muller <muller@ics.u-strasbg.fr>

         * breakpoint.c (REMOVE_HARDWARE_BREAKPOINT_AT_EXIT): 
         Define to 0 if not defined.
         (REMOVE_HARDWARE_WATCHPOINT_AT_EXIT): 
         Define to 0 if not defined.
         (HARDWARE_REGISTER_RESET): Default to no-op.
         (breakpoint_init_inferior): Use REMOVE_HARDWARE_BREAKPOINT_AT_EXIT
         and REMOVE_HARDWARE_WATCHPOINT_AT_EXIT to conditionally
         remove hardware break and watchpoints at exit.
         Call HARDWARE_REGISTER_RESET macro if defined at exit.

         * config/i386/nm-i386.h: Define HARDWARE_REGISTER_RESET
         macro to call i386_cleanup_dregs().
         Set REMOVE_HARDWARE_BREAKPOINT_AT_EXIT to 1.
         Set REMOVE_HARDWARE_WATCHPOINT_AT_EXIT to 1.

         * go32-nat.c (go32_mourn_inferior): Remove call to i386_cleanup_dregs
         as this is done in breakpoint_init_inferior().

[-- Attachment #2: dregs.diffs --]
[-- Type: text/plain, Size: 2794 bytes --]

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.57
diff -u -p -r1.57 breakpoint.c
--- breakpoint.c	2001/11/11 16:39:59	1.57
+++ breakpoint.c	2002/01/11 11:34:08
@@ -234,6 +234,19 @@ static int executing_breakpoint_commands
 	     B ? (TMP=B->next, 1): 0;	\
 	     B = TMP)
 
+#ifndef REMOVE_HARDWARE_BREAKPOINT_AT_EXIT
+#define REMOVE_HARDWARE_BREAKPOINT_AT_EXIT 0
+#endif
+
+#ifndef REMOVE_HARDWARE_WATCHPOINT_AT_EXIT
+#define REMOVE_HARDWARE_WATCHPOINT_AT_EXIT 0
+#endif
+
+
+#ifndef HARDWARE_REGISTERS_RESET
+#define HARDWARE_REGISTERS_RESET {}
+#endif
+
 /* True if SHIFT_INST_REGS defined, false otherwise.  */
 
 int must_shift_inst_regs =
@@ -1440,7 +1453,17 @@ breakpoint_init_inferior (enum inf_conte
 
   ALL_BREAKPOINTS_SAFE (b, temp)
   {
-    b->inserted = 0;
+   if ((((REMOVE_HARDWARE_BREAKPOINT_AT_EXIT)
+	  && b->type == bp_hardware_breakpoint)
+	 ||((REMOVE_HARDWARE_WATCHPOINT_AT_EXIT)
+	     && (b->type == bp_hardware_watchpoint
+	         || b->type == bp_access_watchpoint
+	         || b->type == bp_read_watchpoint)))
+       && b->inserted
+       && context == inf_exited)
+     remove_breakpoint (b, mark_uninserted);
+
+   b->inserted = 0;
 
     switch (b->type)
       {
@@ -1487,6 +1510,8 @@ breakpoint_init_inferior (enum inf_conte
       warning ("You must reinsert them explicitly.");
       warning_needed = 0;
     }
+  if (context == inf_exited)
+    HARDWARE_REGISTER_RESET;
 }
 
 /* breakpoint_here_p (PC) returns non-zero if an enabled breakpoint
Index: go32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/go32-nat.c,v
retrieving revision 1.26
diff -u -p -r1.26 go32-nat.c
--- go32-nat.c	2001/12/06 08:15:37	1.26
+++ go32-nat.c	2002/01/11 11:34:08
@@ -670,7 +670,7 @@ go32_mourn_inferior (void)
      be nice if GDB itself would take care to remove all breakpoints
      at all times, but it doesn't, probably under an assumption that
      the OS cleans up when the debuggee exits.  */
-  i386_cleanup_dregs ();
+  // i386_cleanup_dregs ();
   go32_kill_inferior ();
   generic_mourn_inferior ();
 }
Index: config/i386/nm-i386.h
===================================================================
RCS file: /cvs/src/src/gdb/config/i386/nm-i386.h,v
retrieving revision 1.2
diff -u -p -r1.2 nm-i386.h
--- nm-i386.h	2001/03/23 16:17:45	1.2
+++ nm-i386.h	2002/01/11 11:34:08
@@ -115,6 +115,12 @@ extern int  i386_remove_hw_breakpoint (C
 
 #define DECR_PC_AFTER_HW_BREAK 0
 
+#define REMOVE_HARDWARE_WATCHPOINT_AT_EXIT 1
+
+#define REMOVE_HARDWARE_BREAKPOINT_AT_EXIT 1
+
+#define HARDWARE_REGISTERS_RESET i386_cleanup_dregs ()
+
 #endif /* I386_USE_GENERIC_WATCHPOINTS */
 
 #endif /* NM_I386_H */

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]



Pierre Muller
Institut Charles Sadron
6,rue Boussingault
F 67083 STRASBOURG CEDEX (France)
mailto:muller@ics.u-strasbg.fr
Phone : (33)-3-88-41-40-07  Fax : (33)-3-88-41-40-99

Re: [RFA/RFC 2] Remove hardware break and watchpoints at program exit.

[Eli Zaretskii]

> Date: Fri, 11 Jan 2002 13:00:44 +0100
> From: Pierre Muller <muller@cerbere.u-strasbg.fr>
> 
>    In breakpoint_init_inferior
> I added code that conditionally removes hardware watch and breakpoint 
> if the context is inf_exited, i.e. at exit of the debugged program.

Thanks.  I like this approach much better, especially because I never
understood why does GDB do certain cleanups when the debuggee exits,
but doesn't do other, similar cleanups.

> I still kept the call to a generic hardware removal function
> and testing the i386 case, I could see that even though the dr_mirror
> array is zeored out, the dr_control_mirror and
> dr_status_mirror still aren't set to zero.
> For dr_control_register, this is due to an error in the I386_DR_DISABLE
> because that macro only resets the active bit, but not the size and type bits 
> associated to that debug register.

Why is this a problem?  Once the active bit is off, the corresponding
debug register is inactive, and its value is not important.

>          * breakpoint.c (REMOVE_HARDWARE_BREAKPOINT_AT_EXIT): 
>          Define to 0 if not defined.
>          (REMOVE_HARDWARE_WATCHPOINT_AT_EXIT): 
>          Define to 0 if not defined.

Why do we need these macros at all?  Why not remove the breakpoints
and watchpoints unconditionally?  Does anyone see any problem?

> --- go32-nat.c	2001/12/06 08:15:37	1.26
> +++ go32-nat.c	2002/01/11 11:34:08
> @@ -670,7 +670,7 @@ go32_mourn_inferior (void)
>       be nice if GDB itself would take care to remove all breakpoints
>       at all times, but it doesn't, probably under an assumption that
>       the OS cleans up when the debuggee exits.  */
> -  i386_cleanup_dregs ();
> +  // i386_cleanup_dregs ();

Please don't make such changes.  If you want to remove some code, just
remove it, don't comment it away: it looks ad-hoc and not clean.

Re: [RFA/RFC 2] Remove hardware break and watchpoints at program exit.

[Pierre Muller]

At 13:35 11/01/2002 , Eli Zaretskii a écrit:
> > Date: Fri, 11 Jan 2002 13:00:44 +0100
> > From: Pierre Muller <muller@cerbere.u-strasbg.fr>
> > 
> >    In breakpoint_init_inferior
> > I added code that conditionally removes hardware watch and breakpoint 
> > if the context is inf_exited, i.e. at exit of the debugged program.
>
>Thanks.  I like this approach much better, especially because I never
>understood why does GDB do certain cleanups when the debuggee exits,
>but doesn't do other, similar cleanups.
>
> > I still kept the call to a generic hardware removal function
> > and testing the i386 case, I could see that even though the dr_mirror
> > array is zeored out, the dr_control_mirror and
> > dr_status_mirror still aren't set to zero.
> > For dr_control_register, this is due to an error in the I386_DR_DISABLE
> > because that macro only resets the active bit, but not the size and type bits 
> > associated to that debug register.
>
>Why is this a problem?  Once the active bit is off, the corresponding
>debug register is inactive, and its value is not important.

I agree that it isn't important,
it was just for cleanness...

> >          * breakpoint.c (REMOVE_HARDWARE_BREAKPOINT_AT_EXIT): 
> >          Define to 0 if not defined.
> >          (REMOVE_HARDWARE_WATCHPOINT_AT_EXIT): 
> >          Define to 0 if not defined.
>
>Why do we need these macros at all?  Why not remove the breakpoints
>and watchpoints unconditionally?  Does anyone see any problem?

Because some target might need to access the 
inferior registers to remove the watchpoint, and this
isn't possible if the program has really already completed.

> > --- go32-nat.c        2001/12/06 08:15:37     1.26
> > +++ go32-nat.c        2002/01/11 11:34:08
> > @@ -670,7 +670,7 @@ go32_mourn_inferior (void)
> >       be nice if GDB itself would take care to remove all breakpoints
> >       at all times, but it doesn't, probably under an assumption that
> >       the OS cleans up when the debuggee exits.  */
> > -  i386_cleanup_dregs ();
> > +  // i386_cleanup_dregs ();
>
>Please don't make such changes.  If you want to remove some code, just
>remove it, don't comment it away: it looks ad-hoc and not clean.

I did that more or less to make you react,
but I can't just remove the line as the comment before 
concerns the call to i386_cleanup_dregs()

Should I remove the comment also?



Pierre Muller
Institut Charles Sadron
6,rue Boussingault
F 67083 STRASBOURG CEDEX (France)
mailto:muller@ics.u-strasbg.fr
Phone : (33)-3-88-41-40-07  Fax : (33)-3-88-41-40-99

Re: [RFA/RFC 2] Remove hardware break and watchpoints at program exit.

[Pierre Muller]

At 13:35 11/01/2002 , Eli Zaretskii a écrit:
> > Date: Fri, 11 Jan 2002 13:00:44 +0100
> > From: Pierre Muller <muller@cerbere.u-strasbg.fr>
> > 
> >    In breakpoint_init_inferior
> > I added code that conditionally removes hardware watch and breakpoint 
> > if the context is inf_exited, i.e. at exit of the debugged program.
>
>Thanks.  I like this approach much better, especially because I never
>understood why does GDB do certain cleanups when the debuggee exits,
>but doesn't do other, similar cleanups.
>
> > I still kept the call to a generic hardware removal function
> > and testing the i386 case, I could see that even though the dr_mirror
> > array is zeored out, the dr_control_mirror and
> > dr_status_mirror still aren't set to zero.
> > For dr_control_register, this is due to an error in the I386_DR_DISABLE
> > because that macro only resets the active bit, but not the size and type bits 
> > associated to that debug register.
>
>Why is this a problem?  Once the active bit is off, the corresponding
>debug register is inactive, and its value is not important.

  The code does already generate a claim for i386 linux:
the linux code tries to write the dr registers using ptrace
but as the process is already finished, this fails.
See i386-linux-nt.c line 741.

How could this be solved? 
By adding a test  
if (target_has_execution )
  ptrace(....)

If this is correct, the same code should also be used for i386-bsd-nat.c




Pierre Muller
Institut Charles Sadron
6,rue Boussingault
F 67083 STRASBOURG CEDEX (France)
mailto:muller@ics.u-strasbg.fr
Phone : (33)-3-88-41-40-07  Fax : (33)-3-88-41-40-99

Re: [RFA/RFC 2] Remove hardware break and watchpoints at program exit.

[Eli Zaretskii]

> Date: Fri, 11 Jan 2002 13:56:42 +0100
> From: Pierre Muller <muller@cerbere.u-strasbg.fr>
> 
> > >          * breakpoint.c (REMOVE_HARDWARE_BREAKPOINT_AT_EXIT): 
> > >          Define to 0 if not defined.
> > >          (REMOVE_HARDWARE_WATCHPOINT_AT_EXIT): 
> > >          Define to 0 if not defined.
> >
> >Why do we need these macros at all?  Why not remove the breakpoints
> >and watchpoints unconditionally?  Does anyone see any problem?
> 
> Because some target might need to access the 
> inferior registers to remove the watchpoint, and this
> isn't possible if the program has really already completed.

??? If a target accesses watchpoints of a dead and mourned debuggee,
it must have a bug, no?

> > > --- go32-nat.c        2001/12/06 08:15:37     1.26
> > > +++ go32-nat.c        2002/01/11 11:34:08
> > > @@ -670,7 +670,7 @@ go32_mourn_inferior (void)
> > >       be nice if GDB itself would take care to remove all breakpoints
> > >       at all times, but it doesn't, probably under an assumption that
> > >       the OS cleans up when the debuggee exits.  */
> > > -  i386_cleanup_dregs ();
> > > +  // i386_cleanup_dregs ();
> >
> >Please don't make such changes.  If you want to remove some code, just
> >remove it, don't comment it away: it looks ad-hoc and not clean.
> 
> I did that more or less to make you react,
> but I can't just remove the line as the comment before 
> concerns the call to i386_cleanup_dregs()
> 
> Should I remove the comment also?

Yes, of course: if code goes away, its comment should go away as well.

Re: [RFA/RFC 2] Remove hardware break and watchpoints at program exit.

[Eli Zaretskii]

> Date: Fri, 11 Jan 2002 14:38:40 +0100
> From: Pierre Muller <muller@cerbere.u-strasbg.fr>
> 
>   The code does already generate a claim for i386 linux:
> the linux code tries to write the dr registers using ptrace
> but as the process is already finished, this fails.
> See i386-linux-nt.c line 741.

Why does the Linux port try to do that after the debuggee has died?

Re: [RFA/RFC 2] Remove hardware break and watchpoints at program exit.

[Pierre Muller]

At 15:03 11/01/2002 , Eli Zaretskii a écrit:
> > Date: Fri, 11 Jan 2002 14:38:40 +0100
> > From: Pierre Muller <muller@cerbere.u-strasbg.fr>
> > 
> >   The code does already generate a claim for i386 linux:
> > the linux code tries to write the dr registers using ptrace
> > but as the process is already finished, this fails.
> > See i386-linux-nt.c line 741.
>
>Why does the Linux port try to do that after the debuggee has died?

There was no problem before my patch,
because before my patch, 
remove_breakpoint was never called for a dead debuggee.
So its just that the code was not adapted to my change...
Anyway this is no big deal, I will try to send a new patch handling this very soon.


Pierre Muller
Institut Charles Sadron
6,rue Boussingault
F 67083 STRASBOURG CEDEX (France)
mailto:muller@ics.u-strasbg.fr
Phone : (33)-3-88-41-40-07  Fax : (33)-3-88-41-40-99

[RFA/RFC 3] Remove hardware break and watchpoints at program exit.

[Pierre Muller]

[-- Attachment #1: Type: text/plain, Size: 1577 bytes --]

After the  previous discussion about  the 
hardware watch and breakpoint removal 
about the debuggee exited. I slightly modifed my patch
by adding a test to avoid calling ptrace
in i386-linux-nat.c, i386bsd-nat.c and x86-64-linux-nat.c
i386_linux_dr_set, i386bsd_dr_set and x86_64_linux_dr_set functions
if taget_has_execution is 0.


I have a little doubt about the default value of the
HARDWARE_REGISTER_RESET macro.
What should be used as a no-op default ?



2002-01-09  Pierre Muller <muller@ics.u-strasbg.fr>

          * breakpoint.c (REMOVE_HARDWARE_BREAKPOINT_AT_EXIT): 
          Define to 0 if not defined.
          (REMOVE_HARDWARE_WATCHPOINT_AT_EXIT): 
          Define to 0 if not defined.
          (HARDWARE_REGISTER_RESET): Default to no-op.
          (breakpoint_init_inferior): Use REMOVE_HARDWARE_BREAKPOINT_AT_EXIT
          and REMOVE_HARDWARE_WATCHPOINT_AT_EXIT to conditionally
          remove hardware break and watchpoints at exit.
          Call HARDWARE_REGISTER_RESET macro if defined at exit.

          * config/i386/nm-i386.h: Define HARDWARE_REGISTER_RESET
          macro to call i386_cleanup_dregs().
          Set REMOVE_HARDWARE_BREAKPOINT_AT_EXIT to 1.
          Set REMOVE_HARDWARE_WATCHPOINT_AT_EXIT to 1.

          * go32-nat.c (go32_mourn_inferior): Remove call to i386_cleanup_dregs
          as this is done in breakpoint_init_inferior().
	* i386-linux-nat.c (i386_linux_dr_set): Don't call ptrace if target_has_execution is zero.
	* i386bsd-nat.c (i386bsd_dr_set): Likewise.
	* x86-64-linux-nat.c (x86_64_linux_dr_set): Likewise.

[-- Attachment #2: dregs.diffs --]
[-- Type: text/plain, Size: 5608 bytes --]

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.57
diff -u -p -r1.57 breakpoint.c
--- breakpoint.c	2001/11/11 16:39:59	1.57
+++ breakpoint.c	2002/01/11 16:19:11
@@ -234,6 +234,19 @@ static int executing_breakpoint_commands
 	     B ? (TMP=B->next, 1): 0;	\
 	     B = TMP)
 
+#ifndef REMOVE_HARDWARE_BREAKPOINT_AT_EXIT
+#define REMOVE_HARDWARE_BREAKPOINT_AT_EXIT 0
+#endif
+
+#ifndef REMOVE_HARDWARE_WATCHPOINT_AT_EXIT
+#define REMOVE_HARDWARE_WATCHPOINT_AT_EXIT 0
+#endif
+
+
+#ifndef HARDWARE_REGISTERS_RESET
+#define HARDWARE_REGISTERS_RESET {}
+#endif
+
 /* True if SHIFT_INST_REGS defined, false otherwise.  */
 
 int must_shift_inst_regs =
@@ -1440,7 +1453,17 @@ breakpoint_init_inferior (enum inf_conte
 
   ALL_BREAKPOINTS_SAFE (b, temp)
   {
-    b->inserted = 0;
+   if ((((REMOVE_HARDWARE_BREAKPOINT_AT_EXIT)
+	  && b->type == bp_hardware_breakpoint)
+	 ||((REMOVE_HARDWARE_WATCHPOINT_AT_EXIT)
+	     && (b->type == bp_hardware_watchpoint
+	         || b->type == bp_access_watchpoint
+	         || b->type == bp_read_watchpoint)))
+       && b->inserted
+       && context == inf_exited)
+     remove_breakpoint (b, mark_uninserted);
+
+   b->inserted = 0;
 
     switch (b->type)
       {
@@ -1487,6 +1510,8 @@ breakpoint_init_inferior (enum inf_conte
       warning ("You must reinsert them explicitly.");
       warning_needed = 0;
     }
+  if (context == inf_exited)
+    HARDWARE_REGISTER_RESET;
 }
 
 /* breakpoint_here_p (PC) returns non-zero if an enabled breakpoint
Index: go32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/go32-nat.c,v
retrieving revision 1.26
diff -u -p -r1.26 go32-nat.c
--- go32-nat.c	2001/12/06 08:15:37	1.26
+++ go32-nat.c	2002/01/11 16:19:11
@@ -663,14 +663,6 @@ go32_create_inferior (char *exec_file, c
 static void
 go32_mourn_inferior (void)
 {
-  /* We need to make sure all the breakpoint enable bits in the DR7
-     register are reset when the inferior exits.  Otherwise, if they
-     rerun the inferior, the uncleared bits may cause random SIGTRAPs,
-     failure to set more watchpoints, and other calamities.  It would
-     be nice if GDB itself would take care to remove all breakpoints
-     at all times, but it doesn't, probably under an assumption that
-     the OS cleans up when the debuggee exits.  */
-  i386_cleanup_dregs ();
   go32_kill_inferior ();
   generic_mourn_inferior ();
 }
Index: i386-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-linux-nat.c,v
retrieving revision 1.34
diff -u -p -r1.34 i386-linux-nat.c
--- i386-linux-nat.c	2002/01/09 22:32:16	1.34
+++ i386-linux-nat.c	2002/01/11 16:19:12
@@ -738,8 +738,9 @@ i386_linux_dr_set (int regnum, unsigned 
   tid = PIDGET (inferior_ptid);
 
   errno = 0;
-  ptrace (PT_WRITE_U, tid,
-	  offsetof (struct user, u_debugreg[regnum]), value);
+  if (target_has_execution)
+    ptrace (PT_WRITE_U, tid,
+	    offsetof (struct user, u_debugreg[regnum]), value);
   if (errno != 0)
     perror_with_name ("Couldn't write debug register");
 }
Index: i386bsd-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386bsd-nat.c,v
retrieving revision 1.11
diff -u -p -r1.11 i386bsd-nat.c
--- i386bsd-nat.c	2002/01/05 18:36:32	1.11
+++ i386bsd-nat.c	2002/01/11 16:19:12
@@ -298,8 +298,9 @@ i386bsd_dr_set (int regnum, unsigned int
 {
   struct dbreg dbregs;
 
-  if (ptrace (PT_GETDBREGS, PIDGET (inferior_ptid),
-              (PTRACE_ARG3_TYPE) &dbregs, 0) == -1)
+  if (target_has_execution
+      && ptrace (PT_GETDBREGS, PIDGET (inferior_ptid),
+                (PTRACE_ARG3_TYPE) &dbregs, 0) == -1))
     perror_with_name ("Couldn't get debug registers");
 
   /* For some mysterious reason, some of the reserved bits in the
@@ -309,8 +310,9 @@ i386bsd_dr_set (int regnum, unsigned int
 
   DBREG_DRX ((&dbregs), regnum) = value;
 
-  if (ptrace (PT_SETDBREGS, PIDGET (inferior_ptid),
-              (PTRACE_ARG3_TYPE) &dbregs, 0) == -1)
+  if (target_has_execution
+      && ptrace (PT_SETDBREGS, PIDGET (inferior_ptid),
+                (PTRACE_ARG3_TYPE) &dbregs, 0) == -1))
     perror_with_name ("Couldn't write debug registers");
 }
 
Index: x86-64-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/x86-64-linux-nat.c,v
retrieving revision 1.2
diff -u -p -r1.2 x86-64-linux-nat.c
--- x86-64-linux-nat.c	2002/01/08 00:59:31	1.2
+++ x86-64-linux-nat.c	2002/01/11 16:19:12
@@ -73,8 +73,9 @@ x86_64_linux_dr_set (int regnum, unsigne
   tid = PIDGET (inferior_ptid);
 
   errno = 0;
-  ptrace (PT_WRITE_U, tid,
-	  offsetof (struct user, u_debugreg[regnum]), value);
+  if (target_has_execution)
+    ptrace (PT_WRITE_U, tid,
+	    offsetof (struct user, u_debugreg[regnum]), value);
   if (errno != 0)
     perror_with_name ("Couldn't write debug register");
 }
Index: config/i386/nm-i386.h
===================================================================
RCS file: /cvs/src/src/gdb/config/i386/nm-i386.h,v
retrieving revision 1.2
diff -u -p -r1.2 nm-i386.h
--- nm-i386.h	2001/03/23 16:17:45	1.2
+++ nm-i386.h	2002/01/11 16:19:12
@@ -115,6 +115,12 @@ extern int  i386_remove_hw_breakpoint (C
 
 #define DECR_PC_AFTER_HW_BREAK 0
 
+#define REMOVE_HARDWARE_WATCHPOINT_AT_EXIT 1
+
+#define REMOVE_HARDWARE_BREAKPOINT_AT_EXIT 1
+
+#define HARDWARE_REGISTERS_RESET i386_cleanup_dregs ()
+
 #endif /* I386_USE_GENERIC_WATCHPOINTS */
 
 #endif /* NM_I386_H */

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]



Pierre Muller
Institut Charles Sadron
6,rue Boussingault
F 67083 STRASBOURG CEDEX (France)
mailto:muller@ics.u-strasbg.fr
Phone : (33)-3-88-41-40-07  Fax : (33)-3-88-41-40-99

Re: [RFA/RFC 3] Remove hardware break and watchpoints at program exit.

[Mark Kettenis]

Pierre Muller <muller@cerbere.u-strasbg.fr> writes:

> After the  previous discussion about  the 
> hardware watch and breakpoint removal 
> about the debuggee exited. I slightly modifed my patch
> by adding a test to avoid calling ptrace
> in i386-linux-nat.c, i386bsd-nat.c and x86-64-linux-nat.c
> i386_linux_dr_set, i386bsd_dr_set and x86_64_linux_dr_set functions
> if taget_has_execution is 0.

That should not be necessary.  Those functions shouldn't be called
when there is no debuggee.  I suspect that there is something wrong
with the hardware breakpoint/watchpoint implementation at a much
higher level.

Mark

Re: [RFA/RFC 3] Remove hardware break and watchpoints at program exit.

[muller]

At 14:19 12/01/02 +0100, Mark Kettenis wrote:
>Pierre Muller <muller@cerbere.u-strasbg.fr> writes:
>
>> After the  previous discussion about  the 
>> hardware watch and breakpoint removal 
>> about the debuggee exited. I slightly modifed my patch
>> by adding a test to avoid calling ptrace
>> in i386-linux-nat.c, i386bsd-nat.c and x86-64-linux-nat.c
>> i386_linux_dr_set, i386bsd_dr_set and x86_64_linux_dr_set functions
>> if taget_has_execution is 0.
>
>That should not be necessary.  Those functions shouldn't be called
>when there is no debuggee.  I suspect that there is something wrong
>with the hardware breakpoint/watchpoint implementation at a much
>higher level.

 There was no call tp ptrace before my patch,
but as Eli thought that the right solution would be to remove the
hardware break and watchpoints at exit, I tried to implement this.

 But that implementation then ran into that 
problem that the code in i386-linux-nat.c, 
i386bsd-nat.c and x86-64-linux-nat.c
implicitly supposed that the debuggee was still running...

 So here there is a conflict between 
i386-nat code that would work cleaner if 
we remove the hardware break and watchpoint at exit
(and which works correctly for go32v2
and my still uncommitted patch for win32-nat)
and the code that use ptrace.

 I did quite agree with Eli that its cleaner
to remove those breakpoints, but if you prefer, I could
resubmit a patch where the macros that
trigger the removal are only set for go32v2 and
win32 targets. As long as we also call i386_cleanup_dregs
function, it should solve the bug present in 5.1
version, by which a watchpoint does not work anymore after
a rerun of the debuggee.

Re: [RFA/RFC 3] Remove hardware break and watchpoints at program exit.

[Eli Zaretskii]

On 12 Jan 2002, Mark Kettenis wrote:

> Those functions shouldn't be called when there is no debuggee.  I
> suspect that there is something wrong with the hardware
> breakpoint/watchpoint implementation at a much higher level.

The problem, IMHO, is that GDB's application level doesn't tell the
target-specific low-end that it's time to clean up the watchpoint
related data structures.  The DJGPP port uses mourn_inferior to do
that, but this might not be a good general solution, especially since
so many popular targets use the generic version of mourning.

I think we need to have code that explicitly cleans up watchpoints
when the debuggee exits.