Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Keith Seitz <keiths@redhat.com>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [rfc] physname cross-check  [Re: [RFA] Typedef'd method parameters [0/4]]
Date: Tue, 17 May 2011 18:15:00 -0000	[thread overview]
Message-ID: <4DD2BB36.5000704@redhat.com> (raw)
In-Reply-To: <20110516154851.GA24555@host1.jankratochvil.net>

On 05/16/2011 08:48 AM, Jan Kratochvil wrote:

> I wrote a cross-check of what GDB thinks is the physname (which is in
> demangled canonical form) vs. what GCC thinkgs is the physname (which needs to
> be converted from mangled form and canonicalized).

That's a great idea! "Why didn't I think of that?" (famous last words)

> It reports for me 34524 unique failures on libwebkit.so.debug.  (Sure such
> count is caused only by a few physname computation bugs.)

I will start looking into these and filing/fixing bugs when my plate 
opens up here in the next day or two.

> Therefore I would propose a sinful idea to temporarily just use
> DW_AT_linkage_name if it is available to ever release gdb-7.3 and make
> DW_AT_linkage_name-less GDB a feature for gdb-7.4.  After all such cross-check
> should exist anyway for verifying both GCC and GDB bugs this way.

For me, what really matters is what is best for users. Is reverting 
dwarf2_physname better or worse than DW_AT_MIPS_linkage_name? That's a 
difficult question to answer in a black-and-white way, really, but my 
instincts (which could be wrong, of course) tell me that 
MIPS_linkage_name is the greater of two evils.

> cat>1.h<<EOH
> struct x {};
> class C { public: void m (x *xp); };
> EOH
> cat>1.C<<EOH
> #include "1.h"
> C c;
> int main () { c.m(0); }
> EOH
> cat>1b.C<<EOH
> #include "1.h"
> void C::m (x *xp) {}
> EOH

I've fixed this bug. This was just introduced in this patchset because 
the typeprinter did not recognize that in C++, "struct" = "class". [To 
be clear, it was *my* patch which introduced this inequality.] I have 
fixed this my patchset, and I have added a test for it.

Unfortunately, I must reiterate that I am a slave to the test suite. 
While I try my best to test everything that comes to mind, ultimately, 
not everything comes to (my) mind.

> Just it has some other existing testsuite regressions:
> 	gdb.cp/bs15503.exp gdb.cp/cp-relocate.exp gdb.cp/cpexprs.exp
> 	gdb.java/jmisc.exp gdb.java/jprint.exp

Using DW_AT_MIPS_linkage_name will not pass cpexprs.exp without some 
hacking; the demangled name will need to be re-parsed (to remove 
typedefs -- that might be a bit easier with this patchset) and 
subsequently canonicalize the result.

I really see this as an even bigger risk than keeping the current code. 
And then there's constructors -- no version of GCC that I've seen 
outputs DW_AT_MIPS_linkage_name for ctors, so they would still have to 
be computed in some way.

> What do you think?

In the end, it probably doesn't really matter what I think. :-) IMO, 
this all boils down to risk management. Which path is least risky for 
users and most conducive to moving forward?

This is a decision for you and other maintainers to consider.

Keith


  reply	other threads:[~2011-05-17 18:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-21 21:15 [RFA] Typedef'd method parameters [0/4] Keith Seitz
2011-04-25 20:53 ` Tom Tromey
2011-05-12 21:28 ` Keith Seitz
2011-05-16 15:49   ` [rfc] physname cross-check [Re: [RFA] Typedef'd method parameters [0/4]] Jan Kratochvil
2011-05-17 18:15     ` Keith Seitz [this message]
2011-05-17 18:33       ` Jan Kratochvil
2011-05-17 19:04         ` Keith Seitz
2011-05-17 21:01       ` Tom Tromey
2011-05-19 23:04       ` [rfc] physname cross-check #2 Jan Kratochvil

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=4DD2BB36.5000704@redhat.com \
    --to=keiths@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.com \
    /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