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

Keith Seitz
2011-05-17T18:15:39Z

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