Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference

Message ID CAGyQ6gy9eObF+29DSibKj4xxiw9NpoWtarVJTtd-j_A7LBnktA@mail.gmail.com
State New, archived
Headers

Commit Message

Siva Chandra Reddy Oct. 16, 2014, 7:01 p.m. UTC
  The patch updated with a test case is attached.

gdb/ChangeLog:

        * gnu-v3-abi.c (gnuv3_pass_by_reference): Call TYPE_TARGET_TYPE
        on the arg type of a constructor only if it is of reference type.

gdb/testsuite/ChangeLog:

        * gdb.cp/non-trivial-retval.cc: Add a test case.
        * gdb.cp/non-trivial-retval.exp: Add a test.
  

Comments

Pedro Alves Oct. 16, 2014, 8:42 p.m. UTC | #1
On 10/16/2014 08:01 PM, Siva Chandra wrote:
> The patch updated with a test case is attached.

Thank you very much.  FAOD, I'm expecting Doug will review this.

Thanks,
Pedro Alves
  
Siva Chandra Reddy Oct. 23, 2014, 7:09 p.m. UTC | #2
On Thu, Oct 16, 2014 at 12:01 PM, Siva Chandra <sivachandra@google.com> wrote:
> The patch updated with a test case is attached.
>
> gdb/ChangeLog:
>
>         * gnu-v3-abi.c (gnuv3_pass_by_reference): Call TYPE_TARGET_TYPE
>         on the arg type of a constructor only if it is of reference type.
>
> gdb/testsuite/ChangeLog:
>
>         * gdb.cp/non-trivial-retval.cc: Add a test case.
>         * gdb.cp/non-trivial-retval.exp: Add a test.

Ping.
  
Doug Evans Oct. 24, 2014, 5:31 a.m. UTC | #3
On Thu, Oct 23, 2014 at 12:09 PM, Siva Chandra <sivachandra@google.com> wrote:
> On Thu, Oct 16, 2014 at 12:01 PM, Siva Chandra <sivachandra@google.com> wrote:
>> The patch updated with a test case is attached.
>>
>> gdb/ChangeLog:
>>
>>         * gnu-v3-abi.c (gnuv3_pass_by_reference): Call TYPE_TARGET_TYPE
>>         on the arg type of a constructor only if it is of reference type.
>>
>> gdb/testsuite/ChangeLog:
>>
>>         * gdb.cp/non-trivial-retval.cc: Add a test case.
>>         * gdb.cp/non-trivial-retval.exp: Add a test.
>
> Ping.

LGTM with one nit.

+class B1
+{
+public:
+  B1 () {}
+  B1 (int i);  /* Put this decl before the copy-ctor decl.  */
+  B1 (const B1 &obj);
+
+  int b1;
+};

Can you elaborate on why "Put this decl before ..."?
  
Siva Chandra Reddy Oct. 24, 2014, 12:56 p.m. UTC | #4
On Thu, Oct 23, 2014 at 10:31 PM, Doug Evans <dje@google.com> wrote:
>>> gdb/ChangeLog:
>>>
>>>         * gnu-v3-abi.c (gnuv3_pass_by_reference): Call TYPE_TARGET_TYPE
>>>         on the arg type of a constructor only if it is of reference type.
>>>
>>> gdb/testsuite/ChangeLog:
>>>
>>>         * gdb.cp/non-trivial-retval.cc: Add a test case.
>>>         * gdb.cp/non-trivial-retval.exp: Add a test.
>
> LGTM with one nit.
>
> +class B1
> +{
> +public:
> +  B1 () {}
> +  B1 (int i);  /* Put this decl before the copy-ctor decl.  */
> +  B1 (const B1 &obj);
> +
> +  int b1;
> +};
>
> Can you elaborate on why "Put this decl before ..."?

Thank you. Pushed after adding a detailed comment:
3433cfa51f6397231ffe2b2c69298eff89179769
  

Patch

diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c
index a6c6f9f..b960aa3 100644
--- a/gdb/gnu-v3-abi.c
+++ b/gdb/gnu-v3-abi.c
@@ -1320,13 +1320,15 @@  gnuv3_pass_by_reference (struct type *type)
 	if (TYPE_NFIELDS (fieldtype) == 2)
 	  {
 	    struct type *arg_type = TYPE_FIELD_TYPE (fieldtype, 1);
-	    struct type *arg_target_type;
 
-	    arg_target_type = check_typedef (TYPE_TARGET_TYPE (arg_type));
+	    if (TYPE_CODE (arg_type) == TYPE_CODE_REF)
+	      {
+		struct type *arg_target_type;
 
-	    if (TYPE_CODE (arg_type) == TYPE_CODE_REF
-		&& class_types_same_p (arg_target_type, type))
-	      return 1;
+	        arg_target_type = check_typedef (TYPE_TARGET_TYPE (arg_type));
+		if (class_types_same_p (arg_target_type, type))
+		  return 1;
+	      }
 	  }
       }
 
diff --git a/gdb/testsuite/gdb.cp/non-trivial-retval.cc b/gdb/testsuite/gdb.cp/non-trivial-retval.cc
index 8382f40..aeb7875 100644
--- a/gdb/testsuite/gdb.cp/non-trivial-retval.cc
+++ b/gdb/testsuite/gdb.cp/non-trivial-retval.cc
@@ -63,6 +63,33 @@  f2 (int i1, int i2)
   return b;
 }
 
+class B1
+{
+public:
+  B1 () {}
+  B1 (int i);  /* Put this decl before the copy-ctor decl.  */
+  B1 (const B1 &obj);
+
+  int b1;
+};
+
+B1::B1 (const B1 &obj)
+{
+  b1 = obj.b1;
+}
+
+B1::B1 (int i) : b1 (i) { }
+
+B1
+f22 (int i1, int i2)
+{
+  B1 b1;
+
+  b1.b1 = i1 + i2;
+
+  return b1;
+}
+
 class C
 {
 public:
diff --git a/gdb/testsuite/gdb.cp/non-trivial-retval.exp b/gdb/testsuite/gdb.cp/non-trivial-retval.exp
index 7934946..3450a94 100644
--- a/gdb/testsuite/gdb.cp/non-trivial-retval.exp
+++ b/gdb/testsuite/gdb.cp/non-trivial-retval.exp
@@ -32,5 +32,6 @@  gdb_continue_to_breakpoint "Break here"
 
 gdb_test "p f1 (i1, i2)" ".* = {a = 123}" "p f1 (i1, i2)"
 gdb_test "p f2 (i1, i2)" ".* = {b = 123}" "p f2 (i1, i2)"
+gdb_test "p f22 (i1, i2)" ".* = {b1 = 123}" "p f22 (i1, i2)"
 gdb_test "p f3 (i1, i2)" ".* = {.* c = 123}" "p f3 (i1, i2)"
 gdb_test "p f4 (i1, i2)" ".* = {.* e = 123}" "p f4 (i1, i2)"