Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference
Commit Message
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
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
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
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.
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.
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.
@@ -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;
+ }
}
}