diff mbox

gdb/testsuite: Fix typos in infcall-nested-structs.c

Message ID m3pnj77a5s.fsf@oc0404454431.ibm.com
State New, archived
Headers

Commit Message

Andreas Arnez Oct. 8, 2019, 11:23 a.m. UTC 
  Some of the comparison functions in infcall-nested-structs.c contain
redundant comparisons like a.<some_field> == a.<some_field> instead of
a.<some_field> == b.<some_field>.  They were introduced with this commit:

  36eb4c5f9bbe6 - "infcall-nested-structs: Test up to five fields"

Fix the redundant comparisons.

gdb/testsuite/ChangeLog:

	* gdb.base/infcall-nested-structs.c (cmp_struct_02_01)
	(cmp_struct_02_02, cmp_struct_04_01, cmp_struct_04_02)
	(cmp_struct_05_01, cmp_struct_static_02_01)
	(cmp_struct_static_04_01, cmp_struct_static_06_01): Fix redundant
	comparisons.
---
 .../gdb.base/infcall-nested-structs.c         | 20 +++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Tom Tromey Oct. 9, 2019, 5:58 p.m. UTC | #1 
>>>>> "Andreas" == Andreas Arnez <arnez@linux.ibm.com> writes:

Andreas> Some of the comparison functions in infcall-nested-structs.c contain
Andreas> redundant comparisons like a.<some_field> == a.<some_field> instead of
Andreas> a.<some_field> == b.<some_field>.  They were introduced with this commit:

Andreas>   36eb4c5f9bbe6 - "infcall-nested-structs: Test up to five fields"

Andreas> Fix the redundant comparisons.

Andreas> gdb/testsuite/ChangeLog:

Andreas> 	* gdb.base/infcall-nested-structs.c (cmp_struct_02_01)
Andreas> 	(cmp_struct_02_02, cmp_struct_04_01, cmp_struct_04_02)
Andreas> 	(cmp_struct_05_01, cmp_struct_static_02_01)
Andreas> 	(cmp_struct_static_04_01, cmp_struct_static_06_01): Fix redundant
Andreas> 	comparisons.

Thank you.  This looks reasonable to me.  I think you could have put it
in under the obvious rule, even.

Tom
Andreas Arnez Oct. 10, 2019, 10:29 a.m. UTC | #2 
On Wed, Oct 09 2019, Tom Tromey wrote:

>>>>>> "Andreas" == Andreas Arnez <arnez@linux.ibm.com> writes:

[...]

> Andreas> gdb/testsuite/ChangeLog:
>
> Andreas> 	* gdb.base/infcall-nested-structs.c (cmp_struct_02_01)
> Andreas> 	(cmp_struct_02_02, cmp_struct_04_01, cmp_struct_04_02)
> Andreas> 	(cmp_struct_05_01, cmp_struct_static_02_01)
> Andreas> 	(cmp_struct_static_04_01, cmp_struct_static_06_01): Fix redundant
> Andreas> 	comparisons.
>
> Thank you.  This looks reasonable to me.  I think you could have put it
> in under the obvious rule, even.

Yeah, this does seem obvious...  Thanks anyway!

Pushed as 6dfc00411292aa3220b33b6ac9553907e3933b0f.

--
Andreas
Tom de Vries Oct. 10, 2019, 3:23 p.m. UTC | #3 
On 10-10-2019 12:29, Andreas Arnez wrote:
> On Wed, Oct 09 2019, Tom Tromey wrote:
> 
>>>>>>> "Andreas" == Andreas Arnez <arnez@linux.ibm.com> writes:
> 
> [...]
> 
>> Andreas> gdb/testsuite/ChangeLog:
>>
>> Andreas> 	* gdb.base/infcall-nested-structs.c (cmp_struct_02_01)
>> Andreas> 	(cmp_struct_02_02, cmp_struct_04_01, cmp_struct_04_02)
>> Andreas> 	(cmp_struct_05_01, cmp_struct_static_02_01)
>> Andreas> 	(cmp_struct_static_04_01, cmp_struct_static_06_01): Fix redundant
>> Andreas> 	comparisons.
>>
>> Thank you.  This looks reasonable to me.  I think you could have put it
>> in under the obvious rule, even.
> 
> Yeah, this does seem obvious...  Thanks anyway!
> 

Hi,

I see these new failures on x86_64-linux:
...
FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-tc-tf: p/d
check_arg_struct_02_01 (ref_val_struct_02_01)
FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ts-tf: p/d
check_arg_struct_02_01 (ref_val_struct_02_01)
FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ti-tf: p/d
check_arg_struct_02_01 (ref_val_struct_02_01)
...

Thanks,
- Tom
Andreas Arnez Oct. 10, 2019, 5:24 p.m. UTC | #4 
On Thu, Oct 10 2019, Tom de Vries wrote:

> I see these new failures on x86_64-linux:
> ...
> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-tc-tf: p/d
> check_arg_struct_02_01 (ref_val_struct_02_01)
> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ts-tf: p/d
> check_arg_struct_02_01 (ref_val_struct_02_01)
> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ti-tf: p/d
> check_arg_struct_02_01 (ref_val_struct_02_01)
> ...

Maybe the test case caught a real bug then, right?  Or do you see a
problem with the test case?

--
Andreas
Tom de Vries Oct. 10, 2019, 6:30 p.m. UTC | #5 
On 10-10-2019 19:24, Andreas Arnez wrote:
> On Thu, Oct 10 2019, Tom de Vries wrote:
> 
>> I see these new failures on x86_64-linux:
>> ...
>> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-tc-tf: p/d
>> check_arg_struct_02_01 (ref_val_struct_02_01)
>> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ts-tf: p/d
>> check_arg_struct_02_01 (ref_val_struct_02_01)
>> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ti-tf: p/d
>> check_arg_struct_02_01 (ref_val_struct_02_01)
>> ...
> 
> Maybe the test case caught a real bug then, right?  Or do you see a
> problem with the test case?

I think it's a real bug.

I've minimized the types-ti-tf FAIL to:
...
$ cat test.c
typedef int ti;
typedef float tf;
struct struct_02_01
{
  struct { } es1;
  struct {
    struct {
      ti a;
      tf b;
    } s1;
  } s2;
};

struct struct_02_01 ref_val_struct_02_01 = {
  {},
  {
    {
      'a',
      'b'
    }
  }
};

int cmp_struct_02_01 (struct struct_02_01 a, struct struct_02_01 b)
{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == b.s2.s1.b; }

int
check_arg_struct_02_01 (struct struct_02_01 arg) {
  return cmp_struct_02_01 (arg, ref_val_struct_02_01);
}

int
main (void)
{
  return check_arg_struct_02_01 (ref_val_struct_02_01);
}
$ g++ test.c -g
$ ./a.out; echo $?
1
$ gdb a.out -batch -ex start -ex "p check_arg_struct_02_01
(ref_val_struct_02_01)"
Temporary breakpoint 1 at 0x400563: file test.c, line 35.

Temporary breakpoint 1, main () at test.c:35
35        return check_arg_struct_02_01 (ref_val_struct_02_01);
$1 = 0
...

Thanks,
- Tom
Tom de Vries Oct. 10, 2019, 8:26 p.m. UTC | #6 
On 10-10-2019 20:30, Tom de Vries wrote:
> On 10-10-2019 19:24, Andreas Arnez wrote:
>> On Thu, Oct 10 2019, Tom de Vries wrote:
>>
>>> I see these new failures on x86_64-linux:
>>> ...
>>> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-tc-tf: p/d
>>> check_arg_struct_02_01 (ref_val_struct_02_01)
>>> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ts-tf: p/d
>>> check_arg_struct_02_01 (ref_val_struct_02_01)
>>> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ti-tf: p/d
>>> check_arg_struct_02_01 (ref_val_struct_02_01)
>>> ...
>>
>> Maybe the test case caught a real bug then, right?  Or do you see a
>> problem with the test case?
> 
> I think it's a real bug.
> 
> I've minimized the types-ti-tf FAIL to:
> ...
> $ cat test.c
> typedef int ti;
> typedef float tf;
> struct struct_02_01
> {
>   struct { } es1;
>   struct {
>     struct {
>       ti a;
>       tf b;
>     } s1;
>   } s2;
> };
> 
> struct struct_02_01 ref_val_struct_02_01 = {
>   {},
>   {
>     {
>       'a',
>       'b'
>     }
>   }
> };
> 
> int cmp_struct_02_01 (struct struct_02_01 a, struct struct_02_01 b)
> { return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == b.s2.s1.b; }
> 
> int
> check_arg_struct_02_01 (struct struct_02_01 arg) {
>   return cmp_struct_02_01 (arg, ref_val_struct_02_01);
> }
> 
> int
> main (void)
> {
>   return check_arg_struct_02_01 (ref_val_struct_02_01);
> }
> $ g++ test.c -g
> $ ./a.out; echo $?
> 1
> $ gdb a.out -batch -ex start -ex "p check_arg_struct_02_01
> (ref_val_struct_02_01)"
> Temporary breakpoint 1 at 0x400563: file test.c, line 35.
> 
> Temporary breakpoint 1, main () at test.c:35
> 35        return check_arg_struct_02_01 (ref_val_struct_02_01);
> $1 = 0
> ...
> 

The discrepancy is that the code generated by gcc passes the struct in
registers %rdi and %xmm0, but amd64_push_arguments classifies the struct as:
...
(gdb) p theclass
$57 = {AMD64_INTEGER, AMD64_INTEGER}
...
and therefore passes it in %rdi and %rsi.

Thanks,
- Tom
Tom de Vries Oct. 10, 2019, 9:07 p.m. UTC | #7 
On 10-10-2019 22:26, Tom de Vries wrote:
> On 10-10-2019 20:30, Tom de Vries wrote:
>> On 10-10-2019 19:24, Andreas Arnez wrote:
>>> On Thu, Oct 10 2019, Tom de Vries wrote:
>>>
>>>> I see these new failures on x86_64-linux:
>>>> ...
>>>> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-tc-tf: p/d
>>>> check_arg_struct_02_01 (ref_val_struct_02_01)
>>>> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ts-tf: p/d
>>>> check_arg_struct_02_01 (ref_val_struct_02_01)
>>>> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ti-tf: p/d
>>>> check_arg_struct_02_01 (ref_val_struct_02_01)
>>>> ...
>>>
>>> Maybe the test case caught a real bug then, right?  Or do you see a
>>> problem with the test case?
>>
>> I think it's a real bug.
>>
>> I've minimized the types-ti-tf FAIL to:
>> ...
>> $ cat test.c
>> typedef int ti;
>> typedef float tf;
>> struct struct_02_01
>> {
>>   struct { } es1;
>>   struct {
>>     struct {
>>       ti a;
>>       tf b;
>>     } s1;
>>   } s2;
>> };
>>
>> struct struct_02_01 ref_val_struct_02_01 = {
>>   {},
>>   {
>>     {
>>       'a',
>>       'b'
>>     }
>>   }
>> };
>>
>> int cmp_struct_02_01 (struct struct_02_01 a, struct struct_02_01 b)
>> { return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == b.s2.s1.b; }
>>
>> int
>> check_arg_struct_02_01 (struct struct_02_01 arg) {
>>   return cmp_struct_02_01 (arg, ref_val_struct_02_01);
>> }
>>
>> int
>> main (void)
>> {
>>   return check_arg_struct_02_01 (ref_val_struct_02_01);
>> }
>> $ g++ test.c -g
>> $ ./a.out; echo $?
>> 1
>> $ gdb a.out -batch -ex start -ex "p check_arg_struct_02_01
>> (ref_val_struct_02_01)"
>> Temporary breakpoint 1 at 0x400563: file test.c, line 35.
>>
>> Temporary breakpoint 1, main () at test.c:35
>> 35        return check_arg_struct_02_01 (ref_val_struct_02_01);
>> $1 = 0
>> ...
>>
> 
> The discrepancy is that the code generated by gcc passes the struct in
> registers %rdi and %xmm0, but amd64_push_arguments classifies the struct as:
> ...
> (gdb) p theclass
> $57 = {AMD64_INTEGER, AMD64_INTEGER}
> ...
> and therefore passes it in %rdi and %rsi.
> 

I've simplified the test-case a bit further, and filed as:
https://sourceware.org/bugzilla/show_bug.cgi?id=25096.

Thanks,
- Tom
Tom de Vries Oct. 11, 2019, 12:19 p.m. UTC | #8 
On 10-10-2019 23:07, Tom de Vries wrote:
> On 10-10-2019 22:26, Tom de Vries wrote:
>> On 10-10-2019 20:30, Tom de Vries wrote:
>>> On 10-10-2019 19:24, Andreas Arnez wrote:
>>>> On Thu, Oct 10 2019, Tom de Vries wrote:
>>>>
>>>>> I see these new failures on x86_64-linux:
>>>>> ...
>>>>> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-tc-tf: p/d
>>>>> check_arg_struct_02_01 (ref_val_struct_02_01)
>>>>> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ts-tf: p/d
>>>>> check_arg_struct_02_01 (ref_val_struct_02_01)
>>>>> FAIL: gdb.base/infcall-nested-structs.exp: l=c++: types-ti-tf: p/d
>>>>> check_arg_struct_02_01 (ref_val_struct_02_01)
>>>>> ...
>>>>
>>>> Maybe the test case caught a real bug then, right?  Or do you see a
>>>> problem with the test case?
>>>
>>> I think it's a real bug.
>>>
>>> I've minimized the types-ti-tf FAIL to:
>>> ...
>>> $ cat test.c
>>> typedef int ti;
>>> typedef float tf;
>>> struct struct_02_01
>>> {
>>>   struct { } es1;
>>>   struct {
>>>     struct {
>>>       ti a;
>>>       tf b;
>>>     } s1;
>>>   } s2;
>>> };
>>>
>>> struct struct_02_01 ref_val_struct_02_01 = {
>>>   {},
>>>   {
>>>     {
>>>       'a',
>>>       'b'
>>>     }
>>>   }
>>> };
>>>
>>> int cmp_struct_02_01 (struct struct_02_01 a, struct struct_02_01 b)
>>> { return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == b.s2.s1.b; }
>>>
>>> int
>>> check_arg_struct_02_01 (struct struct_02_01 arg) {
>>>   return cmp_struct_02_01 (arg, ref_val_struct_02_01);
>>> }
>>>
>>> int
>>> main (void)
>>> {
>>>   return check_arg_struct_02_01 (ref_val_struct_02_01);
>>> }
>>> $ g++ test.c -g
>>> $ ./a.out; echo $?
>>> 1
>>> $ gdb a.out -batch -ex start -ex "p check_arg_struct_02_01
>>> (ref_val_struct_02_01)"
>>> Temporary breakpoint 1 at 0x400563: file test.c, line 35.
>>>
>>> Temporary breakpoint 1, main () at test.c:35
>>> 35        return check_arg_struct_02_01 (ref_val_struct_02_01);
>>> $1 = 0
>>> ...
>>>
>>
>> The discrepancy is that the code generated by gcc passes the struct in
>> registers %rdi and %xmm0, but amd64_push_arguments classifies the struct as:
>> ...
>> (gdb) p theclass
>> $57 = {AMD64_INTEGER, AMD64_INTEGER}
>> ...
>> and therefore passes it in %rdi and %rsi.
>>
> 
> I've simplified the test-case a bit further, and filed as:
> https://sourceware.org/bugzilla/show_bug.cgi?id=25096.
> 

I've submitted a fix for PR24104 (
https://sourceware.org/ml/gdb-patches/2019-10/msg00293.html ) which
marks these 3 FAILs as KFAILs.

Thanks,
- Tom
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.base/infcall-nested-structs.c b/gdb/testsuite/gdb.base/infcall-nested-structs.c
index b6f793e7a3..795ab4a3f9 100644
--- a/gdb/testsuite/gdb.base/infcall-nested-structs.c
+++ b/gdb/testsuite/gdb.base/infcall-nested-structs.c
@@ -141,7 +141,7 @@  int cmp_struct_01_04 (struct struct_01_04 a, struct struct_01_04 b)
 { return a.a == b.a; }
 
 int cmp_struct_02_01 (struct struct_02_01 a, struct struct_02_01 b)
-{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == a.s2.s1.b; }
+{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == b.s2.s1.b; }
 
 int cmp_struct_02_02 (struct struct_02_02 a, struct struct_02_02 b)
 { return a.a == b.a && a.b == b.b; }
@@ -153,8 +153,8 @@  int cmp_struct_02_04 (struct struct_02_04 a, struct struct_02_04 b)
 { return a.a == b.a && a.b == b.b; }
 
 int cmp_struct_04_01 (struct struct_04_01 a, struct struct_04_01 b)
-{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == a.s2.s1.b
-	 && a.s2.s1.c == b.s2.s1.c && a.s2.s1.d == a.s2.s1.d; }
+{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == b.s2.s1.b
+	 && a.s2.s1.c == b.s2.s1.c && a.s2.s1.d == b.s2.s1.d; }
 
 int cmp_struct_04_02 (struct struct_04_02 a, struct struct_04_02 b)
 { return a.a == b.a && a.b == b.b && a.c == b.c && a.d == b.d; }
@@ -167,8 +167,8 @@  int cmp_struct_04_04 (struct struct_04_04 a, struct struct_04_04 b)
 { return a.a == b.a && a.b == b.b && a.c == b.c && a.d == b.d; }
 
 int cmp_struct_05_01 (struct struct_05_01 a, struct struct_05_01 b)
-{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == a.s2.s1.b
-	 && a.s2.s1.c == b.s2.s1.c && a.s2.s1.d == a.s2.s1.d
+{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == b.s2.s1.b
+	 && a.s2.s1.c == b.s2.s1.c && a.s2.s1.d == b.s2.s1.d
 	 && a.s2.s1.e == b.s2.s1.e; }
 
 int cmp_struct_05_02 (struct struct_05_02 a, struct struct_05_02 b)
@@ -187,7 +187,7 @@  int cmp_struct_05_04 (struct struct_05_04 a, struct struct_05_04 b)
 int
 cmp_struct_static_02_01 (struct struct_static_02_01 a,
 			 struct struct_static_02_01 b)
-{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == a.s2.s1.b; }
+{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == b.s2.s1.b; }
 
 int
 cmp_struct_static_02_02 (struct struct_static_02_02 a,
@@ -207,8 +207,8 @@  cmp_struct_static_02_04 (struct struct_static_02_04 a,
 int
 cmp_struct_static_04_01 (struct struct_static_04_01 a,
 			 struct struct_static_04_01 b)
-{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == a.s2.s1.b
-	 && a.s2.s1.c == b.s2.s1.c && a.s2.s1.d == a.s2.s1.d; }
+{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == b.s2.s1.b
+	 && a.s2.s1.c == b.s2.s1.c && a.s2.s1.d == b.s2.s1.d; }
 
 int
 cmp_struct_static_04_02 (struct struct_static_04_02 a,
@@ -229,8 +229,8 @@  cmp_struct_static_04_04 (struct struct_static_04_04 a,
 int
 cmp_struct_static_06_01 (struct struct_static_06_01 a,
 			 struct struct_static_06_01 b)
-{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == a.s2.s1.b
-	 && a.s2.s1.c == b.s2.s1.c && a.s2.s1.d == a.s2.s1.d
+{ return a.s2.s1.a == b.s2.s1.a && a.s2.s1.b == b.s2.s1.b
+	 && a.s2.s1.c == b.s2.s1.c && a.s2.s1.d == b.s2.s1.d
 	 && a.s2.s1.e == b.s2.s1.e && a.f == b.f; }
 
 int