Re: [patch] fix for infinite recursion in lookup_symbol

[Elena Zannoni <ezannoni@cygnus.com>]

I would like to see this in. There are too many divergences already.

Fernando, JimI can one of you commit this?

Unless there is some opposition from JimB. (if he replies within say,
5 hours :-).

Thanks
Elena


Jim Ingham writes:
 > Hi, all,
 > 
 > The following patch fixes an infinite recursion in the variable lookup 
 > code for C++ objects that are specified in mangled form.  The old code 
 > would take the mangled name into lookup_symbol, unmangle it, and then in 
 > this bit pass the mangled name BACK to lookup_symbol, which would 
 > unmangle it and come right back here...  Not a good thing.  The bug 
 > crept in when lookup_symbol was broken up into lookup_symbol and 
 > lookup_symbol_aux, the correction is clearly what was meant.
 > 
 > 2001-01-09  James Ingham  <jingham@inghji.apple.com>
 > 
 >          * symtab.c (lookup_symbol_aux): Call lookup_symbol_aux to 
 > lookup a
 >          mangled symbol rather than recursing into lookup_symbol, since 
 > this
 >          will just re-unmangle the name & call lookup_symbol_aux - 
 > leading to
 >          an infinite recursion.
 > 
 > 
 > Index: symtab.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symtab.c,v
 > retrieving revision 1.15
 > diff -c -w -r1.15 symtab.c
 > *** symtab.c    2000/09/04 08:29:25     1.15
 > --- symtab.c    2001/01/12 19:29:21
 > ***************
 > *** 754,760 ****
 >              {
 >                /* This is a mangled variable, look it up by its
 >                   mangled name.  */
 > !             return lookup_symbol (SYMBOL_NAME (msymbol), block,
 >                                      namespace, is_a_field_of_this, 
 > symtab);
 >              }
 >            /* There are no debug symbols for this file, or we are looking
 > --- 754,760 ----
 >              {
 >                /* This is a mangled variable, look it up by its
 >                   mangled name.  */
 > !             return lookup_symbol_aux (SYMBOL_NAME (msymbol), block,
 >                                      namespace, is_a_field_of_this, 
 > symtab);
 >              }
 >            /* There are no debug symbols for this file, or we are looking
 > 
From cgf@redhat.com Wed Jan 17 13:12:00 2001
From: Christopher Faylor <cgf@redhat.com>
To: gdb-patches@sources.redhat.com
Subject: Re: [patch] fix for infinite recursion in lookup_symbol
Date: Wed, 17 Jan 2001 13:12:00 -0000
Message-id: <20010117161229.B15404@redhat.com>
References: <200101121935.LAA03678@scv2.apple.com> <14950.2567.654425.826984@kwikemart.cygnus.com>
X-SW-Source: 2001-01/msg00158.html
Content-length: 754

On Wed, Jan 17, 2001 at 04:09:27PM -0500, Elena Zannoni wrote:
>
>I would like to see this in. There are too many divergences already.
>
>Fernando, JimI can one of you commit this?
>
>Unless there is some opposition from JimB. (if he replies within say,
>5 hours :-).

Can I just suggest that we check it in now and let JimB yell if he
disapproves?  I think enough experienced eyes have looked at this for
there to be a very small chance that the patch is wrong.

What does everyone think about setting a "vote system" for this kind
of contingency?  We could say that the vote of four gdb engineers with
write-after-approval == one maintainer with the maintainer having
absolute authority to remove patches that they think are incorrect,
of course.

cgf
From nsd@redhat.com Wed Jan 17 13:17: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: Wed, 17 Jan 2001 13:17:00 -0000
Message-id: <200101172125.f0HLPDs01986@rtl.cygnus.com>
References: <3A64CBAE.D480ADB7@cygnus.com>
X-SW-Source: 2001-01/msg00159.html
Content-length: 1086

On 17-Jan-2001, Andrew Cagney wrote:

>> That looks good to me.  It's very similar to what regs.c does, except that
>> it calls set_gdbarch_register_list() instead of set_gdbarch_data().

>The difference is that the data is kept private to the regs.c file
>(those interfaces again).  It isn't possible for external code to go
>grubbing around in internals when you're not looking.

Yes.  I wrote:

>I can deal with the memory waste by:
>
>  1. ignoring it;
>  2. using regs.c's approach with e.g. CLIREGS_INTERNAL instead of
>     REGISTER_LIST;
>  3. implementing set_gdbarch_data().
>
>I'd prefer 2 or 3.  Do you have a preference?

If I understand correctly, you're expressing a preference for 3.  I'll
look into implementing that.

On 17-Jan-2001, Andrew Cagney wrote:

>The nice thing about REGISTER_MODULE_INSTALLED_P() is that it is so
>informative that the caller can conclude nothing about the actual
>module.  This is a good thing :-)

I don't understand whether that means "regcache.c is active" or "regs.c is
active".  I'd rather use REGCACHE_MODULE_ACTIVE_P(), okay?

Nick
From dberlin@redhat.com Wed Jan 17 13:28:00 2001
From: Daniel Berlin <dberlin@redhat.com>
To: Christopher Faylor <cgf@redhat.com>, gdb-patches@sources.redhat.com
Subject: Re: [patch] fix for infinite recursion in lookup_symbol
Date: Wed, 17 Jan 2001 13:28:00 -0000
Message-id: <8890000.979766696@dynamic-addr-83-177.resnet.rochester.edu>
References: <20010117161229.B15404@redhat.com>
X-SW-Source: 2001-01/msg00160.html
Content-length: 1162

--On Wednesday, January 17, 2001 16:12:29 -0500 Christopher Faylor 
<cgf@redhat.com> wrote:

> On Wed, Jan 17, 2001 at 04:09:27PM -0500, Elena Zannoni wrote:
>>
>> I would like to see this in. There are too many divergences already.
>>
>> Fernando, JimI can one of you commit this?
>>
>> Unless there is some opposition from JimB. (if he replies within say,
>> 5 hours :-).
>
> Can I just suggest that we check it in now and let JimB yell if he
> disapproves?  I think enough experienced eyes have looked at this for
> there to be a very small chance that the patch is wrong.
>
> What does everyone think about setting a "vote system" for this kind
> of contingency?  We could say that the vote of four gdb engineers with
> write-after-approval == one maintainer with the maintainer having
> absolute authority to remove patches that they think are incorrect,
> of course.
>
> cgf

I like this idea.
after all, patches can always be reverted or backed out if necessary.
It's unlikely this will happen anyway, since maintainers generally got to 
be maintainers from submitting good patches, and having expertise in GDB 
(except me, I slept my way there).
--Dan