[RFA] Fix troubles with watchpoints in DJGPP

[RFA] Fix troubles with watchpoints in DJGPP

[Pierre Muller (IMAP)]

  I finally managed to be able to compile current CVS
with DJGPP v2.03.

  I also tried to use it but I have several problems,
some are probably DJGPP specific
(bad file descriptor errors when trying to restart 
a ./gdb executable at level 2, meaning inside another gdb
itself being debugged by gdb).

  The reason of that email is the behavior for watchpoints:

  When I set a watchpoint before starting a program in cygwin,
the type is first set to 'software watchpoint'
and then modified to 'hardware_watchpoint' (if the watch memory size
is compatible with debug register limitations for i386) during 
initialization.
  But this does not happen for DJGPP
if I set a watchpoint before starting on a pointer,
it never is changed to a hardware watchpoint.

  After some debugging, I realized that 
DJGPP only calls insert_breakpoints ()
that does call update_watchpoint with reparse set to one,
after pushing go32 target,
while with cygwin, the DLL loaded caused a 
reloading of all breakpoints and triggered a
call to update_watchpoint with reparse = 1.


This one line patch fixes the problem.

Is this patch OK?


Pierre Muller
Pascal language support maintainer for GDB

PS-1) Are there not other native targets, without
dynamic libraries, that will suffer the same troubles?
PS-2) Eli,
do you have anything that could help me debug the
Bad file descriptor problems, like a library recording
file opening/closing using the DJGPP file system extensions?



ChangeLog entry:

2009-05-27  Pierre Muller  <muller@ics.u-strasbg.fr>

        * go32-nat.c (go32_create_inferior): Add call to breakpoint_re_set.

Index: go32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/go32-nat.c,v
retrieving revision 1.76
diff -u -p -r1.76 go32-nat.c
--- go32-nat.c  21 May 2009 15:48:41 -0000      1.76
+++ go32-nat.c  27 May 2009 07:44:50 -0000
@@ -721,6 +721,7 @@ go32_create_inferior (struct target_ops
   add_thread_silent (inferior_ptid);

   clear_proceed_status ();
+  breakpoint_re_set ();
   insert_breakpoints ();
   prog_has_started = 1;
 }
~

Re: [RFA] Fix troubles with watchpoints in DJGPP

[Eli Zaretskii]

> From: "Pierre Muller \(IMAP\)" <muller@ics.u-strasbg.fr>
> Date: Wed, 27 May 2009 09:48:49 +0200
> 
> 
>   I also tried to use it but I have several problems,
> some are probably DJGPP specific
> (bad file descriptor errors when trying to restart 
> a ./gdb executable at level 2, meaning inside another gdb
> itself being debugged by gdb).

Please give me some simple test program and a transcript of a GDB
session that reproduces this problem.  I debug GDB with itself a lot,
and I never saw this.

Also, on what platform (OS and version) is that?

>   After some debugging, I realized that 
> DJGPP only calls insert_breakpoints ()
> that does call update_watchpoint with reparse set to one,
> after pushing go32 target,
> while with cygwin, the DLL loaded caused a 
> reloading of all breakpoints and triggered a
> call to update_watchpoint with reparse = 1.
> 
> 
> This one line patch fixes the problem.
> 
> Is this patch OK?

Thanks, but here, too, I would like a simple test program and a
transcript of a GDB session before and after the patch.

> PS-2) Eli,
> do you have anything that could help me debug the
> Bad file descriptor problems, like a library recording
> file opening/closing using the DJGPP file system extensions?

No, but you can put a breakpoint on the respective library functions,
couldn't you?

RE: [RFA] Fix troubles with watchpoints in DJGPP

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Eli Zaretskii
> Envoyé : Wednesday, May 27, 2009 9:25 PM
> À : Pierre Muller (IMAP)
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFA] Fix troubles with watchpoints in DJGPP
> 
> > From: "Pierre Muller \(IMAP\)" <muller@ics.u-strasbg.fr>
> > Date: Wed, 27 May 2009 09:48:49 +0200
> >
> >
> >   I also tried to use it but I have several problems,
> > some are probably DJGPP specific
> > (bad file descriptor errors when trying to restart
> > a ./gdb executable at level 2, meaning inside another gdb
> > itself being debugged by gdb).
> 
> Please give me some simple test program and a transcript of a GDB
> session that reproduces this problem.  I debug GDB with itself a lot,
> and I never saw this.

  Compile gdb on current CVS head.

in build/gdb

bash-2.05b$ ./gdb ./gdb
GNU gdb (GDB) 6.8.50.20090524-cvs
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "--host=i586-pc-msdosdjgpp --target=djgpp".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
(gdb) watch gdb_tderr
No symbol "gdb_tderr" in current context.
(gdb) watch gdb_stderr
Watchpoint 1: gdb_stderr
(gdb) r
Starting program: e:/cygwin/usr/local/src/gdbcvs/djbuild/gdb/./gdb.exe
Watchpoint 1: gdb_stderr

Old value = <unreadable>
New value = (struct ui_file *) 0x0
0x00001a91 in start ()
(gdb) inf watch
Num     Type           Disp Enb Address    What
1       watchpoint     keep y              gdb_stderr
        breakpoint already hit 1 time
(gdb) q

With patch go32-nat.c file:

bash-2.05b$ ./gdb ./gdb
GNU gdb (GDB) 6.8.50.20090524-cvs
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "--host=i586-pc-msdosdjgpp --target=djgpp".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
(gdb) watch gdb_stderr
Watchpoint 1: gdb_stderr
(gdb) r
Starting program: e:/cygwin/usr/local/src/gdbcvs/djbuild/gdb/./gdb.exe
Hardware watchpoint 1: gdb_stderr

Old value = (struct ui_file *) 0x0
New value = (struct ui_file *) 0x3aa7dc
captured_main (data=0x3a92d0) at ../../purecvs/gdb/main.c:322
322       gdb_stdlog = gdb_stderr;      /* for moment */
(gdb) inf watch
Num     Type           Disp Enb Address    What
1       hw watchpoint  keep y              gdb_stderr
        breakpoint already hit 1 time
(gdb)q

With the patch, the 'watchpoint' 
is successfully promoted into a 'hardware watchpoint'
thanks to this 
breakpoint_re_set call.

> Also, on what platform (OS and version) is that?
> 
> >   After some debugging, I realized that
> > DJGPP only calls insert_breakpoints ()
> > that does call update_watchpoint with reparse set to one,
> > after pushing go32 target,
> > while with cygwin, the DLL loaded caused a
> > reloading of all breakpoints and triggered a
> > call to update_watchpoint with reparse = 1.
> >
> >
> > This one line patch fixes the problem.
> >
> > Is this patch OK?
> 
> Thanks, but here, too, I would like a simple test program and a
> transcript of a GDB session before and after the patch.

I hope the above is enough for you.
 
> > PS-2) Eli,
> > do you have anything that could help me debug the
> > Bad file descriptor problems, like a library recording
> > file opening/closing using the DJGPP file system extensions?
> 
> No, but you can put a breakpoint on the respective library functions,
> couldn't you?

  I discovered that there is already something in
dbgcom.c, but I wanted to have dup and dup2 calls 
be monitored also, as the problem seems related to
handles of that type... But dup and dup2
never generate a call to the FSEXT function,
which make it not useful for that :(

Pierre

Re: [RFA] Fix troubles with watchpoints in DJGPP

[Pedro Alves]

On Wednesday 27 May 2009 08:48:49, Pierre Muller (IMAP) wrote:

> PS-1) Are there not other native targets, without
> dynamic libraries, that will suffer the same troubles?

I think so.  I've just tried on x86_64-linux, with a statically
linked binary (I used gdb.threads/staticthreads, set a watchpoint
on semaphore), and although there are no shared libraries loaded,
the problem is masked by adding the symbols of the vsyscall page
(sysfile-mem.c:add_vsyscall_page).  If I hack that function to
do nothing, I see that same thing you're seeing on djgpp.

Maybe there's a place for a generic fix?  Somewhere after
having opened a connection to the target interface.  I was
thinking of post_create_inferior, but sounds like opening
a connection to a remote target with "target remote" that
happens to not pull in any more symbols (like most embedded
targets) is having the same problem?  Maybe there should be
a target_post_open ...

-- 
Pedro Alves

Re: [RFA] Fix troubles with watchpoints in DJGPP

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Thu, 28 May 2009 00:30:28 +0100
> Cc: "Pierre Muller \(IMAP\)" <muller@ics.u-strasbg.fr>,
>  "'Eli Zaretskii'" <eliz@gnu.org>
> 
> On Wednesday 27 May 2009 08:48:49, Pierre Muller (IMAP) wrote:
> 
> > PS-1) Are there not other native targets, without
> > dynamic libraries, that will suffer the same troubles?
> 
> I think so.  I've just tried on x86_64-linux, with a statically
> linked binary (I used gdb.threads/staticthreads, set a watchpoint
> on semaphore), and although there are no shared libraries loaded,
> the problem is masked by adding the symbols of the vsyscall page
> (sysfile-mem.c:add_vsyscall_page).  If I hack that function to
> do nothing, I see that same thing you're seeing on djgpp.
> 
> Maybe there's a place for a generic fix?  Somewhere after
> having opened a connection to the target interface.  I was
> thinking of post_create_inferior, but sounds like opening
> a connection to a remote target with "target remote" that
> happens to not pull in any more symbols (like most embedded
> targets) is having the same problem?  Maybe there should be
> a target_post_open ...

I'd indeed prefer a generic fix.

Pierre, could you please try making a patch along the lines Pedro
suggests above?

Re: [RFA] Fix troubles with watchpoints in DJGPP

[Eli Zaretskii]

> From: "Pierre Muller" <muller@ics.u-strasbg.fr>
> Cc: <gdb-patches@sourceware.org>
> Date: Wed, 27 May 2009 22:53:31 +0200
> 
> (gdb) watch gdb_stderr
> Watchpoint 1: gdb_stderr
> (gdb) r
> Starting program: e:/cygwin/usr/local/src/gdbcvs/djbuild/gdb/./gdb.exe
> Watchpoint 1: gdb_stderr
> 
> Old value = <unreadable>
> New value = (struct ui_file *) 0x0
> 0x00001a91 in start ()
> (gdb) inf watch
> Num     Type           Disp Enb Address    What
> 1       watchpoint     keep y              gdb_stderr
>         breakpoint already hit 1 time
> (gdb) q

Thanks, but this example is for the second (and the main) problem you
mention in your original message, the one with watchpoints.  You also
reported another problem:

> > > (bad file descriptor errors when trying to restart
> > > a ./gdb executable at level 2, meaning inside another gdb
> > > itself being debugged by gdb).

Can you show an example of these ``bad file descriptor errors''?  I
don't think I ever saw them.

Thanks.

> > > PS-2) Eli,
> > > do you have anything that could help me debug the
> > > Bad file descriptor problems, like a library recording
> > > file opening/closing using the DJGPP file system extensions?
> > 
> > No, but you can put a breakpoint on the respective library functions,
> > couldn't you?
> 
>   I discovered that there is already something in
> dbgcom.c, but I wanted to have dup and dup2 calls 
> be monitored also, as the problem seems related to
> handles of that type... But dup and dup2
> never generate a call to the FSEXT function,
> which make it not useful for that :(

First, I meant to actually put a breakpoint on each of the functions
you are interested in; that doesn't need any FSEXT hooks.  And second,
I don't understand the nature of your problems with FSEXT: dup and
dup2 do make a copy of the FSEXT hook of the original file descriptor,
so if the original descriptor was hooked by an FSEXT, the duplicated
descriptor will be hooked as well.  What am I missing?

RE: [RFA] Fix troubles with watchpoints in DJGPP

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Eli Zaretskii
> Envoyé : Thursday, May 28, 2009 10:21 AM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFA] Fix troubles with watchpoints in DJGPP
> 
> > From: "Pierre Muller" <muller@ics.u-strasbg.fr>
> > Cc: <gdb-patches@sourceware.org>
> > Date: Wed, 27 May 2009 22:53:31 +0200
> >
> > (gdb) watch gdb_stderr
> > Watchpoint 1: gdb_stderr
> > (gdb) r
> > Starting program:
> e:/cygwin/usr/local/src/gdbcvs/djbuild/gdb/./gdb.exe
> > Watchpoint 1: gdb_stderr
> >
> > Old value = <unreadable>
> > New value = (struct ui_file *) 0x0
> > 0x00001a91 in start ()
> > (gdb) inf watch
> > Num     Type           Disp Enb Address    What
> > 1       watchpoint     keep y              gdb_stderr
> >         breakpoint already hit 1 time
> > (gdb) q
> 
> Thanks, but this example is for the second (and the main) problem you
> mention in your original message, the one with watchpoints.  You also
> reported another problem:
> 
> > > > (bad file descriptor errors when trying to restart
> > > > a ./gdb executable at level 2, meaning inside another gdb
> > > > itself being debugged by gdb).
> 
> Can you show an example of these ``bad file descriptor errors''?  I
> don't think I ever saw them.

  This one is more tricky, I only saw it with 3 levels of gdb.
And maybe watchpoints were needed too...
  I will try to find a reproducible way, but it might not be so easy.
 
> Thanks.
> 
> > > > PS-2) Eli,
> > > > do you have anything that could help me debug the
> > > > Bad file descriptor problems, like a library recording
> > > > file opening/closing using the DJGPP file system extensions?
> > >
> > > No, but you can put a breakpoint on the respective library
> functions,
> > > couldn't you?
> >
> >   I discovered that there is already something in
> > dbgcom.c, but I wanted to have dup and dup2 calls
> > be monitored also, as the problem seems related to
> > handles of that type... But dup and dup2
> > never generate a call to the FSEXT function,
> > which make it not useful for that :(
> 
> First, I meant to actually put a breakpoint on each of the functions
> you are interested in; that doesn't need any FSEXT hooks.  And second,
> I don't understand the nature of your problems with FSEXT: dup and
> dup2 do make a copy of the FSEXT hook of the original file descriptor,
> so if the original descriptor was hooked by an FSEXT, the duplicated
> descriptor will be hooked as well.  What am I missing?

  What I am missing is a FSEXT hook call inside dup 
so that I know that the return value of dup is used,
instead with current gdbcom.c code, I will only get a 
call to dbg_fsext when this handle is closed, but 
at that point, I would have no idea where this handle is coming from!

Pierre

Re: [RFA] Fix troubles with watchpoints in DJGPP

[Eli Zaretskii]

> From: "Pierre Muller" <muller@ics.u-strasbg.fr>
> Cc: <gdb-patches@sourceware.org>
> Date: Thu, 28 May 2009 11:31:22 +0200
> 
> > > > > (bad file descriptor errors when trying to restart
> > > > > a ./gdb executable at level 2, meaning inside another gdb
> > > > > itself being debugged by gdb).
> > 
> > Can you show an example of these ``bad file descriptor errors''?  I
> > don't think I ever saw them.
> 
>   This one is more tricky, I only saw it with 3 levels of gdb.
> And maybe watchpoints were needed too...
>   I will try to find a reproducible way, but it might not be so easy.

Thank you.

>   What I am missing is a FSEXT hook call inside dup 
> so that I know that the return value of dup is used,
> instead with current gdbcom.c code, I will only get a 
> call to dbg_fsext when this handle is closed, but 
> at that point, I would have no idea where this handle is coming from!

But you can set a breakpoint on dup itself, can't you?  Then you will
see both the input descriptor and the one it returns to the caller.

[RFA-v2] Fix troubles with watchpoints in DJGPP

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : Thursday, May 28, 2009 1:30 AM
> À : gdb-patches@sourceware.org
> Cc : Pierre Muller (IMAP); 'Eli Zaretskii'
> Objet : Re: [RFA] Fix troubles with watchpoints in DJGPP
> 
> On Wednesday 27 May 2009 08:48:49, Pierre Muller (IMAP) wrote:
> 
> > PS-1) Are there not other native targets, without
> > dynamic libraries, that will suffer the same troubles?
> 
> I think so.  I've just tried on x86_64-linux, with a statically
> linked binary (I used gdb.threads/staticthreads, set a watchpoint
> on semaphore), and although there are no shared libraries loaded,
> the problem is masked by adding the symbols of the vsyscall page
> (sysfile-mem.c:add_vsyscall_page).  If I hack that function to
> do nothing, I see that same thing you're seeing on djgpp.
> 
> Maybe there's a place for a generic fix?  Somewhere after
> having opened a connection to the target interface.  I was
> thinking of post_create_inferior, but sounds like opening
> a connection to a remote target with "target remote" that
> happens to not pull in any more symbols (like most embedded
> targets) is having the same problem?  Maybe there should be
> a target_post_open ...

  I submit here a more general fix
that calls breakpoint_re_set function
after the target is pushed in post_create_inferior.

  This patch works for go32 target, but 
should work for all other native targets that
have no dynamic libraries too.

 Is this OK?

Pierre

2009-06-08  Pierre Muller  <muller@ics.u-strasbg.fr>

	* infcmd.c (post_create_inferior): Call breakpoint_re_set after
target
	is pushed for watchpoint promotion to hardware watchpoint.

Index: src/gdb/infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.245
diff -u -p -r1.245 infcmd.c
--- src/gdb/infcmd.c	7 Jun 2009 16:46:48 -0000	1.245
+++ src/gdb/infcmd.c	8 Jun 2009 07:29:43 -0000
@@ -421,6 +421,9 @@ post_create_inferior (struct target_ops 
 #endif
     }
 
+  /* Call breakpoint_re_set to update watchpoints types.  */
+  breakpoint_re_set ();
+
   observer_notify_inferior_created (target, from_tty);
 }
 

[PING][RFA-v2] Fix troubles with watchpoints in DJGPP

[Pierre Muller]

No one reacted to this second version of my patch...

I still have other hardware watchpoint related problems
that need to be discussed, but this one is really a small patch,
no ?

Pierre


> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pierre Muller
> Envoyé : Monday, June 08, 2009 6:16 PM
> À : 'Pedro Alves'; gdb-patches@sourceware.org
> Cc : 'Eli Zaretskii'
> Objet : [RFA-v2] Fix troubles with watchpoints in DJGPP
> 
> 
> 
> > -----Message d'origine-----
> > De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> > owner@sourceware.org] De la part de Pedro Alves
> > Envoyé : Thursday, May 28, 2009 1:30 AM
> > À : gdb-patches@sourceware.org
> > Cc : Pierre Muller (IMAP); 'Eli Zaretskii'
> > Objet : Re: [RFA] Fix troubles with watchpoints in DJGPP
> >
> > On Wednesday 27 May 2009 08:48:49, Pierre Muller (IMAP) wrote:
> >
> > > PS-1) Are there not other native targets, without
> > > dynamic libraries, that will suffer the same troubles?
> >
> > I think so.  I've just tried on x86_64-linux, with a statically
> > linked binary (I used gdb.threads/staticthreads, set a watchpoint
> > on semaphore), and although there are no shared libraries loaded,
> > the problem is masked by adding the symbols of the vsyscall page
> > (sysfile-mem.c:add_vsyscall_page).  If I hack that function to
> > do nothing, I see that same thing you're seeing on djgpp.
> >
> > Maybe there's a place for a generic fix?  Somewhere after
> > having opened a connection to the target interface.  I was
> > thinking of post_create_inferior, but sounds like opening
> > a connection to a remote target with "target remote" that
> > happens to not pull in any more symbols (like most embedded
> > targets) is having the same problem?  Maybe there should be
> > a target_post_open ...
> 
>   I submit here a more general fix
> that calls breakpoint_re_set function
> after the target is pushed in post_create_inferior.
> 
>   This patch works for go32 target, but
> should work for all other native targets that
> have no dynamic libraries too.
> 
>  Is this OK?
> 
> Pierre
> 
> 2009-06-08  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	* infcmd.c (post_create_inferior): Call breakpoint_re_set after
> target
> 	is pushed for watchpoint promotion to hardware watchpoint.
> 
> Index: src/gdb/infcmd.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infcmd.c,v
> retrieving revision 1.245
> diff -u -p -r1.245 infcmd.c
> --- src/gdb/infcmd.c	7 Jun 2009 16:46:48 -0000	1.245
> +++ src/gdb/infcmd.c	8 Jun 2009 07:29:43 -0000
> @@ -421,6 +421,9 @@ post_create_inferior (struct target_ops
>  #endif
>      }
> 
> +  /* Call breakpoint_re_set to update watchpoints types.  */
> +  breakpoint_re_set ();
> +
>    observer_notify_inferior_created (target, from_tty);
>  }
> 

Re: [PING][RFA-v2] Fix troubles with watchpoints in DJGPP

[Pedro Alves]

On Tuesday 16 June 2009 23:37:38, Pierre Muller wrote:
> 
> No one reacted to this second version of my patch...

My reaction was that the patch looked OK, 
but please could you expand the comment some more to
explain a bit better why we need this:

+  /* Call breakpoint_re_set to update watchpoints types.  */
+  breakpoint_re_set ();

This almost looks like:

+ /* Increment variable by one.  */
+ i++;

;-)

breakpoint_re_set is very likely to be something we
will be wanting to split further, make smarter and/or
eliminate, so having its non-obvious uses nicely described
is a good thing, IMO.

> I still have other hardware watchpoint related problems
> that need to be discussed, but this one is really a small patch,
> no ?

-- 
Pedro Alves

RE: [PING][RFA-v2] Fix troubles with watchpoints in DJGPP

[Pierre Muller]

Would this be OK then?

Pierre

PS: It could be optimized in the sense that it should only be called
if it has not been called yet.


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

	* infcmd.c (post_create_inferior): Call breakpoint_re_set after
target
	is pushed for watchpoint promotion to hardware watchpoint.

Index: src/gdb/infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.245
diff -u -p -r1.245 infcmd.c
--- src/gdb/infcmd.c	7 Jun 2009 16:46:48 -0000	1.245
+++ src/gdb/infcmd.c	8 Jun 2009 07:29:43 -0000
@@ -421,6 +421,13 @@ post_create_inferior (struct target_ops 
 #endif
     }
 
+  /* On systems that load no shared libraries, like DJGPP target,
+     breakpoint_re_set is never called.
+     Call it now so that ordinary watchpoints get a chance to
+     become promoted to hardware watchpoints if the pushed target
+     supports hardware watchpoints.  */
+  breakpoint_re_set ();
+
   observer_notify_inferior_created (target, from_tty);
 }
 

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : Wednesday, June 17, 2009 1:00 AM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org; 'Eli Zaretskii'
> Objet : Re: [PING][RFA-v2] Fix troubles with watchpoints in DJGPP
> 
> On Tuesday 16 June 2009 23:37:38, Pierre Muller wrote:
> >
> > No one reacted to this second version of my patch...
> 
> My reaction was that the patch looked OK,
> but please could you expand the comment some more to
> explain a bit better why we need this:
> 
> +  /* Call breakpoint_re_set to update watchpoints types.  */
> +  breakpoint_re_set ();
> 
> This almost looks like:
> 
> + /* Increment variable by one.  */
> + i++;
> 
> ;-)
> 
> breakpoint_re_set is very likely to be something we
> will be wanting to split further, make smarter and/or
> eliminate, so having its non-obvious uses nicely described
> is a good thing, IMO.
> 
> > I still have other hardware watchpoint related problems
> > that need to be discussed, but this one is really a small patch,
> > no ?
> 
> --
> Pedro Alves

Re: [PING][RFA-v2] Fix troubles with watchpoints in DJGPP

[Pedro Alves]

On Wednesday 17 June 2009 00:20:06, Pierre Muller wrote:
> +  /* On systems that load no shared libraries, like DJGPP target,
> +     breakpoint_re_set is never called.
> +     Call it now so that ordinary watchpoints get a chance to
> +     become promoted to hardware watchpoints if the pushed target
> +     supports hardware watchpoints.  */
> +  breakpoint_re_set ();
> +

Much, much better, thanks.  "systems that load no shared libraries"
still isn't the right predicate, as I demonstrated with the
linux static executable, and the breakpoint_re_set call due to
adding the vsyscall page's symbols.  I'd suggest expanding a bit
more.  Something like:

  /* If the user sets watchpoints before execution having started,
     then she gets software watchpoints, because GDB can't know which
     target will end up being pushed, or if it supports hardware
     watchpoints or not.  breakpoint_re_set takes care of promoting
     watchpoints to hardware watchpoints if possible, however, if this
     new inferior doesn't load shared libraries or we don't pull in
     symbols from any other source on this target/arch,
     breakpoint_re_set is never called.  Call it now so that software
     watchpoints get a chance to be promoted to hardware watchpoints
     if the now pushed target supports hardware watchpoints.  */
  breakpoint_re_set ();

(s/ordinary/software/.  There's nothing unordinary about hardware watchpoints)

-- 
Pedro Alves

RE: [PING][RFA-v2] Fix troubles with watchpoints in DJGPP

[Pierre Muller]

This is perfectly OK for me,
could you check it in?
Otherwise I can check it in tomorrow.

Thanks for your replies

Pierre

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : Wednesday, June 17, 2009 1:46 AM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org; 'Eli Zaretskii'
> Objet : Re: [PING][RFA-v2] Fix troubles with watchpoints in DJGPP
> 
> On Wednesday 17 June 2009 00:20:06, Pierre Muller wrote:
> > +  /* On systems that load no shared libraries, like DJGPP target,
> > +     breakpoint_re_set is never called.
> > +     Call it now so that ordinary watchpoints get a chance to
> > +     become promoted to hardware watchpoints if the pushed target
> > +     supports hardware watchpoints.  */
> > +  breakpoint_re_set ();
> > +
> 
> Much, much better, thanks.  "systems that load no shared libraries"
> still isn't the right predicate, as I demonstrated with the
> linux static executable, and the breakpoint_re_set call due to
> adding the vsyscall page's symbols.  I'd suggest expanding a bit
> more.  Something like:
> 
>   /* If the user sets watchpoints before execution having started,
>      then she gets software watchpoints, because GDB can't know which
>      target will end up being pushed, or if it supports hardware
>      watchpoints or not.  breakpoint_re_set takes care of promoting
>      watchpoints to hardware watchpoints if possible, however, if this
>      new inferior doesn't load shared libraries or we don't pull in
>      symbols from any other source on this target/arch,
>      breakpoint_re_set is never called.  Call it now so that software
>      watchpoints get a chance to be promoted to hardware watchpoints
>      if the now pushed target supports hardware watchpoints.  */
>   breakpoint_re_set ();
> 
> (s/ordinary/software/.  There's nothing unordinary about hardware
> watchpoints)
> 
> --
> Pedro Alves

Re: [PING][RFA-v2] Fix troubles with watchpoints in DJGPP

[Pedro Alves]

On Wednesday 17 June 2009 00:54:26, Pierre Muller wrote:
> This is perfectly OK for me,
> could you check it in?
> Otherwise I can check it in tomorrow.

I've no hurry, check it in tomorrow then.  ;-)

-- 
Pedro Alves

RE: [PING][RFA-v2] Fix troubles with watchpoints in DJGPP

[Pierre Muller]

Committed, thanks.

Pierre
> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : Wednesday, June 17, 2009 2:06 AM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org; 'Eli Zaretskii'
> Objet : Re: [PING][RFA-v2] Fix troubles with watchpoints in DJGPP
> 
> On Wednesday 17 June 2009 00:54:26, Pierre Muller wrote:
> > This is perfectly OK for me,
> > could you check it in?
> > Otherwise I can check it in tomorrow.
> 
> I've no hurry, check it in tomorrow then.  ;-)
> 
> --
> Pedro Alves

For the record, here is what I finally checked in.


2009-06-17  Pierre Muller  <muller@ics.u-strasbg.fr>
	Pedro Alves  <pedro@codesourcery.com>
	
	* infcmd.c (post_create_inferior): Call breakpoint_re_set after
target
	is pushed for watchpoint promotion to hardware watchpoint.

 
Index: infcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/infcmd.c,v
retrieving revision 1.245
diff -u -p -r1.245 infcmd.c
--- infcmd.c	7 Jun 2009 16:46:48 -0000	1.245
+++ infcmd.c	17 Jun 2009 05:58:47 -0000
@@ -421,6 +421,18 @@ post_create_inferior (struct target_ops 
 #endif
     }
 
+  /* If the user sets watchpoints before execution having started,
+     then she gets software watchpoints, because GDB can't know which
+     target will end up being pushed, or if it supports hardware
+     watchpoints or not.  breakpoint_re_set takes care of promoting
+     watchpoints to hardware watchpoints if possible, however, if this
+     new inferior doesn't load shared libraries or we don't pull in
+     symbols from any other source on this target/arch,
+     breakpoint_re_set is never called.  Call it now so that software
+     watchpoints get a chance to be promoted to hardware watchpoints
+     if the now pushed target supports hardware watchpoints.  */
+  breakpoint_re_set ();
+
   observer_notify_inferior_created (target, from_tty);
 }
 

Re: [PING][RFA-v2] Fix troubles with watchpoints in DJGPP

[Joel Brobecker]

>   /* If the user sets watchpoints before execution having started,
>      then she gets software watchpoints, because GDB can't know which
>      target will end up being pushed, or if it supports hardware
>      watchpoints or not.  breakpoint_re_set takes care of promoting
>      watchpoints to hardware watchpoints if possible, however, if this
>      new inferior doesn't load shared libraries or we don't pull in
>      symbols from any other source on this target/arch,
>      breakpoint_re_set is never called.  Call it now so that software
>      watchpoints get a chance to be promoted to hardware watchpoints
>      if the now pushed target supports hardware watchpoints.  */
>   breakpoint_re_set ();

Best Comment Of The Year Award! I love comments like these.

-- 
Joel

Re: [PING][RFA-v2] Fix troubles with watchpoints in DJGPP

[DJ Delorie]

> >   /* If the user sets watchpoints before execution having started,
> >      then she gets software watchpoints, because GDB can't know which
> >      target will end up being pushed, or if it supports hardware
> >      watchpoints or not.  breakpoint_re_set takes care of promoting
> >      watchpoints to hardware watchpoints if possible, however, if this
> >      new inferior doesn't load shared libraries or we don't pull in
> >      symbols from any other source on this target/arch,
> >      breakpoint_re_set is never called.  Call it now so that software
> >      watchpoints get a chance to be promoted to hardware watchpoints
> >      if the now pushed target supports hardware watchpoints.  */
> >   breakpoint_re_set ();
> 
> Best Comment Of The Year Award! I love comments like these.

Agreed.  I tell people I'm mentoring, comments should explain *why*,
the code already says *what*.  This is an excellent example of that.