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: [patch 0/3] Re: [RFA] c++/11734 revisited (and c++/12273)


On 02/21/2011 03:06 AM, Jan Kratochvil wrote:

pre-physname:
GNU gdb (GDB) 7.1.50.20100309-cvs
(gdb) b A::outer::foo (int)
Breakpoint 1 at 0x4005a3: file ./gdb.cp/ovsrch3.cc, line 23.
(gdb) b A::outer::foo (void)
Breakpoint 2 at 0x40058c: file ./gdb.cp/ovsrch2.cc, line 23.
(gdb) b A::outer::foo (char *)
Breakpoint 3 at 0x4005c0: file ./gdb.cp/ovsrch4.cc, line 23.

patched:
GNU gdb (GDB) 7.2.50.20110220-cvs
(gdb) b A::outer::foo (int)
the class A::outer does not have any method named foo (int)

This is caused by one of the consequences of dwarf2_physname: we need to have all user input canonicalized before calling lookup_symbol[*]. This is a problem I have wondered about since the dwarf2_physname patchset was originally committed, and now we have a concrete example of how to trigger this.


IIRC, the only functional changes (excluding requested changes) to the patchset are:

- canonicalization of user input in find_methods (linespec.c)
- add tests to ovsrch.exp et al to cover the regressions you've identified

It assumes get_gdb_completer_quote_characters() is just "'" as currently is.
It cannot be changed now anyway and for future it should be rather removed
than extended.  It was already hardcoded into decode_compound before physname
patch.

Yes, that was sloppy on my part. I apologize. I have reintroduced the use of get_gdb_completer_quote_characters. IMO there's something to be said for consistency. One day, we'll want to identify the places where this assumption is made, and using get_gdb_completer_quote_characters is undoubtedly going to be the key to doing it.


Ãxcessive backslash.

Fixed all of these.


Some comment and/or removal wrt get_gdb_completer_quote_characters for '.
(OK, this ' hardcoded in decode_compound was present even before physname,
I can clean it up afterwards.)

If I have understood you correctly, I've removed the quote character and tested independently for it using get_gdb_completer_quote_characters.


*p can == '\0' here, it works - but I would prefer for better readability:

Fixed.


00000000004004ea W _ZNK1C1mEv
00000000004004ea W C::m() const

pre-physname:
GNU gdb (GDB) 7.1.50.20100309-cvs
(gdb) b C::m if C::m()
Breakpoint 1 at 0x4004f2: file 3.C, line 4.
(gdb) b 'C::m' if C::m()
Breakpoint 1 at 0x4004f2: file 3.C, line 4.

patched:
GNU gdb (GDB) 7.2.50.20110220-cvs
(gdb) b C::m if C::m()
the class C does not have any method named m if C::m()
(gdb) b 'C::m' if C::m()
the class C does not have any method named m' if C::m()

This is also fixed, and I have added a test for this to ovsrch.exp.


00000000004004ea W _ZNK1C1mEv
00000000004004ea W C::m() const

pre-physname:
GNU gdb (GDB) 7.1.50.20100309-cvs
(gdb) b 'C::m()  const'
Breakpoint 1 at 0x4004f2: file 3.C, line 4.

patched:
GNU gdb (GDB) 7.2.50.20110220-cvs
(gdb) b 'C::m()  const'
Junk at end of arguments.

This is a regression.

This is also a consequence of (the lack of) canonicalization. I've modified ovsrch.exp et al to test for this.


Probably not meaningful for SYM_CLASS but isn't here appropriate rather
SYMBOL_NATURAL_NAME?  If SYMBOL_LINKAGE_NAME would be a mangled name for the
class fully qualified name it would not work.  But linkage name of a class
type does not make sense so the suggested change is rather for code
readability.

Changed.


+# Copyright 2010 Free Software Foundation, Inc.

2011

Fixed.



if [prepare_for_testing ... ] { return -1 }

Fixed.


[nitpick] gdb_breakpoint already always prints FAIL if it fails.

Grr. Deep down, I knew that to be the case. For some reason, it looked odd, so I added the fail clause. Fixed.


+ Copyright 2010 Free Software Foundation, Inc.

2011

Fixed.


+ set method "${class}::foo\($ovld\)"

Excessive backslashes.

Fixed.



[nitpick] gdb_breakpoint already always prints FAIL if it fails.
(and below)

Fixed.


if [prepare_for_testing ... ] {
     return -1
}

Fixed.


Sorry for the review delay.

It's a big patchset. It's not like I have nothing else to do! ;-)


I'm not sure whether it would be easier to diff the old and new patchsets or just attach the whole thing. But I'm erring on the side of "too much information."

Keith

ChangeLog
2011-02-24  Keith Seitz  <keiths@redhat.com>

	* linespec.c (find_methods): Canonicalize NAME before looking
	up the symbol.

	PR c++/12273
	* linespec.c (locate_first_half): Keep overload information, too.
	(decode_compound): Use a string to represent break characters
	to escape the loop.
	If P points to a break character, do not increment it.
	For C++ and Java, keep overload information and relevant keywords.
	If we cannot find a symbol, search the minimal symbols.

	PR c++/11734
	* linespec.c (decode_compound): Rename SAVED_ARG to
	THE_REAL_SAVED_ARG.
	Make a copy of THE_REAL_SAVED_ARG in SAVED_ARG and strip
	single-quotes.
	Pass a valid block to lookup_symbol.
	(lookup_prefix_sym): Likewise.
	(find_method): Construct search name based on SYM_CLASS instead
	of SAVED_ARG.
	* psymtab.c (lookup_partial_symbol): Add language parameter.
	(lookup_symbol_aux_psymtabs): Likewise.
	Don't assume that the psymtab we found was the right one. Search
	for the desired symbol in the symtab to be certain.
	(psymtab_search_name): New function.
	(lookup_partial_symbol): Use psymtab_search_name.
	Add language parameter.
	(read_symtabs_for_function): Add language parameter and pass to
	lookup_partial_symbol.
	(find_symbol_file_from_partial): Likewise.
	* symfile.h (struct quick_symbol_functions): Add language parameter
	to lookup_symbol, expand_symtabs_for_function, and find_symbol_file.
	* cp-support.c (make_symbol_overload_list): Update above API
	changes.
	* symtab.c (lookup_symbol_aux_quick): Pass the current language
	to the quick symbol functions.
	(basic_lookup_transparent_type_quick): Likewise.
	(find_main_filename): Likewise.
	* dwarf2_read.c (dw2_lookup_symbol): Add langauge parameter.
	(dw2_expand_symtabs_for_function): Likewise.
	(dw2_find_symbol_file): Likewise.

testsuite/ChangeLog
[unchanged from last time]

Keith

Attachment: 11734-12273-3.patch
Description: Text document


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