* [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference
@ 2014-10-16 14:38 Siva Chandra
2014-10-16 18:01 ` Pedro Alves
2014-10-16 19:01 ` Siva Chandra
0 siblings, 2 replies; 11+ messages in thread
From: Siva Chandra @ 2014-10-16 14:38 UTC (permalink / raw)
To: Doug Evans, gdb-patches
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
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.
[-- Attachment #2: gnuv3_pass_by_reference_v1.txt --]
[-- Type: text/plain, Size: 781 bytes --]
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;
+ }
}
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference
2014-10-16 14:38 [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference Siva Chandra
@ 2014-10-16 18:01 ` Pedro Alves
2014-10-16 18:10 ` Siva Chandra
2014-10-16 19:01 ` Siva Chandra
1 sibling, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2014-10-16 18:01 UTC (permalink / raw)
To: Siva Chandra, Doug Evans, gdb-patches
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference
2014-10-16 18:01 ` Pedro Alves
@ 2014-10-16 18:10 ` Siva Chandra
2014-10-16 18:16 ` Sergio Durigan Junior
0 siblings, 1 reply; 11+ messages in thread
From: Siva Chandra @ 2014-10-16 18:10 UTC (permalink / raw)
To: Pedro Alves; +Cc: Doug Evans, gdb-patches
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference
2014-10-16 18:10 ` Siva Chandra
@ 2014-10-16 18:16 ` Sergio Durigan Junior
2014-10-16 18:18 ` Siva Chandra
0 siblings, 1 reply; 11+ messages in thread
From: Sergio Durigan Junior @ 2014-10-16 18:16 UTC (permalink / raw)
To: Siva Chandra; +Cc: Pedro Alves, Doug Evans, gdb-patches
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.
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference
2014-10-16 18:16 ` Sergio Durigan Junior
@ 2014-10-16 18:18 ` Siva Chandra
2014-10-16 18:28 ` Sergio Durigan Junior
0 siblings, 1 reply; 11+ messages in thread
From: Siva Chandra @ 2014-10-16 18:18 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: Pedro Alves, Doug Evans, gdb-patches
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference
2014-10-16 18:18 ` Siva Chandra
@ 2014-10-16 18:28 ` Sergio Durigan Junior
0 siblings, 0 replies; 11+ messages in thread
From: Sergio Durigan Junior @ 2014-10-16 18:28 UTC (permalink / raw)
To: Siva Chandra; +Cc: Pedro Alves, Doug Evans, gdb-patches
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.
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference
2014-10-16 14:38 [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference Siva Chandra
2014-10-16 18:01 ` Pedro Alves
@ 2014-10-16 19:01 ` Siva Chandra
2014-10-16 20:42 ` Pedro Alves
2014-10-23 19:09 ` Siva Chandra
1 sibling, 2 replies; 11+ messages in thread
From: Siva Chandra @ 2014-10-16 19:01 UTC (permalink / raw)
To: Doug Evans, gdb-patches; +Cc: Sergio Durigan Junior, Pedro Alves
[-- Attachment #1: Type: text/plain, Size: 348 bytes --]
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.
[-- Attachment #2: gnuv3_pass_by_reference_v2.txt --]
[-- Type: text/plain, Size: 1986 bytes --]
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)"
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference
2014-10-16 19:01 ` Siva Chandra
@ 2014-10-16 20:42 ` Pedro Alves
2014-10-23 19:09 ` Siva Chandra
1 sibling, 0 replies; 11+ messages in thread
From: Pedro Alves @ 2014-10-16 20:42 UTC (permalink / raw)
To: Siva Chandra, Doug Evans, gdb-patches; +Cc: Sergio Durigan Junior
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference
2014-10-16 19:01 ` Siva Chandra
2014-10-16 20:42 ` Pedro Alves
@ 2014-10-23 19:09 ` Siva Chandra
2014-10-24 5:31 ` Doug Evans
1 sibling, 1 reply; 11+ messages in thread
From: Siva Chandra @ 2014-10-23 19:09 UTC (permalink / raw)
To: Doug Evans, gdb-patches
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference
2014-10-23 19:09 ` Siva Chandra
@ 2014-10-24 5:31 ` Doug Evans
2014-10-24 12:56 ` Siva Chandra
0 siblings, 1 reply; 11+ messages in thread
From: Doug Evans @ 2014-10-24 5:31 UTC (permalink / raw)
To: Siva Chandra; +Cc: gdb-patches
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 ..."?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference
2014-10-24 5:31 ` Doug Evans
@ 2014-10-24 12:56 ` Siva Chandra
0 siblings, 0 replies; 11+ messages in thread
From: Siva Chandra @ 2014-10-24 12:56 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches
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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-10-24 12:56 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-16 14:38 [PATCH] Guard a call to TYPE_TARGET_TYPE in gnuv3_pass_by_reference Siva Chandra
2014-10-16 18:01 ` Pedro Alves
2014-10-16 18:10 ` Siva Chandra
2014-10-16 18:16 ` Sergio Durigan Junior
2014-10-16 18:18 ` Siva Chandra
2014-10-16 18:28 ` Sergio Durigan Junior
2014-10-16 19:01 ` Siva Chandra
2014-10-16 20:42 ` Pedro Alves
2014-10-23 19:09 ` Siva Chandra
2014-10-24 5:31 ` Doug Evans
2014-10-24 12:56 ` Siva Chandra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).