| Message ID | CAGyQ6gzAMpiA8Hwt1=DqmFxXjfe44nqGsCqsj-fx=ObJbgPAsA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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.
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; + } } }
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.