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

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

[Andreas Arnez]

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(-)

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
-- 
2.17.0

Re: [PATCH] gdb/testsuite: Fix typos in infcall-nested-structs.c

[Tom Tromey]

>>>>> "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

Re: [PATCH] gdb/testsuite: Fix typos in infcall-nested-structs.c

[Andreas Arnez]

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

Re: [PATCH] gdb/testsuite: Fix typos in infcall-nested-structs.c

[Tom de Vries]

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

Re: [PATCH] gdb/testsuite: Fix typos in infcall-nested-structs.c

[Andreas Arnez]

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

Re: [PATCH] gdb/testsuite: Fix typos in infcall-nested-structs.c

[Tom de Vries]

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

Re: [PATCH] gdb/testsuite: Fix typos in infcall-nested-structs.c

[Tom de Vries]

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

Re: [PATCH] gdb/testsuite: Fix typos in infcall-nested-structs.c

[Tom de Vries]

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

Re: [PATCH] gdb/testsuite: Fix typos in infcall-nested-structs.c

[Tom de Vries]

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

[PATCH][gdb/tdep] Fix inferior call arg passing for amd64

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 2780 bytes --]

[ was: Re: [PATCH] gdb/testsuite: Fix typos in infcall-nested-structs.c ]

On 11-10-2019 14:19, Tom de Vries wrote:
> 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
> 

And here's a follow-up patch that fixes PR25096.

OK for trunk?

Thanks,
- Tom

[-- Attachment #2: 0002-gdb-tdep-Fix-inferior-call-arg-passing-for-amd64.patch --]
[-- Type: text/x-patch, Size: 9933 bytes --]

[gdb/tdep] Fix inferior call arg passing for amd64

With "[gdb/tdep] Fix 'Unexpected register class' assert in
amd64_push_arguments" as proposed here (
https://sourceware.org/ml/gdb-patches/2019-10/msg00293.html ) applied, we have
12 KFAILS for PR tdep/25096.

A minimal version of the failure looks like this.  Consider test.c:
...
struct s { int c; struct { int a; float b; } s1; };
struct s ref = { 0, { 'a', 'b' } };

int __attribute__((noinline,noclone)) check (struct s arg)
{ return arg.s1.a == 'a' && arg.s1.b == 'b' && arg.c == 0; }

int main (void)
{ return check (ref); }
...

When calling 'check (ref)' from main, we have '1' as expected:
...
$ g++ test.c -g ; ./a.out ; echo $?
1
...

But when calling 'check (ref)' from the gdb prompt, we get '0':
...
$ gdb a.out -batch -ex start -ex "p check (ref)"
Temporary breakpoint 1 at 0x400518: file test.c, line 8.

Temporary breakpoint 1, main () at test.c:8
8       { return check (ref); }
$1 = 0
...

The layout of struct s is this:
- the field c occupies 4 bytes at offset 0,
- the s1.a field occupies 4 bytes at offset 4, and
- the s1.b field occupies 4 bytes at offset 8.

When compiling at -O2, we can see from the disassembly of main:
...
  4003f0:       48 8b 3d 31 0c 20 00    mov    0x200c31(%rip),%rdi \
                                               # 601028 <ref>
  4003f7:       f3 0f 10 05 31 0c 20    movss  0x200c31(%rip),%xmm0 \
                                               # 601030 <ref+0x8>
  4003fe:       00
  4003ff:       e9 ec 00 00 00          jmpq   4004f0 <_Z5check1s>
...
that check is called with fields c and s1.a passed in %rdi, and s1.b passed
in %xmm0.

However, the classification in theclass (a variable representing the first and
second eightbytes, to put it in SYSV X86_64 psABI terms) in
amd64_push_arguments is incorrect:
...
(gdb) p theclass
$1 = {AMD64_INTEGER, AMD64_INTEGER}
...
and therefore the struct is passed using %rdi and %rsi instead of using %rdi
and %xmm0, which explains the failure.

The reason that we're misclassifying the argument in amd64_classify_aggregate
has to do with how nested struct are handled.

Rather than using fields c and s1.a for the first eightbyte, and using field
s1.b for the second eightbyte, instead field c is used for the first
eightbyte, and fields s1.a and s1.b are classified together in an intermediate
eightbyte, which is then used to merge with both the first and second
eightbyte.

Fix this by factoring out a new function amd64_classify_aggregate_field, and
letting it recursively handle fields of nested structs.

Tested on x86_64-linux.

Tested with g++ 4.8.5, 7.4.1, 8.3.1, 9.2.1.

Tested with clang++ 5.0.2 (which requires removing additional_flags=-Wno-psabi
and adding additional_flags=-Wno-deprecated).

gdb/ChangeLog:

2019-10-11  Tom de Vries  <tdevries@suse.de>

	PR tdep/25096
	* amd64-tdep.c (amd64_classify_aggregate_field): Factor out of ...
	(amd64_classify_aggregate): ... here.
	(amd64_classify_aggregate_field): Handled fiels of nested structs
	recursively.

gdb/testsuite/ChangeLog:

2019-10-11  Tom de Vries  <tdevries@suse.de>

	PR tdep/25096
	* gdb.base/infcall-nested-structs.exp: Remove PR25096 KFAILs.

---
 gdb/amd64-tdep.c                                  | 114 +++++++++++++---------
 gdb/testsuite/gdb.base/infcall-nested-structs.exp |   8 --
 2 files changed, 67 insertions(+), 55 deletions(-)

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 9006ec0167a..67a4d3f11a9 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -579,6 +579,72 @@ amd64_has_unaligned_fields (struct type *type)
   return false;
 }
 
+/* Classify field I of TYPE starting at BITOFFSET according to the rules for
+   structures and union types, and store the result in THECLASS.  */
+
+static void
+amd64_classify_aggregate_field (struct type *type, int i,
+				enum amd64_reg_class theclass[2],
+				unsigned int bitoffset)
+{
+  struct type *subtype = check_typedef (TYPE_FIELD_TYPE (type, i));
+  int bitpos = bitoffset + TYPE_FIELD_BITPOS (type, i);
+  int pos = bitpos / 64;
+  enum amd64_reg_class subclass[2];
+  int bitsize = TYPE_FIELD_BITSIZE (type, i);
+  int endpos;
+
+  if (bitsize == 0)
+    bitsize = TYPE_LENGTH (subtype) * 8;
+  endpos = (bitpos + bitsize - 1) / 64;
+
+  /* Ignore static fields, or empty fields, for example nested
+     empty structures.*/
+  if (field_is_static (&TYPE_FIELD (type, i)) || bitsize == 0)
+    return;
+
+  if (TYPE_CODE (subtype) == TYPE_CODE_STRUCT
+      || TYPE_CODE (subtype) == TYPE_CODE_UNION)
+    {
+      /* Each field of an object is classified recursively.  */
+      int j;
+      for (j = 0; j < TYPE_NFIELDS (subtype); j++)
+	amd64_classify_aggregate_field (subtype, j, theclass, bitpos);
+      return;
+    }
+
+  gdb_assert (pos == 0 || pos == 1);
+
+  amd64_classify (subtype, subclass);
+  theclass[pos] = amd64_merge_classes (theclass[pos], subclass[0]);
+  if (bitsize <= 64 && pos == 0 && endpos == 1)
+    /* This is a bit of an odd case:  We have a field that would
+       normally fit in one of the two eightbytes, except that
+       it is placed in a way that this field straddles them.
+       This has been seen with a structure containing an array.
+
+       The ABI is a bit unclear in this case, but we assume that
+       this field's class (stored in subclass[0]) must also be merged
+       into class[1].  In other words, our field has a piece stored
+       in the second eight-byte, and thus its class applies to
+       the second eight-byte as well.
+
+       In the case where the field length exceeds 8 bytes,
+       it should not be necessary to merge the field class
+       into class[1].  As LEN > 8, subclass[1] is necessarily
+       different from AMD64_NO_CLASS.  If subclass[1] is equal
+       to subclass[0], then the normal class[1]/subclass[1]
+       merging will take care of everything.  For subclass[1]
+       to be different from subclass[0], I can only see the case
+       where we have a SSE/SSEUP or X87/X87UP pair, which both
+       use up all 16 bytes of the aggregate, and are already
+       handled just fine (because each portion sits on its own
+       8-byte).  */
+    theclass[1] = amd64_merge_classes (theclass[1], subclass[0]);
+  if (pos == 0)
+    theclass[1] = amd64_merge_classes (theclass[1], subclass[1]);
+}
+
 /* Classify TYPE according to the rules for aggregate (structures and
    arrays) and union types, and store the result in CLASS.  */
 
@@ -619,53 +685,7 @@ amd64_classify_aggregate (struct type *type, enum amd64_reg_class theclass[2])
 		  || TYPE_CODE (type) == TYPE_CODE_UNION);
 
       for (i = 0; i < TYPE_NFIELDS (type); i++)
-	{
-	  struct type *subtype = check_typedef (TYPE_FIELD_TYPE (type, i));
-	  int pos = TYPE_FIELD_BITPOS (type, i) / 64;
-	  enum amd64_reg_class subclass[2];
-	  int bitsize = TYPE_FIELD_BITSIZE (type, i);
-	  int endpos;
-
-	  if (bitsize == 0)
-	    bitsize = TYPE_LENGTH (subtype) * 8;
-	  endpos = (TYPE_FIELD_BITPOS (type, i) + bitsize - 1) / 64;
-
-	  /* Ignore static fields, or empty fields, for example nested
-	     empty structures.*/
-	  if (field_is_static (&TYPE_FIELD (type, i)) || bitsize == 0)
-	    continue;
-
-	  gdb_assert (pos == 0 || pos == 1);
-
-	  amd64_classify (subtype, subclass);
-	  theclass[pos] = amd64_merge_classes (theclass[pos], subclass[0]);
-	  if (bitsize <= 64 && pos == 0 && endpos == 1)
-	    /* This is a bit of an odd case:  We have a field that would
-	       normally fit in one of the two eightbytes, except that
-	       it is placed in a way that this field straddles them.
-	       This has been seen with a structure containing an array.
-
-	       The ABI is a bit unclear in this case, but we assume that
-	       this field's class (stored in subclass[0]) must also be merged
-	       into class[1].  In other words, our field has a piece stored
-	       in the second eight-byte, and thus its class applies to
-	       the second eight-byte as well.
-
-	       In the case where the field length exceeds 8 bytes,
-	       it should not be necessary to merge the field class
-	       into class[1].  As LEN > 8, subclass[1] is necessarily
-	       different from AMD64_NO_CLASS.  If subclass[1] is equal
-	       to subclass[0], then the normal class[1]/subclass[1]
-	       merging will take care of everything.  For subclass[1]
-	       to be different from subclass[0], I can only see the case
-	       where we have a SSE/SSEUP or X87/X87UP pair, which both
-	       use up all 16 bytes of the aggregate, and are already
-	       handled just fine (because each portion sits on its own
-	       8-byte).  */
-	    theclass[1] = amd64_merge_classes (theclass[1], subclass[0]);
-	  if (pos == 0)
-	    theclass[1] = amd64_merge_classes (theclass[1], subclass[1]);
-	}
+	amd64_classify_aggregate_field (type, i, theclass, 0);
     }
 
   /* 4. Then a post merger cleanup is done:  */
diff --git a/gdb/testsuite/gdb.base/infcall-nested-structs.exp b/gdb/testsuite/gdb.base/infcall-nested-structs.exp
index 957eb31bdc2..982149f42c1 100644
--- a/gdb/testsuite/gdb.base/infcall-nested-structs.exp
+++ b/gdb/testsuite/gdb.base/infcall-nested-structs.exp
@@ -132,10 +132,6 @@ proc run_tests { lang types } {
 	    continue
 	}
 
-	if { $lang == "c++" && $name == "struct_02_01"
-	     && [regexp "^types-(tf-t(c|s|i)|t(c|s|i)-tf)" $types match] } {
-	    setup_kfail gdb/25096 "x86_64-*-linux*"
-	}
 	gdb_test "p/d check_arg_${name} (ref_val_${name})" "= 1"
 
 	set refval [ get_valueof "" "ref_val_${name}" "" ]
@@ -147,10 +143,6 @@ proc run_tests { lang types } {
 	    set answer [ get_valueof "" "rtn_str_${name} ()" "XXXX"]
 	    verbose -log "Answer: ${answer}"
 
-	    if { $lang == "c++" && $name == "struct_02_01"
-		 && [regexp "^types-(tf-t(c|s|i)|t(c|s|i)-tf)" $types match] } {
-		setup_kfail gdb/25096 "x86_64-*-linux*"
-	    }
 	    gdb_assert [string eq ${answer} ${refval}] ${test}
 	} else {
 	    unresolved $test