Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* RE: Patch for gdb5.0; enable hardware watchpoints on UnixWare
       [not found] ` <NCBBLMGKIKDGJMEOMNMEMEELHEAA.john@Calva.COM>
@ 2000-10-31  4:32   ` Eli Zaretskii
  2000-10-31  6:31     ` John Hughes
  0 siblings, 1 reply; 3+ messages in thread
From: Eli Zaretskii @ 2000-10-31  4:32 UTC (permalink / raw)
  To: john; +Cc: gdb-patches

>   From: "John Hughes" <john@Calva.COM>
>   Date: Tue, 31 Oct 2000 12:20:13 +0100
> 
>   could you steal the svr42mp specific
>   bits of it for 5.1?  (debug registers held in pr_family element of
>   proc status, PCSDEBUG operation to write debug registers).

I will try, but I cannot promise this.  What I'm trying to do is to
write generic code for managing the x86 debug registers, and provide
hooks for target-specific code to use that.  People who maintain
specific targets will then have to revisit their current
implementations and rework them so that they use the code I write.

I _will_ try to make the generic code general enough to be easily
usable by all targets which already have watchpoint support.  In
particular, I will take a look at your code (which I already stashed
away for my reference), to make sure svr42mp will fit into that
framework.

>   1. Why not zap the waddr arg to go32_..._watchpoint?  It's not used.

It might be there because GDB needs that argument for some other
(non-x86) target.  I will investigate this when I come to finalizing
the API.

>   2. In go32_insert_aligned_watchpoint (and ...remove_aligned...) you
>      have a "switch" to work out the len_bits for the watchpoint, then a
>      loop to find an existing watchpoint that might match, then an
>      "if" to check for non-aligned watchpoints.  (Which could not have
>      matched in the loop).

I think you are looking at an old version of go32-nat.c.  The
functions in version one I have have a different structure, so most of
your comments do not apply.  Thanks anyway, I will make a good use of
those which do apply ;-)

>   3. You're not using the DR_FIRSADDR/DR_LASTADDR defines.

Sorry, I'm not sure what defines are you talking about, and what are
DR_FIRSTADDR and DR_LASTADDR in the first place.

In general, the underlying system calls used by DJGPP ignore some of
the bits of the debug registers, so the code might not use them
correctly, or not at all.  While I generalize the code, I will
bring it up to date with the letter of Intel's manuals.

>   4. You have a define DR_RW_READWRITE, other people (SVR3/SVR4/SVR5) seem
>      to have DR_RW_READ with the same value (0x3).

READWRITE is correct, I've just checked this with the Intel manuals.
READ is 0x2.  It is possible that the systems which define READ to be
0x3 have stale code, dating back to 386/486 CPUs, where 0x2 is
undefined.

>   5. Not everybody has DR_CONTROL_MASK.
>
>	   #define DR_CONTROL_MASK ((1<<DR_CONTROL_SIZE) - 1)

That's just a #define, isn't it?  I can always define it, since the
underlying functionality (the bits extracted by this mask) are
supported, right?  Or do I miss something?

Thanks again for your comments.
From msalter@redhat.com Tue Oct 31 05:15:00 2000
From: Mark Salter <msalter@redhat.com>
To: eliz@is.elta.co.il
Cc: jtc@redback.com, gdb-patches@sources.redhat.com
Subject: Re: Remote watchpoint support.
Date: Tue, 31 Oct 2000 05:15:00 -0000
Message-id: <200010311317.e9VDHpw08879@deneb.localdomain>
References: <200010301416.e9UEG6m18981@deneb.localdomain> <5mg0leoyg6.fsf@jtc.redback.com> <200010310906.EAA21820@indy.delorie.com>
X-SW-Source: 2000-10/msg00298.html
Content-length: 714

>>>>> Eli Zaretskii writes:

>> From: jtc@redback.com (J.T. Conklin)
>> Date: 30 Oct 2000 12:39:53 -0800
>> 
>> One thing I dislike about it is that the debug agent never explicitly
>> tells GDB that a watchpoint has been hit, but rather it is implicitly
>> coded in the return value of the query.

> I agree that this is not a good design.

Ok. It seems pretty clear that there is consensus on getting the watchpoint
data address (and possibly other info) when the target stops and not through
a separate query packet. I will try to work up a patch today that does this.
I will probably limit it to the 'T' packet for now. The 'T' packet is already
documented as being extendible for this sort of thing.

--Mark


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: Patch for gdb5.0; enable hardware watchpoints on UnixWare
  2000-10-31  4:32   ` Patch for gdb5.0; enable hardware watchpoints on UnixWare Eli Zaretskii
@ 2000-10-31  6:31     ` John Hughes
  2000-10-31  9:10       ` Eli Zaretskii
  0 siblings, 1 reply; 3+ messages in thread
From: John Hughes @ 2000-10-31  6:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

> >   could you steal the svr42mp specific bits of it for 5.1?
> >   (debug registers held in pr_family element of proc status,
> >   PCSDEBUG operation to write debug registers).
> 
> I will try, but I cannot promise this.

OK.
 
> I _will_ try to make the generic code general enough to be easily
> usable by all targets which already have watchpoint support.

What will you need?  A macro to get the debug registers for a pid
and one to put 'em back?

> >   1. Why not zap the waddr arg to go32_..._watchpoint?  It's not used.
> 
> It might be there because GDB needs that argument for some other
> (non-x86) target. 

It isn't.  GDB doesn't call go32_insert_aligned_watchpoint,
go32_insert_unaligned_watchpoint and so on.  They are only called from the
go32_insert_watchpoint, go32_remove_watchpoint in go32-nat.c (And
go32_insert_watchpoint, go32_remove_watchpoint are called from macros
in tm-xxx.h or nm-xxx,h).

> >   2. In go32_insert_aligned_watchpoint...
> 
> I think you are looking at an old version of go32-nat.c.

My comments refer to the version labeled '1.6' in the cvsweb.  Is there
a later one I could look at?  (Only if you want, please tell me to go
away if I'm wasting your time).

> While I generalize the code, I will bring it up to date with the letter
> of Intel's manuals.

So you're going to have your own defines for DR_... and not use the
system ones?  I guess that was why your code didn't match up to what
I find in sys/debugreg.h on my system.

-- 
John Hughes <john@Calva.COM>, 
        CalvaEDI SA.                            Tel: +33-1-4313-3131
        66 rue du Moulin de la Pointe,          Fax: +33-1-4313-3139
        75013 PARIS.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Patch for gdb5.0; enable hardware watchpoints on UnixWare
  2000-10-31  6:31     ` John Hughes
@ 2000-10-31  9:10       ` Eli Zaretskii
  0 siblings, 0 replies; 3+ messages in thread
From: Eli Zaretskii @ 2000-10-31  9:10 UTC (permalink / raw)
  To: john; +Cc: gdb-patches

> From: "John Hughes" <john@Calva.COM>
> Date: Tue, 31 Oct 2000 15:30:23 +0100
>  
> > I _will_ try to make the generic code general enough to be easily
> > usable by all targets which already have watchpoint support.
> 
> What will you need?  A macro to get the debug registers for a pid
> and one to put 'em back?

No, I think that's too tailored to some targets and won't work for
some of the others.  I think we need functions to pass the debug
registers between GDB and the system-dependent interface between the
debugger and the debuggee.  For example, i386v-nat.c uses ptrace, but
go32-nat.c doesn't.  (In fact, if you look at go32-nat.c, you probably
won't understand how the heck do the debug register wind up in the
debuggee, because the code to do that is not in GDB's sources, it's in
a special library supplied with DJGPP.)

> > >   1. Why not zap the waddr arg to go32_..._watchpoint?  It's not used.
> > 
> > It might be there because GDB needs that argument for some other
> > (non-x86) target. 
> 
> It isn't.

Yes, you are right.  waddr is a left-over from i386v-nat.c, which is
where the go32-nat.c code originated.  Thanks for pointing this out.

> > >   2. In go32_insert_aligned_watchpoint...
> > 
> > I think you are looking at an old version of go32-nat.c.
> 
> My comments refer to the version labeled '1.6' in the cvsweb.  Is there
> a later one I could look at?

That's the latest version.  Hmm...  Oh, I see what happened: you were
talking about go32_insert_aligned_watchpoint, whereas I was looking at
go32_remove_aligned_watchpoint (because you said that it has the same
problems).  Sorry about that misunderstanding.

Yeah, okay, I think you are right, and the code could be made
simpler.  Thanks for pointing that out.  (That code was written by
several people over several years, and it shows.)

> > While I generalize the code, I will bring it up to date with the letter
> > of Intel's manuals.
> 
> So you're going to have your own defines for DR_... and not use the
> system ones?  I guess that was why your code didn't match up to what
> I find in sys/debugreg.h on my system.

I was unaware of sys/debugreg.h until now.  Is it present on all Unix
x86-based systems?  If not, we will need some configury magic to
handle this.

DJGPP certainly doesn't have sys/debugreg.h, and I don't know how
about Cygwin.

What do people think about relative merits of using sys/debugreg.h vs
having the definitions inside GDB (or both)?
From larry@smith-house.org Tue Oct 31 09:19:00 2000
From: Larry Smith <larry@smith-house.org>
To: Fernando Nasser <fnasser@cygnus.com>
Cc: gdb-patches@sources.redhat.com, insight@sources.redhat.com
Subject: Re: [RFA] 100494 fix
Date: Tue, 31 Oct 2000 09:19:00 -0000
Message-id: <39FEFF6D.57CC24A0@smith-house.org>
References: <200010272126.RAA02584@ozma.smith-house.org> <39FDDFB0.D995A63E@cygnus.com>
X-SW-Source: 2000-10/msg00301.html
Content-length: 5283

Fernando Nasser wrote:
> 
> Good catch Larry.  But please add a labeled frame around the radiobuttons,
> will you?  The target selection dialog expansion box does look weird with
> two square buttons and two rounds with no explanation.
> 
> "Run method" sounds right for the frame label.
> 
> If you really want to fix this, make sure the option is set to "run" and
> the buttons greyed if the target "exec" is selected.


Okay, this is a bit more involved than the previous patch,
but it seems to do what Fernando wants.  It adds a new var
to the header file to save the pathnames for the run method
buttons, and when the target selection is changed to "exec",
these buttons are greyed out (and "run" selected) and when
it is anything else, the buttons are active and set to what-
ever the program would have set them to.

This has NOT been checked in or committed.  Permission to
do so?

> cvs diff -r 1.2 targetselection.ith
Index: targetselection.ith
===================================================================
RCS file: /cvs/src/src/gdb/gdbtk/library/targetselection.ith,v
retrieving revision 1.2
diff -r1.2 targetselection.ith
57a58,59
> 
>     variable run_method
> 


> cvs diff -r 1.5 targetselection.itb
Index: targetselection.itb
===================================================================
RCS file: /cvs/src/src/gdb/gdbtk/library/targetselection.itb,v
retrieving revision 1.5
diff -r1.5 targetselection.itb
489a490,493
>   set rm_frame [iwidgets::labeledframe $frame.run_method -labelpos nw -labeltext "Run Method" ]
>   set run_method [ $rm_frame childsite ]
> 
>   set rm_label [label $frame.label -text "Run Method:"]
491c495
<   radiobutton $frame.cont -text {Continue from Last Stop} -value 1
-variable $var \
---
>   radiobutton $run_method.cont -text {Continue from Last Stop} -value 1 -variable $var \
495c499
<   radiobutton $frame.run -text {Run Program} -value 1 -variable $var \
---
>   radiobutton $run_method.run -text {Run Program} -value 1 -variable $var \
501,504c505,516
<   grid $frame.attach $frame.run -sticky w
<   grid $frame.load   $frame.cont -sticky w
<   grid $frame.afterl -sticky we -columnspan 2
<   grid $frame.aftere -sticky we -columnspan 2
---
> 
>   grid $frame.label -column 1 -row 0 -sticky w
>   grid $frame.attach -column 0 -row 1 -ipady 2
>   grid $frame.load -column 0 -row 2 -ipady 2
> 
>   grid $run_method.run -column 0 -row 1 -sticky w -ipady 2
>   grid $run_method.cont -column 0 -row 2 -sticky w -ipady 2
>   
>   grid $rm_frame -column 1 -row 1 -rowspan 2 -sticky nsew -ipady 2
> 
>   grid $frame.afterl -row 4 -sticky we -columnspan 2 -ipady 2
>   grid $frame.aftere -sticky we -columnspan 2 -ipady 2
724a737,745
> 
>   if { "$target" == "exec" } {
>     $run_method.run configure -state disabled -value 1
>     $run_method.cont configure -state disabled
>   } else {
>     $run_method.run configure -state normal
>     $run_method.cont configure -state normal
>   }
> 
> 




> 
> Cheers,
> Fernando
> 
> P.S.: Insight patches go to the insight list, not gdb-patches as the people
> that wrote the code watch it more closely and there is a larger group of
> users and contributors in this list that do not subscribe to gdb-patches.
> 
> Larry Smith wrote:
> >
> > The following patch is for 100494, changing a pair of checkbuttons
> > to radiobuttons.  Since the proper behaviour was already enforced
> > this is really only cosmetic, and changes no current control flow.
> >
> > I apologise for being unaware of proper protocol for doing patches
> > on sourceware, this has already been checked in.  I will try to be
> > more circumspect in the future...
> >
> > regards,
> > Larry
> >
> > 2000-10-26  Larry Smith  <lsmith@redhat.com>
> >
> >         * change targetselection.itb: Run Program and Continue From Last Stop
> >         are now radio buttons rather than checkbuttons
> >
> > Index: targetselection.itb
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/gdbtk/library/targetselection.itb,v
> > retrieving revision 1.4
> > retrieving revision 1.5
> > diff -u -r1.4 -r1.5
> > --- targetselection.itb 2000/09/12 20:04:11     1.4
> > +++ targetselection.itb 2000/10/26 20:47:57     1.5
> > @@ -488,11 +488,11 @@
> >    checkbutton $frame.load -text {Download Program} -variable $var
> >
> >    set var [pref varname gdb/src/run_cont]
> > -  checkbutton $frame.cont -text {Continue from Last Stop} -variable $var \
> > +  radiobutton $frame.cont -text {Continue from Last Stop} -value 1 -variable $var \
> >      -command [code $this set_run run]
> >
> >    set var [pref varname gdb/src/run_run]
> > -  checkbutton $frame.run -text {Run Program} -variable $var \
> > +  radiobutton $frame.run -text {Run Program} -value 1 -variable $var \
> >      -command [code $this set_run cont]
> >
> >    # The after attaching command entry
> 
> --
> Fernando Nasser
> Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
> 2323 Yonge Street, Suite #300
> Toronto, Ontario   M4P 2C9

-- 
 .-.    .-. .---. .---. .-..-. | "Bill Gates is just a monocle
 | |__ / | \| |-< | |-<  >  /  | and a Persian Cat away from
 `----'`-^-'`-'`-'`-'`-' `-'   | being one of the bad guys in a
       My opinions only.       | James Bond movie." -- D Miller
From spolk@redhat.com Tue Oct 31 09:23:00 2000
From: Syd Polk <spolk@redhat.com>
To: lsmith@redhat.com
Cc: Fernando Nasser <fnasser@cygnus.com>, gdb-patches@sources.redhat.com, insight@sources.redhat.com
Subject: Re: [RFA] 100494 fix
Date: Tue, 31 Oct 2000 09:23:00 -0000
Message-id: <39FF00D9.670FE177@redhat.com>
References: <200010272126.RAA02584@ozma.smith-house.org> <39FDDFB0.D995A63E@cygnus.com> <39FEFF6D.57CC24A0@smith-house.org>
X-SW-Source: 2000-10/msg00302.html
Content-length: 5837

Need a ChangeLog entry.

Now that the official policy has been clarified, patches should be submitted
just to insight@sources.redhat.com, not gdb-patches@sources.redhat.com.

And when you get a ChangeLog entry, please post a new message with [RFA]
starting the subject.

Larry Smith wrote:
> 
> Fernando Nasser wrote:
> >
> > Good catch Larry.  But please add a labeled frame around the radiobuttons,
> > will you?  The target selection dialog expansion box does look weird with
> > two square buttons and two rounds with no explanation.
> >
> > "Run method" sounds right for the frame label.
> >
> > If you really want to fix this, make sure the option is set to "run" and
> > the buttons greyed if the target "exec" is selected.
> 
> Okay, this is a bit more involved than the previous patch,
> but it seems to do what Fernando wants.  It adds a new var
> to the header file to save the pathnames for the run method
> buttons, and when the target selection is changed to "exec",
> these buttons are greyed out (and "run" selected) and when
> it is anything else, the buttons are active and set to what-
> ever the program would have set them to.
> 
> This has NOT been checked in or committed.  Permission to
> do so?
> 
> > cvs diff -r 1.2 targetselection.ith
> Index: targetselection.ith
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/library/targetselection.ith,v
> retrieving revision 1.2
> diff -r1.2 targetselection.ith
> 57a58,59
> >
> >     variable run_method
> >
> 
> > cvs diff -r 1.5 targetselection.itb
> Index: targetselection.itb
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/library/targetselection.itb,v
> retrieving revision 1.5
> diff -r1.5 targetselection.itb
> 489a490,493
> >   set rm_frame [iwidgets::labeledframe $frame.run_method -labelpos nw -labeltext "Run Method" ]
> >   set run_method [ $rm_frame childsite ]
> >
> >   set rm_label [label $frame.label -text "Run Method:"]
> 491c495
> <   radiobutton $frame.cont -text {Continue from Last Stop} -value 1
> -variable $var \
> ---
> >   radiobutton $run_method.cont -text {Continue from Last Stop} -value 1 -variable $var \
> 495c499
> <   radiobutton $frame.run -text {Run Program} -value 1 -variable $var \
> ---
> >   radiobutton $run_method.run -text {Run Program} -value 1 -variable $var \
> 501,504c505,516
> <   grid $frame.attach $frame.run -sticky w
> <   grid $frame.load   $frame.cont -sticky w
> <   grid $frame.afterl -sticky we -columnspan 2
> <   grid $frame.aftere -sticky we -columnspan 2
> ---
> >
> >   grid $frame.label -column 1 -row 0 -sticky w
> >   grid $frame.attach -column 0 -row 1 -ipady 2
> >   grid $frame.load -column 0 -row 2 -ipady 2
> >
> >   grid $run_method.run -column 0 -row 1 -sticky w -ipady 2
> >   grid $run_method.cont -column 0 -row 2 -sticky w -ipady 2
> >
> >   grid $rm_frame -column 1 -row 1 -rowspan 2 -sticky nsew -ipady 2
> >
> >   grid $frame.afterl -row 4 -sticky we -columnspan 2 -ipady 2
> >   grid $frame.aftere -sticky we -columnspan 2 -ipady 2
> 724a737,745
> >
> >   if { "$target" == "exec" } {
> >     $run_method.run configure -state disabled -value 1
> >     $run_method.cont configure -state disabled
> >   } else {
> >     $run_method.run configure -state normal
> >     $run_method.cont configure -state normal
> >   }
> >
> >
> 
> >
> > Cheers,
> > Fernando
> >
> > P.S.: Insight patches go to the insight list, not gdb-patches as the people
> > that wrote the code watch it more closely and there is a larger group of
> > users and contributors in this list that do not subscribe to gdb-patches.
> >
> > Larry Smith wrote:
> > >
> > > The following patch is for 100494, changing a pair of checkbuttons
> > > to radiobuttons.  Since the proper behaviour was already enforced
> > > this is really only cosmetic, and changes no current control flow.
> > >
> > > I apologise for being unaware of proper protocol for doing patches
> > > on sourceware, this has already been checked in.  I will try to be
> > > more circumspect in the future...
> > >
> > > regards,
> > > Larry
> > >
> > > 2000-10-26  Larry Smith  <lsmith@redhat.com>
> > >
> > >         * change targetselection.itb: Run Program and Continue From Last Stop
> > >         are now radio buttons rather than checkbuttons
> > >
> > > Index: targetselection.itb
> > > ===================================================================
> > > RCS file: /cvs/src/src/gdb/gdbtk/library/targetselection.itb,v
> > > retrieving revision 1.4
> > > retrieving revision 1.5
> > > diff -u -r1.4 -r1.5
> > > --- targetselection.itb 2000/09/12 20:04:11     1.4
> > > +++ targetselection.itb 2000/10/26 20:47:57     1.5
> > > @@ -488,11 +488,11 @@
> > >    checkbutton $frame.load -text {Download Program} -variable $var
> > >
> > >    set var [pref varname gdb/src/run_cont]
> > > -  checkbutton $frame.cont -text {Continue from Last Stop} -variable $var \
> > > +  radiobutton $frame.cont -text {Continue from Last Stop} -value 1 -variable $var \
> > >      -command [code $this set_run run]
> > >
> > >    set var [pref varname gdb/src/run_run]
> > > -  checkbutton $frame.run -text {Run Program} -variable $var \
> > > +  radiobutton $frame.run -text {Run Program} -value 1 -variable $var \
> > >      -command [code $this set_run cont]
> > >
> > >    # The after attaching command entry
> >
> > --
> > Fernando Nasser
> > Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
> > 2323 Yonge Street, Suite #300
> > Toronto, Ontario   M4P 2C9
> 
> --
>  .-.    .-. .---. .---. .-..-. | "Bill Gates is just a monocle
>  | |__ / | \| |-< | |-<  >  /  | and a Persian Cat away from
>  `----'`-^-'`-'`-'`-'`-' `-'   | being one of the bad guys in a
>        My opinions only.       | James Bond movie." -- D Miller
From msnyder@redhat.com Tue Oct 31 11:24:00 2000
From: Michael Snyder <msnyder@redhat.com>
To: Eli Zaretskii <eliz@is.elta.co.il>
Cc: john@Calva.COM, Peter.Schauer@Regent.E-Technik.TU-Muenchen.DE, gdb-patches@sources.redhat.com
Subject: Re: Patch for gdb5.0; enable hardware watchpoints on UnixWare
Date: Tue, 31 Oct 2000 11:24:00 -0000
Message-id: <39FF1BA8.16E3@redhat.com>
References: <NCBBLMGKIKDGJMEOMNMEEEDMHEAA.john@Calva.COM> <200010310906.EAA21817@indy.delorie.com>
X-SW-Source: 2000-10/msg00303.html
Content-length: 1115

Eli Zaretskii wrote:
> 
> > From: "John Hughes" <john@Calva.COM>
> > Date: Mon, 30 Oct 2000 17:11:58 +0100
> >
> > > But all the i386v4-nat.c code has to be conditionalized on
> > > UNIXWARE (or better yet, moved to a new i386sco-nat.c file), as
> > > i386v4-nat.c is used by x86 Solaris as well, and Solaris uses page
> > > faulting instead of debug registers for watchpoints, and would not
> > > compile with your patches installed.
> >
> > Aha.  Here's a fixed version of the patch; the i386v4-nat.c changes
> > are conditional on UNIXWARE.
> 
> Note that I'm working on a unified watchpoint implementation for all
> ia32 targets, based on the code written for the DJGPP port (see
> go32-nat.c).  This implementation will allow to watch large areas (up
> to 16 bytes) and debug register sharing via reference counts (required
> for watching overlapping areas) on all ia32 platforms.
> 
> I am trying to make this happen for v5.1, so perhaps changes like this
> one should be witheld and not committed, at least until we decide that
> I'm not living up to my promises.

Is there a reason for them to be withheld?
From msnyder@redhat.com Tue Oct 31 11:51:00 2000
From: Michael Snyder <msnyder@redhat.com>
To: Mark Kettenis <kettenis@wins.uva.nl>
Cc: gdb-patches@sourceware.cygnus.com, kevinb@cygnus.com
Subject: Re: [PATCH]: Extend SVR4 shlib debug changes to SH3 and other targets
Date: Tue, 31 Oct 2000 11:51:00 -0000
Message-id: <39FF22BE.2E8E@redhat.com>
References: <200010310102.RAA24876@cleaver.cygnus.com> <200010310937.e9V9b4604953@debye.wins.uva.nl>
X-SW-Source: 2000-10/msg00304.html
Content-length: 2518

Tweaked per feedback and checked in.

Mark Kettenis wrote:
> 
>    From: Michael Snyder <msnyder@cygnus.com>
>    Date: Mon, 30 Oct 2000 17:02:01 -0800 (PST)
> 
>    This change moves the definition of SVR4_SHARED_LIBS into
>    config/tm-linux.h, so that it will affect all linuxen.
>    I then add a new file config/sh/tm-linux.h which includes
>    config/tm-linux.h (as well as sh/tm-sh.h).  Finally I add
>    target-specific definitions of fetch_link_map_offsets, so
>    that shared libraries can be cross-debugged.
> 
>    2000-10-30  Michael Snyder  <msnyder@cleaver.cygnus.com>
> 
>            * config/sh/tm-linux.h: New file.  Include generic tm-linux.h,
>            plus tm-sh.h, then define SVR4_FETCH_LINK_MAP_OFFSETS to use
>            the sh target function instead of the default link map offsets.
>            * config/sh/sh.mt: Add solib.o and solib-svr4.o to TDEPFILES.
>            Use sh/tm-linux.h instead of sh/tm-sh.h.
>            * sh-tdep.c (sh_linux_svr4_fetch_link_map_offsets):
>            New function.  Construct target-specific link map offsets.
>            * i386-linux-tdep.c (i386_linux_svr4_fetch_link_map_offsets:
>            New function.  Construct target-specific link map offsets.
>            * config/i386/tm-linux.h: Use above function instead of default.
> 
>    2000-10-30  Michael Snyder  <msnyder@cleaver.cygnus.com>
> 
>            * config/i386/tm-linux.h: Remove definition of SVR4_SHARED_LIBS,
>            and inclusion of solib.h.  Move up into ../tm-linux.h.
>            config/tm-linux.h: Define SVR4_SHARED_LIBS, include solib.h.
> 
> Looks good to me.  One minor nit though:
> 
>    Index: config/tm-linux.h
>    ===================================================================
>    RCS file: /cvs/src/src/gdb/config/tm-linux.h,v
>    retrieving revision 1.1.1.1
>    diff -c -3 -p -r1.1.1.1 tm-linux.h
>    *** tm-linux.h       1999/12/07 03:56:08     1.1.1.1
>    --- tm-linux.h       2000/10/31 00:57:12
>    ***************
>    *** 34,36 ****
>    --- 34,42 ----
>      /* We need this file for the SOLIB_TRAMPOLINE stuff. */
> 
>      #include "tm-sysv4.h"
>    +
>    + /* We define this if link.h is available, because with ELF we use SVR4 style
>    +    shared libraries.  FIXME: move up into config/tm-linux.h?  */
>    +
>    + #define SVR4_SHARED_LIBS
>    + #include "solib.h"         /* Support for shared libraries. */
> 
> Please update the comment.  The FIXME can go now, and the blurb about
> link.h isn't true anymore isn't it?
From msnyder@cygnus.com Tue Oct 31 14:04:00 2000
From: Michael Snyder <msnyder@cygnus.com>
To: gdb-patches@sources.redhat.com
Subject: [RFA] Encapsulate method of searching for a solib.
Date: Tue, 31 Oct 2000 14:04:00 -0000
Message-id: <200010312203.OAA25580@cleaver.cygnus.com>
X-SW-Source: 2000-10/msg00305.html
Content-length: 9273

The enable_break function from solib-svr4.c needs to use the same
search method for shared libs as is used in solib.c (utilizing
SOLIB_ABSOLUTE_PREFIX and SOLIB_SEARCH_PATH), so I've abstracted
this method into a function and cleaned it up a bit.

2000-10-31  Michael Snyder  <msnyder@cleaver.cygnus.com>

        * solib.c (solib_open): New function.  Abstracts the code from
        solib_map_sections that searches for a solib file.
        (solib_map_sections): Call solib_open instead of working inline.
        * solib-svr4.c (enable_break): Call solib_open to find the
        dynamic linker's base address.
        * solib-svr4.h (solib_open): Export the declaration.

Index: solib.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/solib.c,v
retrieving revision 1.154
diff -c -3 -p -r1.154 solib.c
*** solib.c	2000/10/31 20:19:49	1.154
--- solib.c	2000/10/31 21:59:52
*************** static char *solib_search_path = NULL;
*** 66,71 ****
--- 66,150 ----
  
  /*
  
+    GLOBAL FUNCTION
+ 
+    solib_open -- Find a shared library file and open it.
+ 
+    SYNOPSIS
+ 
+    int solib_open (char *in_patname, char **found_pathname);
+ 
+    DESCRIPTION
+ 
+    Global variable SOLIB_ABSOLUTE_PREFIX is used as a prefix directory
+    to search for shared libraries if they have an absolute path.
+ 
+    Global variable SOLIB_SEARCH_PATH is used as a prefix directory
+    (or set of directories, as in LD_LIBRARY_PATH) to search for all
+    shared libraries if not found in SOLIB_ABSOLUTE_PREFIX.
+ 
+    Search order:
+    * If path is absolute, look in SOLIB_ABSOLUTE_PREFIX.
+    * If path is absolute, look for it literally (unmodified).
+    * Look in SOLIB_SEARCH_PATH.
+    * Look in inferior's $PATH.
+    * Look in inferior's $LD_LIBRARY_PATH.
+ 
+    RETURNS
+    
+    file handle for opened solib, or -1 for failure.  */
+ 
+ int
+ solib_open (char *in_pathname, char **found_pathname)
+ {
+   int found_file = -1;
+   char *temp_pathname = NULL;
+ 
+   if (solib_absolute_prefix != NULL &&
+       ROOTED_P (in_pathname))
+     {
+       int  prefix_len = strlen (solib_absolute_prefix);
+ 
+       /* Remove trailing slashes from absolute prefix.  */
+       while (prefix_len > 0 && SLASH_P (solib_absolute_prefix[prefix_len - 1]))
+ 	prefix_len--;
+ 
+       /* Cat the prefixed pathname together.  */
+       temp_pathname = alloca (prefix_len + strlen (in_pathname) + 1);
+       strncpy (temp_pathname, solib_absolute_prefix, prefix_len);
+       temp_pathname[prefix_len] = '\0';
+       strcat (temp_pathname, in_pathname);
+ 
+       /* Now see if we can open it.  */
+       found_file = open (temp_pathname, O_RDONLY, 0);
+     }
+ 
+   /* If not found, next search the solib_search_path (if any).  */
+   if (found_file < 0 && solib_search_path != NULL)
+     found_file = openp (solib_search_path,
+ 			1, in_pathname, O_RDONLY, 0, &temp_pathname);
+ 
+   /* If not found, next search the inferior's $PATH environment variable. */
+   if (found_file < 0 && solib_search_path != NULL)
+     found_file = openp (get_in_environ (inferior_environ, "PATH"),
+ 			1, in_pathname, O_RDONLY, 0, &temp_pathname);
+ 
+   /* If not found, next search the inferior's $LD_LIBRARY_PATH 
+      environment variable. */
+   if (found_file < 0 && solib_search_path != NULL)
+     found_file = openp (get_in_environ (inferior_environ, "LD_LIBRARY_PATH"),
+ 			1, in_pathname, O_RDONLY, 0, &temp_pathname);
+ 
+   /* Done.  If not found, tough luck.  Return found_file and 
+      (optionally) found_pathname.  */
+   if (found_pathname != NULL)
+     *found_pathname = strsave (temp_pathname);
+   return found_file;
+ }
+ 
+ 
+ /*
+ 
     LOCAL FUNCTION
  
     solib_map_sections -- open bfd and build sections for shared lib
*************** solib_map_sections (PTR arg)
*** 104,152 ****
  
    filename = tilde_expand (so->so_name);
  
-   if (solib_absolute_prefix && ROOTED_P (filename))
-     /* Prefix shared libraries with absolute filenames with
-        SOLIB_ABSOLUTE_PREFIX.  */
-     {
-       char *pfxed_fn;
-       int pfx_len;
- 
-       pfx_len = strlen (solib_absolute_prefix);
- 
-       /* Remove trailing slashes.  */
-       while (pfx_len > 0 && SLASH_P (solib_absolute_prefix[pfx_len - 1]))
- 	pfx_len--;
- 
-       pfxed_fn = xmalloc (pfx_len + strlen (filename) + 1);
-       strcpy (pfxed_fn, solib_absolute_prefix);
-       strcat (pfxed_fn, filename);
-       free (filename);
- 
-       filename = pfxed_fn;
-     }
- 
    old_chain = make_cleanup (free, filename);
  
-   scratch_chan = -1;
- 
-   if (solib_search_path)
-     scratch_chan = openp (solib_search_path,
- 			  1, filename, O_RDONLY, 0, &scratch_pathname);
-   if (scratch_chan < 0)
-     scratch_chan = openp (get_in_environ (inferior_environ, "PATH"),
- 			  1, filename, O_RDONLY, 0, &scratch_pathname);
-   if (scratch_chan < 0)
-     {
-       scratch_chan = openp (get_in_environ
- 			    (inferior_environ, "LD_LIBRARY_PATH"),
- 			    1, filename, O_RDONLY, 0, &scratch_pathname);
-     }
    if (scratch_chan < 0)
      {
        perror_with_name (filename);
      }
-   /* Leave scratch_pathname allocated.  abfd->name will point to it.  */
  
    abfd = bfd_fdopenr (scratch_pathname, gnutarget, scratch_chan);
    if (!abfd)
      {
--- 183,197 ----
  
    filename = tilde_expand (so->so_name);
  
    old_chain = make_cleanup (free, filename);
+   scratch_chan = solib_open (filename, &scratch_pathname);
  
    if (scratch_chan < 0)
      {
        perror_with_name (filename);
      }
  
+   /* Leave scratch_pathname allocated.  abfd->name will point to it.  */
    abfd = bfd_fdopenr (scratch_pathname, gnutarget, scratch_chan);
    if (!abfd)
      {
*************** solib_map_sections (PTR arg)
*** 154,165 ****
        error ("Could not open `%s' as an executable file: %s",
  	     scratch_pathname, bfd_errmsg (bfd_get_error ()));
      }
    /* Leave bfd open, core_xfer_memory and "info files" need it.  */
    so->abfd = abfd;
    abfd->cacheable = true;
  
!   /* copy full path name into so_name, so that later symbol_file_add can find
!      it */
    if (strlen (scratch_pathname) >= SO_NAME_MAX_PATH_SIZE)
      error ("Full path name length of shared library exceeds SO_NAME_MAX_PATH_SIZE in so_list structure.");
    strcpy (so->so_name, scratch_pathname);
--- 199,211 ----
        error ("Could not open `%s' as an executable file: %s",
  	     scratch_pathname, bfd_errmsg (bfd_get_error ()));
      }
+ 
    /* Leave bfd open, core_xfer_memory and "info files" need it.  */
    so->abfd = abfd;
    abfd->cacheable = true;
  
!   /* copy full path name into so_name, so that later symbol_file_add
!      can find it */
    if (strlen (scratch_pathname) >= SO_NAME_MAX_PATH_SIZE)
      error ("Full path name length of shared library exceeds SO_NAME_MAX_PATH_SIZE in so_list structure.");
    strcpy (so->so_name, scratch_pathname);
Index: solib-svr4.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/solib-svr4.c,v
retrieving revision 1.3
diff -c -3 -p -r1.3 solib-svr4.c
*** solib-svr4.c	2000/10/31 20:19:49	1.3
--- solib-svr4.c	2000/10/31 21:59:52
*************** enable_break (void)
*** 1269,1275 ****
        unsigned int interp_sect_size;
        char *buf;
        CORE_ADDR load_addr;
!       bfd *tmp_bfd;
        CORE_ADDR sym_addr = 0;
  
        /* Read the contents of the .interp section into a local buffer;
--- 1269,1277 ----
        unsigned int interp_sect_size;
        char *buf;
        CORE_ADDR load_addr;
!       bfd *tmp_bfd = NULL;
!       int tmp_fd = -1;
!       char *tmp_pathname = NULL;
        CORE_ADDR sym_addr = 0;
  
        /* Read the contents of the .interp section into a local buffer;
*************** enable_break (void)
*** 1287,1293 ****
           to find any magic formula to find it for Solaris (appears to
           be trivial on GNU/Linux).  Therefore, we have to try an alternate
           mechanism to find the dynamic linker's base address.  */
!       tmp_bfd = bfd_openr (buf, gnutarget);
        if (tmp_bfd == NULL)
  	goto bkpt_at_symbol;
  
--- 1289,1299 ----
           to find any magic formula to find it for Solaris (appears to
           be trivial on GNU/Linux).  Therefore, we have to try an alternate
           mechanism to find the dynamic linker's base address.  */
! 
!       tmp_fd  = solib_open (buf, &tmp_pathname);
!       if (tmp_fd >= 0)
! 	tmp_bfd = bfd_fdopenr (tmp_pathname, gnutarget, tmp_fd);
! 
        if (tmp_bfd == NULL)
  	goto bkpt_at_symbol;
  
Index: solib-svr4.h
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/solib-svr4.h,v
retrieving revision 1.2
diff -c -3 -p -r1.2 solib-svr4.h
*** solib-svr4.h	2000/10/30 22:03:33	1.2
--- solib-svr4.h	2000/10/31 21:59:52
*************** struct link_map_offsets
*** 64,69 ****
--- 64,72 ----
      int l_name_size;
    };
  
+ /* Find solib binary file and open it.  */
+ extern int solib_open (char *in_pathname, char **found_pathname);
+ 
  #ifndef SVR4_FETCH_LINK_MAP_OFFSETS
  extern struct link_map_offsets *default_svr4_fetch_link_map_offsets (void);
  #define SVR4_FETCH_LINK_MAP_OFFSETS() default_svr4_fetch_link_map_offsets ()
From msnyder@cygnus.com Tue Oct 31 14:09:00 2000
From: Michael Snyder <msnyder@cygnus.com>
To: gdb-patches@sources.redhat.com
Subject: [RFA]: Initialize solib search psth from environ variable
Date: Tue, 31 Oct 2000 14:09:00 -0000
Message-id: <200010312209.OAA25598@cleaver.cygnus.com>
X-SW-Source: 2000-10/msg00306.html
Content-length: 1076

This patch would check the environment variables SOLIB_SEARCH_PATH
and SOLIB_ABSOLUTE_PREFIX once, at start-up, and use them to set the
corresponding gdb state variables.

2000-10-31  Michael Snyder  <msnyder@cleaver.cygnus.com>

        * solib.c (initialize_solib): Initialize solib_absolute_prefix
        and solib_search_path from the environment variables
        SOLIB_ABSOLUTE_PREFIX and SOLIB_SEARCH_PATH, if they're defined.

Index: solib.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/solib.c,v
retrieving revision 1.155
diff -c -3 -p -r1.155 solib.c
*** solib.c	2000/10/31 22:04:12	1.155
--- solib.c	2000/10/31 22:08:17
*************** This takes precedence over the environme
*** 810,813 ****
--- 810,819 ----
  		  &setlist),
       &showlist);
  
+   /* check for solib path environment variables */
+   if ((tmp = getenv ("SOLIB_ABSOLUTE_PREFIX")) != NULL)
+     solib_absolute_prefix = strsave (tmp);
+ 
+   if ((tmp = getenv ("SOLIB_SEARCH_PATH")) != NULL)
+     solib_search_path = strsave (tmp);
  }
From jtc@redback.com Tue Oct 31 15:14:00 2000
From: jtc@redback.com (J.T. Conklin)
To: Eli Zaretskii <eliz@is.elta.co.il>
Cc: msalter@redhat.com, gdb-patches@sources.redhat.com
Subject: Re: Remote watchpoint support.
Date: Tue, 31 Oct 2000 15:14:00 -0000
Message-id: <5mpukgli1x.fsf@jtc.redback.com>
References: <200010301416.e9UEG6m18981@deneb.localdomain> <5mg0leoyg6.fsf@jtc.redback.com> <200010310906.EAA21820@indy.delorie.com>
X-SW-Source: 2000-10/msg00307.html
Content-length: 2051

>>>>> "Eli" == Eli Zaretskii <eliz@delorie.com> writes:
>> The second is the return value of 0 if the target stopped for another
>> reason.  I (now) know that this is the current behavior of the target_
>> stopped_data_address() macro, but it use the one address that I think
>> might be very important to watch on targets where the zero page isn't
>> unmapped (in which case a reference would cause a exception/trap).

Eli> I would actually be very happy if we could change the interface of
Eli> target_stopped_data_address so that zero would not be treated
Eli> specially.  One problem with the current design is that I cannot catch
Eli> NULL pointer dereferences on Windows, where the null page cannot be
Eli> unmapped from the address space of DJGPP programs.  This prevents me
Eli> from finding such bugs by setting a watchpoint at address 0.

Did I just hear an echo?  :-)  I'm glad you agree that this is a bug.

>> While it might not be possible to fix in GDB's internals for some
>> time

Eli> What would it take to change that?  Perhaps it would be a good
Eli> idea to have the list of the obstacles, in case someone might try
Eli> to negotiate them one by one.

I was being pessimistic, perhaps overly so, in my assessment of fixing
GDB to emphasize my point that we shouldn't constrain the remote debug
protocol around internal implementation details.  Given a bit of hard
work, many limitations within GDB can be easily fixed; but the remote
protocol has a permanence that makes it worth our while to get it right
the first time.  Otherwise we'll forever be saddled with supporting 
old stubs with the flawed implementation.

In this case, I don't think it would be that difficult to fix.  For a 
first cut, I'd change the macro to:

        target_stopped_data_address(&addr)

It would return 0 if GDB didn't stop because of a watchpoint, and 1
(and set addr) if it did.

I don't know if there are any other lurking problems for watching
address 0.  I'm sure they'd show up pretty soon.

        --jtc

-- 
J.T. Conklin
RedBack Networks
From kevinb@cygnus.com Tue Oct 31 16:23:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: Michael Snyder <msnyder@cygnus.com>, gdb-patches@sources.redhat.com
Subject: Re: [RFA] Encapsulate method of searching for a solib.
Date: Tue, 31 Oct 2000 16:23:00 -0000
Message-id: <1001101002345.ZM4449@ocotillo.lan>
References: <200010312203.OAA25580@cleaver.cygnus.com> <msnyder@cygnus.com>
X-SW-Source: 2000-10/msg00308.html
Content-length: 912

On Oct 31,  2:03pm, Michael Snyder wrote:

> The enable_break function from solib-svr4.c needs to use the same
> search method for shared libs as is used in solib.c (utilizing
> SOLIB_ABSOLUTE_PREFIX and SOLIB_SEARCH_PATH), so I've abstracted
> this method into a function and cleaned it up a bit.
> 
> 2000-10-31  Michael Snyder  <msnyder@cleaver.cygnus.com>
> 
>         * solib.c (solib_open): New function.  Abstracts the code from
>         solib_map_sections that searches for a solib file.
>         (solib_map_sections): Call solib_open instead of working inline.
>         * solib-svr4.c (enable_break): Call solib_open to find the
>         dynamic linker's base address.
>         * solib-svr4.h (solib_open): Export the declaration.

This looks good to me, but Jim Blandy is the official maintainer of
these files, so if you want "official maintainer approval", you'll
have to check with him.

Kevin
From kevinb@cygnus.com Tue Oct 31 16:28:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: Michael Snyder <msnyder@cygnus.com>, gdb-patches@sources.redhat.com
Subject: Re: [RFA] Encapsulate method of searching for a solib.
Date: Tue, 31 Oct 2000 16:28:00 -0000
Message-id: <1001101002807.ZM4469@ocotillo.lan>
References: <200010312203.OAA25580@cleaver.cygnus.com> <msnyder@cygnus.com>
X-SW-Source: 2000-10/msg00309.html
Content-length: 405

On Oct 31,  2:03pm, Michael Snyder wrote:

>         * solib-svr4.h (solib_open): Export the declaration.

Woops.  Now that I stare at it a bit longer, I think that the
exported declaration ought to go somewhere else.  I suggest
solist.h.  (Actually, I think we ought to rename solist.h to
solib-private.h.  The intent is that it should only be included
by solib.c and the various solib backends.)

Kevin
From msnyder@redhat.com Tue Oct 31 16:29:00 2000
From: Michael Snyder <msnyder@redhat.com>
To: Kevin Buettner <kevinb@cygnus.com>
Cc: Michael Snyder <msnyder@cygnus.com>, gdb-patches@sources.redhat.com
Subject: Re: [RFA] Encapsulate method of searching for a solib.
Date: Tue, 31 Oct 2000 16:29:00 -0000
Message-id: <39FF63EB.690D@redhat.com>
References: <200010312203.OAA25580@cleaver.cygnus.com> <1001101002807.ZM4469@ocotillo.lan>
X-SW-Source: 2000-10/msg00310.html
Content-length: 503

Kevin Buettner wrote:
> 
> On Oct 31,  2:03pm, Michael Snyder wrote:
> 
> >         * solib-svr4.h (solib_open): Export the declaration.
> 
> Woops.  Now that I stare at it a bit longer, I think that the
> exported declaration ought to go somewhere else.  I suggest
> solist.h.  (Actually, I think we ought to rename solist.h to
> solib-private.h.  The intent is that it should only be included
> by solib.c and the various solib backends.)

You're the second one to say so.  OK, I'll make that change.
From msnyder@cygnus.com Tue Oct 31 16:33:00 2000
From: Michael Snyder <msnyder@cygnus.com>
To: gdb-patches@sources.redhat.com
Subject: [RFA] Call SOLIB_CREATE_INFERIOR_HOOK for remote targets.
Date: Tue, 31 Oct 2000 16:33:00 -0000
Message-id: <200011010033.QAA25682@cleaver.cygnus.com>
X-SW-Source: 2000-10/msg00311.html
Content-length: 1231

I want to add this call to remote_open, so that we can debug shared
libraries on a remote target.  This works together with Kevin Buettner's
recent changes to the solib code.

2000-10-31  Michael Snyder  <msnyder@cleaver.cygnus.com>

        * remote.c (remote_open): Call SOLIB_CREATE_INFERIOR_HOOK
        (if defined), so that shared libraries can be detected.

Index: remote.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/remote.c,v
retrieving revision 1.290
diff -c -3 -p -r1.290 remote.c
*** remote.c	2000/10/25 12:55:39	1.290
--- remote.c	2000/11/01 00:31:00
***************
*** 51,56 ****
--- 51,57 ----
  
  #include <signal.h>
  #include "serial.h"
+ #include "gdbcore.h" /* for exec_bfd */
  
  /* Prototypes for local functions */
  static void cleanup_sigint_signal_handler (void *dummy);
*************** static void
*** 2000,2005 ****
--- 2001,2010 ----
  remote_open (char *name, int from_tty)
  {
    remote_open_1 (name, from_tty, &remote_ops, 0);
+ #ifdef SOLIB_CREATE_INFERIOR_HOOK
+   if (exec_bfd) 	/* no use without one... */
+     SOLIB_CREATE_INFERIOR_HOOK (inferior_pid);
+ #endif
  }
  
  /* Just like remote_open, but with asynchronous support. */
From kevinb@cygnus.com Tue Oct 31 16:36:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: Michael Snyder <msnyder@cygnus.com>, gdb-patches@sources.redhat.com
Subject: Re: [RFA]: Initialize solib search psth from environ variable
Date: Tue, 31 Oct 2000 16:36:00 -0000
Message-id: <1001101003556.ZM4496@ocotillo.lan>
References: <200010312209.OAA25598@cleaver.cygnus.com> <msnyder@cygnus.com>
X-SW-Source: 2000-10/msg00312.html
Content-length: 704

On Oct 31,  2:09pm, Michael Snyder wrote:

> This patch would check the environment variables SOLIB_SEARCH_PATH
> and SOLIB_ABSOLUTE_PREFIX once, at start-up, and use them to set the
> corresponding gdb state variables.
> 
> 2000-10-31  Michael Snyder  <msnyder@cleaver.cygnus.com>
> 
>         * solib.c (initialize_solib): Initialize solib_absolute_prefix
>         and solib_search_path from the environment variables
>         SOLIB_ABSOLUTE_PREFIX and SOLIB_SEARCH_PATH, if they're defined.

I think this one's okay too so long as we've all agreed that concept
of initializing bits of gdb state from environment variables is
acceptable.

(But again, Jim Blandy is the official maintainer...)

Kevin
From ac131313@cygnus.com Tue Oct 31 18:32:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: Michael Snyder <msnyder@cygnus.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] Call SOLIB_CREATE_INFERIOR_HOOK for remote targets.
Date: Tue, 31 Oct 2000 18:32:00 -0000
Message-id: <39FF7CDB.839743CD@cygnus.com>
References: <200011010033.QAA25682@cleaver.cygnus.com>
X-SW-Source: 2000-10/msg00314.html
Content-length: 1438

Michael Snyder wrote:
> 
> I want to add this call to remote_open, so that we can debug shared
> libraries on a remote target.  This works together with Kevin Buettner's
> recent changes to the solib code.
> 
> 2000-10-31  Michael Snyder  <msnyder@cleaver.cygnus.com>
> 
>         * remote.c (remote_open): Call SOLIB_CREATE_INFERIOR_HOOK
>         (if defined), so that shared libraries can be detected.

FYI, There are currently several remote opens so they would each needed
to be updated.

	Andrew

> Index: remote.c
> ===================================================================
> RCS file: /cvs/cvsfiles/devo/gdb/remote.c,v
> retrieving revision 1.290
> diff -c -3 -p -r1.290 remote.c
> *** remote.c    2000/10/25 12:55:39     1.290
> --- remote.c    2000/11/01 00:31:00
> ***************
> *** 51,56 ****
> --- 51,57 ----
> 
>   #include <signal.h>
>   #include "serial.h"
> + #include "gdbcore.h" /* for exec_bfd */
> 
>   /* Prototypes for local functions */
>   static void cleanup_sigint_signal_handler (void *dummy);
> *************** static void
> *** 2000,2005 ****
> --- 2001,2010 ----
>   remote_open (char *name, int from_tty)
>   {
>     remote_open_1 (name, from_tty, &remote_ops, 0);
> + #ifdef SOLIB_CREATE_INFERIOR_HOOK
> +   if (exec_bfd)       /* no use without one... */
> +     SOLIB_CREATE_INFERIOR_HOOK (inferior_pid);
> + #endif
>   }
> 
>   /* Just like remote_open, but with asynchronous support. */
From ac131313@cygnus.com Tue Oct 31 18:32:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: Michael Snyder <msnyder@cygnus.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA]: Initialize solib search psth from environ variable
Date: Tue, 31 Oct 2000 18:32:00 -0000
Message-id: <39FF65A0.6E019C53@cygnus.com>
References: <200010312209.OAA25598@cleaver.cygnus.com>
X-SW-Source: 2000-10/msg00313.html
Content-length: 2085

Michael Snyder wrote:
> 
> This patch would check the environment variables SOLIB_SEARCH_PATH
> and SOLIB_ABSOLUTE_PREFIX once, at start-up, and use them to set the
> corresponding gdb state variables.
> 
> 2000-10-31  Michael Snyder  <msnyder@cleaver.cygnus.com>
> 
>         * solib.c (initialize_solib): Initialize solib_absolute_prefix
>         and solib_search_path from the environment variables
>         SOLIB_ABSOLUTE_PREFIX and SOLIB_SEARCH_PATH, if they're defined.

As it currently stands, I don't like this patch.  It introduces (more)
obscure undocumented behavour into GDB and its CLI.

I think there first needs to be a clear policy on the question of should
a GDB (native or cross) take information from the host environment (1). 
Then, if the decision is yes, a clearly documented convention on how to
implement this behavour (nameing convention, semantics, ...).

My personal preference is to not add this feature.  I think that
.gdbinit files and explicit command line options/commands would be more
useful.  Having GDB silently pick things up from a host environment
(especially in a test situtation) will just make things more confusing.

	enjoy,
		Andrew

(1) I should note that a native GDB does take environment-variables from
the ``target'' environment.  It is just that, in a native situtation,
the distinction between host and target gets pretty blured.

> Index: solib.c
> ===================================================================
> RCS file: /cvs/cvsfiles/devo/gdb/solib.c,v
> retrieving revision 1.155
> diff -c -3 -p -r1.155 solib.c
> *** solib.c     2000/10/31 22:04:12     1.155
> --- solib.c     2000/10/31 22:08:17
> *************** This takes precedence over the environme
> *** 810,813 ****
> --- 810,819 ----
>                   &setlist),
>        &showlist);
> 
> +   /* check for solib path environment variables */
> +   if ((tmp = getenv ("SOLIB_ABSOLUTE_PREFIX")) != NULL)
> +     solib_absolute_prefix = strsave (tmp);
> +
> +   if ((tmp = getenv ("SOLIB_SEARCH_PATH")) != NULL)
> +     solib_search_path = strsave (tmp);
>   }
From cgf@redhat.com Tue Oct 31 20:31:00 2000
From: Christopher Faylor <cgf@redhat.com>
To: gdb-patches@sources.redhat.com
Subject: Re: [RFA]: Initialize solib search psth from environ variable
Date: Tue, 31 Oct 2000 20:31:00 -0000
Message-id: <20001031233120.G6215@redhat.com>
References: <200010312209.OAA25598@cleaver.cygnus.com> <39FF65A0.6E019C53@cygnus.com>
X-SW-Source: 2000-10/msg00315.html
Content-length: 1722

On Wed, Nov 01, 2000 at 11:36:48AM +1100, Andrew Cagney wrote:
>Michael Snyder wrote:
>> 
>> This patch would check the environment variables SOLIB_SEARCH_PATH
>> and SOLIB_ABSOLUTE_PREFIX once, at start-up, and use them to set the
>> corresponding gdb state variables.
>> 
>> 2000-10-31  Michael Snyder  <msnyder@cleaver.cygnus.com>
>> 
>>         * solib.c (initialize_solib): Initialize solib_absolute_prefix
>>         and solib_search_path from the environment variables
>>         SOLIB_ABSOLUTE_PREFIX and SOLIB_SEARCH_PATH, if they're defined.
>
>As it currently stands, I don't like this patch.  It introduces (more)
>obscure undocumented behavour into GDB and its CLI.
>
>I think there first needs to be a clear policy on the question of should
>a GDB (native or cross) take information from the host environment (1). 
>Then, if the decision is yes, a clearly documented convention on how to
>implement this behavour (nameing convention, semantics, ...).
>
>My personal preference is to not add this feature.  I think that
>.gdbinit files and explicit command line options/commands would be more
>useful.  Having GDB silently pick things up from a host environment
>(especially in a test situtation) will just make things more confusing.

I agree with this.  When I first started working on Cygwin, I pushed to
have all of the various environment variables eliminated in favor of
one environment variable called 'CYGWIN'.  I didn't want to engender
the overhead of reading a file in Cygwin's case, or I might have lobbied
for that, too.

If we do decide to use environment variables, how about just using one?
In that case we could have something like:

export GDB='solib_absolute_prefix solib_search_path'

cgf
From ac131313@cygnus.com Tue Oct 31 23:00:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: "Christopher G. Faylor" <cgf@cygnus.com>
Cc: GDB Patches <gdb-patches@sourceware.cygnus.com>
Subject: People can subscribe to gdb-prs via the web
Date: Tue, 31 Oct 2000 23:00:00 -0000
Message-id: <39FFBE2F.AE53AEB4@cygnus.com>
X-SW-Source: 2000-10/msg00316.html
Content-length: 320

FYI,

I've added gdb-prs to the list of subscribe options on the home page
(and after three attempts it appears to work :-)
http://sources.redhat.com/gdb/#mailing-lists

This doesn't answer the gdb-prs->gdb question though.

Once the fine tuning has finished you should definitly announce this on
gdb-announce.

	Andrew
From toddpw@best.com Wed Nov 01 00:11:00 2000
From: Todd Whitesel <toddpw@best.com>
To: Peter.Schauer@regent.e-technik.tu-muenchen.de (Peter.Schauer)
Cc: aoliva@redhat.com, Peter.Schauer@regent.e-technik.tu-muenchen.de, msnyder@redhat.com, kettenis@wins.uva.nl, gdb-patches@sourceware.cygnus.com, rfolden@cygnus.com, ebachalo@cygnus.com, jld@cygnus.com
Subject: Re: [DISCUSS]: cross-debugging shared libraries
Date: Wed, 01 Nov 2000 00:11:00 -0000
Message-id: <200011010809.AAA26460@shell17.ba.best.com>
References: <200010261009.MAA09821@reisser.regent.e-technik.tu-muenchen.de>
X-SW-Source: 2000-11/msg00000.html
Content-length: 317

> Looks better, but I still think that GDB should use getenv only if absolutely
> necessary.

Strongly Agree.

Especially when using one GDB to debug another, environment cross-talk can
be a real headache.

GCC_EXEC_PREFIX seems like a neat idea, but then cross-compilation happened.

Todd Whitesel
toddpw @ best.com
From kettenis@wins.uva.nl Wed Nov 01 01:12:00 2000
From: Mark Kettenis <kettenis@wins.uva.nl>
To: eliz@is.elta.co.il
Cc: john@Calva.COM, gdb-patches@sources.redhat.com
Subject: Re: Patch for gdb5.0; enable hardware watchpoints on UnixWare
Date: Wed, 01 Nov 2000 01:12:00 -0000
Message-id: <200011010912.eA19C1j05992@debye.wins.uva.nl>
References: <NCBBLMGKIKDGJMEOMNMEGEEOHEAA.john@Calva.COM> <200010311710.MAA22110@indy.delorie.com>
X-SW-Source: 2000-11/msg00001.html
Content-length: 1287

   Date: Tue, 31 Oct 2000 12:10:42 -0500 (EST)
   From: Eli Zaretskii <eliz@delorie.com>

   > > While I generalize the code, I will bring it up to date with the letter
   > > of Intel's manuals.
   > 
   > So you're going to have your own defines for DR_... and not use the
   > system ones?  I guess that was why your code didn't match up to what
   > I find in sys/debugreg.h on my system.

   I was unaware of sys/debugreg.h until now.  Is it present on all Unix
   x86-based systems?  If not, we will need some configury magic to
   handle this.

   DJGPP certainly doesn't have sys/debugreg.h, and I don't know how
   about Cygwin.

   What do people think about relative merits of using sys/debugreg.h vs
   having the definitions inside GDB (or both)?

Forget about sys/debugreg.h.  Some Linux systems have it, some don't.
And any dependency on target-specific header files is a PITA when
building a cross-debugger.  IMHO having definitions inside GDB that
match the Intel documentation is probably the best thing to do.  I
don't think that the layout of the debug registers differs between any
x86 targets.  Any differences in register numbering should be
addressed by the system-dependcent layer, and any native version of
that code could use sys/debugreg.h if it wants.

Mark
From eliz@delorie.com Wed Nov 01 01:27:00 2000
From: Eli Zaretskii <eliz@delorie.com>
To: msnyder@redhat.com
Cc: john@Calva.COM, Peter.Schauer@Regent.E-Technik.TU-Muenchen.DE, gdb-patches@sources.redhat.com
Subject: Re: Patch for gdb5.0; enable hardware watchpoints on UnixWare
Date: Wed, 01 Nov 2000 01:27:00 -0000
Message-id: <200011010924.EAA22677@indy.delorie.com>
References: <NCBBLMGKIKDGJMEOMNMEEEDMHEAA.john@Calva.COM> <200010310906.EAA21817@indy.delorie.com> <39FF1BA8.16E3@redhat.com>
X-SW-Source: 2000-11/msg00002.html
Content-length: 1159

> Date: Tue, 31 Oct 2000 11:21:12 -0800
> From: Michael Snyder <msnyder@redhat.com>
> 
> Eli Zaretskii wrote:
> > 
> > Note that I'm working on a unified watchpoint implementation for all
> > ia32 targets, based on the code written for the DJGPP port (see
> > go32-nat.c).  This implementation will allow to watch large areas (up
> > to 16 bytes) and debug register sharing via reference counts (required
> > for watching overlapping areas) on all ia32 platforms.
> > 
> > I am trying to make this happen for v5.1, so perhaps changes like this
> > one should be witheld and not committed, at least until we decide that
> > I'm not living up to my promises.
> 
> Is there a reason for them to be withheld?

I thought what I wrote was the reason.  But if you think it's not a
reason, or that it's not important enough, by all means go ahead and
commit the changes; that's why I said ``perhaps'' in my last sentence.

In case my wording wasn't clear: adding yet another subtly
incompatible watchpoint implementation for ia32 target might be
largely a wasted effort, since they all will have to be revisited
when/if the generalized code is finished and accepted.
From eliz@delorie.com Wed Nov 01 01:28:00 2000
From: Eli Zaretskii <eliz@delorie.com>
To: msnyder@redhat.com
Cc: john@Calva.COM, Peter.Schauer@Regent.E-Technik.TU-Muenchen.DE, gdb-patches@sources.redhat.com
Subject: Re: Patch for gdb5.0; enable hardware watchpoints on UnixWare
Date: Wed, 01 Nov 2000 01:28:00 -0000
Message-id: <200011010924.EAA22680@indy.delorie.com>
References: <NCBBLMGKIKDGJMEOMNMEEEDMHEAA.john@Calva.COM> <200010310906.EAA21817@indy.delorie.com> <39FF1BA8.16E3@redhat.com>
X-SW-Source: 2000-11/msg00003.html
Content-length: 3038

> Date: Tue, 31 Oct 2000 11:21:12 -0800
> From: Michael Snyder <msnyder@redhat.com>
> 
> Eli Zaretskii wrote:
> > 
> > Note that I'm working on a unified watchpoint implementation for all
> > ia32 targets, based on the code written for the DJGPP port (see
> > go32-nat.c).  This implementation will allow to watch large areas (up
> > to 16 bytes) and debug register sharing via reference counts (required
To: jtc@redback.com
CC: msalter@redhat.com, gdb-patches@sources.redhat.com
In-reply-to: <5mpukgli1x.fsf@jtc.redback.com>
Subject: Re: Remote watchpoint support.
Reply-to: Eli Zaretskii <eliz@is.elta.co.il>
References: <200010301416.e9UEG6m18981@deneb.localdomain> <5mg0leoyg6.fsf@jtc.redback.com> <200010310906.EAA21820@indy.delorie.com> <5mpukgli1x.fsf@jtc.redback.com>
--text follows this line--
> From: jtc@redback.com (J.T. Conklin)
> Date: 31 Oct 2000 15:14:34 -0800
> 
> Did I just hear an echo?  :-)

Yes.  Isn't it true that echoes are so reassuring? ;-)

> In this case, I don't think it would be that difficult to fix.  For a 
> first cut, I'd change the macro to:
> 
>         target_stopped_data_address(&addr)
> 
> It would return 0 if GDB didn't stop because of a watchpoint, and 1
> (and set addr) if it did.

That's what I had in mind.  I thought you saw some specific problems
to make such a change.

In addition, I think it might be a good idea for breakpoint.c to use
STOPPED_BY_WATCHPOINT in the loop where it tests whether any of the
watchpoints triggered.  Using target_stopped_data_address for that, as
GDB does now, is inefficient: since the target end doesn't know
whether GDB needs the address or not, it is forced to do lots of
redundant work interrogating the debug interface.  The target end
needs to find out, for each one of the watchpoints, whether or not it
triggered, because the target_stopped_data_address API doesn't include
the information about a specific watchpoint.  If you have several
active watchpoints, this tends to an O(n^2) behavior, since GDB itself
loops over all the watchpoints.

(I'm guessing that this API peculiarity dates back to the first
platforms where watchpoints became available, where watchpoints
watched a very large area.)

Is there any reason why GDB couldn't use STOPPED_BY_WATCHPOINT inside
bpstat_stop_status?
> > for watching overlapping areas) on all ia32 platforms.
> > 
> > I am trying to make this happen for v5.1, so perhaps changes like this
> > one should be witheld and not committed, at least until we decide that
> > I'm not living up to my promises.
> 
> Is there a reason for them to be withheld?

I thought what I wrote was the reason.  But if you think it's not a
reason, or that it's not important enough, by all means go ahead and
commit the changes; that's why I said ``perhaps'' in my last sentence.

In case my wording wasn't clear: adding yet another subtly
incompatible watchpoint implementation for ia32 target might be
largely a wasted effort, since they all will have to be revisited
when/if the generalized code is finished and accepted.
From eliz@delorie.com Wed Nov 01 01:51:00 2000
From: Eli Zaretskii <eliz@delorie.com>
To: jtc@redback.com
Cc: msalter@redhat.com, gdb-patches@sources.redhat.com
Subject: Re: Remote watchpoint support.
Date: Wed, 01 Nov 2000 01:51:00 -0000
Message-id: <200011010951.EAA22695@indy.delorie.com>
References: <200010301416.e9UEG6m18981@deneb.localdomain> <5mg0leoyg6.fsf@jtc.redback.com> <200010310906.EAA21820@indy.delorie.com> <5mpukgli1x.fsf@jtc.redback.com>
X-SW-Source: 2000-11/msg00004.html
Content-length: 1481

> From: jtc@redback.com (J.T. Conklin)
> Date: 31 Oct 2000 15:14:34 -0800
> 
> Did I just hear an echo?  :-)

Yes.  Isn't it true that echoes are so reassuring? ;-)

> In this case, I don't think it would be that difficult to fix.  For a 
> first cut, I'd change the macro to:
> 
>         target_stopped_data_address(&addr)
> 
> It would return 0 if GDB didn't stop because of a watchpoint, and 1
> (and set addr) if it did.

That's what I had in mind.  I thought you saw some specific problems
to make such a change.

In addition, I think it might be a good idea for breakpoint.c to use
STOPPED_BY_WATCHPOINT in the loop where it tests whether any of the
watchpoints triggered.  Using target_stopped_data_address for that, as
GDB does now, is inefficient: since the target end doesn't know
whether GDB needs the address or not, it is forced to do lots of
redundant work interrogating the debug interface.  The target end
needs to find out, for each one of the watchpoints, whether or not it
triggered, because the target_stopped_data_address API doesn't include
the information about a specific watchpoint.  If you have several
active watchpoints, this tends to an O(n^2) behavior, since GDB itself
loops over all the watchpoints.

(I'm guessing that this API peculiarity dates back to the first
platforms where watchpoints became available, where watchpoints
watched a very large area.)

Is there any reason why GDB couldn't use STOPPED_BY_WATCHPOINT inside
bpstat_stop_status?
From ac131313@cygnus.com Wed Nov 01 02:49:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: jtc@redback.com
Cc: Eli Zaretskii <eliz@is.elta.co.il>, msalter@redhat.com, gdb-patches@sources.redhat.com
Subject: Re: Remote watchpoint support.
Date: Wed, 01 Nov 2000 02:49:00 -0000
Message-id: <39FFF3F9.255AD722@cygnus.com>
References: <200010301416.e9UEG6m18981@deneb.localdomain> <5mg0leoyg6.fsf@jtc.redback.com> <200010310906.EAA21820@indy.delorie.com> <5mpukgli1x.fsf@jtc.redback.com>
X-SW-Source: 2000-11/msg00005.html
Content-length: 439

"J.T. Conklin" wrote:

> In this case, I don't think it would be that difficult to fix.  For a
> first cut, I'd change the macro to:
> 
>         target_stopped_data_address(&addr)
> 
> It would return 0 if GDB didn't stop because of a watchpoint, and 1
> (and set addr) if it did.

The trend has been towards (well ok, I've in gdbarch used):

	int target_stopped_data_address_p ();
	void target_stopped_data_address (..);

enjoy,
	Andrew
From ac131313@cygnus.com Wed Nov 01 03:05:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: jtc@redback.com
Cc: Michael Snyder <msnyder@redhat.com>, Mark Salter <msalter@redhat.com>, gdb-patches@sources.redhat.com
Subject: Re: Remote watchpoint support.
Date: Wed, 01 Nov 2000 03:05:00 -0000
Message-id: <39FFF7A3.92440071@cygnus.com>
References: <200010301416.e9UEG6m18981@deneb.localdomain> <39FDCF55.46BD@redhat.com> <5m66maoxdo.fsf@jtc.redback.com>
X-SW-Source: 2000-11/msg00006.html
Content-length: 1934

"J.T. Conklin" wrote:
> 
> >>>>> "Michael" == Michael Snyder <msnyder@redhat.com> writes:
> 
> >> Mark Salter wrote:
> >> I'm adding hw break/watchpoint support to a remote target. I'm using
> >> the Z-packet support that is in remote.c to install/remove the
> >> watchpoints and breakpoints. When a watchpoint is triggered by a
> >> read access, GDB needs a way to tell if the target stopped because
> >> of the watchpoint. This is done through a target specific function:
> 
> Michael> That really sounds like more of a gdb thing.  xxx-tdep.c
> Michael> should have the knowledge about the registers, though, not
> Michael> remote.c.
> 
> Well, that's one thing that Mark's change has going for it.  Like the
> "Z" packet, the proposed "qWaddr" packet knows nothing about how the
> target implements hardware break/watchpoints.  Those details are left
> up to the debug agent.  While it's probably processor debug registers,
> it could be an external device that speaks the remote debug protocol
> and talks to the target over BDM or some other on-chip-debugging tech-
> nology.

Yes.

There are two largely independant cases:

	o	gdb talking to a fully
		functioned target.

		Here the target is responsible for
		controling things like break/watch
		points and consequently is also
		responsible for returning indications
		that a break/watchpoint was reached.

		A remote target that supports Z packets
		is an example of this.  (only as
		Mark has pointed out, it has limited
		functionality)


	o	gdb talking to a minimal
		raw target.

		Here gdb implements the break/watch
		point functionality by directly
		accesing things like registers.

		x86 watchpoint registers is (?) an
		example of this one.

Before anyone tries to argue that one or the other is better, I think
they both are.  They solve different problems :-).  Now if only the
target vector was sufficiently clean to allow these to be stacked ....
:-)

	Andrew
From ac131313@cygnus.com Wed Nov 01 03:19:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: Kevin Buettner <kevinb@cygnus.com>
Cc: Michael Snyder <msnyder@cygnus.com>, gdb-patches@sources.redhat.com
Subject: Re: [RFA] Encapsulate method of searching for a solib.
Date: Wed, 01 Nov 2000 03:19:00 -0000
Message-id: <39FFFAD2.2357785@cygnus.com>
References: <200010312203.OAA25580@cleaver.cygnus.com> <1001101002807.ZM4469@ocotillo.lan>
X-SW-Source: 2000-11/msg00007.html
Content-length: 749

Kevin Buettner wrote:
> 
> On Oct 31,  2:03pm, Michael Snyder wrote:
> 
> >         * solib-svr4.h (solib_open): Export the declaration.
> 
> Woops.  Now that I stare at it a bit longer, I think that the
> exported declaration ought to go somewhere else.  I suggest
> solist.h.  (Actually, I think we ought to rename solist.h to
> solib-private.h.  The intent is that it should only be included
> by solib.c and the various solib backends.)

I'd suggest any thoughts of renaming the file to solib-private.h be
discussed separatly.  There are a number of other places in GDB which
may also benefit from a better separation of public/private interfaces. 
So if we're going to adopt a change we might as well think a little
first :-)

	enjoy,
		Andrew
From ac131313@cygnus.com Wed Nov 01 03:36:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: jtc@redback.com
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: [RFA]: Changes to move dcache above target vector functions.
Date: Wed, 01 Nov 2000 03:36:00 -0000
Message-id: <39FFFED9.226B5E4F@cygnus.com>
References: <5md7hjnd5v.fsf@jtc.redback.com>
X-SW-Source: 2000-11/msg00008.html
Content-length: 911

"J.T. Conklin" wrote:
> 
> Early last month, I trolled for comments about the enclosed patch.
> The only bite I got was about my suggestion that reset is a higher
> level concept and probably deserves a separate command / target
> function.
> 
> I'd seeking approval to commit the enclosed patch.  It's the same as
> the earlier one, except I "fixed" the problem with invalidating the
> dcache in ocd.c's bdm_reset_command().  This was fixed by accessing
> target_dcache in ocd.c.  This is an abstraction layer violation, but
> it seemed the most expedient way to handle the problem.  Support for a
> reset command would make this hack unnecessary.  I'm open to changing
> this if necessary.

This is certainly ok with me.  As they say two giant leaps forward and a
small step back.

Just mention the reset command issue (with a URL link) in the TODO file.

	Andrew

(Yes I did notice the pre-existing goto :-)
From ac131313@cygnus.com Wed Nov 01 03:53:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: Orjan Friberg <orjan.friberg@axis.com>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: Submission of a new gdb port
Date: Wed, 01 Nov 2000 03:53:00 -0000
Message-id: <3A0002D9.2BAD4903@cygnus.com>
References: <39D837F0.9E376923@axis.com>
X-SW-Source: 2000-11/msg00009.html
Content-length: 2866

Orjan Friberg wrote:
> 
> I am planning the submission of a new gdb port, and I would appreciate
> some guidance as to the best procedure to do this.
> 
> The port is for an embedded system. The current status of the code is
> that there are no deprecated macros, but I have not yet switched to the
> multi-arch framework. During development, I used the 2000-05-19-release
> source tree, but I just updated to the latest CVS sources.
> 
> * Will I be working with anyone in particular? Will the code be
> scrutinized in the same way patches are?

You'll need to work with a number of people (check the S390 posts). 
Some parts of a GDB release are part of GDB but some are not.  BFD, for
instance, is really part of binutils so changes to that go to the
binutils@sources.cygnus.com mailing list.  Top level changes are
discussed in src/MAINTAINERS.

In terms of process, I'd not follow IBM's lead with the s390 - they are
in a somewhat unusual situtation.  The 68hc11 is probably more typical
of what should happen - the basic thing was committed and then as second
and later passes various issues were resolved.

I'd be looking to breakdown your submittion in to smaller chunks and
then farming them out.  On GDB, once the basics such as file names have
been resolved, there can be a fairly large degree of flexability in now
things are integrated.

> * How stable is the multi-arch framework? Are there any catches
> switching to it? (Will multi-arching allow me to debug APIs that only
> differ in their respective sizes of a double?)

Definitly useable.  Many targets have basicly converted, and yes, it
lets you do things like change the size of a double (and a lot more
:-).  The MIPS target knows how to debug well over 30 different ISA/ABI
combinations....

> * Which currently supported target would be the best example for
> architectural issues? Already being multi-arched, I reckon that the d10v
> and the mn10300 are good candidates.

Yes.

> * Should I submit test results from the DejaGnu testsuite? Should I
> submit patches for XFAILs specific to my target?

People will be interested in testsuite results but, for the most part,
it is the responsibility of the target developer to keep track of such
things.

You should probably also note that an XFAIL doesn't mean ``I know this
doesn't work for my target''.  Rather it means something like
``technical issues outside of GDBs control make fixing this
impossible''.

> * The 4.18 port, on which the 5.0 port is heavily based, was developed
> by another person at my company (for which an assign.future has been
> submitted previously). How do I give proper credit to him?

If they authored the original files then probably mention them as part
of the copyright (along with your company and you :-).

> Thanks for bearing with me on this.

Not a problem,  sory to be so slow in responding.

	Andrew
From ac131313@cygnus.com Wed Nov 01 04:30:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: GDB Patches <gdb-patches@sourceware.cygnus.com>
Subject: [patch] Move h/w watchpoints to 5.2. + misc
Date: Wed, 01 Nov 2000 04:30:00 -0000
Message-id: <3A000B86.1A4E67BD@cygnus.com>
X-SW-Source: 2000-11/msg00010.html
Content-length: 3063

FYI,

Per my previous e-mail, the hardware watchpoint stuff that H.J. raised
is part of the 5.2 release criteria.  If it makes it in 5.1 then it is a
bonus.

	Andrew
Wed Nov  1 23:07:29 2000  Andrew Cagney  <cagney@b1.cygnus.com>

	* TODO: Document cleanup to function monitor.dumpregs.  Move
 	hardware watchpoint notes to 5.2.

Index: TODO
===================================================================
RCS file: /cvs/src/src/gdb/TODO,v
retrieving revision 1.51
diff -p -r1.51 TODO
*** TODO	2000/10/23 22:49:28	1.51
--- TODO	2000/11/01 12:26:40
*************** cycle.  People hope to have these proble
*** 12,37 ****
  
  --
  
- Hardware watchpint problems on x86 OSes, including Linux:
- 
- 1. Delete/disable hardware watchpoints should free hardware debug
- registers. 
- 2. Watch for different values on a viariable with one hardware debug
- register.
- 
- According to Eli Zaretskii <eliz@delorie.com>:
- 
- These are not GDB/ia32 issues per se: the above features are all
- implemented in the DJGPP port of GDB and work in v5.0.  Every
- x86-based target should be able to lift the relevant parts of
- go32-nat.c and use them almost verbatim.  You get debug register
- sharing through reference counts, and the ability to watch large
- regions (up to 16 bytes) using multiple registers.  (The required
- infrastructure in high-level GDB application code, mostly in
- breakpoint.c, is also working since v5.0.)
- 
- --
- 
  RFD: infrun.c: No bpstat_stop_status call after proceed over break?
  http://sourceware.cygnus.com/ml/gdb-patches/2000-q1/msg00665.html
  
--- 12,17 ----
*************** Fix at least one thread bug.
*** 256,261 ****
--- 236,264 ----
  
  --
  
+ Hardware watchpint [sic] problems on x86 OSes, including Linux:
+ http://sources.redhat.com/ml/gdb/2000-09/msg00005.html
+ 
+ H.J. Lu writes:
+ 
+ 1. Delete/disable hardware watchpoints should free hardware debug
+ registers. 
+ 2. Watch for different values on a viariable with one hardware debug
+ register.
+ 
+ According to Eli Zaretskii <eliz@delorie.com>:
+ 
+ These are not GDB/ia32 issues per se: the above features are all
+ implemented in the DJGPP port of GDB and work in v5.0.  Every
+ x86-based target should be able to lift the relevant parts of
+ go32-nat.c and use them almost verbatim.  You get debug register
+ sharing through reference counts, and the ability to watch large
+ regions (up to 16 bytes) using multiple registers.  (The required
+ infrastructure in high-level GDB application code, mostly in
+ breakpoint.c, is also working since v5.0.)
+ 
+ --
+ 
  		GDB 5.2 - New features
  		======================
  
*************** Rename read_register{,_pid}() to read_un
*** 801,806 ****
--- 804,813 ----
  
  --
  
+ Change monitor.h:struct monitor.dumpregs to a void function.
+ 
+ --
+ 
  			Symbol Support
  			==============
  
*************** an error (or is interrupted).
*** 1785,1790 ****
--- 1792,1799 ----
  
  Remove the range and type checking code and documentation, if not
  going to implement.
+ 
+ --
  
  # Local Variables:
  # mode: text
From brolley@redhat.com Wed Nov 01 08:13:00 2000
From: Dave Brolley <brolley@redhat.com>
To: gdb-patches@sources.redhat.com
Subject: Re: Patch:RFA:sim testsuite: Options Embedded in Test Cases
Date: Wed, 01 Nov 2000 08:13:00 -0000
Message-id: <3A004145.69155672@redhat.com>
References: <39FDE57F.A237E2A5@redhat.com>
X-SW-Source: 2000-11/msg00011.html
Content-length: 862

Approved by Frank Eigler and committed.

Dave

Dave Brolley wrote:
> 
> Hi,
> 
> The attached patch addresses two issues:
> 
> 1) Yet another attempt at passing the correct cpu to the assembler
> when more than one machine is being tested. This time I've defined a
> global variable 'cpu_option' which the caller can set to indicate the
> name of the option which specifies the cpu.
> 
> 2) Resetting options between test cases. Currently options from
> previous test cases are not cleared and sometimes they leak into
> subsequent tests. Specifically unsetting them seems to be the only way
> to make the "if [info exists <option>]" tests work properly.
> 
> Tested against m32r (which does not specify 'cpu_option', but has more
> than one machine) and two internal ports (one of which uses
> 'cpu_option' and has mutiple machines to test).
> 
> OK to commit?
From larry@smith-house.org Wed Nov 01 12:58:00 2000
From: Larry Smith <larry@smith-house.org>
To: gdb-patches@sources.redhat.com
Subject: [RFA] 100473 fix
Date: Wed, 01 Nov 2000 12:58:00 -0000
Message-id: <200011012058.PAA11799@ozma.smith-house.org>
X-SW-Source: 2000-11/msg00012.html
Content-length: 2936

The request was to permit wrapping of text in the console widget
rather then requiring the horizontal scrollbar.  Jim Ingham had
partially implemented this, but it was left unfinished and did
not work as advertised.  The real problem was, in order to enable
wrapping, one had to first tell the scrolledtext widget not to
enable horizontal scrolling.  Once that was done, the interior
text widget would obliging wrap text as desired.  This feature
was previously only available by editing the global prefs file,
I've added a checkbutton to the global prefs page to permit a
user to enable this feature with less hassle.

2000-11-01  Larry Smith  <lsmith@redhat.com>

        * console.itb (_build_win): Added code to remove horizontal
        scrollbar by setting -hscrollmode none, which enables the
        console widget to wrap text properly.
        * globalpref.itb (build_win): added checkbutton to enable
        wrapping text in the console window.

Index: console.itb
===================================================================
RCS file: /cvs/src/src/gdb/gdbtk/library/console.itb,v
retrieving revision 1.3
diff -u -r1.3 console.itb
--- console.itb	2000/04/14 14:45:37	1.3
+++ console.itb	2000/11/01 20:51:50
@@ -34,12 +34,19 @@
 }
   
 body Console::_build_win {} {
-  iwidgets::scrolledtext $itk_interior.stext -hscrollmode dynamic \
+  set wrap [pref get gdb/console/wrap]
+
+  if { $wrap } {
+    set hsm none
+  } else {
+    set hsm dynamic
+  }
+  iwidgets::scrolledtext $itk_interior.stext -hscrollmode $hsm \
     -vscrollmode dynamic -textbackground white
 
   set _twin [$itk_interior.stext component text]
 
-  if {[pref get gdb/console/wrap]} {
+  if {$wrap} {
     $_twin configure -wrap word
   } else {
     $_twin configure -wrap none
Index: globalpref.itb
===================================================================
RCS file: /cvs/src/src/gdb/gdbtk/library/globalpref.itb,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 globalpref.itb
--- globalpref.itb	2000/02/07 00:19:42	1.1.1.1
+++ globalpref.itb	2000/11/01 20:51:51
@@ -189,13 +189,20 @@
     pack $frame.use_icons.cb -pady 10 -side left -fill none 
   }
 
+  # console wrap
+  frame $frame.consolewrap
+  checkbutton $frame.consolewrap.cw -text "wrap text in console window" \
+    -variable [pref varname gdb/console/wrap]
+  pack $frame.consolewrap.cw -pady 10 -side left -fill none
+
   pack $frame.icons.lab $frame.icons.cb -side left
   pack $frame.icons -side top -padx 10 -pady 10
   pack $frame.tracing -side top -fill x -expand 0 -side bottom
   pack $frame.browser -side top -fill x -expand 0 -side bottom
   if {$tcl_platform(platform) == "unix"} {
-  pack $frame.use_icons -side top -fill x -expand 0 -side bottom
+    pack $frame.use_icons -side top -fill x -expand 0 -side bottom
   }
+  pack $frame.consolewrap -side top -fill x -expand 0 -side bottom
   pack $frame.d -side top -fill both -expand yes
 
   # make buttons
From fnasser@cygnus.com Wed Nov 01 13:08:00 2000
From: Fernando Nasser <fnasser@cygnus.com>
To: Larry Smith <larry@smith-house.org>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] 100473 fix
Date: Wed, 01 Nov 2000 13:08:00 -0000
Message-id: <3A008639.6C582889@cygnus.com>
References: <200011012058.PAA11799@ozma.smith-house.org>
X-SW-Source: 2000-11/msg00013.html
Content-length: 3313

Very nice!  Please check it in.

Fernando



Larry Smith wrote:
> 
> The request was to permit wrapping of text in the console widget
> rather then requiring the horizontal scrollbar.  Jim Ingham had
> partially implemented this, but it was left unfinished and did
> not work as advertised.  The real problem was, in order to enable
> wrapping, one had to first tell the scrolledtext widget not to
> enable horizontal scrolling.  Once that was done, the interior
> text widget would obliging wrap text as desired.  This feature
> was previously only available by editing the global prefs file,
> I've added a checkbutton to the global prefs page to permit a
> user to enable this feature with less hassle.
> 
> 2000-11-01  Larry Smith  <lsmith@redhat.com>
> 
>         * console.itb (_build_win): Added code to remove horizontal
>         scrollbar by setting -hscrollmode none, which enables the
>         console widget to wrap text properly.
>         * globalpref.itb (build_win): added checkbutton to enable
>         wrapping text in the console window.
> 
> Index: console.itb
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/library/console.itb,v
> retrieving revision 1.3
> diff -u -r1.3 console.itb
> --- console.itb 2000/04/14 14:45:37     1.3
> +++ console.itb 2000/11/01 20:51:50
> @@ -34,12 +34,19 @@
>  }
> 
>  body Console::_build_win {} {
> -  iwidgets::scrolledtext $itk_interior.stext -hscrollmode dynamic \
> +  set wrap [pref get gdb/console/wrap]
> +
> +  if { $wrap } {
> +    set hsm none
> +  } else {
> +    set hsm dynamic
> +  }
> +  iwidgets::scrolledtext $itk_interior.stext -hscrollmode $hsm \
>      -vscrollmode dynamic -textbackground white
> 
>    set _twin [$itk_interior.stext component text]
> 
> -  if {[pref get gdb/console/wrap]} {
> +  if {$wrap} {
>      $_twin configure -wrap word
>    } else {
>      $_twin configure -wrap none
> Index: globalpref.itb
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/library/globalpref.itb,v
> retrieving revision 1.1.1.1
> diff -u -r1.1.1.1 globalpref.itb
> --- globalpref.itb      2000/02/07 00:19:42     1.1.1.1
> +++ globalpref.itb      2000/11/01 20:51:51
> @@ -189,13 +189,20 @@
>      pack $frame.use_icons.cb -pady 10 -side left -fill none
>    }
> 
> +  # console wrap
> +  frame $frame.consolewrap
> +  checkbutton $frame.consolewrap.cw -text "wrap text in console window" \
> +    -variable [pref varname gdb/console/wrap]
> +  pack $frame.consolewrap.cw -pady 10 -side left -fill none
> +
>    pack $frame.icons.lab $frame.icons.cb -side left
>    pack $frame.icons -side top -padx 10 -pady 10
>    pack $frame.tracing -side top -fill x -expand 0 -side bottom
>    pack $frame.browser -side top -fill x -expand 0 -side bottom
>    if {$tcl_platform(platform) == "unix"} {
> -  pack $frame.use_icons -side top -fill x -expand 0 -side bottom
> +    pack $frame.use_icons -side top -fill x -expand 0 -side bottom
>    }
> +  pack $frame.consolewrap -side top -fill x -expand 0 -side bottom
>    pack $frame.d -side top -fill both -expand yes
> 
>    # make buttons

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9
From msalter@redhat.com Wed Nov 01 13:29:00 2000
From: Mark Salter <msalter@redhat.com>
To: gdb-patches@sources.redhat.com
Subject: remote watchpoint support
Date: Wed, 01 Nov 2000 13:29:00 -0000
Message-id: <200011012131.eA1LVa514840@deneb.localdomain>
X-SW-Source: 2000-11/msg00014.html
Content-length: 8643

Ok. Here's a patch that allows a remote target to pass watchpoint
information back to gdb using the T packet. It also fixes the T
packet handling to ignore other optional info which may not be
recognized. This changes the behavior to match the documentation.

While testing this, I discovered that GDB tries to read data from
the watched address before the watchpoints are removed. This causes
another watchpoint exception so the stub returns an error response
to the read memory request.

--Mark


Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.744
diff -p -r1.744 ChangeLog
*** ChangeLog	2000/10/31 19:35:03	1.744
--- ChangeLog	2000/11/01 21:06:26
***************
*** 1,3 ****
--- 1,10 ----
+ 2000-11-01  Mark Salter  <msalter@redhat.com>
+ 
+ 	* remote.c (remote_wait): Add support to extract optional
+ 	watchpoint information from T-packet. Ignore unrecognized
+ 	optional info in T-packet.
+ 	(remote_async_wait): Ditto.
+ 
  2000-10-30  Michael Snyder  <msnyder@cleaver.cygnus.com>
  
  	* config/sh/tm-linux.h: New file.  Include generic tm-linux.h, 
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.26
diff -p -r1.26 remote.c
*** remote.c	2000/10/23 22:49:28	1.26
--- remote.c	2000/11/01 21:06:29
*************** void _initialize_remote (void);
*** 204,209 ****
--- 204,216 ----
  
  /* */
  
+ /* This is set to the data address of the access causing the target
+  * to stop for a watchpoint. */
+ static CORE_ADDR remote_watch_data_address;
+ 
+ /* This is non-zero if taregt stopped for a watchpoint. */
+ static int remote_stopped_by_watchpoint_p;
+ 
  static struct target_ops remote_ops;
  
  static struct target_ops extended_remote_ops;
*************** struct gdb_ext_thread_info
*** 963,969 ****
  
  #define BUF_THREAD_ID_SIZE (OPAQUETHREADBYTES*2)
  
! char *unpack_varlen_hex (char *buff, int *result);
  
  static char *unpack_nibble (char *buf, int *val);
  
--- 970,976 ----
  
  #define BUF_THREAD_ID_SIZE (OPAQUETHREADBYTES*2)
  
! char *unpack_varlen_hex (char *buff, ULONGEST *result);
  
  static char *unpack_nibble (char *buf, int *val);
  
*************** stub_unpack_int (char *buff, int fieldle
*** 1084,1090 ****
  
  char *
  unpack_varlen_hex (char *buff,	/* packet to parse */
! 		   int *result)
  {
    int nibble;
    int retval = 0;
--- 1091,1097 ----
  
  char *
  unpack_varlen_hex (char *buff,	/* packet to parse */
! 		   ULONGEST *result)
  {
    int nibble;
    int retval = 0;
*************** remote_wait (int pid, struct target_wait
*** 2593,2598 ****
--- 2600,2606 ----
  {
    unsigned char *buf = alloca (PBUFSIZ);
    int thread_num = -1;
+   ULONGEST ul;
  
    status->kind = TARGET_WAITKIND_EXITED;
    status->value.integer = 0;
*************** remote_wait (int pid, struct target_wait
*** 2610,2615 ****
--- 2618,2625 ----
        if (target_wait_loop_hook)
  	(*target_wait_loop_hook) ();
  
+       remote_stopped_by_watchpoint_p = 0;
+ 
        switch (buf[0])
  	{
  	case 'E':		/* Error of some sort */
*************** Packet: '%s'\n",
*** 2647,2656 ****
  			       p, buf);
  		    if (strncmp ((const char *) p, "thread", p1 - p) == 0)
  		      {
! 			p_temp = unpack_varlen_hex (++p1, &thread_num);
  			record_currthread (thread_num);
  			p = (unsigned char *) p_temp;
  		      }
  		  }
  		else
  		  {
--- 2657,2682 ----
  			       p, buf);
  		    if (strncmp ((const char *) p, "thread", p1 - p) == 0)
  		      {
! 			p_temp = unpack_varlen_hex (++p1, &ul);
! 			thread_num = ul;
  			record_currthread (thread_num);
  			p = (unsigned char *) p_temp;
  		      }
+ 		    else if ((strncmp ((const char *) p, "watch", p1 - p) == 0)
+ 			     || (strncmp ((const char *) p, "rwatch", p1 - p) == 0)
+ 			     || (strncmp ((const char *) p, "awatch", p1 - p) == 0))
+ 		      {
+ 			remote_stopped_by_watchpoint_p = 1;
+ 			p = unpack_varlen_hex (++p1, &ul);
+ 			remote_watch_data_address = (CORE_ADDR)ul;
+ 		      }
+ 		    else
+ 		      {
+ 			/* silently skip unknown optional info */
+ 			p_temp = (unsigned char *) strchr ((const char *) p1+1, ';');
+ 			if (p_temp)
+ 			  p = p_temp;
+ 		      }
  		  }
  		else
  		  {
*************** remote_async_wait (int pid, struct targe
*** 2808,2813 ****
--- 2834,2840 ----
  {
    unsigned char *buf = alloca (PBUFSIZ);
    int thread_num = -1;
+   ULONGEST ul;
  
    status->kind = TARGET_WAITKIND_EXITED;
    status->value.integer = 0;
*************** remote_async_wait (int pid, struct targe
*** 2831,2836 ****
--- 2858,2865 ----
        if (target_wait_loop_hook)
  	(*target_wait_loop_hook) ();
  
+       remote_stopped_by_watchpoint_p = 0;
+ 
        switch (buf[0])
  	{
  	case 'E':		/* Error of some sort */
*************** Packet: '%s'\n",
*** 2868,2877 ****
  			       p, buf);
  		    if (strncmp ((const char *) p, "thread", p1 - p) == 0)
  		      {
! 			p_temp = unpack_varlen_hex (++p1, &thread_num);
  			record_currthread (thread_num);
  			p = (unsigned char *) p_temp;
  		      }
  		  }
  		else
  		  {
--- 2897,2922 ----
  			       p, buf);
  		    if (strncmp ((const char *) p, "thread", p1 - p) == 0)
  		      {
! 			p_temp = unpack_varlen_hex (++p1, &ul);
! 			thread_num = ul;
  			record_currthread (thread_num);
  			p = (unsigned char *) p_temp;
  		      }
+ 		    else if ((strncmp ((const char *) p, "watch", p1 - p) == 0)
+ 			     || (strncmp ((const char *) p, "rwatch", p1 - p) == 0)
+ 			     || (strncmp ((const char *) p, "awatch", p1 - p) == 0))
+ 		      {
+ 			remote_stopped_by_watchpoint_p = 1;
+ 			p = unpack_varlen_hex (++p1, &ul);
+ 			remote_watch_data_address = (CORE_ADDR)ul;
+ 		      }
+ 		    else
+ 		      {
+ 			/* silently skip unknown optional info */
+ 			p_temp = (unsigned char *) strchr ((const char *) p1+1, ';');
+ 			if (p_temp)
+ 			  p = p_temp;
+ 		      }
  		  }
  		else
  		  {
*************** remote_remove_hw_breakpoint (CORE_ADDR a
*** 4484,4489 ****
--- 4529,4551 ----
      }
    internal_error ("remote_remove_watchpoint: reached end of function");
  }
+ 
+ /* Returns data address if stopped by watchpoint.
+    Zero otherwise. */
+ CORE_ADDR
+ remote_stopped_data_address (void)
+ {
+     if (remote_stopped_by_watchpoint_p)
+ 	return remote_watch_data_address;
+     return (CORE_ADDR)0;
+ }
+ 
+ int
+ remote_stopped_by_watchpoint(void)
+ {
+     return remote_stopped_by_watchpoint_p;
+ }
+ 
  
  /* Some targets are only capable of doing downloads, and afterwards
     they switch to the remote serial protocol.  This function provides
Index: doc/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/doc/ChangeLog,v
retrieving revision 1.51
diff -p -r1.51 ChangeLog
*** ChangeLog	2000/10/16 07:34:02	1.51
--- ChangeLog	2000/11/01 21:06:31
***************
*** 1,3 ****
--- 1,8 ----
+ 2000-11-01  Mark Salter  <msalter@redhat.com>
+ 
+ 	* gdb.texinfo (Protocol): Document T packet extension to 
+ 	allow watchpoint address reporting.
+ 
  2000-10-16  Eli Zaretskii  <eliz@is.elta.co.il>
  
  	* gdb.texinfo (Contributors, MIPS Embedded): Minor spelling
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.28
diff -p -r1.28 gdb.texinfo
*** gdb.texinfo	2000/10/16 07:34:02	1.28
--- gdb.texinfo	2000/11/01 21:06:40
*************** conventions is used.
*** 9390,9399 ****
  @var{AA} = two hex digit signal number; @var{n...} = register number
  (hex), @var{r...}  = target byte ordered register contents, size defined
  by @code{REGISTER_RAW_SIZE}; @var{n...} = @samp{thread}, @var{r...} =
! thread process ID, this is a hex integer; @var{n...} = other string not
! starting with valid hex digit.  @value{GDBN} should ignore this
! @var{n...}, @var{r...} pair and go on to the next.  This way we can
! extend the protocol.
  
  @item @code{W}@var{AA}
  @tab
--- 9390,9400 ----
  @var{AA} = two hex digit signal number; @var{n...} = register number
  (hex), @var{r...}  = target byte ordered register contents, size defined
  by @code{REGISTER_RAW_SIZE}; @var{n...} = @samp{thread}, @var{r...} =
! thread process ID, this is a hex integer; @var{n...} = (@samp{watch} | 
! @samp{rwatch} | @samp{awatch}, @var{r...} = data address, this is a hex
! integer; @var{n...} = other string not starting with valid hex digit.
! @value{GDBN} should ignore this @var{n...}, @var{r...} pair and go on
! to the next.  This way we can extend the protocol.
  
  @item @code{W}@var{AA}
  @tab
From jezra@emc.com Wed Nov 01 13:34:00 2000
From: "Josef Ezra" <jezra@emc.com>
To: <gdb-patches@sourceware.cygnus.com>
Cc: <shagam@emc.com>, <sgordon@emc.com>
Subject: patch suggestion: conditional collect command
Date: Wed, 01 Nov 2000 13:34:00 -0000
Message-id: <009301c0444b$a5565b70$6c219fa8@lss.emc.com>
X-SW-Source: 2000-11/msg00015.html
Content-length: 22918

(had some mail problems, hoping it won't be sent more then once)

 This patch should allow conditional collect
expressions in tracepoints. The format is:

> collect expr2 if expr3
or
> collect expr1, expr2 if expr3, expr4 ....

in both cases, the expression expr2 will be collected
only if expr3 turned out to be true.

NOTE 1: if this patch will be accepted, the collect
        command's legend should be updated.
NOTE 2: targets should protect themselves from infinite
        loops.
NOTE 3: in gen_binop, I thought that pointer/integer
        operations are too powerful to be disallowed,
        fixme if I'm wrong.

Thanks -
Josef Ezra


*ax.h: (ax_append_const) new function
*ax-general.c: ditto
*ax-gdb.h: (gen_trace_for_expr) additional 'if' parameter
*ax-gdb.c: (require_rvalue) minor bug fixed
*ax-gdb.c: (gen_binop) allow ptr/int operation
*ax-gdb.c: (gen_expr) added compare operations (EQUAL, GTR etc.)
*ax-gdb.c: (gen_trace_for_expr) additional 'if' parameter, create
conditional trace
*ax-gdb.c: (agent_command) manipulate 'expr if expr' expresstions
*tracepoint.c: (validate_actionline) allow 'collect expr if expr' commands
*tracepoint.c: (encode_actions) ditto
*tracepoint.c: (trace_dump_command) outputing conditional traces




Index: ax-gdb.h
===================================================================
RCS file: /emc/ucode/cvsroot_0/sgdb6/gdb/ax-gdb.h,v
retrieving revision 1.1
diff -c -5 -p -r1.1 ax-gdb.h
*** ax-gdb.h 2000/09/07 16:34:30 1.1
--- ax-gdb.h 2000/10/26 21:07:55
*************** extern struct agent_expr *expr_to_addres
*** 107,114 ****
     The result will use the `trace' and `trace_quick' bytecodes to
     record the value of all memory touched by the expression, and leave
     no values on the stack.  The caller can then use the ax_reqs
     function to discover which registers the expression uses.  */
  extern struct agent_expr *gen_trace_for_expr PARAMS ((CORE_ADDR,
!       struct expression *));

  #endif /* AX_GDB_H */
--- 107,123 ----
     The result will use the `trace' and `trace_quick' bytecodes to
     record the value of all memory touched by the expression, and leave
     no values on the stack.  The caller can then use the ax_reqs
     function to discover which registers the expression uses.  */
  extern struct agent_expr *gen_trace_for_expr PARAMS ((CORE_ADDR,
!       struct expression *,
!                                                       struct expression
*));
!

  #endif /* AX_GDB_H */
Index: ax.h
===================================================================
RCS file: /emc/ucode/cvsroot_0/sgdb6/gdb/ax.h,v
retrieving revision 1.1
retrieving revision 1.2
diff -c -5 -p -r1.1 -r1.2
*** ax.h 2000/09/07 16:34:30 1.1
--- ax.h 2000/10/19 13:33:25 1.2
*************** extern int ax_goto PARAMS ((struct agent
*** 174,183 ****
--- 180,193 ----
  extern void ax_label PARAMS ((struct agent_expr * EXPR, int patch, int
target));

  /* Assemble code to push a constant on the stack.  */
  extern void ax_const_l PARAMS ((struct agent_expr * EXPR, LONGEST l));
  extern void ax_const_d PARAMS ((struct agent_expr * EXPR, LONGEST d));
+
+
+ /* just add n low bytes of val to expression chain - jezra EMC */
+ extern void ax_append_const PARAMS ((struct agent_expr * x, unsigned long
val, int n));

  /* Assemble code to push the value of register number REG on the
     stack.  */
  extern void ax_reg PARAMS ((struct agent_expr * EXPR, int REG));


Index: ax-gdb.c
===================================================================
RCS file: /emc/ucode/cvsroot_0/sgdb6/gdb/ax-gdb.c,v
retrieving revision 1.1
diff -c -5 -p -r1.1 ax-gdb.c
*** ax-gdb.c 2000/09/07 16:34:30 1.1
--- ax-gdb.c 2000/11/01 15:27:06
*************** require_rvalue (ax, value)
*** 697,707 ****
        /* It's already an rvalue.  */
        break;

      case axs_lvalue_memory:
        /* The top of stack is the address of the object.  Dereference.  */
!       gen_fetch (ax, value->type);
        break;

      case axs_lvalue_register:
        /* There's nothing on the stack, but value->u.reg is the
           register number containing the value.
--- 697,707 ----
        /* It's already an rvalue.  */
        break;

      case axs_lvalue_memory:
        /* The top of stack is the address of the object.  Dereference.  */
!       gen_fetch (ax, CHECK_TYPEDEF( value->type ));
        break;

      case axs_lvalue_register:
        /* There's nothing on the stack, but value->u.reg is the
           register number containing the value.
*************** gen_binop (ax, value, value1, value2, op
*** 1106,1117 ****
       enum agent_op op, op_unsigned;
       int may_carry;
       char *name;
  {
    /* We only handle INT op INT.  */
!   if ((value1->type->code != TYPE_CODE_INT)
!       || (value2->type->code != TYPE_CODE_INT))
      error ("Illegal combination of types in %s.", name);

    ax_simple (ax,
       TYPE_UNSIGNED (value1->type) ? op_unsigned : op);
    if (may_carry)
--- 1106,1120 ----
       enum agent_op op, op_unsigned;
       int may_carry;
       char *name;
  {
    /* We only handle INT op INT.  */
!   /* FIXME: mmm.., ptr/int operations are too powerful to be disabled */
!   if (((value1->type->code != TYPE_CODE_INT) &&
!        (value1->type->code != TYPE_CODE_PTR)) ||
!       ((value2->type->code != TYPE_CODE_INT) &&
!        (value2->type->code != TYPE_CODE_PTR)))
      error ("Illegal combination of types in %s.", name);

    ax_simple (ax,
       TYPE_UNSIGNED (value1->type) ? op_unsigned : op);
    if (may_carry)
*************** gen_expr (pc, ax, value)
*** 1565,1574 ****
--- 1569,1586 ----
    }

    /* Otherwise, go ahead and generate code for it.  */
    switch (op)
      {
+       /* Binary Comparation operators */
+     case BINOP_EQUAL: /* == */
+     case BINOP_NOTEQUAL: /* != */
+     case BINOP_LESS: /* <  */
+     case BINOP_GTR: /* >  */
+     case BINOP_LEQ: /* <= */
+     case BINOP_GEQ:        /* >= */
+
        /* Binary arithmetic operators.  */
      case BINOP_ADD:
      case BINOP_SUB:
      case BINOP_MUL:
      case BINOP_DIV:
*************** gen_expr (pc, ax, value)
*** 1583,1592 ****
--- 1595,1627 ----
        gen_expr (pc, ax, &value2);
        gen_usual_unary (ax, &value2);
        gen_usual_arithmetic (ax, &value1, &value2);
        switch (op)
  {
+         case BINOP_EQUAL: /* == */
+           gen_binop (ax, value, &value1, &value2,
+                      aop_equal, aop_equal, 0, "isequal");
+           break;
+         case BINOP_NOTEQUAL: /* != */
+           gen_binop (ax, value, &value1, &value2,
+                      aop_equal, aop_equal, 0, "isequal");
+           ax_simple (ax, aop_log_not);
+           break;
+         case BINOP_GTR: /* >  */
+           ax_simple (ax, aop_swap);
+         case BINOP_LESS: /* <  */
+           gen_binop (ax, value, &value1, &value2,
+                      aop_less_signed, aop_less_unsigned, 1, "isless");
+           break;
+         case BINOP_LEQ: /* <= */
+           ax_simple (ax, aop_swap);
+         case BINOP_GEQ:        /* >= */
+           gen_binop (ax, value, &value1, &value2,
+                      aop_less_signed, aop_less_unsigned, 1, "isless");
+           ax_simple (ax, aop_log_not);
+           break;
+
  case BINOP_ADD:
    gen_add (ax, value, &value1, &value2, "addition");
    break;
  case BINOP_SUB:
    gen_sub (ax, value, &value1, &value2);
*************** expr_to_address_and_size (expr)
*** 1843,1865 ****
     The result will use the `trace' and `trace_quick' bytecodes to
     record the value of all memory touched by the expression.  The
     caller can then use the ax_reqs function to discover which
     registers it relies upon.  */
  struct agent_expr *
! gen_trace_for_expr (scope, expr)
       CORE_ADDR scope;
       struct expression *expr;
  {
    struct cleanup *old_chain = 0;
    struct agent_expr *ax = new_agent_expr (scope);
    union exp_element *pc;
    struct axs_value value;

    old_chain = make_cleanup ((make_cleanup_func) free_agent_expr, ax);

-   pc = expr->elts;
    trace_kludge = 1;
    gen_expr (&pc, ax, &value);

    /* Make sure we record the final object, and get rid of it.  */
    gen_traced_pop (ax, &value);

--- 1878,1912 ----
     The result will use the `trace' and `trace_quick' bytecodes to
     record the value of all memory touched by the expression.  The
     caller can then use the ax_reqs function to discover which
     registers it relies upon.  */
  struct agent_expr *
! gen_trace_for_expr (scope, expr, ifexpr)
       CORE_ADDR scope;
       struct expression *expr;
+      struct expression *ifexpr;
  {
    struct cleanup *old_chain = 0;
    struct agent_expr *ax = new_agent_expr (scope);
    union exp_element *pc;
    struct axs_value value;

    old_chain = make_cleanup ((make_cleanup_func) free_agent_expr, ax);

    trace_kludge = 1;
+   if (ifexpr != NULL)
+     {
+       pc = ifexpr->elts;
+       gen_expr (&pc, ax, &value) ;
+       require_rvalue (ax, &value) ;
+       ax_simple (ax, aop_if_goto) ;
+       ax_append_const (ax, (ax->len + 3), 2);
+       ax_simple (ax, aop_end) ;
+       /*                    >>==========>>
+          condition_expr, if_goto, end, collection_expr      */
+     }
+   pc = expr->elts;
    gen_expr (&pc, ax, &value);

    /* Make sure we record the final object, and get rid of it.  */
    gen_traced_pop (ax, &value);

*************** agent_command (exp, from_tty)
*** 1909,1918 ****
--- 2019,2030 ----
  {
    struct cleanup *old_chain = 0;
    struct expression *expr;
    struct agent_expr *agent;
    struct frame_info *fi = get_current_frame (); /* need current scope */
+   struct expression *ifexpr = NULL;
+   char *ifexp ;

    /* We don't deal with overlay debugging at the moment.  We need to
       think more carefully about this.  If you copy this code into
       another command, change the error message; the user shouldn't
       have to know anything about agent expressions.  */
*************** agent_command (exp, from_tty)
*** 1920,1932 ****
      error ("GDB can't do agent expression translation with overlays.");

    if (exp == 0)
      error_no_arg ("expression to translate");

    expr = parse_expression (exp);
    old_chain = make_cleanup ((make_cleanup_func) free_current_contents,
&expr);
!   agent = gen_trace_for_expr (fi->pc, expr);
    make_cleanup ((make_cleanup_func) free_agent_expr, agent);
    ax_print (gdb_stdout, agent);

    /* It would be nice to call ax_reqs here to gather some general info
       about the expression, and then print out the result.  */
--- 2032,2052 ----
      error ("GDB can't do agent expression translation with overlays.");

    if (exp == 0)
      error_no_arg ("expression to translate");

+   if ((ifexp = strstr (exp, " if ")) != NULL)   /* is it an if command?
*/
+     {
+       ifexpr = parse_expression (ifexp + 4);
+       *ifexp = '\0';
+     }
    expr = parse_expression (exp);
    old_chain = make_cleanup ((make_cleanup_func) free_current_contents,
&expr);
!   if (ifexpr)
!     old_chain = make_cleanup ((make_cleanup_func) free_current_contents,
&ifexpr);
!
!   agent = gen_trace_for_expr (fi->pc, expr, ifexpr);
    make_cleanup ((make_cleanup_func) free_agent_expr, agent);
    ax_print (gdb_stdout, agent);

    /* It would be nice to call ax_reqs here to gather some general info
       about the expression, and then print out the result.  */
Index: ax-general.c
===================================================================
RCS file: /emc/ucode/cvsroot_0/sgdb6/gdb/ax-general.c,v
retrieving revision 1.1
diff -c -5 -p -r1.1 ax-general.c
*** ax-general.c 2000/09/07 16:34:30 1.1
--- ax-general.c 2000/11/01 14:28:05
*************** grow_expr (x, n)
*** 78,87 ****
--- 78,96 ----
        x->buf = xrealloc (x->buf, x->size);
      }
  }


+ void
+ ax_append_const (x, val, n)
+      struct agent_expr *x;
+      unsigned long val;
+      int n;
+ {
+   append_const (x, (LONGEST) val, n);
+ }
+
  /* Append the low N bytes of VAL as an N-byte integer to the
     expression X, in big-endian order.  */
  static void
  append_const (x, val, n)
       struct agent_expr *x;
Index: tracepoint.c
===================================================================
RCS file: /emc/ucode/cvsroot_0/sgdb6/gdb/tracepoint.c,v
retrieving revision 1.1
diff -c -5 -p -r1.1 tracepoint.c
*** tracepoint.c 2000/09/07 16:35:13 1.1
--- tracepoint.c 2000/11/01 16:59:41
*************** validate_actionline (line, t)
*** 932,942 ****
--- 995,1007 ----
       char **line;
       struct tracepoint *t;
  {
    struct cmd_list_element *c;
    struct expression *exp = NULL;
+   struct expression *ifexp = NULL;
    struct cleanup *old_chain = NULL;
+
    char *p;

    for (p = *line; isspace ((int) *p);)
      p++;

*************** validate_actionline (line, t)
*** 967,986 ****

    if (*p == '$') /* look for special pseudo-symbols */
      {
        if ((0 == strncasecmp ("reg", p + 1, 3)) ||
    (0 == strncasecmp ("arg", p + 1, 3)) ||
!   (0 == strncasecmp ("loc", p + 1, 3)))
  {
    p = strchr (p, ',');
    continue;
  }
        /* else fall thru, treat p as an expression and parse it! */
      }
    exp = parse_exp_1 (&p, block_for_pc (t->address), 1);
    old_chain = make_cleanup ((make_cleanup_func) free_current_contents,
      &exp);

    if (exp->elts[0].opcode == OP_VAR_VALUE)
      {
        if (SYMBOL_CLASS (exp->elts[2].symbol) == LOC_CONST)
  {
--- 1032,1100 ----

    if (*p == '$') /* look for special pseudo-symbols */
      {
        if ((0 == strncasecmp ("reg", p + 1, 3)) ||
    (0 == strncasecmp ("arg", p + 1, 3)) ||
!   (0 == strncasecmp ("loc", p + 1, 3)))
  {
    p = strchr (p, ',');
    continue;
  }
        /* else fall thru, treat p as an expression and parse it! */
      }
    exp = parse_exp_1 (&p, block_for_pc (t->address), 1);
+           if (*p)
+             {
+               while (isspace ((int) *p)) p ++;
+               if ( p[0] == 'i' &&
+                    p[1] == 'f' &&
+                    p[2] == ' ')
+                 {                                       /* this is an if
command  */
+                   p += 3;
+                   ifexp = parse_exp_1 (&p, block_for_pc (t->address), 1);
+                 }
+             }
+
    old_chain = make_cleanup ((make_cleanup_func) free_current_contents,
      &exp);
+           if (ifexp)
+             old_chain = make_cleanup ((make_cleanup_func)
free_current_contents,
+     &ifexp);

    if (exp->elts[0].opcode == OP_VAR_VALUE)
      {
        if (SYMBOL_CLASS (exp->elts[2].symbol) == LOC_CONST)
  {
*************** validate_actionline (line, t)
*** 997,1007 ****
  }
      }

    /* we have something to collect, make sure that the expr to
       bytecode translator can handle it and that it's not too long */
!   aexpr = gen_trace_for_expr (t->address, exp);
    (void) make_cleanup ((make_cleanup_func) free_agent_expr, aexpr);

    if (aexpr->len > MAX_AGENT_EXPR_LEN)
      error ("expression too complicated, try simplifying");

--- 1111,1121 ----
  }
      }

    /* we have something to collect, make sure that the expr to
       bytecode translator can handle it and that it's not too long */
!   aexpr = gen_trace_for_expr (t->address, exp, ifexp);
    (void) make_cleanup ((make_cleanup_func) free_agent_expr, aexpr);

    if (aexpr->len > MAX_AGENT_EXPR_LEN)
      error ("expression too complicated, try simplifying");

*************** encode_actions (t, tdp_actions, stepping
*** 1514,1532 ****
       char ***stepping_actions;
  {
    static char tdp_buff[2048], step_buff[2048];
    char *action_exp;
    struct expression *exp = NULL;
    struct action_line *action;
    int i;
    value_ptr tempval;
    struct collection_list *collect;
    struct cmd_list_element *cmd;
    struct agent_expr *aexpr;
    long frame_reg, frame_offset;

-
    clear_collection_list (&tracepoint_list);
    clear_collection_list (&stepping_list);
    collect = &tracepoint_list;

    *tdp_actions = NULL;
--- 1629,1647 ----
       char ***stepping_actions;
  {
    static char tdp_buff[2048], step_buff[2048];
    char *action_exp;
    struct expression *exp = NULL;
+   struct expression *ifexp = NULL;
    struct action_line *action;
    int i;
    value_ptr tempval;
    struct collection_list *collect;
    struct cmd_list_element *cmd;
    struct agent_expr *aexpr;
    long frame_reg, frame_offset;

    clear_collection_list (&tracepoint_list);
    clear_collection_list (&stepping_list);
    collect = &tracepoint_list;

    *tdp_actions = NULL;
*************** encode_actions (t, tdp_actions, stepping
*** 1540,1550 ****
        action_exp = action->action;
        while (isspace ((int) *action_exp))
  action_exp++;

        if (*action_exp == '#') /* comment line */
! return;

        cmd = lookup_cmd (&action_exp, cmdlist, "", -1, 1);
        if (cmd == 0)
  error ("Bad action list item: %s", action_exp);

--- 1655,1665 ----
        action_exp = action->action;
        while (isspace ((int) *action_exp))
  action_exp++;

        if (*action_exp == '#') /* comment line */
! continue ;

        cmd = lookup_cmd (&action_exp, cmdlist, "", -1, 1);
        if (cmd == 0)
  error ("Bad action list item: %s", action_exp);

*************** encode_actions (t, tdp_actions, stepping
*** 1582,1625 ****
  }
        else
  {
    unsigned long addr, len;
    struct cleanup *old_chain = NULL;
!   struct cleanup *old_chain1 = NULL;
    struct agent_reqs areqs;
!
    exp = parse_exp_1 (&action_exp, block_for_pc (t->address), 1);
    old_chain = make_cleanup ((make_cleanup_func)
      free_current_contents, &exp);

!   switch (exp->elts[0].opcode)
!     {
!     case OP_REGISTER:
        i = exp->elts[1].longconst;
        if (info_verbose)
  printf_filtered ("OP_REGISTER: ");
        add_register (collect, i);
!       break;

!     case UNOP_MEMVAL:
        /* safe because we know it's a simple expression */
        tempval = evaluate_expression (exp);
        addr = VALUE_ADDRESS (tempval) + VALUE_OFFSET (tempval);
        len = TYPE_LENGTH (check_typedef (exp->elts[1].type));
        add_memrange (collect, -1, addr, len);
!       break;

!     case OP_VAR_VALUE:
        collect_symbol (collect,
        exp->elts[2].symbol,
        frame_reg,
        frame_offset);
!       break;

!     default: /* full-fledged expression */
!       aexpr = gen_trace_for_expr (t->address, exp);

        old_chain1 = make_cleanup ((make_cleanup_func)
  free_agent_expr, aexpr);

        ax_reqs (aexpr, &areqs);
        if (areqs.flaw != agent_flaw_none)
--- 1697,1804 ----
  }
        else
  {
    unsigned long addr, len;
    struct cleanup *old_chain = NULL;
!     struct cleanup *old_chain1 = NULL;
    struct agent_reqs areqs;
    exp = parse_exp_1 (&action_exp, block_for_pc (t->address), 1);
+                   if (*action_exp)                              /* is it
an if command?  */
+                     {
+                       while (isspace ((int) *action_exp)) action_exp ++;
+                       if ( action_exp[0] == 'i' &&
+                            action_exp[1] == 'f' &&
+                            action_exp[2] == ' ')
+                         {                                       /* this is
an if command  */
+                           action_exp += 3;
+                           ifexp = parse_exp_1 (&action_exp, block_for_pc
(t->address), 1);
+                         }
+                     }
+
    old_chain = make_cleanup ((make_cleanup_func)
      free_current_contents, &exp);
+                   if (ifexp)
+                     old_chain = make_cleanup ((make_cleanup_func)
+                                               free_current_contents,
&ifexp);

!                   if (!ifexp  && exp->elts[0].opcode == OP_REGISTER)
!                     {
        i = exp->elts[1].longconst;
        if (info_verbose)
  printf_filtered ("OP_REGISTER: ");
        add_register (collect, i);
!                     }

!                   else if (!ifexp && exp->elts[0].opcode == UNOP_MEMVAL)
!     {
        /* safe because we know it's a simple expression */
        tempval = evaluate_expression (exp);
        addr = VALUE_ADDRESS (tempval) + VALUE_OFFSET (tempval);
        len = TYPE_LENGTH (check_typedef (exp->elts[1].type));
        add_memrange (collect, -1, addr, len);
!                     }

!                   else if (!ifexp && exp->elts[0].opcode == OP_VAR_VALUE)
!     {
        collect_symbol (collect,
        exp->elts[2].symbol,
        frame_reg,
        frame_offset);
!                     }

!                   else /* full-fledged expression */
!                     {
+                     aexpr = gen_trace_for_expr (t->address, exp, ifexp);
+
        old_chain1 = make_cleanup ((make_cleanup_func)
  free_agent_expr, aexpr);

        ax_reqs (aexpr, &areqs);
        if (areqs.flaw != agent_flaw_none)
*************** encode_actions (t, tdp_actions, stepping
*** 1650,1664 ****
        /* it's used -- record it */
        add_register (collect, ndx1 * 8 + ndx2);
  }
      }
  }
!       break;
!     } /* switch */
    do_cleanups (old_chain);
! } /* do */
!     }
    while (action_exp && *action_exp++ == ',');
  } /* if */
        else if (cmd->function.cfunc == while_stepping_pseudocommand)
  {
    collect = &stepping_list;
--- 1829,1842 ----
        /* it's used -- record it */
        add_register (collect, ndx1 * 8 + ndx2);
  }
      }
  }
!     }
    do_cleanups (old_chain);
! }
!     }   /* do */
    while (action_exp && *action_exp++ == ',');
  } /* if */
        else if (cmd->function.cfunc == while_stepping_pseudocommand)
  {
    collect = &stepping_list;
*************** trace_dump_command (args, from_tty)
*** 2536,2546 ****
       char *args;
       int from_tty;
  {
    struct tracepoint *t;
    struct action_line *action;
!   char *action_exp, *next_comma;
    struct cleanup *old_cleanups;
    int stepping_actions = 0;
    int stepping_frame = 0;

    if (!target_is_remote ())
--- 2722,2732 ----
       char *args;
       int from_tty;
  {
    struct tracepoint *t;
    struct action_line *action;
!   char *action_exp, *next_comma, *ifexpr;
    struct cleanup *old_cleanups;
    int stepping_actions = 0;
    int stepping_frame = 0;

    if (!target_is_remote ())
*************** trace_dump_command (args, from_tty)
*** 2627,2639 ****
--- 2814,2858 ----
        if (next_comma)
  {
    make_cleanup (replace_comma, next_comma);
    *next_comma = '\0';
  }
+                                 /* always try to show conditional traces
*/
+                       if ((ifexpr = strstr (action_exp, " if ")) != NULL)
+                         *ifexpr = '\0' ;
        printf_filtered ("%s = ", action_exp);
        output_command (action_exp, from_tty);
        printf_filtered ("\n");
+                       if (ifexpr != NULL)
+                         *ifexpr = ' ' ;
      }
    if (next_comma)
      *next_comma = ',';
    action_exp = next_comma;
  }



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2000-10-31  9:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <NCBBLMGKIKDGJMEOMNMEEEDMHEAA.john@Calva.COM>
     [not found] ` <NCBBLMGKIKDGJMEOMNMEMEELHEAA.john@Calva.COM>
2000-10-31  4:32   ` Patch for gdb5.0; enable hardware watchpoints on UnixWare Eli Zaretskii
2000-10-31  6:31     ` John Hughes
2000-10-31  9:10       ` Eli Zaretskii

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox