Re: [patch] fix for infinite recursion in lookup_symbol

[Jim Ingham <jingham at apple dot com>]

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