Improve 'this' detection

Improve 'this' detection

[Pedro Alves]

This patch of Daniel's improves THIS detection in C++
method's parameters.  The user visible change is that GDB now
presents the `this' parameter type const qualified.

 (gdb) ptype pmf
 type = int (A::*)(A *, int)

 (gdb) ptype pmf
 type = int (A::*)(A * const, int)

We needed tweaks for GCC and RealView.  For GCC, this works
around GCC PR 43053, where GCC marks THIS as const in
method definitions, but not in the class specifications.
RealView also doesn't mark THIS as const, but it does
output DW_AT_object_pointer, which we can use to cleanly
identify `this'.

I was going to ask if you think we should tweak the testsuite
patch to always expect "const", instead of the current
optional "( const)?", but I remembered not the whole
world is dwarf yet :-)

The patch looked pretty reasonable to me.

Tested on x86_64-linux.

Comments?

-- 
Pedro Alves

2010-07-27  Daniel Jacobowitz  <dan@codesourcery.com>

        gdb/
        * dwarf2read.c (read_subroutine_type): Improve THIS detection,
        handling DW_AT_object_pointer, and workaround GCC PR 43053.

        gdb/testsuite/
        * gdb.cp/member-ptr.exp, gdb.cp/printmethod.exp,
        gdb.dwarf2/member-ptr-forwardref.exp: Adjust.

---
 gdb/dwarf2read.c                                   |   51 ++++++++++++++++++---
 gdb/testsuite/gdb.cp/member-ptr.exp                |   22 ++++-----
 gdb/testsuite/gdb.cp/printmethod.exp               |    4 -
 gdb/testsuite/gdb.dwarf2/member-ptr-forwardref.exp |    2 
 4 files changed, 59 insertions(+), 20 deletions(-)

Index: src/gdb/dwarf2read.c
===================================================================
--- src.orig/gdb/dwarf2read.c	2010-07-27 20:28:25.000000000 +0100
+++ src/gdb/dwarf2read.c	2010-07-27 21:50:37.000000000 +0100
@@ -7319,11 +7319,17 @@ read_subroutine_type (struct die_info *d
 	{
 	  if (child_die->tag == DW_TAG_formal_parameter)
 	    {
-	      /* Dwarf2 has no clean way to discern C++ static and non-static
-	         member functions. G++ helps GDB by marking the first
-	         parameter for non-static member functions (which is the
-	         this pointer) as artificial. We pass this information
-	         to dwarf2_add_member_fn via TYPE_FIELD_ARTIFICIAL.  */
+	      struct type *arg_type;
+
+	      /* DWARF version 2 has no clean way to discern C++
+		 static and non-static member functions.  G++ helps
+		 GDB by marking the first parameter for non-static
+		 member functions (which is the this pointer) as
+		 artificial.  We pass this information to
+		 dwarf2_add_member_fn via TYPE_FIELD_ARTIFICIAL.
+
+		 DWARF version 3 added DW_AT_object_pointer, which GCC
+		 4.5 does not yet generate.  */
 	      attr = dwarf2_attr (child_die, DW_AT_artificial, cu);
 	      if (attr)
 		TYPE_FIELD_ARTIFICIAL (ftype, iparams) = DW_UNSND (attr);
@@ -7341,7 +7347,40 @@ read_subroutine_type (struct die_info *d
 			TYPE_FIELD_ARTIFICIAL (ftype, iparams) = 1;
 		    }
 		}
-	      TYPE_FIELD_TYPE (ftype, iparams) = die_type (child_die, cu);
+	      arg_type = die_type (child_die, cu);
+
+	      /* RealView does not mark THIS as const, which the testsuite
+		 expects.  GCC marks THIS as const in method definitions,
+		 but not in the class specifications (GCC PR 43053).  */
+	      if (cu->language == language_cplus && !TYPE_CONST (arg_type)
+		  && TYPE_FIELD_ARTIFICIAL (ftype, iparams))
+		{
+		  int is_this = 0;
+		  struct dwarf2_cu *arg_cu = cu;
+		  const char *name = dwarf2_name (child_die, cu);
+
+		  attr = dwarf2_attr (die, DW_AT_object_pointer, cu);
+		  if (attr)
+		    {
+		      /* If the compiler emits this, use it.  */
+		      if (follow_die_ref (die, attr, &arg_cu) == child_die)
+			is_this = 1;
+		    }
+		  else if (name && strcmp (name, "this") == 0)
+		    /* Function definitions will have the argument names.  */
+		    is_this = 1;
+		  else if (name == NULL && iparams == 0)
+		    /* Declarations may not have the names, so like
+		       elsewhere in GDB, assume an artificial first
+		       argument is "this".  */
+		    is_this = 1;
+
+		  if (is_this)
+		    arg_type = make_cv_type (1, TYPE_VOLATILE (arg_type),
+					     arg_type, 0);
+		}
+
+	      TYPE_FIELD_TYPE (ftype, iparams) = arg_type;
 	      iparams++;
 	    }
 	  child_die = sibling_die (child_die);
Index: src/gdb/testsuite/gdb.cp/member-ptr.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.cp/member-ptr.exp	2010-07-27 20:28:25.000000000 +0100
+++ src/gdb/testsuite/gdb.cp/member-ptr.exp	2010-07-27 21:25:09.000000000 +0100
@@ -396,7 +396,7 @@ gdb_test_multiple "print ((int) pmi) == 
 
 set name "ptype pmf"
 gdb_test_multiple "ptype pmf" $name {
-    -re "type = int \\( ?A::\\*\\)\\(A \\*, int\\)\r\n$gdb_prompt $" {
+    -re "type = int \\( ?A::\\*\\)\\(A \\*( const)?, int\\)\r\n$gdb_prompt $" {
 	pass $name
     }
     -re "type = int \\( ?A::\\*\\)\\(void\\)\r\n$gdb_prompt $" {
@@ -418,7 +418,7 @@ gdb_test_multiple "ptype pmf" $name {
 
 set name "print pmf"
 gdb_test_multiple "print pmf" $name {
-    -re "$vhn = \\(int \\(A::\\*\\)\\(A \\*, int\\)\\) $hex <A::bar\\(int\\)>\r\n$gdb_prompt $" {
+    -re "$vhn = \\(int \\(A::\\*\\)\\(A \\*( const)?, int\\)\\) $hex <A::bar\\(int\\)>\r\n$gdb_prompt $" {
 	pass $name
     }
     -re "$vhn = .*not supported with HP aCC.*\r\n$gdb_prompt $" {
@@ -440,7 +440,7 @@ gdb_test_multiple "print pmf" $name {
 
 set name "ptype pmf_p"
 gdb_test_multiple "ptype pmf_p" $name {
-    -re "type = int \\( ?A::\\*\\*\\)\\(A \\*, int\\)\r\n$gdb_prompt $" {
+    -re "type = int \\( ?A::\\*\\*\\)\\(A \\*( const)?, int\\)\r\n$gdb_prompt $" {
 	pass $name
     }
     -re "type = int \\( ?A::\\*\\*\\)\\(void\\)\r\n$gdb_prompt $" {
@@ -482,7 +482,7 @@ gdb_test_multiple "print pmf_p" $name {
 
 set name "print a.*pmf"
 gdb_test_multiple "print a.*pmf" $name {
-    -re "$vhn = {int \\(A \\*, int\\)} $hex <A::bar\\(int\\)>\r\n$gdb_prompt $" {
+    -re "$vhn = {int \\(A \\*( const)?, int\\)} $hex <A::bar\\(int\\)>\r\n$gdb_prompt $" {
 	pass $name
     }
     -re "Pointers to methods not supported with HP aCC\r\n$gdb_prompt $" {
@@ -504,7 +504,7 @@ gdb_test_multiple "print a.*pmf" $name {
 
 set name "print a_p->*pmf"
 gdb_test_multiple "print a_p->*pmf" $name {
-    -re "$vhn = {int \\(A \\*, int\\)} $hex <A::bar\\(int\\)>\r\n$gdb_prompt $" {
+    -re "$vhn = {int \\(A \\*( const)?, int\\)} $hex <A::bar\\(int\\)>\r\n$gdb_prompt $" {
 	pass $name
     }
     -re "Pointers to methods not supported with HP aCC\r\n$gdb_prompt $" {
@@ -606,7 +606,7 @@ gdb_test_multiple "print (a.*pmf)(3)" $n
     }
 }
 
-gdb_test "ptype a.*pmf" "type = int \\(A \\*, int\\)"
+gdb_test "ptype a.*pmf" "type = int \\(A \\*( const)?, int\\)"
 
 # Print out a pointer to data member which requires looking into
 # a base class.
@@ -618,9 +618,9 @@ gdb_test "print diamond.*diamond_pmi" "$
 
 # These two have a different object adjustment, but call the same method.
 gdb_test "print diamond.*left_pmf" \
-    "$vhn = {int \\(Diamond \\*\\)} $hex <Base::get_x\\((void|)\\)>"
+    "$vhn = {int \\(Diamond \\*( const)?\\)} $hex <Base::get_x\\((void|)\\)>"
 gdb_test "print diamond.*right_pmf" \
-    "$vhn = {int \\(Diamond \\*\\)} $hex <Base::get_x\\((void|)\\)>"
+    "$vhn = {int \\(Diamond \\*( const)?\\)} $hex <Base::get_x\\((void|)\\)>"
 
 gdb_test "print (diamond.*left_pmf) ()" "$vhn = 77"
 gdb_test "print (diamond.*right_pmf) ()" "$vhn = 88"
@@ -628,9 +628,9 @@ gdb_test "print (diamond.*right_pmf) ()"
 # These two point to different methods, although they have the same
 # virtual table offsets.
 gdb_test "print diamond.*left_vpmf" \
-    "$vhn = {int \\(Diamond \\*\\)} $hex <Left::vget\\((void|)\\)>"
+    "$vhn = {int \\(Diamond \\*( const)?\\)} $hex <Left::vget\\((void|)\\)>"
 gdb_test "print diamond.*right_vpmf" \
-    "$vhn = {int \\(Diamond \\*\\)} $hex <Right::vget\\((void|)\\)>"
+    "$vhn = {int \\(Diamond \\*( const)?\\)} $hex <Right::vget\\((void|)\\)>"
 
 gdb_test "print (diamond.*left_vpmf) ()" "$vhn = 177"
 gdb_test "print (diamond.*left_base_vpmf) ()" "$vhn = 2077"
@@ -658,5 +658,5 @@ gdb_test "print null_pmi = &A::j" "$vhn 
 gdb_test "print null_pmi = 0" "$vhn = NULL"
 
 gdb_test "print null_pmf" "$vhn = NULL"
-gdb_test "print null_pmf = &A::foo" "$vhn = \\(int \\(A::\\*\\)\\(A \\*, int\\)\\) $hex <A::foo ?\\(int\\)>"
+gdb_test "print null_pmf = &A::foo" "$vhn = \\(int \\(A::\\*\\)\\(A \\*( const)?, int\\)\\) $hex <A::foo ?\\(int\\)>"
 gdb_test "print null_pmf = 0" "$vhn = NULL"
Index: src/gdb/testsuite/gdb.cp/printmethod.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.cp/printmethod.exp	2010-07-27 20:28:25.000000000 +0100
+++ src/gdb/testsuite/gdb.cp/printmethod.exp	2010-07-27 21:25:09.000000000 +0100
@@ -62,10 +62,10 @@ gdb_continue_to_breakpoint "end of const
 # The first of these is for PR gdb/653.
 
 gdb_test "print theA->virt" \
-    "\\$\[0-9\]* = {void \\(A \\*\\)} $hex <A::virt\\((void|)\\)>" \
+    "\\$\[0-9\]* = {void \\(A \\* const\\)} $hex <A::virt\\((void|)\\)>" \
     "print virtual method."
 gdb_test "print theA->nonvirt" \
-    "\\$\[0-9\]* = {void \\(A \\*\\)} $hex <A::nonvirt\\((void|)\\)>" \
+    "\\$\[0-9\]* = {void \\(A \\* const\\)} $hex <A::nonvirt\\((void|)\\)>" \
     "print nonvirtual method."
 
 gdb_exit
Index: src/gdb/testsuite/gdb.dwarf2/member-ptr-forwardref.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.dwarf2/member-ptr-forwardref.exp	2010-07-27 20:28:25.000000000 +0100
+++ src/gdb/testsuite/gdb.dwarf2/member-ptr-forwardref.exp	2010-07-27 21:25:09.000000000 +0100
@@ -45,4 +45,4 @@ gdb_test "show cp-abi" {The currently se
 
 gdb_load ${binfile}
 
-gdb_test "ptype c" "type = struct C {\[\r\n \t\]*private:\[\r\n \t\]*int \\(C::\\*fp\\)\\(C \\*\\);\[\r\n \t\]*}"
+gdb_test "ptype c" "type = struct C {\[\r\n \t\]*private:\[\r\n \t\]*int \\(C::\\*fp\\)\\(C \\*( const)?\\);\[\r\n \t\]*}"

Re: Improve 'this' detection

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> This patch of Daniel's improves THIS detection in C++
Pedro> method's parameters.  The user visible change is that GDB now
Pedro> presents the `this' parameter type const qualified.

Pedro> The patch looked pretty reasonable to me.

Pedro> Comments?

Looks reasonable to me too.

Pedro> +		 DWARF version 3 added DW_AT_object_pointer, which GCC
Pedro> +		 4.5 does not yet generate.  */

Do we want GCC to emit this?  If so, I can file a bug.

Tom

Re: Improve 'this' detection

[Daniel Jacobowitz]

On Wed, Jul 28, 2010 at 10:08:45AM -0600, Tom Tromey wrote:
> Pedro> +		 DWARF version 3 added DW_AT_object_pointer, which GCC
> Pedro> +		 4.5 does not yet generate.  */
> 
> Do we want GCC to emit this?  If so, I can file a bug.

IMO, it ought to.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Improve 'this' detection

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <dan@codesourcery.com> writes:

Daniel> On Wed, Jul 28, 2010 at 10:08:45AM -0600, Tom Tromey wrote:
Pedro> +		 DWARF version 3 added DW_AT_object_pointer, which GCC
Pedro> +		 4.5 does not yet generate.  */

Tom> Do we want GCC to emit this?  If so, I can file a bug.

Daniel> IMO, it ought to.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45110

Tom

Re: Improve 'this' detection

[Pedro Alves]

On Wednesday 28 July 2010 17:08:45, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> Pedro> This patch of Daniel's improves THIS detection in C++
> Pedro> method's parameters.  The user visible change is that GDB now
> Pedro> presents the `this' parameter type const qualified.
> 
> Pedro> The patch looked pretty reasonable to me.
> 
> Pedro> Comments?
> 
> Looks reasonable to me too.

Great, thanks.  I've applied it.

-- 
Pedro Alves