Re: [RFA] New register definition interface

[Andrew Cagney <ac131313@cygnus.com>]

BTW, Is there a working example of this code at work?  It is very hard
to comment on interfaces without (partially :-) working examples.  The
PowerPC perhaphs?

	enjoy,
		Andrew
From ac131313@cygnus.com Sat Jan 13 18:38:00 2001
From: Andrew Cagney <ac131313@cygnus.com>
To: Nick Duffek <nsd@redhat.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] New register definition interface
Date: Sat, 13 Jan 2001 18:38:00 -0000
Message-id: <3A6110CB.D5E3CE0B@cygnus.com>
References: <3A5FB8E3.8E778C41@cygnus.com> <200101131951.f0DJpHo04858@rtl.cygnus.com>
X-SW-Source: 2001-01/msg00113.html
Content-length: 1124

Nick Duffek wrote:
> 
> On 13-Jan-2001, Andrew Cagney wrote:
> 
> >I'm confused.  Your saying that the CLI changes depend on regs.c?
> 
> Yes.

I think this is wrong then.  It should be possible to just tweek an
existing target and have it make use of of your new info-registers code.

A quick glance suggests that the code is relying on register attribue
information not currently available through the existing register
interface.  All that hopefully needs to be done is for that interface to
be formally specified (added to gdbarch?).

You may even find a willing volunteer to do the work.

> >It should be possible to code the cli so that it doesn't specificly
> >depend on regs.c.
> 
> It is possible.  I'll look into changing that, but for now, can it be
> committed without that modification?  This is an internal implementation
> issue that won't affect clients of either module, and I'd really like to
> make the interface available.

Unfortunatly the problem isn't an internal implementation issue.  Rather
it is that the CLI is making assumptions about unpublished (as in not in
gdbarch) reg interfaces.

	Andrew
From ac131313@cygnus.com Sat Jan 13 18:53:00 2001
From: Andrew Cagney <ac131313@cygnus.com>
To: Nick Duffek <nsd@redhat.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] New register definition interface
Date: Sat, 13 Jan 2001 18:53:00 -0000
Message-id: <3A611431.C70732@cygnus.com>
References: <3A5F0871.E77E5294@cygnus.com> <200101131858.f0DIw4004850@rtl.cygnus.com>
X-SW-Source: 2001-01/msg00114.html
Content-length: 431

BTW,

In reading the coding standard I would interpret ``try to avoid'' as
``don't do this in new code''. For instance where the standard
indicates:

	Try to avoid assignments inside if-conditions. For example, don't write
this: 

	if ((foo = (char *) malloc (sizeof *foo)) == 0)
	  fatal ("virtual memory exhausted");

I'd interpret it as don't do this in new code.  Even though there are
many examples in existing code.

	Andrew
From nsd@redhat.com Sat Jan 13 23:54:00 2001
From: Nick Duffek <nsd@redhat.com>
To: ac131313@cygnus.com
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] New register definition interface
Date: Sat, 13 Jan 2001 23:54:00 -0000
Message-id: <200101140801.f0E81if04993@rtl.cygnus.com>
References: <3A60F18C.335A7444@cygnus.com>
X-SW-Source: 2001-01/msg00115.html
Content-length: 364

On 14-Jan-2001, Andrew Cagney wrote:

>FYI

>Just an asside, some coding comments (this code is likely to have a very
>long life time and be looked over by many eyes so best to get it right
>straight up).

Sorry Andrew, I didn't realize your email wasn't CCed gdb-patches when I
CCed my response there.  (I am intending to CC _this_ response there,
though.)

Nick
From nsd@redhat.com Sat Jan 13 23:56:00 2001
From: Nick Duffek <nsd@redhat.com>
To: ac131313@cygnus.com
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] New register definition interface
Date: Sat, 13 Jan 2001 23:56:00 -0000
Message-id: <200101140803.f0E83re04998@rtl.cygnus.com>
References: <3A610CD4.FD75BCF4@cygnus.com>
X-SW-Source: 2001-01/msg00116.html
Content-length: 291

On 14-Jan-2001, Andrew Cagney wrote:

>BTW, Is there a working example of this code at work?

Yes, but not that I can publish at the moment.

>The PowerPC perhaphs?

I've been considering that.  The various PPC chips have varied register
sets that this interface should handle nicely.

Nick
From mrg@cygnus.com Sun Jan 14 03:59:00 2001
From: matthew green <mrg@cygnus.com>
To: gdb-patches@sources.redhat.com
Subject: Re: patch for compiles that don't define "unix"
Date: Sun, 14 Jan 2001 03:59:00 -0000
Message-id: <22097.979473536@cygnus.com>
X-SW-Source: 2001-01/msg00117.html
Content-length: 145

hi folks.

could someone please approve & apply the patch from

	http://sources.redhat.com/ml/gdb-patches/2000-12/msg00156.html

thanks,


.mrg.
From fnasser@redhat.com Sun Jan 14 08:11:00 2001
From: Fernando Nasser <fnasser@redhat.com>
To: matthew green <mrg@cygnus.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: patch for compiles that don't define "unix"
Date: Sun, 14 Jan 2001 08:11:00 -0000
Message-id: <3A61CF62.34A9FFE2@redhat.com>
References: <22097.979473536@cygnus.com>
X-SW-Source: 2001-01/msg00118.html
Content-length: 960

Hi Matthew,

Sorry for the delay.

This is not the proper fix though.  The rdi-share subdirectory is
supposed to 
contain code shared with ARM, so we shouldn't make local modifications
in there
(unless absolutely necessary).

In this case we are better off with a more general fix that will solve
all possible
occurrences of the lack of "unix" problem.

You should define this symbol through some configuration test that
results in
a -Dunix being generated.  This way you do not change the files in a
"share"
directory and fix this all around and even catch something that people
add in
the future.

Regards,
Fernando



matthew green wrote:
> 
> hi folks.
> 
> could someone please approve & apply the patch from
> 
>         http://sources.redhat.com/ml/gdb-patches/2000-12/msg00156.html
> 
> thanks,
> 
> .mrg.

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9
From ac131313@cygnus.com Sun Jan 14 16:15:00 2001
From: Andrew Cagney <ac131313@cygnus.com>
To: Nick Duffek <nsd@redhat.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] New register definition interface
Date: Sun, 14 Jan 2001 16:15:00 -0000
Message-id: <3A62408E.A745B2D6@cygnus.com>
References: <3A610CD4.FD75BCF4@cygnus.com> <200101140803.f0E83re04998@rtl.cygnus.com>
X-SW-Source: 2001-01/msg00119.html
Content-length: 2252

Nick Duffek wrote:
> 
> On 14-Jan-2001, Andrew Cagney wrote:
> 
> >BTW, Is there a working example of this code at work?
> 
> Yes, but not that I can publish at the moment.
> 
> >The PowerPC perhaphs?
> 
> I've been considering that.  The various PPC chips have varied register
> sets that this interface should handle nicely.

It is much easier to justify the acceptance of your changes if there is
a real live working example.  Especially one that demonstrates how easy
(?) it is to update an existing target.

At present there is no reason to hurry with the change.

Could I strongly encourage you to do this in two separate steps:

	o	sort out the cli/cli* stuff so that
		it is independant of regs.c

		demonstrate this

	o	then sort out regs.c

I know this is more onerous on your part, however, I can see significant
long term benefits in sorting out cliregs so that it is independant of
regs.c.  To put it another way, if that work isn't done, then I suspect
that, apart from the new obscure ports comming out of Red Hat, your work
will just sit on a shelf gathering dust :-(.  I'll try to explain why
I'm somewhat negative.

Both MI and Insight (GDBtk) have been screaming out for a mechanism that
added formatting information to the register description.  I think your
cliregs code provides a proof-of-concept of this.

Unfortunatly, cliregs assumes your custom regs.c when obtaining that
information.  That in turn means that for an existing target to provide
those annotations it must completly re-implement its register interface
(and as they say in the clasics, if it ain't broke, don't fix it :-( ). 
Simiarly, for MI and Insight, this new annotation information might look
useful, however, since it is only provided by a few new and obscure
targets that use a custom regs.c there is no real incentive to exploit
it.

If instead, published interfaces (based around GDBARCH and REGNUM) were
created and cliregs used those then I'd expect both MI and Insight to
also quickly adopt them - they wouldn't be reliant on a custom regs.c. 
Similarly, since only an incremental change to the target - add the new
methods - would be needed, it would be very easily to tweek existing
targets so that they too could benefit from this code.

	Andrew
From fnasser@redhat.com Mon Jan 15 07:37:00 2001
From: Fernando Nasser <fnasser@redhat.com>
To: Fernando Nasser <fnasser@cygnus.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: RFA: [infrun.c] Fix to "nexti".
Date: Mon, 15 Jan 2001 07:37:00 -0000
Message-id: <3A6318BF.A9A43A10@redhat.com>
References: <3A54D5D2.CCA3E45E@redhat.com> <3A59DE42.5295C9A5@cygnus.com>
X-SW-Source: 2001-01/msg00120.html
Content-length: 2628

If there is a maintainer for this code please speak up.

Otherwise I will check this in under the "obvious fix" rule.

This thing is broken, is annoying lots of people and this patch
has been submitted (and reposted again) a long time ago.

Fernando


Fernando Nasser wrote:
> 
> Ping!
> 
> Fernando Nasser wrote:
> >
> > A "nexti" inside a function prologue currently == continue.
> > This has been broken for quite a while (24-Oct-95).
> >
> > Here is the fix.
> >
> >         * infrun.c (handle_inferior_event): Handle "nexti" inside function
> >         prologues.
> >
> > --
> > Fernando Nasser
> > Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
> > 2323 Yonge Street, Suite #300
> > Toronto, Ontario   M4P 2C9
> >
> >
> > Index: infrun.c
> > ===================================================================
> > RCS file: /cvs/cvsfiles/devo/gdb/infrun.c,v
> > retrieving revision 1.277
> > diff -c -p -r1.277 infrun.c
> > *** infrun.c    2000/02/29 07:17:52     1.277
> > --- infrun.c    2001/01/04 19:39:44
> > *************** handle_inferior_event (struct execution_
> > *** 2738,2748 ****
> >         {
> >         /* It's a subroutine call.  */
> >
> > !       if (step_over_calls == STEP_OVER_NONE)
> >           {
> >             /* I presume that step_over_calls is only 0 when we're
> >                supposed to be stepping at the assembly language level
> >                ("stepi").  Just stop.  */
> >             stop_step = 1;
> >             print_stop_reason (END_STEPPING_RANGE, 0);
> >             stop_stepping (ecs);
> > --- 2738,2753 ----
> >         {
> >         /* It's a subroutine call.  */
> >
> > !       if ((step_over_calls == 0)
> > !           || ((step_range_end == 1)
> > !               && in_prologue (prev_pc, ecs->stop_func_start)))
> >           {
> >             /* I presume that step_over_calls is only 0 when we're
> >                supposed to be stepping at the assembly language level
> >                ("stepi").  Just stop.  */
> > +           /* Also, maybe we just did a "nexti" inside a prolog,
> > +                so we thought it was a subroutine call but it was not.
> > +                Stop as well.  FENN */
> >             stop_step = 1;
> >             print_stop_reason (END_STEPPING_RANGE, 0);
> >             stop_stepping (ecs);
> 
> --
> Fernando Nasser
> Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
> 2323 Yonge Street, Suite #300
> Toronto, Ontario   M4P 2C9

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9
From cgf@redhat.com Mon Jan 15 08:36:00 2001
From: Christopher Faylor <cgf@redhat.com>
To: gdb-patches@sources.redhat.com
Subject: Re: RFA: [infrun.c] Fix to "nexti".
Date: Mon, 15 Jan 2001 08:36:00 -0000
Message-id: <20010115113635.B26739@redhat.com>
References: <3A54D5D2.CCA3E45E@redhat.com> <3A59DE42.5295C9A5@cygnus.com> <3A6318BF.A9A43A10@redhat.com>
X-SW-Source: 2001-01/msg00121.html
Content-length: 287

On Mon, Jan 15, 2001 at 10:35:27AM -0500, Fernando Nasser wrote:
>If there is a maintainer for this code please speak up.
>
>Otherwise I will check this in under the "obvious fix" rule.

Do we really have an "obvious fix" rule?  It seems that there is
some confusion on this issue.

cgf
From kevinb@cygnus.com Mon Jan 15 09:50:00 2001
From: Kevin Buettner <kevinb@cygnus.com>
To: gdb-patches@sources.redhat.com
Subject: Re: RFA: [infrun.c] Fix to "nexti".
Date: Mon, 15 Jan 2001 09:50:00 -0000
Message-id: <1010115175046.ZM6315@ocotillo.lan>
References: <3A54D5D2.CCA3E45E@redhat.com> <3A59DE42.5295C9A5@cygnus.com> <3A6318BF.A9A43A10@redhat.com> <20010115113635.B26739@redhat.com> <cgf@redhat.com>
X-SW-Source: 2001-01/msg00122.html
Content-length: 462

On Jan 15, 11:36am, Christopher Faylor wrote:

> On Mon, Jan 15, 2001 at 10:35:27AM -0500, Fernando Nasser wrote:
> >If there is a maintainer for this code please speak up.
> >
> >Otherwise I will check this in under the "obvious fix" rule.
> 
> Do we really have an "obvious fix" rule?  It seems that there is
> some confusion on this issue.

We do not have an "obvious fix" rule.  See

    http://sources.redhat.com/ml/gdb-patches/2000-04/msg00468.html

Kevin
From mrg@cygnus.com Mon Jan 15 10:29:00 2001
From: matthew green <mrg@cygnus.com>
To: Fernando Nasser <fnasser@redhat.com>
Cc: gdb-patches@sources.redhat.com
Subject: re: patch for compilers that don't define "unix" 
Date: Mon, 15 Jan 2001 10:29:00 -0000
Message-id: <8062.979583347@cygnus.com>
References: <3A61CF62.34A9FFE2@redhat.com>
X-SW-Source: 2001-01/msg00123.html
Content-length: 1402

   
   This is not the proper fix though.  The rdi-share subdirectory is supposed to 
   contain code shared with ARM, so we shouldn't make local modifications in there
   (unless absolutely necessary).


how about this then?  tested on netbsd/i386 and solaris 2.6.  you'll need to
regenerate `configure' after applying this patch.


thanks.



2001-01-15  matthew green  <mrg@redhat.com>

	* configure.in: Define missing `__unix' if `__unix__' is present.
	* configure: Regenerate.



Index: configure.in
===================================================================
RCS file: /cvs/src/src/gdb/configure.in,v
retrieving revision 1.53
diff -p -r1.53 configure.in
*** configure.in	2000/12/21 16:16:17	1.53
--- configure.in	2001/01/15 18:21:54
*************** if test x${want_included_regex} = xtrue;
*** 703,708 ****
--- 703,718 ----
      AC_DEFINE(USE_INCLUDED_REGEX)
  fi
  AC_SUBST(REGEX)
+ 
+ # NetBSD compiler defines __unix__ only; rdi-share needs __unix.
+ AC_CACHE_CHECK([for NetBSD [__unix__]], gdb_cv_missing_netbsd___unix,
+ [AC_EGREP_CPP(lose, [
+ #if defined (__unix__) || !defined (__unix)
+ lose
+ #endif],[gdb_cv_missing_netbsd___unix=yes],[gdb_cv_missing_netbsd___unix=no])])
+ if test x$gdb_cv_missing_netbsd___unix = xyes; then
+   CFLAGS="$CFLAGS -D__unix"
+ fi
  
  # In the Cygwin environment, we need some additional flags.
  AC_CACHE_CHECK([for cygwin], gdb_cv_os_cygwin,
From fnasser@redhat.com Mon Jan 15 10:31:00 2001
From: Fernando Nasser <fnasser@redhat.com>
To: Kevin Buettner <kevinb@cygnus.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: RFA: [infrun.c] Fix to "nexti".
Date: Mon, 15 Jan 2001 10:31:00 -0000
Message-id: <3A63418D.513E833@redhat.com>
References: <3A54D5D2.CCA3E45E@redhat.com> <3A59DE42.5295C9A5@cygnus.com> <3A6318BF.A9A43A10@redhat.com> <20010115113635.B26739@redhat.com> <1010115175046.ZM6315@ocotillo.lan>
X-SW-Source: 2001-01/msg00124.html
Content-length: 1725

Kevin Buettner wrote:
> 
> On Jan 15, 11:36am, Christopher Faylor wrote:
> 
> > On Mon, Jan 15, 2001 at 10:35:27AM -0500, Fernando Nasser wrote:
> > >If there is a maintainer for this code please speak up.
> > >
> > >Otherwise I will check this in under the "obvious fix" rule.
> >
> > Do we really have an "obvious fix" rule?  It seems that there is
> > some confusion on this issue.
> 
> We do not have an "obvious fix" rule.  See
> 
>     http://sources.redhat.com/ml/gdb-patches/2000-04/msg00468.html
> 

If you go throughout the list you'll find that it has been mentioned and 
invoked several times in other messages.  It is not well defined though,
and it does not seem to be written anywhere either.

As it is not written what to do when the maintainers do not respond and 
something is broken.  Do we keep downloading rotten code?  These bugs
are very costly in that people waste hours of work because of them.

The message above mentions a problem with regards to small fixes in
codes outside one's maintainership that go wrong.  Sometimes
the original problem is fixed but some other is created.
That is why we use CVS.  The change can be reversed and the maintainer
(when he/she shows up) has a complete record of what has been changed
and
why (in addition to the list archives).

I don't know what is the better/right solution to the problem, but we do
have a problem now with regarding to bug fixes, specially small fix to 
harmful ones.  We do need a fast track to them and even consider
temporary
fixes (properly marked with FIXME and entries in the TODO file).


-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9
From fnasser@redhat.com Mon Jan 15 10:35:00 2001
From: Fernando Nasser <fnasser@redhat.com>
To: matthew green <mrg@cygnus.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: patch for compilers that don't define "unix"
Date: Mon, 15 Jan 2001 10:35:00 -0000
Message-id: <3A63429D.B915C05A@redhat.com>
References: <8062.979583347@cygnus.com>
X-SW-Source: 2001-01/msg00125.html
Content-length: 1792

matthew green wrote:
> 
> 
>    This is not the proper fix though.  The rdi-share subdirectory is supposed to
>    contain code shared with ARM, so we shouldn't make local modifications in there
>    (unless absolutely necessary).
> 
> how about this then?  tested on netbsd/i386 and solaris 2.6.  you'll need to
> regenerate `configure' after applying this patch.
> 

Thank you Matthew.

I will give it a spin and commit it in a couple of days if nobody
disagrees.

Regards,
Fernando


> thanks.
> 
> 2001-01-15  matthew green  <mrg@redhat.com>
> 
>         * configure.in: Define missing `__unix' if `__unix__' is present.
>         * configure: Regenerate.
> 
> Index: configure.in
> ===================================================================
> RCS file: /cvs/src/src/gdb/configure.in,v
> retrieving revision 1.53
> diff -p -r1.53 configure.in
> *** configure.in        2000/12/21 16:16:17     1.53
> --- configure.in        2001/01/15 18:21:54
> *************** if test x${want_included_regex} = xtrue;
> *** 703,708 ****
> --- 703,718 ----
>       AC_DEFINE(USE_INCLUDED_REGEX)
>   fi
>   AC_SUBST(REGEX)
> +
> + # NetBSD compiler defines __unix__ only; rdi-share needs __unix.
> + AC_CACHE_CHECK([for NetBSD [__unix__]], gdb_cv_missing_netbsd___unix,
> + [AC_EGREP_CPP(lose, [
> + #if defined (__unix__) || !defined (__unix)
> + lose
> + #endif],[gdb_cv_missing_netbsd___unix=yes],[gdb_cv_missing_netbsd___unix=no])])
> + if test x$gdb_cv_missing_netbsd___unix = xyes; then
> +   CFLAGS="$CFLAGS -D__unix"
> + fi
> 
>   # In the Cygwin environment, we need some additional flags.
>   AC_CACHE_CHECK([for cygwin], gdb_cv_os_cygwin,

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9
From msnyder@redhat.com Mon Jan 15 10:46:00 2001
From: Michael Snyder <msnyder@redhat.com>
To: gdb-patches@sources.redhat.com
Subject: [Fwd: RFA: [infrun.c] Fix to "nexti".]
Date: Mon, 15 Jan 2001 10:46:00 -0000
Message-id: <3A634581.9C1C76CD@redhat.com>
X-SW-Source: 2001-01/msg00126.html
Content-length: 747

Christopher Faylor wrote:
>
> On Mon, Jan 15, 2001 at 10:35:27AM -0500, Fernando Nasser wrote:
> >If there is a maintainer for this code please speak up.
> >
> >Otherwise I will check this in under the "obvious fix" rule.
>
> Do we really have an "obvious fix" rule?  It seems that there is
> some confusion on this issue.

We do (I think), but when you're discussing infrun.c, I'm not
sure that any change can be regarded as "obviously correct".
At least not in the wait_for_inferior/handle_event area.

By eyeball, this change looks correct to me, or at least
"not obviously incorrect".  I would like to see it tested,
and perhaps the best way to do that is to apply it and then
notice if there's a sudden uptick in testsuite failures.

Michael
From nsd@redhat.com Mon Jan 15 10:52:00 2001
From: Nick Duffek <nsd@redhat.com>
To: ac131313@cygnus.com
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] New register definition interface
Date: Mon, 15 Jan 2001 10:52:00 -0000
Message-id: <200101151859.f0FIxqG05555@rtl.cygnus.com>
References: <3A62408E.A745B2D6@cygnus.com>
X-SW-Source: 2001-01/msg00127.html
Content-length: 3123

On 15-Jan-2001, Andrew Cagney wrote:

>It is much easier to justify the acceptance of your changes if there is
>a real live working example.  Especially one that demonstrates how easy
>(?) it is to update an existing target.

I agree, but it's not something I'll be able to do immediately.

>	o	sort out the cli/cli* stuff so that
>		it is independant of regs.c

You've convinced me.  I've done most of that work, but due to constraints
in my schedule at the moment, it might be a few weeks before I can post
the tested patch.

There's one technical problem: when an architecture selects one of several
competing module implementations, the other implementations shouldn't
initialize themselves for that architecture.

For example, regcache.c does this in _initialize_regcache():

   register_gdbarch_swap (NULL, 0, build_regcache);

build_regcache() attaches a chunk of malloc()ed memory to each
architecture.  That memory is wasted in architectures that use regs.c
instead of regcache.c.

The same issue applies to cli-regs.c: cliregs_init() attaches memory to
each architecture even if the architecture hasn't set cliregs_info() as
its do_registers_info callback.

Some possible solutions:

  1. Each competing module implementation introduces a gdbarch variable
     USE_<module-implementation> that architectures must set to 1 if
     they're using that implementation.  For example:

        set_gdbarch_do_registers_info (gdbarch, cliregs_info);
        set_gdbarch_use_cliregs (gdbarch, 1);

     cliregs_init() would check USE_CLIREGS before allocating memory.

     This doesn't work well for regcache.c: each existing architecture
     would need to add "set_gdbarch_use_regcache (gdbarch, 1)" to its
     initialization routine, or else each future architecture using regs.c
     would need to call "set_gdbarch_use_regcache (gdbarch, 0)".

  2. Each module introduces a gdbarch variable MODULE_<module-name> that
     architectures must set to an implementation-unique value.  For
     example:

        set_gdbarch_do_registers_info (gdbarch, cliregs_info);
        set_gdbarch_module_registers_info (gdbarch, MODULE_CLIREGS);

     This requires a new header file containing all the MODULE_*
     definitions.

  3. Introduce a gdbarch interface to query function pointers.  Competing
     module implementations can use these to check whether they're being
     used by the current architecture.  For example:

       if (GET_DO_REGISTERS_INFO () != cliregs_info)
         return;
       /* allocate memory */

     To avoid the footprint of a GET_*() function for every gdbarch.sh
     entry, a per-entry flag could be added that controls whether that
     function is needed.

I'm partial to option 3, and I volunteer to implement it if you give
pre-approval to the approach.

Speaking of gdbarch.sh changes, what do you think about the idea of
translating gdbarch.sh into C and automatically generating gdbarch.[ch] as
transient compile-time entities?  Running gdbarch.sh takes about 25
seconds on my 650MHz Pentium, and I often forget to run it after applying
or reverting patches that change it.

Nick
From jingham@apple.com Mon Jan 15 11:39:00 2001
From: Jim Ingham <jingham@apple.com>
To: Daniel Berlin <dberlin@redhat.com>
Cc: <gdb-patches@sources.redhat.com>
Subject: Re: [patch] fix for infinite recursion in lookup_symbol
Date: Mon, 15 Jan 2001 11:39:00 -0000
Message-id: <B6889200.268C%jingham@apple.com>
References: <Pine.LNX.4.31.0101122216180.15353-100000@www.cgsoftware.com>
X-SW-Source: 2001-01/msg00128.html
Content-length: 6895

Daniel,

>> I understand that there is some argument over whether the current
>> implementation of C++ symbol lookup is the right one, but while it is in
>> place, simple fixes to it need to get into the sources quickly.
> 
> I'd happily check them in under an obvious bugfix rule, but I don't want
> to step on any toes.
> I had enough fun doing that trying to figure out what exact areas of code
> C++ maintainer covers, and I still couldn't tell you.
> If someone with definite maintainership over the symbol tables says I can
> check in the fixes, i'll do it. Otherwise, i won't.
> Sorry.

I sympathize, but in this case the fix really is trivial...  This one really
does fall under the obvious bugfix rule if any fix does...

> 
> 
>> It is
>> not sufficient to say that the crash was reported in this forum -
>> Fernando didn't make the connection either, and he is no slouch to say
>> the least...
> 
> Err, i'm curious, if you noticed a problem with crashes trying to print
> C++ objects, why didn't you ask me or Jim, as i'm C++ maintainer, and he's
> symbol table maintainer.

Because we have a fairly hacked version of gdb, including a lot of patches
for Objective C, and I always suspect them first when I see any symbol
related problem.  By the time I was sure this was not the problem, I also
had found the bug & fixed it.  I don't want to send you guys chasing a
phantom problem that only appears in our sources.  (And yes, once MacOS X
makes its way out the door, we will turn out attention to submitting these
patches back into the main sources, but until then we are more of a service
organization for all the other folks at Apple that are madly trying to
finish up MacOS X...)

> 
>>  And most gdb users don't read gdb patches. Sure,
> i'll give you that.
> 
>> 
>> Just like no user error, no matter how stupid, should ever result in a
>> crash, no patch that keeps gdb from crashing should be refused unless
>> the maintainer can come up with another solution quickly.  It is one
>> thing if gdb doesn't find a symbol, or reports some data wrong.  That is
>> bad, but there is leisure to argue over method, since users can
>> generally work around it and still get their job done.  If gdb crashes -
>> particularly on a common code path, then users are just stuck...
> If they use CVS versions, yes.

I am not sure what you mean by this.  We have a fairly hacked version of
GDB, but we work pretty hard to keep re-synching with the CVS sources, so
that when we finally get time to submit patches, we won't have a massive
update to do first...  However, if you mean we should work from CVS +
unapproved patches submitted to the patches mailing list, that is going to
make things very much worse for us.  Then we would have our changes, plus
the official sources, plus some random collection of other patches we aren't
responsible for.  Yuck!  And if you mean stick with 5.0, that would mean we
get pretty out of synch, and then have a big merge job to do every half-year
to year.

> 
>> 
>> If it is really the case that this patch is waiting on Jim's approval,
>> do we need to have a "fast track crash prevention approval mechanism" in
>> the Maintainer system for gdb?
> We do, it's the obvious bugfix rule, but it's hard to say what is
> obvious to everyone.
> I don't feel comfortable making that call right now, I'm pretty tired of
> taking the backlash from the people who think it isn't obvious/needed
> approval which invariably happens.
> 
> As I said, isn't the only patch i've got outstanding, and most of them are
> trivial
> fixes/improvements, because I've not yet submitted some large improvements
> made to memory usage/demangling speed/symbol lookup speed for C++/every
> language except C/infrastructure improvements for C++/dwarf2 improvements.
> Among other improvements. Normally, I would just keep pinging Jim until I
> get a response, but to be honest, all the politicking and crap that
> has happened with regards to the steering committee, etc, has
> kinda demotivated me, and it's hard to motivate myself to contribute to
> gdb outside of work, either large or small improvements.
> I'm sure it'll pass in a month or so.

Okay, I only heard about all the steering committee by looking over Stan &
Klee's shoulders, but it did seem pretty counter-productive.  No slagging on
JimB, either, but it doesn't help that a maintainer of a the relevant
portion of GDB doesn't actually work on it right now :-(

>> 
>> This sort of thing makes gdb look really bad.
> Yeah, but it's only in gdb CVS, don't forget.
> I get the feeling you guys are shipping CVS gdb's to people.
> Well, it's not just a feeling, I have a powerbook with OS X 4F8 (acquired
> through a lot of pain and trouble) on it, and
> I just looked, and it's  GDB 20000912, a CVS version.

As I said above, we have a bunch of changes in our sources, but we try to
keep it synched with gdb from CVS every so often.  There aren't official
releases often enough for them to be a good base.  And this bug has been
around long enough without fixes that if there were more frequent releases
it would still be in one, so I don't see this as viable solution...  So what
are you suggesting we do?

> 
> Which explains why you keep mentioning users, when the bug only exists in
> CVS.
> This is a very bad idea to be doing.

I really don't understand this.  What else are we supposed to be basing our
sources on?  There are two options here, apply patches from the Patches list
in advance of their acceptance, or just stick with "official" releases (i.e.
5.0)

There are lots of good patches, and lots of bad ones, in the patches mailing
list.  It is better, I think, for us to let the "experts" in the Maintainers
group sort them out, and then use the results of this vetting process.  That
IS what the whole Maintainer's (or Helpers??) structure is for.  Seems like
subverting this process will just cause us more trouble.

On the other hand, does anyone think an unmodified 5.0 is a good release to
base our efforts on?  It has been around for a while now, and has its own
share of bugs that HAVE been fixed.  In any case, you don't really WANT most
of your gdb users to stick with the 5.0 release, do you?  That would mean
that the CVS changes are being tested by some set of Red Hat customers, and
that is about all.  That would not be a very good idea.

> However, if you like, I'll happily make sure C++ nicely, if I didn't have
> to go through a ton of  pain to get new builds so I could check it out (IE
> I was seeded the same as the other developers who got 4F8 and friends the
> normal way)
> I know it's in Darwin CVS, but I can't make that compile without the right
> versions of OS X anyway.
> 

We should take this part off line, but the Darwin version of gdb SHOULD
build on 4F8.  I haven't tried it in a while, but I can't think of anything
we have done that would break this.

Jim