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

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

[Sergio Durigan Junior]

Hi,

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"
 

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

[Sergio Durigan Junior]

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

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

[Tom Tromey]

>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

Sergio> And then, printing the value of `var':
Sergio>     (gdb) print 'foo::bar()::var'
Sergio> GDB would fall in an internal_error:

Sergio> This may not be the ideal solution, according to Keith it would
Sergio> be good to implement productions on c-exp.y in order to
Sergio> recognize CLASS::FUNCTION::VARIABLE, but it is a solution which
Sergio> works with what we have today.

I'm not sure about this idea.  Extensions make me a bit nervous, I
suppose.

Sergio> +      else if (sym->flags & (BSF_GLOBAL | BSF_LOCAL | BSF_WEAK | BSF_GNU_UNIQUE))

I think this line is too long.

Sergio> +# Regression test for PR c++/15203 and PR c++/15210
Sergio> +gdb_test "print 'gnu_obj_1::method()::sintvar'" "\\$\[0-9\]+ = 4" \
Sergio> +	   "simple object, static const int, accessing via 'class::method::variable"

I think the second line should only be indented 4 spaces.

Ok with those changes.

Tom

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

[Sergio Durigan Junior]

On Wednesday, March 13 2013, Tom Tromey wrote:

> Sergio> +      else if (sym->flags & (BSF_GLOBAL | BSF_LOCAL | BSF_WEAK | BSF_GNU_UNIQUE))
>
> I think this line is too long.
>
> Sergio> +# Regression test for PR c++/15203 and PR c++/15210
> Sergio> +gdb_test "print 'gnu_obj_1::method()::sintvar'" "\\$\[0-9\]+ = 4" \
> Sergio> +	   "simple object, static const int, accessing via 'class::method::variable"
>
> I think the second line should only be indented 4 spaces.
>
> Ok with those changes.

Thanks, the patch below is what I have actually committed.

        http://sourceware.org/ml/gdb-cvs/2013-03/msg00130.html

-- 
Sergio

Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.15271
diff -u -r1.15271 ChangeLog
--- gdb/ChangeLog	14 Mar 2013 09:08:55 -0000	1.15271
+++ gdb/ChangeLog	14 Mar 2013 11:09:26 -0000
@@ -1,3 +1,13 @@
+2013-03-14  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-14  Yao Qi  <yao@codesourcery.com>
 
 	* tracepoint.c (tfile_write_status): Write 'stop_desc' of trace
Index: gdb/cp-namespace.c
===================================================================
RCS file: /cvs/src/src/gdb/cp-namespace.c,v
retrieving revision 1.65
diff -u -r1.65 cp-namespace.c
--- gdb/cp-namespace.c	31 Jan 2013 18:37:37 -0000	1.65
+++ gdb/cp-namespace.c	14 Mar 2013 11:09:26 -0000
@@ -812,6 +812,11 @@
 	   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 "
Index: gdb/elfread.c
===================================================================
RCS file: /cvs/src/src/gdb/elfread.c,v
retrieving revision 1.148
diff -u -r1.148 elfread.c
--- gdb/elfread.c	6 Mar 2013 21:20:02 -0000	1.148
+++ gdb/elfread.c	14 Mar 2013 11:09:27 -0000
@@ -357,7 +357,8 @@
 	}
       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 +414,7 @@
 	    }
 	  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 +444,7 @@
 	    }
 	  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)
 		    {
Index: gdb/testsuite/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.3583
diff -u -r1.3583 ChangeLog
--- gdb/testsuite/ChangeLog	14 Mar 2013 09:12:20 -0000	1.3583
+++ gdb/testsuite/ChangeLog	14 Mar 2013 11:09:29 -0000
@@ -1,3 +1,11 @@
+2013-03-14  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'.
+
 2013-03-14  Yao Qi  <yao@codesourcery.com>
 
 	* gdb.trace/tstatus.exp (run_trace_experiment): Save the output
Index: gdb/testsuite/gdb.cp/m-static.cc
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/m-static.cc,v
retrieving revision 1.6
diff -u -r1.6 m-static.cc
--- gdb/testsuite/gdb.cp/m-static.cc	12 Feb 2013 20:27:36 -0000	1.6
+++ gdb/testsuite/gdb.cp/m-static.cc	14 Mar 2013 11:09:29 -0000
@@ -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 @@
 
   long method ()
   {
+    static int sintvar = 4;
     static bool svar = true;
+
     keepalive (&svar);
+    keepalive_int (&sintvar);
     return key2;
   }
 };
Index: gdb/testsuite/gdb.cp/m-static.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/m-static.exp,v
retrieving revision 1.21
diff -u -r1.21 m-static.exp
--- gdb/testsuite/gdb.cp/m-static.exp	12 Feb 2013 20:27:36 -0000	1.21
+++ gdb/testsuite/gdb.cp/m-static.exp	14 Mar 2013 11:09:29 -0000
@@ -52,6 +52,11 @@
 
 # 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"
 

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

[Sergio Durigan Junior]

On Thursday, March 14 2013, I wrote:

>         http://sourceware.org/ml/gdb-cvs/2013-03/msg00130.html

BTW, it was not clear, is this patch approved for 7.6 branch too?

Thanks,

-- 
Sergio

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

[Tom Tromey]

>> http://sourceware.org/ml/gdb-cvs/2013-03/msg00130.html

Sergio> BTW, it was not clear, is this patch approved for 7.6 branch too?

Yes.  It looks safe enough to me.

Tom

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

[Sergio Durigan Junior]

On Thursday, March 14 2013, Tom Tromey wrote:

>>> http://sourceware.org/ml/gdb-cvs/2013-03/msg00130.html
>
> Sergio> BTW, it was not clear, is this patch approved for 7.6 branch too?
>
> Yes.  It looks safe enough to me.

Thanks, committed.

        http://sourceware.org/ml/gdb-cvs/2013-03/msg00160.html

-- 
Sergio