Re: patch for PR gdb/574

[Daniel Jacobowitz <drow@mvista.com>]

On Mon, Jul 22, 2002 at 11:42:58AM -0700, david carlton wrote:
> For various reasons, I decided to delve into GDB recently, so I picked
> bug 574 somewhat at random and decided to try to fix it.  Here's a
> patch.
> 
> The problem: GDB tries to access an improper memory location while
> trying to find run time type info on certain code compiled with G++
> 2.something.  In the situation in question, GDB is trying to find the
> vptr of an object that is a subclass of a class with virtual functions
> and static member data.  It looks for the information about the vptr
> at a place where information about the static member data is actually
> stored.  This is a data structure with a useless (and very large)
> bitpos field; adding this bitpos to a pointer causes that pointer to
> point to the middle of nowhere.  Oops.
> 
> The solution: Get rid of the check for
> 
>   if (VALUE_ENCLOSING_TYPE(v) != VALUE_TYPE(v))
> 
> and the surrounding code in gnu-v2-abi.c(gnuv2_value_rtti_type).

I'll review this properly later this evening, but first of all, some
comments of my own.

First, do you have an FSF copyright assignment on file?  I'm not going
to look too closely at your patch until you do, although I can
duplicate your fix from your excellent description without having
copyright problems.

Second of all, VALUE_ENCLOSING_TYPE is a bad idea, as far as I'm
concerned.  Look for it in the ChangeLog and you'll see two entries:
one fixing a bug regarding it, and the other introducing it.  It's in
ChangeLog-1998.  The entry in question is 2569 lines long, and
represents most of the periodically-mentioned HP merge.  There's a lot
of good work in there, and a lot of things that weren't quite such a
good idea in the long run.

Proper fixes for this sort of problem will probably involve removing or
at least substantially modifying the meaning of VALUE_ENCLOSING_TYPE.
I wouldn't have done it at all the same way.... and v3 support has been
getting by without setting it in this case, anyway, as you noticed.

> Some comments:
> 
> * gnu-v3-abi.c(gnuv3_rtti_type) doesn't have any such code, so I don't
>   think this fix is likely to break anything that isn't also broken in
>   gnu-v3-abi.c.
> 
> * It doesn't seem to cause any testsuites to fail that didn't fail
>   before.  (In fact, when I ran it, it turned gdb.base/selftest.exp
>   from a FAIL into a pass, for what that's worth; I don't know if my
>   changes are relevant to that, though.)  I'm not too used to working
>   the testsuites; please rerun them to make sure.

Selftest tends to change based on the optimization level you build GDB
with, among other things.  That's probably why.

> * There are other problems in the surrounding code that are unrelated
>   to this.  When I compiled my test program under g++-3.1, I got an
>   unrelated bad memory access from inside gnu-v3-abi.c.  Also (and
>   this may be related to that bad memory access), there seems to be a
>   difference of opinion among various pieces of code as to whose job
>   it is to demangle symbols that you want to look up in symbol
>   tables.  I'll try to spend some time over the next week or two
>   sorting this out.

The former is more immediately interesting to me.  I'll take a look
later.  For the latter, generally one should look up demangled names.
Especially now that I've hashed the symbol tables based on the
demangled name!

> * I'm not convinced that the code I deleted wouldn't be useful in some
>   circumstances (where we had a pointer to something in the middle of
>   a class); I'll try to spend some time over the next week or two
>   coming up with test cases to see cause the code to fail, and to see
>   if there's a better fix around somewhere.  Nonetheless, given that
>   my patch does fix a known problem and, if there is a problem with
>   enclosing_type, then the current gnu-v3-abi.c code probably has it
>   as well, I think it should be committed.
> 
> * I haven't contacted the submitter of the bug report, but I'd be
>   happy to do so if that is appropriate.
> 
> I'm including a sample program that demonstrates the bug (though the
> submitter of the bug report also submitted a more elaborate test case
> that has some other interesting features; hurrah for good bug
> reports), a ChangeLog entry, and the patch; all will follow my
> signature.
> 
> David Carlton
> carlton@math.stanford.edu

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer