Re: That dwarf2read patch i just submitted

[Kevin Buettner <kevinb@cygnus.com>]

On Oct 23, 12:55pm, Daniel Berlin wrote:

> Kevin Buettner <kevinb@cygnus.com> writes:
> 
> > On Oct 23, 10:02am, Daniel Berlin wrote:
> > 
> > > So no one feels slighted, that last dwarf2 patch I submitted (to add
> > > support for .debug_loc) is based on a patch I found on gdb-patches
> > > that is a few years old, submitted originally by one of the ADA guys.
> > > I cleaned it up so that it works with dwarf2read of today, but if you
> > > want credit, email me, and i'll put your name in the ChangeLog
> > > instead.
> > 
> > Dan,
> > 
> > You should definitely give credit to the original submitter of the
> > patch (whether they step forward or not).  Also, you should make sure
> > that we have a copyright assignment on file for whomever submitted
> > that patch.
> 
> The problem is I can't seem to find it in the archives anymore using
> the search engine, or else I would have. I really don't want to go
> back month by month, trying to find the patch again.

It looks to me like it was

    http://sources.redhat.com/ml/gdb-patches/1999-q2/msg00054.html

The author of this patch is Brian Nettleton.

> The patch is actually <30 lines of code, and thus, shouldn't require
> copyright assignment.

I'm not sure what the metric is for requiring a copyright assignment,
but I thought it was lower than 30.  (Perhaps Andrew could let us know?)

Anyway, you're off by an order of magnitude.  If I'm looking at the
right patch, it's actually 322 lines long.  The number of lines
affected by this patch is also greater than 30.  The locblock function
and corresponding comment alone are 39 lines.

> I also modified every line of the patch, literally.

It is true that you modified many of the lines, but you didn't touch
*all* of them.

E.g, with the exception of the comment (which you split to be on two
lines), the following section from Brian's patch appears to be identical
to what you submitted:

+ {
+   char *loc_ptr;
+   struct dwarf_block *result;
+ 
+   switch (attr->form)
+     {
+     case DW_FORM_block:
+     case DW_FORM_block1:
+     case DW_FORM_block2:
+     case DW_FORM_block4:
+       return DW_BLOCK(attr);
+     case DW_FORM_ref_addr:
+     case DW_FORM_ref_udata:
+       /* return the first block in the location list for now */
+       loc_ptr = dwarf_loc_buffer + dwarf2_get_ref_die_offset(attr);

Even if you had modified every line from Brian's patch, he should
still be credited for the changes.  It's a maintainer's job to adapt a
patch so that it works with current sources.  Adapting a patch that is
several years old may well require substantial modification, but the
original submitter should still be given credit.  (And if he's to be
given credit, we must then observe the FSF's mandates regarding
copyright assignments.)

Kevin
From ac131313@cygnus.com Mon Oct 23 10:59:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: David B Anderson <davea@quasar.engr.sgi.com>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: spelling corrections: request approval
Date: Mon, 23 Oct 2000 10:59:00 -0000
Message-id: <39F47B09.1F8A7C23@cygnus.com>
References: <200010141539.IAA32359@quasar.engr.sgi.com>
X-SW-Source: 2000-10/msg00151.html
Content-length: 757

David B Anderson wrote:
> 
> Requesting approval for checkin of the following
> spelling corrections in comments.
> 
> My last request had too many different maintainers
> in one spelling correction list, so no one person wanted
> to approve it (is my interpretation/guess).
> And, too, the set of corrections was, well, large.
> 
> So I'm trying to break this down sensibly into separate
> requests.

Rather than require that you get approval for each of these cleanups, I
see no reason why you shouldn't follow the strategy that KevinB has
adopted for PARAMS.  Post the RFA/RFC for the cleanups and then, a day
or so later, just commit the patch.

Baring the occasional *colourful* comment, I can't see any problems :-)

	thanks for doing this,

		Andrew
From msnyder@redhat.com Mon Oct 23 11:03:00 2000
From: Michael Snyder <msnyder@redhat.com>
To: josef ezra <jezra@emc.com>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: tracepoints implementation: bug in byte code generating.
Date: Mon, 23 Oct 2000 11:03:00 -0000
Message-id: <39F47D62.2B5B@redhat.com>
References: <00c801c02c9a$fe671c90$6c219fa8@lss.emc.com> <39D8D1CE.3059@redhat.com> <004e01c039d8$774c1d50$6c219fa8@lss.emc.com> <39EF3020.3F6A@redhat.com> <005b01c039f6$f46b1390$6c219fa8@lss.emc.com> <39EF381C.7EE6@redhat.com> <006101c039f9$df09df60$6c219fa8@lss.emc.com> <5mvguo5spm.fsf@jtc.redback.com> <008501c03aad$46c9d700$6c219fa8@lss.emc.com> <39F090F3.2DDC@redhat.com> <009001c03adb$b46a7b60$6c219fa8@lss.emc.com> <39F0BF1B.74DD@redhat.com> <003d01c03b0e$42568040$961919ac@lss.emc.com>
X-SW-Source: 2000-10/msg00152.html
Content-length: 5987

> 
> > Since this discussion now involves only tracepoints,
> > after this reply I will continue it in private email
> > unies someone on the list pipes up and expresses an
> > interest in keeping it public.  OK with you Josef?
> >

Putting gdb-patches back IN the Cc: list, at one reader's
request.  Oh well...  ;-)

> > Josef Ezra wrote:
> > >
> > > Let me try to list the features that I would like to have, if you think
> that
> > > other tracepoints clients may be interested, I will be more then happy
> to
> > > cooperate.
> > >
> > > -In actions:
> > > -- Trace/ref different memory ranges.
> >
> > Not sure what you mean by the above.
> 
> As I mentioned before, our target is working with two memory spaces: regular
> and shared. It is very common to have a pointer in regular memory that
> points to a structure in shared memory which one of it's field is a pointer
> to different structure in shared memory which I would like to trace.
> Something like:
> -  Special-trace (from other memory space) X bytes in offset Y from:
>      - Special-trace-ref (size is pointer size) in offset Z from:
>            - My-regular-memory-pointer

If we add support for separate memory spaces to GDB, 
then I would favor adding it to the traceing syntax
as well.  Otherwise not.  I don't want to start throwing
a bunch of stuff into trace that would only be supported
on this target or that one.  And if GDB can't support
multi-address-spaces, then you wouldn't have a standard
way to display your results.

The whole strength of the tracing implementation,
in my opinion, is that it uses the same expression
syntax as the rest of GDB.  Therefore whatever data
you collect can be used by the rest of GDB without
modification.


> > > -- Stop all traces on condition.
> >
> > That would be useful, sure.
> >
> > > -- Save back trace functions chain. (I can't save all the stack,
> > > but I can save a limited array of function addresses)
> >
> > Tracepoints can already do that.  I do it regularly in demos.
> > All you need to do is define a memory range relative to the
> > stack pointer register.  Capture all that memory, and you can
> > do backtraces, examine the local variables of your caller,
> > and pretty much everything else you would expect.
> >
> This might me perfect for most targets, but in our environment there are big
> stack gaps every time C function is calling assembly function and vice
> versa. Dedicating most of the trace buffer to save the stack, I can't back
> trace more then two or three functions. I considered that as a way to
> compress backtrace data (paying the price of losing information).

Then just collect two chunks of memory instead of one.
Trace can already do that, it doesn't need a special 
extension.  If we start putting in everybody's idea 
of an extension that benefits only one target, we're
gonna end up with a mess.

There's always gonna be a limit to how much stack you can
collect.  You only have so much room in your collection
buffer.  If you want to be able to trace back more than
a few stack levels, it will of course cost you, no matter
what kind of standard or non-standard stack you have.

> > > -In output:
> > > --Show/set memory from different memory space - (already suggested by
> J.T.C)
> >
> > Fine, but not related to traceipoints.
> >
> > > --Display (if traced) the functions backtrace array.
> >
> > Can already do that.  See above.
> >
> > > -I also need a user interface for conditional trace commands. The Stub
> > > specification has provisions for aop_goto but I could not find a way to
> > > access this through GDB.
> >
> > If you mean something like:
> > if (a == b) then collect c
> > yes, we'd like to have that.
> >
> > If you mean something like
> > if (a == b) then activate tracepoint N
> > then yes, we'd like to have that too.
> 
> Well, I had meant the first 'cause it can be done with the existing byte
> code, but having the other one could be nice too.
> 
> >
> > > -As a reply to "QT_Frame" I would like to get the passcount with the
> frame
> >
> > Hitcount, I assume you mean.  Passcount is something different.
> > Yes, I'm sure we'd like that too.
> >
> > With all of these, however, we would have to talk about
> > the syntax for the remote protocol.  We would not want
> > new syntax to break old existing stubs or debuggers.
> >
> > > and tracepoint numbers.
> >
> > Mmmmm... do we need that?  The frame info ALWAYS contains
> > the PC, and the PC would generally uniquely identify a
> > tracepoint.
> >
> 
> I guess I haven't thought enough about this one. How a different probed able
> command that will return list of tracepoints with their hitcounts?  Would it
> be too noisy?

You can do that now, I think.  Well, you could, if you just
had the hitcounts.  It's just a matter of processing the data
once you get it.  You can either write gdb scripts or tcl/tk
code to produce whatever kind of report or display you  want.

> > > -In the future I will like to have a trace/ref of long address with
> > > base-offset parameters.
> >
> > Have to explain to me what you mean.
> 
> Oh', it is just a special-trace-64-bits and special-trace-ref-64-bits
> commands for letting the target know it has to use bigger address. (in my
> case it means the target should pop two items from the stack instead of
> one). At the gdb side the user should be able to determine separately the
> base and the offset address.

I don't understand -- is this another reference to your separate
address spaces?  If the target architecture supports long pointers
and short pointers, then GDB should already know about that
(although I can't think of an architecture for which this is true.)

I think you need to make this concept into something that can
be supported in the C expression language.  Trace should 
support anything that can be expressed as a C expression.

> > > thanks for your comments - Josef Ezra
> >
> > No problem.  Happy that someone else is working on it.
> >
> > Michael
> >
> Josef
From jezra@emc.com Mon Oct 23 13:00:00 2000
From: "Josef Ezra" <jezra@emc.com>
To: "Michael Snyder" <msnyder@redhat.com>
Cc: <gdb-patches@sourceware.cygnus.com>, <shagam@emc.com>, <sgordon@emc.com>
Subject: Re: tracepoints implementation: bug in byte code generating.
Date: Mon, 23 Oct 2000 13:00:00 -0000
Message-id: <004001c03d2c$14209970$6c219fa8@lss.emc.com>
References: <00c801c02c9a$fe671c90$6c219fa8@lss.emc.com> <39D8D1CE.3059@redhat.com> <004e01c039d8$774c1d50$6c219fa8@lss.emc.com> <39EF3020.3F6A@redhat.com> <005b01c039f6$f46b1390$6c219fa8@lss.emc.com> <39EF381C.7EE6@redhat.com> <006101c039f9$df09df60$6c219fa8@lss.emc.com> <5mvguo5spm.fsf@jtc.redback.com> <008501c03aad$46c9d700$6c219fa8@lss.emc.com> <39F090F3.2DDC@redhat.com> <009001c03adb$b46a7b60$6c219fa8@lss.emc.com> <39F0BF1B.74DD@redhat.com> <003d01c03b0e$42568040$961919ac@lss.emc.com> <39F47D62.2B5B@redhat.com>
X-SW-Source: 2000-10/msg00153.html
Content-length: 5934

> > > > -In actions:
> > > > -- Trace/ref different memory ranges.
> > >
> > > Not sure what you mean by the above.
> >
> > As I mentioned before, our target is working with two memory spaces:
regular
> > and shared. It is very common to have a pointer in regular memory that
> > points to a structure in shared memory which one of it's field is a
pointer
> > to different structure in shared memory which I would like to trace.
> > Something like:
> > -  Special-trace (from other memory space) X bytes in offset Y from:
> >      - Special-trace-ref (size is pointer size) in offset Z from:
> >            - My-regular-memory-pointer
>
> If we add support for separate memory spaces to GDB,
> then I would favor adding it to the traceing syntax
> as well.  Otherwise not.  I don't want to start throwing
> a bunch of stuff into trace that would only be supported
> on this target or that one.  And if GDB can't support
> multi-address-spaces, then you wouldn't have a standard
> way to display your results.
>
> The whole strength of the tracing implementation,
> in my opinion, is that it uses the same expression
> syntax as the rest of GDB.  Therefore whatever data
> you collect can be used by the rest of GDB without
> modification.
>

What is the appropriate generic interface to trace and show other memory
spaces? I can use a good advice here.

> > > > -- Stop all traces on condition.
> > >
> > > That would be useful, sure.
> > >
> > > > -- Save back trace functions chain. (I can't save all the stack,
> > > > but I can save a limited array of function addresses)
> > >
> > > Tracepoints can already do that.  I do it regularly in demos.
> > > All you need to do is define a memory range relative to the
> > > stack pointer register.  Capture all that memory, and you can
> > > do backtraces, examine the local variables of your caller,
> > > and pretty much everything else you would expect.
> > >
> > This might me perfect for most targets, but in our environment there are
big
> > stack gaps every time C function is calling assembly function and vice
> > versa. Dedicating most of the trace buffer to save the stack, I can't
back
> > trace more then two or three functions. I considered that as a way to
> > compress backtrace data (paying the price of losing information).
>
> Then just collect two chunks of memory instead of one.
> Trace can already do that, it doesn't need a special
> extension.  If we start putting in everybody's idea
> of an extension that benefits only one target, we're
> gonna end up with a mess.
>
> There's always gonna be a limit to how much stack you can
> collect.  You only have so much room in your collection
> buffer.  If you want to be able to trace back more than
> a few stack levels, it will of course cost you, no matter
> what kind of standard or non-standard stack you have.
>

I withdraw this suggestion.

> > > > -In output:
> > > > --Show/set memory from different memory space - (already suggested
by
> > J.T.C)
> > >
> > > Fine, but not related to traceipoints.
> > >
> > > > --Display (if traced) the functions backtrace array.
> > >
> > > Can already do that.  See above.
> > >
> > > > -I also need a user interface for conditional trace commands. The
Stub
> > > > specification has provisions for aop_goto but I could not find a way
to
> > > > access this through GDB.
> > >
> > > If you mean something like:
> > > if (a == b) then collect c
> > > yes, we'd like to have that.
> > >
> > > If you mean something like
> > > if (a == b) then activate tracepoint N
> > > then yes, we'd like to have that too.
> >
> > Well, I had meant the first 'cause it can be done with the existing byte
> > code, but having the other one could be nice too.
> >
> > >
> > > > -As a reply to "QT_Frame" I would like to get the passcount with the
> > frame
> > >
> > > Hitcount, I assume you mean.  Passcount is something different.
> > > Yes, I'm sure we'd like that too.
> > >
> > > With all of these, however, we would have to talk about
> > > the syntax for the remote protocol.  We would not want
> > > new syntax to break old existing stubs or debuggers.
> > >
> > > > and tracepoint numbers.
> > >
> > > Mmmmm... do we need that?  The frame info ALWAYS contains
> > > the PC, and the PC would generally uniquely identify a
> > > tracepoint.
> > >
> >
> > I guess I haven't thought enough about this one. How a different probed
able
> > command that will return list of tracepoints with their hitcounts?
Would it
> > be too noisy?
>
> You can do that now, I think.  Well, you could, if you just
> had the hitcounts.  It's just a matter of processing the data
> once you get it.  You can either write gdb scripts or tcl/tk
> code to produce whatever kind of report or display you  want.

Do you mean  by walking through every frame? Wouldn't that be too clumsy?

> > > > -In the future I will like to have a trace/ref of long address with
> > > > base-offset parameters.
> > >
> > > Have to explain to me what you mean.
> >
> > Oh', it is just a special-trace-64-bits and special-trace-ref-64-bits
> > commands for letting the target know it has to use bigger address. (in
my
> > case it means the target should pop two items from the stack instead of
> > one). At the gdb side the user should be able to determine separately
the
> > base and the offset address.
>
> I don't understand -- is this another reference to your separate
> address spaces?  If the target architecture supports long pointers
> and short pointers, then GDB should already know about that
> (although I can't think of an architecture for which this is true.)
>
> I think you need to make this concept into something that can
> be supported in the C expression language.  Trace should
> support anything that can be expressed as a C expression.
>

I withdraw this suggestion too (unless others are facing the need to capture
64 bits pointers (to another memory space) in 32 bits architecture)

- Josef Ezra