Resend: [RFA] Fix problem with i386 watchpoints after restarting

Resend: [RFA] Fix problem with i386 watchpoints after restarting

[Pierre Muller]

I sent this message 9 days ago, 
but didn't get any answer,
maybe the error was to forget to CC to breakpoint maintainers?
So I added Michael Synder, Jim Blandy and Andrew Cagney,
together with Mark Kettenis (as i386 target maintainer) on CC.

<<Start of original message>>

After much efforts to find a reasonable solution for
the bug that I already explained several times,
I think that I found an easy solution.
See http://sources.redhat.com/ml/gdb-patches/2001-11/msg00613.html
and all related threads.

In short, most i386 targets using the common hardware watchpoint code do 
not call i386_cleanup_dregs 
and this leads to big troubles because 
on a second run of a debugged program the watchpoint is not
inserted correctly.

This patch does call i386_cleanup_dregs for 
all i386 targets that define
I386_USE_GENERIC_WATCHPOINTS
by simply defining a custom 
child_post_startup_inferior function
in the part of i386-nat.c code
that depends on the
I386_USE_GENERIC_WATCHPOINTS
conditional.

Only drawback of that is that this 
function does not seem to be 
multi-arch compatible, 
but does it make sense to have 
native code multi-arched?


ChangeLog entry:

2002-06-04  Pierre Muller  <muller@ics.u-strasbg.fr>

	* i386-nat.c (child_post_startup_inferior): New function
	calling i386_cleanup_dregs if
	I386_USE_GENERIC_WATCHPOINTS is defined.
	* config/i386/nm-i386.h: define CHILD_POST_STARTUP_INFERIOR
	conditional to acknowledge that i386-nat.c has its
	own child_post_startup_inferior function.

Index: i386-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-nat.c,v
retrieving revision 1.4
diff -u -p -r1.4 i386-nat.c
--- i386-nat.c  21 Sep 2001 07:16:03 -0000      1.4
+++ i386-nat.c  4 Jun 2002 10:58:17 -0000
@@ -230,6 +230,12 @@ i386_cleanup_dregs (void)
    dr_status_mirror  = 0;
  }

+void
+child_post_startup_inferior (ptid_t ptid)
+{
+  i386_cleanup_dregs ();
+}
+
  /* Print the values of the mirrored debug registers.
     This is called when maint_show_dr is non-zero.  To set that
     up, type "maint show-debug-regs" at GDB's prompt.  */
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
--- config/i386/nm-i386.h       23 Mar 2001 16:17:45 -0000      1.2
+++ config/i386/nm-i386.h       4 Jun 2002 10:58:18 -0000
@@ -30,6 +30,9 @@
  #define TARGET_HAS_HARDWARE_WATCHPOINTS
  #endif

+/* Use child_post_startup_inferior to reset all debug registers */
+#define CHILD_POST_STARTUP_INFERIOR
+
  /* Clear the reference counts and forget everything we knew about DRi.  */
  extern void i386_cleanup_dregs (void);



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: Resend: [RFA] Fix problem with i386 watchpoints after restarting

[Mark Kettenis]

Sorry, I somehow missed your earlier message.  This seems to me as an
acceptable way to solve the problem (although I still think there is
something wrong with GDB's mechanism of removing hardware breakpoints.
But: could you add the reason why this is needed to the comment.
Minor nit: Please end your sentences with a '.' and put two spaces
after it.

Thanks,

Mark

Re: Resend: [RFA] Fix problem with i386 watchpoints after restarting

[Pierre Muller]

At 12:16 13/06/2002 , Mark Kettenis a écrit:
>Sorry, I somehow missed your earlier message.  This seems to me as an
>acceptable way to solve the problem (although I still think there is
>something wrong with GDB's mechanism of removing hardware breakpoints.
>But: could you add the reason why this is needed to the comment.
>Minor nit: Please end your sentences with a '.' and put two spaces
>after it.
OK, I slightly modifed the patch
by adding a comment to i386-nat.c 
and correcting the comment in the nm-i386.h file.

Can I check this in?

2002-06-17 Pierre Muller  <muller@ics.u-strasbg.fr>

         * i386-nat.c (child_post_startup_inferior): New function
         calling i386_cleanup_dregs if
         I386_USE_GENERIC_WATCHPOINTS is defined.
         * config/i386/nm-i386.h: define CHILD_POST_STARTUP_INFERIOR
         conditional to acknowledge that i386-nat.c has its
         own child_post_startup_inferior function.

Index: i386-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-nat.c,v
retrieving revision 1.4
diff -r1.4 i386-nat.c
232a233,240
 > /* Reset all debug registers at each new startup
 >    to avoid missing watchpoints after restart.  */
 > void
 > child_post_startup_inferior (ptid_t ptid)
 > {
 >   i386_cleanup_dregs ();
 > }
 > 
Index: config/i386/nm-i386.h
===================================================================
RCS file: /cvs/src/src/gdb/config/i386/nm-i386.h,v
retrieving revision 1.2
diff -r1.2 nm-i386.h
117a118,121
 > /* child_post_startup_inferior used to
 >    reset all debug registers by calling i386_cleanup_dregs ().  */ 
 > #define CHILD_POST_STARTUP_INFERIOR
 > 



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: Resend: [RFA] Fix problem with i386 watchpoints after restarting

[Pierre Muller]

At 15:35 17/06/2002 , Pierre Muller a écrit:
>At 12:16 13/06/2002 , Mark Kettenis a écrit:
> >Sorry, I somehow missed your earlier message.  This seems to me as an
> >acceptable way to solve the problem (although I still think there is
> >something wrong with GDB's mechanism of removing hardware breakpoints.
> >But: could you add the reason why this is needed to the comment.
> >Minor nit: Please end your sentences with a '.' and put two spaces
> >after it.
>OK, I slightly modifed the patch
>by adding a comment to i386-nat.c 
>and correcting the comment in the nm-i386.h file.
>
>Can I check this in?

After getting approval by a private mail, I have
checked this in.

Mark suggested that I should add a test case for 
that bug, but I never really looked at the testsuite ...
And it does not work on cygwin target...

  The following simple test can be added :

 >>>Start of watchpoint_restart check >>>>

int x;

int
main ()

{
   x = 1;
   x= 2;
   return 0;
}
 >>>Start of watchpoint_restart check >>>>

The test would then simply consist of 
1) compiling the source above.
2) starting gdb with the produced executable.
3) set a watchpoint on x variable.
4) start program with 'run'
5) We should get a stop because watchpoint x was hit (value changed from 0 to 1)
6) 'continue'
7) we should get a second watchpoint when x changes from 1 back to 2.
8)'continue'
9) we should get to end of execution.
10) start with 'run' again.
11) to 15) same as 5) to 9)
16) exit test.


By modifing the source only slightly, we could also check for 
the other watchpoint bug that I reported and sent a patch for:
see http://sources.redhat.com/ml/gdb-patches/2002-06/msg00394.html)
I didn't get any answer about that RFA yet 

 >>>Start of watchpoint_restart check >>>>

int x;

int
main ()

{
   x = 1;
   x= 2;
   x = 1;
   return 0;
}
 >>>Start of watchpoint_restart check >>>>

compile this program statically you will see that 
at second run only the x=2 is hit currently
because the last value of previous run is 
kept in the val field of the breakpoint structure.
If compiling normally the problem is hidden because the startup value of x is
reloaded at the time the dynamic libraries are loaded.
(This also means that you will never see that bug on win32 target as
DLLs are always loaded there!)


>2002-06-17 Pierre Muller  <muller@ics.u-strasbg.fr>
>
>          * i386-nat.c (child_post_startup_inferior): New function
>          calling i386_cleanup_dregs if
>          I386_USE_GENERIC_WATCHPOINTS is defined.
>          * config/i386/nm-i386.h: define CHILD_POST_STARTUP_INFERIOR
>          conditional to acknowledge that i386-nat.c has its
>          own child_post_startup_inferior function.
>
>Index: i386-nat.c
>===================================================================
>RCS file: /cvs/src/src/gdb/i386-nat.c,v
>retrieving revision 1.4
>diff -r1.4 i386-nat.c
>232a233,240
>  > /* Reset all debug registers at each new startup
>  >    to avoid missing watchpoints after restart.  */
>  > void
>  > child_post_startup_inferior (ptid_t ptid)
>  > {
>  >   i386_cleanup_dregs ();
>  > }
>  > 
>Index: config/i386/nm-i386.h
>===================================================================
>RCS file: /cvs/src/src/gdb/config/i386/nm-i386.h,v
>retrieving revision 1.2
>diff -r1.2 nm-i386.h
>117a118,121
>  > /* child_post_startup_inferior used to
>  >    reset all debug registers by calling i386_cleanup_dregs ().  */ 
>  > #define CHILD_POST_STARTUP_INFERIOR
>  > 



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: Resend: [RFA] Fix problem with i386 watchpoints after restarting

[Daniel Jacobowitz]

On Thu, Jul 04, 2002 at 03:12:46PM +0200, Pierre Muller wrote:
> At 15:35 17/06/2002 , Pierre Muller a écrit:
> >At 12:16 13/06/2002 , Mark Kettenis a écrit:
> > >Sorry, I somehow missed your earlier message.  This seems to me as an
> > >acceptable way to solve the problem (although I still think there is
> > >something wrong with GDB's mechanism of removing hardware breakpoints.
> > >But: could you add the reason why this is needed to the comment.
> > >Minor nit: Please end your sentences with a '.' and put two spaces
> > >after it.
> >OK, I slightly modifed the patch
> >by adding a comment to i386-nat.c 
> >and correcting the comment in the nm-i386.h file.
> >
> >Can I check this in?
> 
> After getting approval by a private mail, I have
> checked this in.

Is this appropriate for the branch also?  My instincts say yes.


-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: Resend: [RFA] Fix problem with i386 watchpoints after restarting

[Pierre Muller]

At 18:25 04/07/2002 , Daniel Jacobowitz a écrit:
>On Thu, Jul 04, 2002 at 03:12:46PM +0200, Pierre Muller wrote:
> > At 15:35 17/06/2002 , Pierre Muller a écrit:
> > >At 12:16 13/06/2002 , Mark Kettenis a écrit:
> > > >Sorry, I somehow missed your earlier message.  This seems to me as an
> > > >acceptable way to solve the problem (although I still think there is
> > > >something wrong with GDB's mechanism of removing hardware breakpoints.
> > > >But: could you add the reason why this is needed to the comment.
> > > >Minor nit: Please end your sentences with a '.' and put two spaces
> > > >after it.
> > >OK, I slightly modifed the patch
> > >by adding a comment to i386-nat.c 
> > >and correcting the comment in the nm-i386.h file.
> > >
> > >Can I check this in?
> > 
> > After getting approval by a private mail, I have
> > checked this in.
>
>Is this appropriate for the branch also?  My instincts say yes.
Isn't 5.2.1 aleady frozen?

I would also be in favor of putting it into the branch, but I don't know who should decide this....


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: Resend: [RFA] Fix problem with i386 watchpoints after restarting

[Andrew Cagney]

> At 18:25 04/07/2002 , Daniel Jacobowitz a écrit:
> 
>>On Thu, Jul 04, 2002 at 03:12:46PM +0200, Pierre Muller wrote:
> 
>> > At 15:35 17/06/2002 , Pierre Muller a écrit:
> 
>> > >At 12:16 13/06/2002 , Mark Kettenis a écrit:
> 
>> > > >Sorry, I somehow missed your earlier message.  This seems to me as an
>> > > >acceptable way to solve the problem (although I still think there is
>> > > >something wrong with GDB's mechanism of removing hardware breakpoints.
>> > > >But: could you add the reason why this is needed to the comment.
>> > > >Minor nit: Please end your sentences with a '.' and put two spaces
>> > > >after it.
> 
>> > >OK, I slightly modifed the patch
>> > >by adding a comment to i386-nat.c 
>> > >and correcting the comment in the nm-i386.h file.
>> > >
>> > >Can I check this in?
> 
>> > 
>> > After getting approval by a private mail, I have
>> > checked this in.
> 
>>
>>Is this appropriate for the branch also?  My instincts say yes.
> 
> Isn't 5.2.1 aleady frozen?

No.  I keep going to do this but someone turns up with a new patch :-)

> I would also be in favor of putting it into the branch, but I don't know who should decide this....

Something for the i386 maintainers to decide.

Andrew

Re: Resend: [RFA] Fix problem with i386 watchpoints after restarting

[Mark Kettenis]

   Date: Mon, 08 Jul 2002 14:45:02 -0400
   From: Andrew Cagney <ac131313@ges.redhat.com>

   > I would also be in favor of putting it into the branch, but I
   > don't know who should decide this....

   Something for the i386 maintainers to decide.

I have no objections (and if we one counts the number of i386 targets
that I'm supposed to maintain, I guess I have the majority vote ;-)).

So please, go ahead!

Mark

Re: Resend: [RFA] Fix problem with i386 watchpoints after restarting

[Eli Zaretskii]

On Mon, 8 Jul 2002, Mark Kettenis wrote:

>    Date: Mon, 08 Jul 2002 14:45:02 -0400
>    From: Andrew Cagney <ac131313@ges.redhat.com>
> 
>    > I would also be in favor of putting it into the branch, but I
>    > don't know who should decide this....
> 
>    Something for the i386 maintainers to decide.
> 
> I have no objections

Me neither.

Re: Resend: [RFA] Fix problem with i386 watchpoints after restarting

[Pierre Muller]

At 07:07 09/07/2002 , Eli Zaretskii a écrit:

>On Mon, 8 Jul 2002, Mark Kettenis wrote:
>
> >    Date: Mon, 08 Jul 2002 14:45:02 -0400
> >    From: Andrew Cagney <ac131313@ges.redhat.com>
> > 
> >    > I would also be in favor of putting it into the branch, but I
> >    > don't know who should decide this....
> > 
> >    Something for the i386 maintainers to decide.
> > 
> > I have no objections
>
>Me neither.

OK, I commited this also to gdb_2_2-branch



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: Resend: [RFA] Fix problem with i386 watchpoints after restarting

[Pierre Muller]

At 10:26 09/07/2002 , Pierre Muller a écrit:
>At 07:07 09/07/2002 , Eli Zaretskii a écrit:
>
> >On Mon, 8 Jul 2002, Mark Kettenis wrote:
> >
> > >    Date: Mon, 08 Jul 2002 14:45:02 -0400
> > >    From: Andrew Cagney <ac131313@ges.redhat.com>
> > > 
> > >    > I would also be in favor of putting it into the branch, but I
> > >    > don't know who should decide this....
> > > 
> > >    Something for the i386 maintainers to decide.
> > > 
> > > I have no objections
> >
> >Me neither.
>
>OK, I commited this also to gdb_2_2-branch


Whoops, I meant gdb_5_2-branch of course.


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: Resend: [RFA] Fix problem with i386 watchpoints after restarting

[Andrew Cagney]

> At 10:26 09/07/2002 , Pierre Muller a écrit:

>>OK, I commited this also to gdb_2_2-branch
> 
> 
> 
> Whoops, I meant gdb_5_2-branch of course.

:-)

Pierre, can you also add something to the branch NEWS file mentioning 
this change?

Michal, same for the x86-64 stuff you pulled in?

To speak from experience, the first question GDB gets hit with, when 
making a X.X.1 release, is ``what changed'' and why isn't it mentioned 
in the NEWS file + announcement file.

(Hmm, I'll re-sync the trunk and branch NEWS files since I've got them 
out of sync).

enjoy,
Andrew

Re: Resend: [RFA] Fix problem with i386 watchpoints after restarting

[Michal Ludvig]

Andrew Cagney wrote:
> Pierre, can you also add something to the branch NEWS file mentioning 
> this change?
> 
> Michal, same for the x86-64 stuff you pulled in?

I added the following to NEWS in the branch:

Dwarf2 .debug_frame & .eh_frame handler improved in many ways.
Surprisingly enough, it works now.
By Michal Ludvig, imported from mainline.

Michal Ludvig
-- 
* SuSE CR, s.r.o     * mludvig@suse.cz
* +420 2 9654 5373   * http://www.suse.cz