Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Daniel Jacobowitz <drow@false.org>
To: Chris Moller <cmoller@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: Patch for PR 9399
Date: Thu, 10 Dec 2009 18:41:00 -0000	[thread overview]
Message-ID: <20091210184102.GA16828@caradoc.them.org> (raw)
In-Reply-To: <4B1FBDF4.4040801@redhat.com>

Tom's approved this.  That's fine.  But for posterity:

On Wed, Dec 09, 2009 at 10:10:44AM -0500, Chris Moller wrote:
> >>You can see what the patch does by compiling -g virtfunc.cc, gdb-ing
> >>it, breaking in the return stmt at // marker1, and doing things like
> >>"print o.do_print()".  Without the patch, gdb tries to access
> >>location 0x0; with the patch it does the right thing.  (There are
> >>more tests in virtfunc2.exp)
> >>
> >
> >Where does the access to 0x0 come from?  Is it inside
> >search_struct_field?
> 
> Ultimately, yes.  Without the patch, the thread ultimately gets to
> 
>      if (BASETYPE_VIA_VIRTUAL (type, i))
> 
> in search_struct_field and then to the memcpy about 30 lines later
> that extracts a new value struct.  That main_type of that value
> doesn't include a field for the virtual function, so it's never
> found, and ultimately returns a null pointer.  I haven't a clue why
> it works that way--for a while I was working on the assumption that
> the DWARF reader was screwing up, but if it is, it's too subtle for
> me.

I couldn't follow this and haven't had time to debug it myself yet.
But we shouldn't dereference any null (target) pointers in this.  If
we do, there's a bug in some routine this function calls.

> >  I wouldn't expect value_cast_structs to do any
> >cast in this case,
> 
> value_cast_structs only does nothing if both TYPE_NAME()s are null.
> I was wondering if, back when the code was originally written, if
> there never was a case when both TYPE_NAME()s were non-null--it's the
> only way, other than simple oversight, I can account for that case
> not being covered.

No, it looks exactly like I intended.  If we have two classes, we
check if A is a base class of B, then if B is a base class of A.
If A == B, neither of those checks will be true, and we'll return
without doing anything.

So the problem is that one of those base class checks is calling
error().  That's not supposed to happen.

-- 
Daniel Jacobowitz
CodeSourcery


  reply	other threads:[~2009-12-10 18:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-09 13:32 Chris Moller
2009-12-09 14:05 ` Daniel Jacobowitz
2009-12-09 15:10   ` Chris Moller
2009-12-10 18:41     ` Daniel Jacobowitz [this message]
2009-12-10 19:04       ` Tom Tromey
2009-12-10 19:09         ` Chris Moller
2009-12-10 19:13         ` Daniel Jacobowitz
2009-12-10 16:59 ` Tom Tromey
2009-12-10 18:33   ` Chris Moller
2009-12-10 18:55     ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091210184102.GA16828@caradoc.them.org \
    --to=drow@false.org \
    --cc=cmoller@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox