diff mbox

Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference

Message ID CAGyQ6gzAMpiA8Hwt1=DqmFxXjfe44nqGsCqsj-fx=ObJbgPAsA@mail.gmail.com
State New, archived
Headers

Commit Message

Siva Chandra Reddy Oct. 16, 2014, 2:38 p.m. UTC 
  A call to TYPE_TARGET_TYPE was being done without checking if the type
does have a target type. This was introduced by my commit:
82c48ac732edb0155288a93ef3dd39625ff2d2e1

The attached patch fixes it. This probably qualifies as an obvious
fix, but just in case.

 2014-10-16  Siva Chandra Reddy  <sivachandra@google.com>

        * 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.

Comments

Pedro Alves Oct. 16, 2014, 6:01 p.m. UTC | #1 
Hi Siva,

On 10/16/2014 03:38 PM, Siva Chandra wrote:
> A call to TYPE_TARGET_TYPE was being done without checking if the type
> does have a target type. This was introduced by my commit:
> 82c48ac732edb0155288a93ef3dd39625ff2d2e1
> 
> The attached patch fixes it. This probably qualifies as an obvious
> fix, but just in case.
> 
>  2014-10-16  Siva Chandra Reddy  <sivachandra@google.com>
> 
>         * 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.
> 

How did you notice this?  Does an existing test catch it?

Thanks,
Pedro Alves
Siva Chandra Reddy Oct. 16, 2014, 6:10 p.m. UTC | #2 
On Thu, Oct 16, 2014 at 11:01 AM, Pedro Alves <palves@redhat.com> wrote:
>>  2014-10-16  Siva Chandra Reddy  <sivachandra@google.com>
>>
>>         * 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.
>>
>
> How did you notice this?  Does an existing test catch it?

I hit it while I was "using" GDB so to say :)
I had a class which had a copy constructor as well as another
constructor taking an argument. It fails when going over the other
constructor.

Do you think a test case should be added? I did think about it, but
then, should there be a test case for every use of TYPE_TARGET_TYPE? I
thought it was more of a "user mistake" in my original patch.

Thank you,
Siva Chandra
Sergio Durigan Junior Oct. 16, 2014, 6:15 p.m. UTC | #3 
On Thursday, October 16 2014, Siva Chandra wrote:

> Do you think a test case should be added? I did think about it, but
> then, should there be a test case for every use of TYPE_TARGET_TYPE? I
> thought it was more of a "user mistake" in my original patch.

If I may, I think a testcase should be added for this case, yes.  GDB
should not have a problem because a user mistake.
Siva Chandra Reddy Oct. 16, 2014, 6:18 p.m. UTC | #4 
On Thu, Oct 16, 2014 at 11:15 AM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
> On Thursday, October 16 2014, Siva Chandra wrote:
>
>> Do you think a test case should be added? I did think about it, but
>> then, should there be a test case for every use of TYPE_TARGET_TYPE? I
>> thought it was more of a "user mistake" in my original patch.
>
> If I may, I think a testcase should be added for this case, yes.  GDB
> should not have a problem because a user mistake.

I will add and send an updated patch. The user mistake I am talking
about here is me as a developer using "TYPE_TARGET_TYPE" in GDB source
code.
Sergio Durigan Junior Oct. 16, 2014, 6:28 p.m. UTC | #5 
On Thursday, October 16 2014, Siva Chandra wrote:

> I will add and send an updated patch. The user mistake I am talking
> about here is me as a developer using "TYPE_TARGET_TYPE" in GDB source
> code.

Oh, I see now :-).  Anyway, testcases are always welcome.
diff mbox

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;
+	      }
 	  }
       }