[RFA] Patch for incorrect handling of references to pointers [pr1147]

[RFA] Patch for incorrect handling of references to pointers [pr1147]

[Paul Pluzhnikov]

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

Greetings,

Attached patch fixes incorrect handling of "this" when reference
to pointer is used:
  http://sourceware.org/cgi-bin/gnatsweb.pl?cmd=view&database=gdb&pr=1147
and adds a test case for it.

Thanks,
-- 
Paul Pluzhnikov


ChangeLog:

2008-05-08  Paul Pluzhnikov  <ppluzhnikov@google.com>

       * gdb/valopts.c (find_overload_match): Handle references
       to pointers.

testsuite/ChangeLog:

2008-05-08  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.cp/call-c.exp: Test for incorrect handling of reference
	to pointer.
	* gdb.cp/call-c.cc: Likewise.

[-- Attachment #2: patch-1147.txt --]
[-- Type: text/plain, Size: 1227 bytes --]

--- gdb/testsuite/gdb.cp/call-c.cc.orig	2008-05-08 14:01:57.000000000 -0700
+++ gdb/testsuite/gdb.cp/call-c.cc	2008-05-08 13:40:09.851165000 -0700
@@ -21,7 +21,18 @@ int func(int x)
    return x;
 }
 
+struct Foo {
+  Foo() : x_(1) { }
+  int func() const { return x_; }
+ private:
+  int x_;
+};
+
 int main()
 {
+    Foo f;
+    Foo *pf = &f;
+    Foo* &rf = pf;
+    rf->func();
     return func(0);
 }
--- gdb/testsuite/gdb.cp/call-c.exp.orig	2008-05-08 14:01:57.000000000 -0700
+++ gdb/testsuite/gdb.cp/call-c.exp	2008-05-08 13:54:14.497857000 -0700
@@ -44,3 +44,6 @@ gdb_load ${binfile}
 runto_main
 
 gdb_test "print foo(1)" "\\\$$decimal = 1"
+send_gdb "next\nnext\nnext\n"
+gdb_expect -re "$gdb_prompt $"
+gdb_test "print rf->func()" "\\\$$decimal = 1"
--- gdb/valops.c.orig	2008-05-08 14:01:57.000000000 -0700
+++ gdb/valops.c	2008-05-08 13:57:14.320875000 -0700
@@ -1919,7 +1919,8 @@ find_overload_match (struct type **arg_t
   if (objp)
     {
       if (TYPE_CODE (value_type (temp)) != TYPE_CODE_PTR
-	  && TYPE_CODE (value_type (*objp)) == TYPE_CODE_PTR)
+	  && (TYPE_CODE (value_type (*objp)) == TYPE_CODE_PTR
+	      || (TYPE_CODE (value_type (*objp)) == TYPE_CODE_REF)))
 	{
 	  temp = value_addr (temp);
 	}

Re: [RFA] Patch for incorrect handling of references to pointers [pr1147]

[Paul Pluzhnikov]

Ping.
http://sourceware.org/ml/gdb-patches/2008-05/msg00289.html

On Thu, May 8, 2008 at 2:18 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> Greetings,
>
> Attached patch fixes incorrect handling of "this" when reference
> to pointer is used:
>  http://sourceware.org/cgi-bin/gnatsweb.pl?cmd=view&database=gdb&pr=1147
> and adds a test case for it.
>
> Thanks,
> --
> Paul Pluzhnikov
>
>
> ChangeLog:
>
> 2008-05-08  Paul Pluzhnikov  <ppluzhnikov@google.com>
>
>       * gdb/valopts.c (find_overload_match): Handle references
>       to pointers.
>
> testsuite/ChangeLog:
>
> 2008-05-08  Paul Pluzhnikov  <ppluzhnikov@google.com>
>
>        * gdb.cp/call-c.exp: Test for incorrect handling of reference
>        to pointer.
>        * gdb.cp/call-c.cc: Likewise.
>



-- 
Paul Pluzhnikov

Re: [RFA] Patch for incorrect handling of references to pointers [pr1147]

[Paul Pluzhnikov]

Ping.
http://sourceware.org/ml/gdb-patches/2008-05/msg00289.html

Also, just discovered that GMail sends base64 attachments even for
plain text :(

Here it is again as plain text.

--
Paul Pluzhnikov

2008-05-08  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb/valopts.c (find_overload_match): Handle references
	to pointers.

testsuite/ChangeLog:

2008-05-08  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.cp/call-c.exp: Test for incorrect handling of reference
	to pointer.
	* gdb.cp/call-c.cc: Likewise.

--- gdb/testsuite/gdb.cp/call-c.cc.orig	2008-05-08 14:01:57.000000000 -0700
+++ gdb/testsuite/gdb.cp/call-c.cc	2008-05-08 13:40:09.851165000 -0700
@@ -21,7 +21,18 @@ int func(int x)
    return x;
 }
 
+struct Foo {
+  Foo() : x_(1) { }
+  int func() const { return x_; }
+ private:
+  int x_;
+};
+
 int main()
 {
+    Foo f;
+    Foo *pf = &f;
+    Foo* &rf = pf;
+    rf->func();
     return func(0);
 }
--- gdb/testsuite/gdb.cp/call-c.exp.orig	2008-05-08 14:01:57.000000000 -0700
+++ gdb/testsuite/gdb.cp/call-c.exp	2008-05-08 13:54:14.497857000 -0700
@@ -44,3 +44,6 @@ gdb_load ${binfile}
 runto_main
 
 gdb_test "print foo(1)" "\\\$$decimal = 1"
+send_gdb "next\nnext\nnext\n"
+gdb_expect -re "$gdb_prompt $"
+gdb_test "print rf->func()" "\\\$$decimal = 1"
--- gdb/valops.c.orig	2008-05-08 14:01:57.000000000 -0700
+++ gdb/valops.c	2008-05-08 13:57:14.320875000 -0700
@@ -1919,7 +1919,8 @@ find_overload_match (struct type **arg_t
   if (objp)
     {
       if (TYPE_CODE (value_type (temp)) != TYPE_CODE_PTR
-	  && TYPE_CODE (value_type (*objp)) == TYPE_CODE_PTR)
+	  && (TYPE_CODE (value_type (*objp)) == TYPE_CODE_PTR
+	      || (TYPE_CODE (value_type (*objp)) == TYPE_CODE_REF)))
 	{
 	  temp = value_addr (temp);
 	}

Re: [RFA] Patch for incorrect handling of references to pointers [pr1147]

[Paul Pluzhnikov]

Ping.
http://sourceware.org/ml/gdb-patches/2008-05/msg00289.html

-- 
Paul Pluzhnikov

Re: [RFA] Patch for incorrect handling of references to pointers [pr1147]

[Daniel Jacobowitz]

On Thu, May 08, 2008 at 02:18:53PM -0700, Paul Pluzhnikov wrote:
> Greetings,
> 
> Attached patch fixes incorrect handling of "this" when reference
> to pointer is used:
>   http://sourceware.org/cgi-bin/gnatsweb.pl?cmd=view&database=gdb&pr=1147
> and adds a test case for it.

Since there's a PR, please mention it in the changelog and commit
message; it'll show up in gnats automatically.  Assuming we eventually
switch to bugzilla, the same thing will work there.  For instance:

2008-05-08  Paul Pluzhnikov  <ppluzhnikov@google.com>

	PR gdb/1147
	* valopts.c (find_overload_match): Handle references
	to pointers.

> --- gdb/testsuite/gdb.cp/call-c.exp.orig	2008-05-08 14:01:57.000000000 -0700
> +++ gdb/testsuite/gdb.cp/call-c.exp	2008-05-08 13:54:14.497857000 -0700
> @@ -44,3 +44,6 @@ gdb_load ${binfile}
>  runto_main
>  
>  gdb_test "print foo(1)" "\\\$$decimal = 1"
> +send_gdb "next\nnext\nnext\n"
> +gdb_expect -re "$gdb_prompt $"
> +gdb_test "print rf->func()" "\\\$$decimal = 1"

This is an unsafe use of send_gdb, because the gdb_expect might match
only the first of three expected prompts depending when there is a
pause in the output.  In general, relying on multiple nexts makes the
test case hard to modify; can you set a breakpoint and use
gdb_get_line_number to find the line to break on?
 
> --- gdb/valops.c.orig	2008-05-08 14:01:57.000000000 -0700
> +++ gdb/valops.c	2008-05-08 13:57:14.320875000 -0700
> @@ -1919,7 +1919,8 @@ find_overload_match (struct type **arg_t
>    if (objp)
>      {
>        if (TYPE_CODE (value_type (temp)) != TYPE_CODE_PTR
> -	  && TYPE_CODE (value_type (*objp)) == TYPE_CODE_PTR)
> +	  && (TYPE_CODE (value_type (*objp)) == TYPE_CODE_PTR
> +	      || (TYPE_CODE (value_type (*objp)) == TYPE_CODE_REF)))

One more set of parens on the last line than you need.

Sorry about the delay.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Patch for incorrect handling of references to pointers [pr1147]

[Paul Pluzhnikov]

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

On Thu, Jun 5, 2008 at 1:37 PM, Daniel Jacobowitz <drow@false.org> wrote:

> This is an unsafe use of send_gdb, because the gdb_expect might match
> only the first of three expected prompts depending when there is a
> pause in the output.

Thanks,

I believe to have fixed all the problems.
New patch attached.

-- 
Paul Pluzhnikov


ChangeLog:

2008-06-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

	PR gdb/1147
	* gdb/valopts.c (find_overload_match): Handle references
	to pointers.

testsuite/ChangeLog:

2008-06-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.cp/call-c.exp: Test for incorrect handling of reference
	to pointer.
	* gdb.cp/call-c.cc: Likewise.

[-- Attachment #2: gdb-patch-1147-20080605.txt --]
[-- Type: text/plain, Size: 1691 bytes --]

ChangeLog:

2008-06-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

	PR gdb/1147
	* gdb/valopts.c (find_overload_match): Handle references
	to pointers.

testsuite/ChangeLog:

2008-06-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.cp/call-c.exp: Test for incorrect handling of reference
	to pointer.
	* gdb.cp/call-c.cc: Likewise.

--- gdb/valops.c.orig	2008-06-05 13:53:24.000000000 -0700
+++ gdb/valops.c	2008-06-05 13:54:03.143468000 -0700
@@ -1972,7 +1972,8 @@ find_overload_match (struct type **arg_t
   if (objp)
     {
       if (TYPE_CODE (value_type (temp)) != TYPE_CODE_PTR
-	  && TYPE_CODE (value_type (*objp)) == TYPE_CODE_PTR)
+	  && (TYPE_CODE (value_type (*objp)) == TYPE_CODE_PTR
+	      || TYPE_CODE (value_type (*objp)) == TYPE_CODE_REF))
 	{
 	  temp = value_addr (temp);
 	}
--- gdb/testsuite/gdb.cp/call-c.cc.orig	2008-05-08 14:01:57.000000000 -0700
+++ gdb/testsuite/gdb.cp/call-c.cc	2008-06-05 17:30:09.009890000 -0700
@@ -21,7 +21,18 @@ int func(int x)
    return x;
 }
 
+struct Foo {
+  Foo() : x_(1) { }
+  int func() const { return x_; }
+ private:
+  int x_;
+};
+
 int main()
 {
+    Foo f;
+    Foo *pf = &f;
+    Foo* &rf = pf;
+    rf->func(); /* set breakpoint here */
     return func(0);
 }
--- gdb/testsuite/gdb.cp/call-c.exp.orig	2008-05-08 14:01:57.000000000 -0700
+++ gdb/testsuite/gdb.cp/call-c.exp	2008-06-05 17:31:19.700025000 -0700
@@ -43,4 +43,9 @@ gdb_load ${binfile}
 
 runto_main
 
+gdb_test "b [gdb_get_line_number {breakpoint here} ${testfile}.cc ]" \
+	 ".*Breakpoint .*call-c.*"
+
 gdb_test "print foo(1)" "\\\$$decimal = 1"
+gdb_test "continue" ".*breakpoint here.*" "continue to bp"
+gdb_test "print rf->func()" "\\\$$decimal = 1"

Re: [RFA] Patch for incorrect handling of references to pointers [pr1147]

[Daniel Jacobowitz]

On Thu, Jun 05, 2008 at 05:52:02PM -0700, Paul Pluzhnikov wrote:
> I believe to have fixed all the problems.
> New patch attached.

Looks just right to me!  Please feel free to commit this (if you
don't have write access, this might be a good time to request it).

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Patch for incorrect handling of references to pointers [pr1147]

[Paul Pluzhnikov]

On Thu, Jun 5, 2008 at 7:35 PM, Daniel Jacobowitz <drow@false.org> wrote:
>
> Looks just right to me!  Please feel free to commit this (if you
> don't have write access, this might be a good time to request it).

Thanks. Committed.

-- 
Paul Pluzhnikov