This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] 12266 (typedef'd parameters) revisited again - what should go for gdb-7.3?


On 06/07/2011 01:11 PM, Jan Kratochvil wrote:

You have said off-list you have a fix but you still run the real-world .debug
files regression tests on top of it.

A fix, but not a complete fix. I am running tests against libstdc++, and that is uncovering some additional problems.


I believe the replace_typedefs part (DEMANGLE_COMPONENT_* typedef
replacements) should be cross-checked against `set debug check-physname on'
which I guess hasn't been done yet.

I agree, and I will do that when I can successfully get through libstdc++ (and libwebkit, others?).


The question (at least for me) is now which parts of this patch should go for
7.3 and which only for 7.4+:

It can be split into the parts before before:
1/4: changes cp-name-parser.y to not use shared memory.
2/4: differences between print names and "linkage names" (physnames).
3/4: cp_canonicalize_no_typedefs to decode_variable and decode_compound
4/4: tests for this new feature.

#2 is quite independent of the others. #1 is only needed for #3 (as is #4). IMO, #3 is a long ways away, and as it addresses new features, it is far too risky to include in 7.3.


So the only one I would consider is #2, which addresses differences in physname/DW_AT_linkage_name and "print" names. If that has actual benefits to something today, we can decide to include it. You have, however, found other ways to "skin the cat," so this might not be needed until the rest of the 12266 patch is really ready to go in.

PR 12506 is I believe fixed by 2/4.  But for compilers featuring
DW_AT_MIPS_linkage_name (=G++) it is fixed also by the more general:
	Re: [patch] Follow DW_AT_linkage_name for methods #2
	http://sourceware.org/ml/gdb-patches/2011-06/msg00040.html
And I believe I could still find a countercase where DW_AT_linkage_name is
more correct than the current physname code.

I would say this part is really at your/maintainer's discretion(s). I have been working under the assumption that we were all in agreement about moving to preferring DW_AT_linkage_name over physnames.


In this context, I would say (as I always do), take the least risky approach, and I do not think today that cp_canonicalize_string_no_typedefs is the least risky. Like physnames, it works in a great many instances, but it is far from perfect/bug-free.

ctors/dtors do not have DW_AT_MIPS_linkage_name but for class based naming
(C::C or C::~C) one can drop to the minimal symbols (the fallback needs to
work even for other cases):
	[patch 1/2] physname reg.: linespec minsym fallback
	http://sourceware.org/ml/gdb-patches/2011-06/msg00079.html
and one IMO cannot use linespec to refer to ctors/dtors (only to regular
methods) during variable base naming (var.C or var.~C) - where the minsym
fallback is not applicable and where decode_compound could be useful.

Additionally, I would even suggest adding a fallback to decode_variable if decode_compound fails. This is, I believe, the biggest linespec regression honey pot we've seen.


The 3/4 is a new feature for PR 12266.  GDB could never
do `break f(typedefparam)', I do not think it needs to go for 7.3 so late.

Absolutely agreed.


Keith


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]