Re: [rfc] physname cross-check [Re: [RFA] Typedef'd method parameters [0/4]]

[Keith Seitz <keiths at redhat dot com>]

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