Re: Proposal: convert function definitions to prototyped form

[Mark Kettenis <kettenis@wins.uva.nl>]

   Date: Fri, 2 Jun 2000 00:50:19 -0700
   From: Kevin Buettner <kevinb@cygnus.com>

   Comma separated list with differing number of stars on the parameter
   names (sparc-tdep.c):

    static branch_type
   -isbranch (instruction, addr, target)
   -     long instruction;
   -     CORE_ADDR addr, *target;
   +isbranch (long instruction, CORE_ADDR addr, CORE_ADDR * target)
    {

I guess you should tweak it some more such that it outputs

   CORE_ADDR *target

instead of

   CORE_ADDR * target

(note the spurious space between * and target).

Mark
From kevinb@cygnus.com Fri Jun 02 08:16:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: Mark Kettenis <kettenis@wins.uva.nl>
Cc: gdb@sourceware.cygnus.com
Subject: Re: Proposal: convert function definitions to prototyped form
Date: Fri, 02 Jun 2000 08:16:00 -0000
Message-id: <1000602151553.ZM30578@ocotillo.lan>
References: <1000602075018.ZM29997@ocotillo.lan> <200006021226.e52CQ2I01239@delius.kettenis.local> <kettenis@wins.uva.nl>
X-SW-Source: 2000-06/msg00016.html
Content-length: 997

On Jun 2,  2:26pm, Mark Kettenis wrote:

>    Date: Fri, 2 Jun 2000 00:50:19 -0700
>    From: Kevin Buettner <kevinb@cygnus.com>
> 
>    Comma separated list with differing number of stars on the parameter
>    names (sparc-tdep.c):
> 
>     static branch_type
>    -isbranch (instruction, addr, target)
>    -     long instruction;
>    -     CORE_ADDR addr, *target;
>    +isbranch (long instruction, CORE_ADDR addr, CORE_ADDR * target)
>     {
> 
> I guess you should tweak it some more such that it outputs
> 
>    CORE_ADDR *target
> 
> instead of
> 
>    CORE_ADDR * target
> 
> (note the spurious space between * and target).

I noticed that.  The space was put there by ``indent''.  I would very
much like to get rid of that space and it would be easy to make the
perl script postprocess the ``indent'' output.  But in doing so, we
(obviously) generate different output than that of ``indent''.

I suppose the other solution is to fix indent.  :-)

FYI, I'm using GNU indent 2.2.5.

Kevin
From taylor@cygnus.com Fri Jun 02 08:40:00 2000
From: David Taylor <taylor@cygnus.com>
To: Kevin Buettner <kevinb@cygnus.com>
Cc: gdb@sourceware.cygnus.com
Subject: Re: Proposal: convert function definitions to prototyped form
Date: Fri, 02 Jun 2000 08:40:00 -0000
Message-id: <200006021539.LAA25912@texas.cygnus.com>
X-SW-Source: 2000-06/msg00017.html
Content-length: 4312

    Date: Fri, 2 Jun 2000 00:50:19 -0700
    From: Kevin Buettner <kevinb@cygnus.com>
[...]
	2) ``protoize'' fails to convert functions disabled by ifdefs for
	   the given platform.  OTOH, on some other platform(s), these
	   functions might not be disabled and would be converted.  E.g,
	   in breakpoint.c, on my GNU/Linux/x86 machine, protoize failed
	   to convert create_longjmp_breakpoint() which is protected by an
	   "#ifdef GET_LONGJMP_TARGET".

With any solution, either you're going to have to check the results or
you're going to have to hand convert some of the stuff.  It's a
tradeoff.  From what you say below, your script also requires some
later hand conversions.

Does your script produce substantially less that needs to be hand
converted?

	3) ``protoize'' screws up on PTR types.  E.g, in breakpoint.c, it
	   converted

		static int
		cover_target_enable_exception_callback (arg)
		     PTR arg;

	   to

		static int
		cover_target_enable_exception_callback (__builtin_va_list arg)

I consider this to be a more serious problem than the other complaints
you have.

	4) ``protoize'' does not reformat long argument lists.  The lists
	   end up entirely contained on one line.

So... adopt the same solution for it that you chose to adopt for your
script -- run indent on the output!

    For more information on protoize, see the "Running protoize" page:

	http://gcc.gnu.org/onlinedocs/gcc_2.html#SEC48

    and the "Caveats of using protoize" page:

	http://gcc.gnu.org/onlinedocs/gcc_7.html#SEC135

I've used protoize before with good results.  It was a fairly
substantial project, though not as big as gdb.

    Two of my goals in creating the scripts for the PARAMS purging
    activities was that the scripts should be 1) easy to invoke and 2)
    require no hand editing of the output when done.  I.e, you shouldn't
    have to edit any of the files that these scripts touch in order to fix

I trust that "3)" is:

    "be conservative; not convert something if it can't be sure
    of getting it right".

I'd much rather hand convert more of the code than have it make a
subtle but incorrect change.

    errors so that you can build again.  OTOH, the script may leave
    certain portions of the file alone that could possibly have be
    converted had the script been a little smarter.  The things that the
    script fails to convert *will* have to be fixed later on (in order to
    complete the cleanup activity), either by another script, or by hand. 
    For the PARAMS purging activity, I have spent a fair amount of time
    examining the diffs to make sure that this is indeed the case.  (And
    I intend to do the same for the activity in this proposal.)

Good.

    The reason that it is so important to avoid any hand edits is that we
    want people who have local repositories or checked out source trees to
    be able to run these conversion scripts against them so that merges
    will be less painful.  (Even easy.)

Agreed.

    With that said, keeping in mind the problems I noted above, I conclude
    that ``protoize'' is not the appropriate tool for us to use to convert
    function definitions in the gdb sources to their prototyped form.

I only consider 3) -- screwing up on PTR types -- to be serious; the
others seem minor enough.
[...]

    Finally, I should note that I've done a test build on GNU/Linux/x86
    and had no build problems, nor did I see any regressions when I ran
    the testsuite.

    The fix-decls script is below.  I'd be interested in finding out if
    anyone else has a favorite script for doing this sort of thing.  Other
    comments welcome too.

I'd be tempted to do a build before running your script; stash away the
object files; run the script; do another build; compare the object
files...

I consider the lack of (prototyped) function declarations to be more
serious "problem" than the use of old style function definitions in
old code.  I'd like to see:

. declarations near the top of every file (i.e., before the first
  function definition) for every static function in the file.

. a declaration in an included header file for every non static
  function definition and for every non static function used.

. changes to the default configuration to "encourage" developers to
  include the above declarations.
From davea@quasar.engr.sgi.com Fri Jun 02 09:17:00 2000
From: davea@quasar.engr.sgi.com (David B Anderson)
To: gdb@sourceware.cygnus.com
Subject: Re: does GDB support IRIX 64 bit executables?
Date: Fri, 02 Jun 2000 09:17:00 -0000
Message-id: <200006021622.JAA05620@quasar.engr.sgi.com>
X-SW-Source: 2000-06/msg00018.html
Content-length: 952

On: irix 64 bit pointer apps and dwarf2 and gdb

robert somerville <somervil@cadvision.com>
writes:
>i got this patch from : 
>            Benjamin Gamsa <ben@eecg.toronto.edu>

The patch makes no distinction between address-size
and offset-size in the dwarf2, but that is
an essential distinction, I think. 
I think the patch as written
will break alpha-64-bit-pointers-using-dwarf2 in dwarf2read.c

That is, it will break
any non-sgi target with 64bit pointers and dwarf2, 
as *only* SGI extended offsets/lengths in dwarf2 to 64bits for
64bit pointer apps.  And the patch makes no provision for
the coming dwarf2 revision with its compatible allowance
of 32-bit-offset and 64-bit-offset dwarf2 in a single
object file.

I believe this is very easily dealt with.

I hope to get to this to suggest a revised patch soon, but
of course, promises are worthless :-)

Regards,
David B. Anderson davea@sgi.com danderson@acm.org http://reality.sgi.com/davea/
From Will_Lentz@Trimble.COM Fri Jun 02 10:34:00 2000
From: Will Lentz <Will_Lentz@Trimble.COM>
To: Peter Reilley <micrio@mv.com>, gdb@sourceware.cygnus.com
Subject: RE: Questions on GDB
Date: Fri, 02 Jun 2000 10:34:00 -0000
Message-id: <8B0BE50D6F9AD01185A300A0C92BF455088393EE@US01XCH01.Trimble.COM>
X-SW-Source: 2000-06/msg00019.html
Content-length: 1327

Hi Pete,

For a remote PPC target (through rproxy), I use:
 file xyz.elf
 target remote ip.addr:port
 load
 run

It's really cool that you ported the Macraigor DLL to Linux!  I
think more people would use the Wiggler if your port was made
available :-).

Will


> -----Original Message-----
> From: Peter Reilley [ mailto:micrio@mv.com ]
> Sent: Thursday, June 01, 2000 2:24 PM
> To: gdb@sourceware.cygnus.com
> Subject: Questions on GDB
> 
> 
> I have a few questions on the operation of GDB with the Wiggler
> and a PowerPC target.
> 
> I have ported the Macraigor DLL from MS Windows to Linux and
> am attempting to get GDB working.   I can read and write registers
> and memory in the PPC target.   I have a small test program that
> I compiled with GCC configured as a cross compiler for the PPC.
> I can use the "load" command and have the binary load properly
> in the target memory.   I can use the "symbol-file" command to
> load the symbols.   The "file" command will load the binary but
> it seems to have a base address in the host and not the target.
> The "run" and "step" commands do not seem to work.
> 
> Anyway, what are the commands that you used to load and execute
> a binary in a target under Windows.   The documentation that I found
> on the internet is conflicting.
> 
> Thanks,
> Pete.
> 
> 
> 
> 
> 
From jtc@redback.com Fri Jun 02 10:44:00 2000
From: jtc@redback.com (J.T. Conklin)
To: Kevin Buettner <kevinb@cygnus.com>
Cc: Mark Kettenis <kettenis@wins.uva.nl>, gdb@sourceware.cygnus.com
Subject: Re: Proposal: convert function definitions to prototyped form
Date: Fri, 02 Jun 2000 10:44:00 -0000
Message-id: <5mya4om115.fsf@jtc.redback.com>
References: <1000602075018.ZM29997@ocotillo.lan> <200006021226.e52CQ2I01239@delius.kettenis.local> <1000602151553.ZM30578@ocotillo.lan>
X-SW-Source: 2000-06/msg00020.html
Content-length: 799

>>>>> "Kevin" == Kevin Buettner <kevinb@cygnus.com> writes:
Kevin> I noticed that.  The space was put there by ``indent''.  I
Kevin> would very much like to get rid of that space and it would be
Kevin> easy to make the perl script postprocess the ``indent'' output.
Kevin> But in doing so, we (obviously) generate different output than
Kevin> that of ``indent''.
Kevin>
Kevin> I suppose the other solution is to fix indent.  :-)

You can tell indent about all the types defined by typedef with -T
option, and then it won't add the extra space.  It shouldn't be too
difficult to identify all the types.  

It might be useful for us to maintain an indent.pro file that has
these definitions so that additional runs of indent don't add back
the space.

        --jtc

-- 
J.T. Conklin
RedBack Networks
From kevinb@cygnus.com Fri Jun 02 12:10:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: David Taylor <taylor@cygnus.com>
Cc: gdb@sourceware.cygnus.com
Subject: Re: Proposal: convert function definitions to prototyped form
Date: Fri, 02 Jun 2000 12:10:00 -0000
Message-id: <1000602191042.ZM30936@ocotillo.lan>
References: <200006021539.LAA25912@texas.cygnus.com> <taylor@cygnus.com>
X-SW-Source: 2000-06/msg00021.html
Content-length: 7094

On Jun 2, 11:39am, David Taylor wrote:

> With any solution, either you're going to have to check the results or
> you're going to have to hand convert some of the stuff.  It's a
> tradeoff.  From what you say below, your script also requires some
> later hand conversions.

Yes.  But, it's conservative in that it won't break your builds.
(Or at least that's the intent.)

> Does your script produce substantially less that needs to be hand
> converted?

I only ran protoize on breakpoint.c to see what the problems were. 
This was the file that required the most conversion, however, so
hopefully it's a good representative sample.  For this file, protoize
converted 126 function declarations and fix-decls converted 138.  In
addition, not counting reformatting, the protoized version of
breakpoint.c would require 5 hand edits in order to fix problems
introduced in the conversion.  The fix-decls version requires 0 hand
edits.  After conversion, the protoized version of breakpoint.c
still had 15 functions which would need to be converted by hand (or
some other tool) whereas the fix-decls version had only 3.  Here's
a table to summarizing the above:

                        breakpoint.c conversion
                                              | protoize | fix-decls |
----------------------------------------------+----------+-----------+
Functions converted                           |   126    |   138     |
----------------------------------------------+----------+-----------+
Hand edits needed afterwards to fix problems  |          |           |
in the conversion process                     |     5    |     0     |
----------------------------------------------+----------+-----------+
Functions remaining to be converted           |    15    |     3     |
----------------------------------------------+----------+-----------+
Total number of hand edits required           |    20    |     3     |
----------------------------------------------+----------+-----------+

Recall that over all of the gdb sources, fix-decls converted slightly
over 5200 declarations.  I believe that there are only around 140 left
to consider.  This is somewhat higher than I would like and I may be
able to reduce it somewhat by making the script a little bit smarter. 
But I also want to be certain that no errors are introduced in the
process. 

I should note that there are many settings where protoize is a
completely acceptable (and probably better) tool.  With gdb, however,
we have a lot of developers scattered across the globe who need to be
able to deterministically apply the same transformations to their
sources in order to make merges and checkins easier.

With protoize, I think it's going be be difficult to guarantee
determinism since the results will vary depending upon which platform
you use to do the transformation.

> 	3) ``protoize'' screws up on PTR types.  E.g, in breakpoint.c, it
> 	   converted
> 
> 		static int
> 		cover_target_enable_exception_callback (arg)
> 		     PTR arg;
> 
> 	   to
> 
> 		static int
> 		cover_target_enable_exception_callback (__builtin_va_list arg)
> 
> I consider this to be a more serious problem than the other complaints
> you have.

I think the setup issue is important too.  (I don't know if it's more
serious though.)  As I mentioned in my first point (which is no longer
quoted), you have to do a configure in the source directory above gdb
in order to to properly run protoize or else it complains that it
can't find certain header files.  Also, you need to make sure that the
bfd header files are generated before you start.  None of these
problems are insurmountable, but to do things safely with protoize,
and in order to avoid polluting your sources with files that belong in
a build tree, it would be necessary for a script using protoize to
make a complete copy of the source tree somewhere else in order to do
the work.

> 	4) ``protoize'' does not reformat long argument lists.  The lists
> 	   end up entirely contained on one line.
> 
> So... adopt the same solution for it that you chose to adopt for your
> script -- run indent on the output!

Granted.  A suitably imaginative script could identify just the lines
that protoize touched and reformat those.  I don't think we want to
run indent on entire files however, at least not for this activity.

>     For more information on protoize, see the "Running protoize" page:
> 
> 	http://gcc.gnu.org/onlinedocs/gcc_2.html#SEC48
> 
>     and the "Caveats of using protoize" page:
> 
> 	http://gcc.gnu.org/onlinedocs/gcc_7.html#SEC135
> 
> I've used protoize before with good results.  It was a fairly
> substantial project, though not as big as gdb.

Okay.  Out of curiousity, did the project in question have a large
number of active developers?  (This doesn't necessarily matter, but
I think it's probably easier to manage this kind of thing when only
a handful of developers are affected.)

>     Two of my goals in creating the scripts for the PARAMS purging
>     activities was that the scripts should be 1) easy to invoke and 2)
>     require no hand editing of the output when done.  I.e, you shouldn't
>     have to edit any of the files that these scripts touch in order to fix
> 
> I trust that "3)" is:
> 
>     "be conservative; not convert something if it can't be sure
>     of getting it right".

You're right.  I should have mentioned this.

> I'd much rather hand convert more of the code than have it make a
> subtle but incorrect change.

I agree completely.

[...]

>     Finally, I should note that I've done a test build on GNU/Linux/x86
>     and had no build problems, nor did I see any regressions when I ran
>     the testsuite.
> 
>     The fix-decls script is below.  I'd be interested in finding out if
>     anyone else has a favorite script for doing this sort of thing.  Other
>     comments welcome too.
> 
> I'd be tempted to do a build before running your script; stash away the
> object files; run the script; do another build; compare the object
> files...

Good idea.  I'll have to see what gcc does to compare object files.
(I don't think a simple cmp works for ELF files.)

> I consider the lack of (prototyped) function declarations to be more
> serious "problem" than the use of old style function definitions in
> old code.  I'd like to see:
> 
> . declarations near the top of every file (i.e., before the first
>   function definition) for every static function in the file.
> 
> . a declaration in an included header file for every non static
>   function definition and for every non static function used.
> 
> . changes to the default configuration to "encourage" developers to
>   include the above declarations.

I don't disagree, but I think that adding prototypes for everything
should be a separate activity.  The question is whether it should
occur before the activity covered by my proposal.  And if it should
occur before, once again, we'll need to find a way to help automate
the process because if we attempt to do it incrementally by hand
over a period of time, it is likely that it'll never get done.

Kevin
From taylor@cygnus.com Fri Jun 02 13:11:00 2000
From: David Taylor <taylor@cygnus.com>
To: Kevin Buettner <kevinb@cygnus.com>
Cc: gdb@sourceware.cygnus.com
Subject: Re: Proposal: convert function definitions to prototyped form
Date: Fri, 02 Jun 2000 13:11:00 -0000
Message-id: <200006022010.QAA25997@texas.cygnus.com>
X-SW-Source: 2000-06/msg00022.html
Content-length: 4318

    Date: Fri, 2 Jun 2000 12:10:42 -0700
    From: Kevin Buettner <kevinb@cygnus.com>

    On Jun 2, 11:39am, David Taylor wrote:

    >       3) ``protoize'' screws up on PTR types.  E.g, in breakpoint.c, it
    >          converted
    > 
    >               static int
    >               cover_target_enable_exception_callback (arg)
    >                    PTR arg;
    > 
    >          to
    > 
    >               static int
    >               cover_target_enable_exception_callback (__builtin_va_list arg)
    > 
    > I consider this to be a more serious problem than the other complaints
    > you have.

    I think the setup issue is important too.  (I don't know if it's more
    serious though.)  As I mentioned in my first point (which is no longer
    quoted), you have to do a configure in the source directory above gdb

Sorry, I didn't consider it relevant to what I was replying to.

    in order to to properly run protoize or else it complains that it
    can't find certain header files.  Also, you need to make sure that the
    bfd header files are generated before you start.  None of these
    problems are insurmountable, but to do things safely with protoize,
    and in order to avoid polluting your sources with files that belong in
    a build tree, it would be necessary for a script using protoize to
    make a complete copy of the source tree somewhere else in order to do
    the work.

I don't consider the setup issue to be a big issue.  It requires some
time, but it doesn't require much human time.  Configure in the source
tree, then build.  Setup done.

During the commit phase, you don't commit anything that is new -- you
only commit files that existed prior to doing the configure.  No
separate tree needed.

    >       4) ``protoize'' does not reformat long argument lists.  The lists
    >          end up entirely contained on one line.
    > 
    > So... adopt the same solution for it that you chose to adopt for your
    > script -- run indent on the output!

    Granted.  A suitably imaginative script could identify just the lines
    that protoize touched and reformat those.  I don't think we want to
    run indent on entire files however, at least not for this activity.

Since Stan previously ran indent on all of the files, re-running it
*should* produce no change for stuff that hasn't changed.  Should
produce no change.  I'm not saying it won't.  If it changes its own
output when presented with it as input, I would consider that a bug.

    > I've used protoize before with good results.  It was a fairly
    > substantial project, though not as big as gdb.

    Okay.  Out of curiousity, did the project in question have a large
    number of active developers?  (This doesn't necessarily matter, but
    I think it's probably easier to manage this kind of thing when only
    a handful of developers are affected.)

It was at a former company.  There were a small number of active
developers all within the company.  The total project was around
200-250K lines of code.  There was a great deal less use of #if within
the sources than there is in GDB...

[...]
    I don't disagree, but I think that adding prototypes for everything
    should be a separate activity.  The question is whether it should
    occur before the activity covered by my proposal.  And if it should
    occur before, once again, we'll need to find a way to help automate
    the process because if we attempt to do it incrementally by hand
    over a period of time, it is likely that it'll never get done.

I agree that it should be a separate activity.

I think that your script is a good thing.  And due to the amount of
hand editing that appears to be necessary with protoize vs the amount
that appears to be necessary with your script, I think we should use
your script.

Ultimately, though, since you're doing the work, you get to select the
tool.  So long as it is fairly reliable and reasonably quick (quick in
terms of *your time*, not in terms of *elapsed time*), it doesn't
ultimately matter to me what tool you use.  (Which is not to say that
I am not interested in knowing what tool you use -- I am interested.)

Like I said, I think your script is a good thing.  And I look forward
to you using it to protoize the bulk of the GDB sources.

    Kevin

David
From kettenis@wins.uva.nl Fri Jun 02 13:58:00 2000
From: Mark Kettenis <kettenis@wins.uva.nl>
To: gdb@sourceware.cygnus.com
Subject: handling unexpected debugging unformation
Date: Fri, 02 Jun 2000 13:58:00 -0000
Message-id: <200006022058.e52KwS213519@delius.kettenis.local>
X-SW-Source: 2000-06/msg00023.html
Content-length: 1842

Struggling with the misconfigured GCC 2.95.2 used by FreeBSD/Alpha as
its system compiler revealed some weaknesses in GDB with respect to
unexpected debugging information.  Some extra stabs present in the
stabs-in-ecoff .mdebug section of an Alpha ELF executable caused GDB
to crash.

The problematic stabs in question are:

* N_BINCL stabs: These cause add_new_header_files to be called, but
  since init_header_files() hasn't been called for stabs-in-ecoff,
  GDB tries to dereference a null pointer.  There are several possible
  solutions, and I'll need some help choosing between them :-).  Hence
  a few questions:

  - Should N_BINCL stabs be supported in stabs-in-ecoff?
  - Is it acceptable to export free_header_files() and
    init_header_files() from dbxread.c?
  - Shouldn't mdebugread.c:elfmdebug_build_psymtabs() call
    stabsread_new_init() and buildsym_new_init()?
  - What about mdebug_build_psymtabs()?

* An extra blank N_SO stab (emitted by GCC for normal stabs-in-ELF):
  mdebugread.c:psymtab_to_symtab_1() iterates over all stabs symbols,
  calling dbxread.c:process_one_symbol() for them.  If
  process_one_symbol() sees an N_SO stab it will call end_symtab() if
  we've seen an N_SO before, and then calls start_symtab() if the N_SO
  stab isn't blank.

  After the iteration psymtab_to_symtab_1() also calls end_symtab()
  and uses its return value.  The problem is that the blank N_SO stab
  already finishes creating the symbol table.  The end_symtab() in
  psymtab_to_symtab_1() tries to do that again, freeing already freed
  memory blocks, which again crashed GDB.

  (The extra blank N_SO stab in normal ELF objects seems never to be
  passed to process_one_symbal()).

  Is there a way we can make this a bit more robust?  Would setting
  subfiles to NULL in buildsymtab.c:end_symtab() be sufficient?

Mark