Re: [PATCH] Fix for PR c++/15203 and PR c++/15210

[Sergio Durigan Junior <sergiodj at redhat dot com>]

On Saturday, March 09 2013, I wrote:

> Hi,

I was going to ping later, but maybe this patch would be a good
candidate for inclusion in the 7.6 release, so: ping.

> This bug was reported internally at our Bugzilla, along with a proposed
> fix.  After talking to Keith about it, he investigated and came up with
> another patch needed to really fix the issue on CVS HEAD.
>
> The first part of the fix is the patch to cp-namespace.c.  It handles
> the case when we are accessing a static variable inside a function
> (inside a class) by the full linespec (is it right, Keith?).  E.g.:
>
>     class foo
>     {
>     public:
>         int bar()
>         {
>             static int var = 0;
>         }
>     };
>
> And then, printing the value of `var':
>
>     (gdb) print 'foo::bar()::var'
>
> GDB would fall in an internal_error:
>
>     gdb/cp-namespace.c:816: internal-error: cp_lookup_nested_symbol called on a non-aggregate type.
>
> This is because `cp_lookup_nested_symbol' is not handling the case when
> TYPE_CODE is either _FUNC or _METHOD.  This patch fixes it by returning
> NULL in this case.
>
> The second part of the fix is the patch to elfread.c.  It is needed
> because the BSF_GNU_UNIQUE flag was added to some symbols in
> <http://sourceware.org/ml/binutils/2009-06/msg00016.html>.  Because of
> that, (still) the command:
>
>     (gdb) print 'foo::bar()::var'
>
> where `var' is a static variable returns:
>
>     "No symbol "foo::bar()::var" in current context."
>
> So with the second patch applied the command finally DTRT:
>
>     (gdb) print 'foo::bar()::var'
>     $1 = 0
>
> This may not be the ideal solution, according to Keith it would be good
> to implement productions on c-exp.y in order to recognize
> CLASS::FUNCTION::VARIABLE, but it is a solution which works with what we
> have today.
>
> I regtested it in Fedora 17 x86_64 with -m64 and -m32, including
> gdbserver, without regressions.
>
> OK to apply (before the 7.6 release)?
>
> -- 
> Sergio
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index e02f01b..03ba4e2 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,13 @@
> +2013-03-09  Keith Seitz  <keiths@redhat.com>
> +	    Alan Matsuoka  <alanm@redhat.com>
> +
> +	PR c++/15203
> +	PR c++/15210
> +	* cp-namespace.c (cp_lookup_nested_symbol): Handle TYPE_CODE_FUNC and
> +	TYPE_CODE_METHOD.
> +	* elfread.c (elf_symtab_read): Handle BSF_GNU_UNIQUE for certain
> +	symbols.
> +
>  2013-03-08  Yao Qi  <yao@codesourcery.com>
>  
>  	* tracepoint.c (_initialize_tracepoint): Indent the code.
> diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
> index 279021e..add4ccb 100644
> --- a/gdb/cp-namespace.c
> +++ b/gdb/cp-namespace.c
> @@ -812,6 +812,11 @@ cp_lookup_nested_symbol (struct type *parent_type,
>  	   base classes.  */
>  	return find_symbol_in_baseclass (parent_type, nested_name, block);
>        }
> +
> +    case TYPE_CODE_FUNC:
> +    case TYPE_CODE_METHOD:
> +      return NULL;
> +
>      default:
>        internal_error (__FILE__, __LINE__,
>  		      _("cp_lookup_nested_symbol called "
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index 6ab3a6a..3f0c64a 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -357,7 +357,7 @@ elf_symtab_read (struct objfile *objfile, int type,
>  	}
>        else if (sym->flags & BSF_SECTION_SYM)
>  	continue;
> -      else if (sym->flags & (BSF_GLOBAL | BSF_LOCAL | BSF_WEAK))
> +      else if (sym->flags & (BSF_GLOBAL | BSF_LOCAL | BSF_WEAK | BSF_GNU_UNIQUE))
>  	{
>  	  struct minimal_symbol *msym;
>  
> @@ -413,7 +413,7 @@ elf_symtab_read (struct objfile *objfile, int type,
>  	    }
>  	  else if (sym->section->flags & SEC_CODE)
>  	    {
> -	      if (sym->flags & (BSF_GLOBAL | BSF_WEAK))
> +	      if (sym->flags & (BSF_GLOBAL | BSF_WEAK | BSF_GNU_UNIQUE))
>  		{
>  		  if (sym->flags & BSF_GNU_INDIRECT_FUNCTION)
>  		    ms_type = mst_text_gnu_ifunc;
> @@ -443,7 +443,7 @@ elf_symtab_read (struct objfile *objfile, int type,
>  	    }
>  	  else if (sym->section->flags & SEC_ALLOC)
>  	    {
> -	      if (sym->flags & (BSF_GLOBAL | BSF_WEAK))
> +	      if (sym->flags & (BSF_GLOBAL | BSF_WEAK | BSF_GNU_UNIQUE))
>  		{
>  		  if (sym->section->flags & SEC_LOAD)
>  		    {
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 19058dc..58e03ee 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,11 @@
> +2013-03-09  Sergio Durigan Junior  <sergiodj@redhat.com>
> +
> +	PR c++/15203
> +	PR c++/15210
> +	* gdb.cp/m-static.cc (keepalive_int): New function.
> +	(gnu_obj_1::method): New variable `sintvar', call `keepalive_int'.
> +	* gdb.cp/m-static.exp: New test for `sintvar'.
> +
>  2012-03-08  Stan Shebs  <stan@codesourcery.com>
>  	    Hafiz Abid Qadeer  <abidh@codesourcery.com>
>  
> diff --git a/gdb/testsuite/gdb.cp/m-static.cc b/gdb/testsuite/gdb.cp/m-static.cc
> index 8472988..eadbdfc 100644
> --- a/gdb/testsuite/gdb.cp/m-static.cc
> +++ b/gdb/testsuite/gdb.cp/m-static.cc
> @@ -3,6 +3,7 @@
>  enum region { oriental, egyptian, greek, etruscan, roman };
>  
>  void keepalive(bool *var) { }
> +void keepalive_int (int *var) { }
>  
>  // Test one.
>  class gnu_obj_1
> @@ -21,8 +22,11 @@ public:
>  
>    long method ()
>    {
> +    static int sintvar = 4;
>      static bool svar = true;
> +
>      keepalive (&svar);
> +    keepalive_int (&sintvar);
>      return key2;
>    }
>  };
> diff --git a/gdb/testsuite/gdb.cp/m-static.exp b/gdb/testsuite/gdb.cp/m-static.exp
> index ae4b2ad..4624e9e 100644
> --- a/gdb/testsuite/gdb.cp/m-static.exp
> +++ b/gdb/testsuite/gdb.cp/m-static.exp
> @@ -52,6 +52,11 @@ gdb_continue_to_breakpoint "end of constructors"
>  
>  # One.
>  
> +# simple object, static const int, accessing via 'class::method::variable'
> +# Regression test for PR c++/15203 and PR c++/15210
> +gdb_test "print 'gnu_obj_1::method()::sintvar'" "\\$\[0-9\]+ = 4" \
> +	   "simple object, static const int, accessing via 'class::method::variable"
> +
>  # simple object, static const bool
>  gdb_test "print test1.test" "\\$\[0-9\]* = true" "simple object, static const bool"

-- 
Sergio