Re: [RFA] c++/11734 revisited

Jan Kratochvil
2011-01-24T17:37:59Z

On Tue, 14 Dec 2010 21:02:48 +0100, Keith Seitz wrote:
> On 12/09/2010 01:44 PM, Tom Tromey wrote:
> >>>>>>"Keith" == Keith Seitz<keiths@redhat.com> writes:
> >The return type of this function should be just "char *".
> >Otherwise you will need casts when freeing.
>
> Okay.

psymtab_search_name still inappropriately returns `const char *'; plus the part
at the caller.

> Revised patch attached.

As a summary:

"physname" patch I call:
42284fdf9d8cdb20c8e833bdbdb2b56977fea525
http://sourceware.org/ml/gdb-cvs/2010-03/msg00082.html
dwarf2_physname patchset:
[RFA] dwarf2_physname FINAL
http://sourceware.org/ml/gdb-patches/2010-03/msg00220.html

Original pre-physname code for pr11734::foo() did not call decode_compound but
called decode_variable which resolved it from the fully qualified name.

post-physname code tried to resolve pr11734::foo() via decode_compound which
did not support such case and it fails now on HEAD.

Alternatively to your patch the current HEAD can be modified to call
decode_variable again as in pre-physname era which resolves it correctly but
for some reason you prefer to resolve pr11734::foo() via decode_compound
instead which makes sense.

> testsuite/ChangeLog
> 2010-12-14 Keith Seitz <keiths@redhat.com>
>
> PR c++/11734
> * gdb.cp/pr11734.exp: New test.
> * gdb.cp/pr11734.h: New file.
> * gdb.cp/pr11734-1.cc: New file.
> * gdb.cp/pr11734-2.cc: New file.
> * gdb.cp/pr11734-3.cc: New file.
> * gdb.cp/pr11734-4.cc: New file.

Please choose arbitrary words instead of prNUMBER for the testcase filename.

> decode_compound (char **argptr, int funfirstline, char ***canonical,
[...]
> + /* If the user specified "'foo::bar(baz)'" (note the quotes -- often
> + added to workaround completer issues) -- saved_arg will be
> + encapsulated in single-quotes. They are superfluous, so just strip
> + them off. */
> + if (*saved_arg == '\'')
> + {
> + char *end = skip_quoted (saved_arg);

Missing empty line (when it has been already patched by Michael Snyder in the
"white space" series).

> + memmove (saved_arg, saved_arg + 1, end - saved_arg);
> + memmove (end - 2, end, strlen (saved_arg) + 1);

Here is overrun as skip_quoted does not need a trailing quote.
For: (gdb) break '.
*******
mudflap violation 16 (check/read): time=1295814377.349729 ptr=0x320e6e2 size=2
pc=0x7f2af65f89a1 location=`(memmove src)'
/usr/lib64/libmudflap.so.0(__mf_check+0x41) [0x7f2af65f89a1]
/usr/lib64/libmudflap.so.0(__mfwrap_memmove+0x170) [0x7f2af65fa1a0]
../gdb() [0x69faa3]
Nearby object 1: checked region begins 2B into and ends 1B after
mudflap object 0x88759c0: name=`alloca region'
bounds=[0x320e6e0,0x320e6e2] size=3 area=heap check=3r/1w liveness=4
alloc time=1295814377.349463 pc=0x7f2af65f80e1
/usr/lib64/libmudflap.so.0(__mf_register+0x41) [0x7f2af65f80e1]
/usr/lib64/libmudflap.so.0(__mf_wrap_alloca_indirect+0x1b5) [0x7f2af65f9c25]
../gdb() [0x69f932]
../gdb(decode_line_1+0x804) [0x69bb1b]
number of nearby objects: 1
saved_arg=<>
*******

> lookup_symbol_aux_psymtabs (struct objfile *objfile,
[...]
> + if (stab->primary)
> + {
> + struct blockvector *bv = BLOCKVECTOR (stab);
> + struct block *block = BLOCKVECTOR_BLOCK (bv, block_index);

Empty line.

> + sym = lookup_block_symbol (block, name, domain);
> + }

> +static const char *

The `const char *' code cleanup discussed above.

> +psymtab_search_name (const char *name, enum language language)
> +{
> + switch (language)
> + {
> + case language_cplus:
> + case language_java:
> + {
> + if (strchr (name, '('))
> + {
> + char *ret = cp_remove_params (name);

Empty line.

> + if (ret)
> + return ret;
> + }
> + }

Missing `break;' here, it is a bit fragile against future modifications.

> +
> + default:
> + break;
> + }
> +
> + return xstrdup (name);
> +}
> +
> /* Look, in partial_symtab PST, for symbol whose natural name is NAME.
> Check the global symbols if GLOBAL, the static symbols if not. */
>
> static struct partial_symbol *
> lookup_partial_symbol (struct partial_symtab *pst, const char *name,
> - int global, domain_enum domain)
> + int global, domain_enum domain, enum language language)
> {
> struct partial_symbol **start, **psym;
> struct partial_symbol **top, **real_top, **bottom, **center;
> int length = (global ? pst->n_global_syms : pst->n_static_syms);
> int do_linear_search = 1;
> + const char *search_name;

The `const char *' code cleanup discussed above.

> + struct cleanup *cleanup;
>
> if (length == 0)
> {
> return (NULL);
> }
> +
> + search_name = psymtab_search_name (name, language);
> + cleanup = make_cleanup (xfree, (void *) search_name);

The `const char *' code cleanup discussed above.

> + Please email any bugs, comments, and/or additions to this file to:
> + bug-gdb@gnu.org */

This part is obsolete (and should be removed in few places).

AFAIK Tom was also reviewing it.

Thanks,
Jan