Re: [RFA] c++/11734 revisited

[Jan Kratochvil <jan.kratochvil@redhat.com>]

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