[patch] stabs assertion failure symbol_get_demangled_name

[patch] stabs assertion failure symbol_get_demangled_name

[sami wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 661 bytes --]

Pedro reported a crash of code compiled with stabs on

gdb_assert(gsymbol->language_specific.cplus_specific != NULL);

in symbol_get_demangled_name. The problem was that 
stabsread.c:define_symbol was calling cp_scan_for_anonymous_namespaces 
before setting the name. I corrected that but there other case, and for 
those I changed the assertion into an if statement that returns null 
since symbol_natural_name depends on symbol_get_demangled_name to return 
null in order to fall back to gsymbol->name.

This was tested by running the test suite and compiling
member-ptr with -gstabs breaking on main executing a few commands and 
printing some values.

Sami

[-- Attachment #2: stabs.patch --]
[-- Type: text/plain, Size: 1575 bytes --]

Fixed stabs cplus_specific issue

2010-07-28  Sami Wagiaalla  <swagiaal@redhat.com>

	* symtab.c (symbol_get_demangled_name): Remove assertion and
	return NULL when language_specific.cplus_specific is not initialized.
	* stabsread.c (define_symbol): Set the name before calling 
	cp_scan_for_anonymous_namespaces.
	
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index b62156c..7a68d7a 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -712,7 +712,6 @@ define_symbol (CORE_ADDR valu, char *string, int desc, int type,
 	  memcpy (name, string, p - string);
 	  name[p - string] = '\0';
 	  new_name = cp_canonicalize_string (name);
-	  cp_scan_for_anonymous_namespaces (sym);
 	}
       if (new_name != NULL)
 	{
@@ -721,6 +720,10 @@ define_symbol (CORE_ADDR valu, char *string, int desc, int type,
 	}
       else
 	SYMBOL_SET_NAMES (sym, string, p - string, 1, objfile);
+
+      if (SYMBOL_LANGUAGE (sym) == language_cplus)
+	cp_scan_for_anonymous_namespaces (sym);
+
     }
   p++;
 
diff --git a/gdb/symtab.c b/gdb/symtab.c
index ec0e809..822c89c 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -381,10 +381,10 @@ char *
 symbol_get_demangled_name (const struct general_symbol_info *gsymbol)
 {
   if (gsymbol->language == language_cplus)
-    {
-      gdb_assert (gsymbol->language_specific.cplus_specific != NULL);
+    if (gsymbol->language_specific.cplus_specific != NULL)
       return gsymbol->language_specific.cplus_specific->demangled_name;
-    }
+    else
+      return NULL;
   else
     return gsymbol->language_specific.mangled_lang.demangled_name;
 }

Re: [patch] stabs assertion failure symbol_get_demangled_name

[Pedro Alves]

On Wednesday 28 July 2010 21:16:17, sami wagiaalla wrote:
> Pedro reported a crash of code compiled with stabs on
> 
> gdb_assert(gsymbol->language_specific.cplus_specific != NULL);
> 
> in symbol_get_demangled_name. The problem was that 
> stabsread.c:define_symbol was calling cp_scan_for_anonymous_namespaces 
> before setting the name. 

> I corrected that but there other case, and for 
> those I changed the assertion into an if statement that returns null 
> since symbol_natural_name depends on symbol_get_demangled_name to return 
> null in order to fall back to gsymbol->name.

I tried this myself to check what symbol this was, and why doesn't this
assert trigger with dwarf.  It's a typedef symbol.  The dwarf reader
ends up with a call to symbol_set_demangled_name with a NULL name,
while the stabs reader never calls it, and, symbol_set_demangled_name with
a NULL name always allocates gsymbol->language_specific.cplus_specific even
in that case.

void
symbol_set_demangled_name (struct general_symbol_info *gsymbol,
                           char *name,
                           struct objfile *objfile)
{
  if (gsymbol->language == language_cplus)
    {
      if (gsymbol->language_specific.cplus_specific == NULL)
	symbol_init_cplus_specific (gsymbol, objfile);

      gsymbol->language_specific.cplus_specific->demangled_name = name;
    }
  else
    gsymbol->language_specific.mangled_lang.demangled_name = name;
}

I guess we could avoid a number of allocations if when
(gsymbol->language_specific.cplus_specific == NULL && name == NULL)
is true, we didn't allocate the cplus_specific bit.


> 2010-07-28  Sami Wagiaalla  <swagiaal@redhat.com>
> 
>         * symtab.c (symbol_get_demangled_name): Remove assertion and
>         return NULL when language_specific.cplus_specific is not initialized.
>         * stabsread.c (define_symbol): Set the name before calling 
>         cp_scan_for_anonymous_namespaces.

Okay, thanks.   Small formatting issue pointed out below.

> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -381,10 +381,10 @@ char *
>  symbol_get_demangled_name (const struct general_symbol_info *gsymbol)
>  {
>    if (gsymbol->language == language_cplus)
> -    {
> -      gdb_assert (gsymbol->language_specific.cplus_specific != NULL);
> +    if (gsymbol->language_specific.cplus_specific != NULL)
>        return gsymbol->language_specific.cplus_specific->demangled_name;
> -    }
> +    else
> +      return NULL;
>    else
>      return gsymbol->language_specific.mangled_lang.demangled_name;
>  }

In cases like this (if as only statement of if), it's preferred to
leave the outer {}'s in place, to future proof against dangling
else problems:

    if (gsymbol->language == language_cplus)
      {
        if (gsymbol->language_specific.cplus_specific != NULL)
          return gsymbol->language_specific.cplus_specific->demangled_name;
        else
          return NULL;
      }
    else
      return gsymbol->language_specific.mangled_lang.demangled_name;

-- 
Pedro Alves

Re: [patch] stabs assertion failure symbol_get_demangled_name

[sami wagiaalla]

> ...there other case, and for
> those I changed the assertion into an if statement that returns null
> since symbol_natural_name depends on symbol_get_demangled_name to return
> null in order to fall back to gsymbol->name.
>

I looked into this some more. At least of of the other cases occurs 
when, during symbol construction, the stabs reader uses 
SYMBOL_SET_LINKAGE_NAME bypassing symbol_set_names.

by removing the assertion eventually the name that was set through 
SYMBOL_SET_LINKAGE_NAME (gsymbol->name) will be returned.

The other option is to eliminate the use of SYMBOL_SET_LINKAGE_NAME.