STOPPED_BY_WATCHPOINT peculiarity

STOPPED_BY_WATCHPOINT peculiarity

[Doug Evans]

Something is not right, or at least confusing, in watchpoint-land.

This code in infrun.c is odd:

    /* It may be possible to simply continue after a watchpoint.  */
    if (HAVE_CONTINUABLE_WATCHPOINT)
      STOPPED_BY_WATCHPOINT (ecs->ws);

STOPPED_BY_WATCHPOINT is a predicate.
Therefore at first glance this code is pointless.

Things are slightly less confusing by recognizing that in the
process of computing STOPPED_BY_WATCHPOINT some debugging printf's
may get printed.  e.g. grep for maint_show_dr in
i386-nat.c:i386_stopped_data_address.

nm-i386.h:
#define STOPPED_BY_WATCHPOINT(W)       (i386_stopped_data_address () != 0)

Is that all there is to it?

If so, a comment should be added, maybe something like

-    /* It may be possible to simply continue after a watchpoint.  */
+    /* It may be possible to simply continue after a watchpoint.
+       While at first glance this code is pointless, STOPPED_BY_WATCHPOINT
+       is called in case there are any maintenance debugging printf's.  */
     if (HAVE_CONTINUABLE_WATCHPOINT)
       STOPPED_BY_WATCHPOINT (ecs->ws);

Re: STOPPED_BY_WATCHPOINT peculiarity

[Daniel Jacobowitz]

On Tue, Apr 02, 2002 at 01:54:47PM -0800, Doug Evans wrote:
> Something is not right, or at least confusing, in watchpoint-land.
> 
> This code in infrun.c is odd:
> 
>     /* It may be possible to simply continue after a watchpoint.  */
>     if (HAVE_CONTINUABLE_WATCHPOINT)
>       STOPPED_BY_WATCHPOINT (ecs->ws);
> 
> STOPPED_BY_WATCHPOINT is a predicate.
> Therefore at first glance this code is pointless.
> 
> Things are slightly less confusing by recognizing that in the
> process of computing STOPPED_BY_WATCHPOINT some debugging printf's
> may get printed.  e.g. grep for maint_show_dr in
> i386-nat.c:i386_stopped_data_address.
> 
> nm-i386.h:
> #define STOPPED_BY_WATCHPOINT(W)       (i386_stopped_data_address () != 0)
> 
> Is that all there is to it?
> 
> If so, a comment should be added, maybe something like
> 
> -    /* It may be possible to simply continue after a watchpoint.  */
> +    /* It may be possible to simply continue after a watchpoint.
> +       While at first glance this code is pointless, STOPPED_BY_WATCHPOINT
> +       is called in case there are any maintenance debugging printf's.  */
>      if (HAVE_CONTINUABLE_WATCHPOINT)
>        STOPPED_BY_WATCHPOINT (ecs->ws);

What astonishing timing... I believe there is more going on here, and I
was in the middle of looking at this code just a moment ago.  See the
test failure on i386-linux in gdb.c++/annota2.exp (watch a.x).  We have
a problem actually correctly detecting that we are stopped by a
watchpoint.

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

Re: STOPPED_BY_WATCHPOINT peculiarity

[Daniel Jacobowitz]

On Tue, Apr 02, 2002 at 05:01:45PM -0500, Daniel Jacobowitz wrote:
> On Tue, Apr 02, 2002 at 01:54:47PM -0800, Doug Evans wrote:
> > Something is not right, or at least confusing, in watchpoint-land.
> > 
> > This code in infrun.c is odd:
> > 
> >     /* It may be possible to simply continue after a watchpoint.  */
> >     if (HAVE_CONTINUABLE_WATCHPOINT)
> >       STOPPED_BY_WATCHPOINT (ecs->ws);
> > 
> > STOPPED_BY_WATCHPOINT is a predicate.
> > Therefore at first glance this code is pointless.
> > 
> > Things are slightly less confusing by recognizing that in the
> > process of computing STOPPED_BY_WATCHPOINT some debugging printf's
> > may get printed.  e.g. grep for maint_show_dr in
> > i386-nat.c:i386_stopped_data_address.
> > 
> > nm-i386.h:
> > #define STOPPED_BY_WATCHPOINT(W)       (i386_stopped_data_address () != 0)
> > 
> > Is that all there is to it?
> > 
> > If so, a comment should be added, maybe something like
> > 
> > -    /* It may be possible to simply continue after a watchpoint.  */
> > +    /* It may be possible to simply continue after a watchpoint.
> > +       While at first glance this code is pointless, STOPPED_BY_WATCHPOINT
> > +       is called in case there are any maintenance debugging printf's.  */
> >      if (HAVE_CONTINUABLE_WATCHPOINT)
> >        STOPPED_BY_WATCHPOINT (ecs->ws);
> 
> What astonishing timing... I believe there is more going on here, and I
> was in the middle of looking at this code just a moment ago.  See the
> test failure on i386-linux in gdb.c++/annota2.exp (watch a.x).  We have
> a problem actually correctly detecting that we are stopped by a
> watchpoint.

Actually, I take that back.  STOPPED_BY_WATCHPOINT has nothing to do
with the problem I'm working on.  This line came in far enough back
that the public tree's CVS history appears to be useless; it's been
through some reformats but that's it.  Might want to ask a Cygnus
person to dig around.

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

Re: STOPPED_BY_WATCHPOINT peculiarity

[Eli Zaretskii]

> Date: Tue, 2 Apr 2002 13:54:47 -0800
> From: Doug Evans <dje@transmeta.com>
> 
>     /* It may be possible to simply continue after a watchpoint.  */
>     if (HAVE_CONTINUABLE_WATCHPOINT)
>       STOPPED_BY_WATCHPOINT (ecs->ws);
> 
> STOPPED_BY_WATCHPOINT is a predicate.
> Therefore at first glance this code is pointless.

Who said predicates cannot have side effects?

Anyway, this code is there in GDB for as long as I can remember.
Presumably, it's a survivor from the days when watchpoints were added
to GDB for Sparclet or some such.

I don't mind the extra comment, though.  My only request is to change
the wording of the comment so that the kind of use in i386-nat.c is an
example of how it could be used, not the original purpose of this code
(which is unknown to me).  IMHO, the comment should serve as a warning
against the temptation of removing it, or otherwise rewriting it in
ways that could break ecisting usage.

Thanks.

Re: STOPPED_BY_WATCHPOINT peculiarity

[Doug Evans]

Eli Zaretskii writes:
 > > Date: Tue, 2 Apr 2002 13:54:47 -0800
 > > From: Doug Evans <dje@transmeta.com>
 > > 
 > >     /* It may be possible to simply continue after a watchpoint.  */
 > >     if (HAVE_CONTINUABLE_WATCHPOINT)
 > >       STOPPED_BY_WATCHPOINT (ecs->ws);
 > > 
 > > STOPPED_BY_WATCHPOINT is a predicate.
 > > Therefore at first glance this code is pointless.
 > 
 > Who said predicates cannot have side effects?

They can, but given the choice they don't.
It's poor programming practice.
[pedantic: setting aside predicates that do result caching and the like]

Re: STOPPED_BY_WATCHPOINT peculiarity

[Michael Snyder]

Daniel Jacobowitz wrote:
> 
> On Tue, Apr 02, 2002 at 05:01:45PM -0500, Daniel Jacobowitz wrote:
> > On Tue, Apr 02, 2002 at 01:54:47PM -0800, Doug Evans wrote:
> > > Something is not right, or at least confusing, in watchpoint-land.
> > >
> > > This code in infrun.c is odd:
> > >
> > >     /* It may be possible to simply continue after a watchpoint.  */
> > >     if (HAVE_CONTINUABLE_WATCHPOINT)
> > >       STOPPED_BY_WATCHPOINT (ecs->ws);
> > >
> > > STOPPED_BY_WATCHPOINT is a predicate.
> > > Therefore at first glance this code is pointless.
> > >
> > > Things are slightly less confusing by recognizing that in the
> > > process of computing STOPPED_BY_WATCHPOINT some debugging printf's
> > > may get printed.  e.g. grep for maint_show_dr in
> > > i386-nat.c:i386_stopped_data_address.
> > >
> > > nm-i386.h:
> > > #define STOPPED_BY_WATCHPOINT(W)       (i386_stopped_data_address () != 0)
> > >
> > > Is that all there is to it?
> > >
> > > If so, a comment should be added, maybe something like
> > >
> > > -    /* It may be possible to simply continue after a watchpoint.  */
> > > +    /* It may be possible to simply continue after a watchpoint.
> > > +       While at first glance this code is pointless, STOPPED_BY_WATCHPOINT
> > > +       is called in case there are any maintenance debugging printf's.  */
> > >      if (HAVE_CONTINUABLE_WATCHPOINT)
> > >        STOPPED_BY_WATCHPOINT (ecs->ws);
> >
> > What astonishing timing... I believe there is more going on here, and I
> > was in the middle of looking at this code just a moment ago.  See the
> > test failure on i386-linux in gdb.c++/annota2.exp (watch a.x).  We have
> > a problem actually correctly detecting that we are stopped by a
> > watchpoint.
> 
> Actually, I take that back.  STOPPED_BY_WATCHPOINT has nothing to do
> with the problem I'm working on.  This line came in far enough back
> that the public tree's CVS history appears to be useless; it's been
> through some reformats but that's it.  Might want to ask a Cygnus
> person to dig around.

Yo!   ;-)

1) History: the line was added in 1995 by Jim Kingdon.
I can't find any explanation in the changelogs or comments.

2) Yes, it is possible for a predicate to have side effects, 
and indeed several implementations of this one (eg. sparc)
cause a register to be read, and one (ia64) causes a register
to be modified.  Based on that, I don't think it is safe to
remove the line, without much more deliberation.