[PATCH v2 1/2] Fix function argument and return value locations

[PATCH v2 1/2] Fix function argument and return value locations

[Hannes Domani]

Fixes these testsuite fails on Windows:
FAIL: gdb.base/callfuncs.exp: p t_float_complex_values(fc1, fc2)
FAIL: gdb.base/callfuncs.exp: p t_float_complex_many_args(fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4)
FAIL: gdb.base/callfuncs.exp: noproto: p t_float_complex_values(fc1, fc2)
FAIL: gdb.base/callfuncs.exp: noproto: p t_float_complex_many_args(fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4)
FAIL: gdb.base/call-sc.exp: p/c fun(); call call-sc-tld
FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tld
FAIL: gdb.base/call-sc.exp: zed L for return; return call-sc-tld
FAIL: gdb.base/call-sc.exp: return foo; return call-sc-tld
FAIL: gdb.base/call-sc.exp: return foo; synchronize pc to main() for 'call-sc-tld'
FAIL: gdb.base/call-sc.exp: return foo; synchronize pc to main() for 'call-sc-tld'
FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tld
FAIL: gdb.base/call-sc.exp: zed L for finish; return call-sc-tld
FAIL: gdb.base/call-sc.exp: finish foo; return call-sc-tld (the program is no longer running)
FAIL: gdb.base/call-sc.exp: value foo finished; return call-sc-tld

For function arguments (callfuncs.exp), only TYPE_CODE_COMPLEX was
missing in the types passed via integer registers.

For return values, there were a lot more issues:
- TYPE_CODE_DECFLOAT is NOT returned via XMM0.
- long double is NOT returned via XMM0.
- but __int128 IS returned via XMM0.
- the comments for TYPE_CODE_FLT state that __m128, __m128i and __m128d are
  returned by XMM0, and this is correct, but it doesn't actually check for
  them, because they are TYPE_CODE_ARRAY with TYPE_VECTOR

So I had to add TYPE_CODE_DECFLOAT to the arguments passed via XMM register,
but I had to remove it from the values returned via XMM0 register.

gdb/ChangeLog:

2020-05-29  Hannes Domani  <ssbssa@yahoo.de>

	* amd64-windows-tdep.c (amd64_windows_passed_by_integer_register):
	Add TYPE_CODE_COMPLEX.
	(amd64_windows_return_value): Fix types returned via XMM0.

gdb/testsuite/ChangeLog:

2020-05-29  Hannes Domani  <ssbssa@yahoo.de>

	* gdb.base/call-sc.c: Fix return struct on stack test case.
	* gdb.base/call-sc.exp: Likewise.
---
v2:
- TYPE_CODE_COMPLEX is actually passed via integer register, not XMM
  register.
---
 gdb/amd64-windows-tdep.c           | 21 +++++++++++++++++----
 gdb/testsuite/gdb.base/call-sc.c   |  6 +++++-
 gdb/testsuite/gdb.base/call-sc.exp | 13 +++++++++++++
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 487dfd45fc..e83d76257f 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -60,6 +60,7 @@ amd64_windows_passed_by_integer_register (struct type *type)
       case TYPE_CODE_RVALUE_REF:
       case TYPE_CODE_STRUCT:
       case TYPE_CODE_UNION:
+      case TYPE_CODE_COMPLEX:
 	return (TYPE_LENGTH (type) == 1
 		|| TYPE_LENGTH (type) == 2
 		|| TYPE_LENGTH (type) == 4
@@ -299,17 +300,29 @@ amd64_windows_return_value (struct gdbarch *gdbarch, struct value *function,
   switch (type->code ())
     {
       case TYPE_CODE_FLT:
-      case TYPE_CODE_DECFLOAT:
-        /* __m128, __m128i, __m128d, floats, and doubles are returned
-           via XMM0.  */
-        if (len == 4 || len == 8 || len == 16)
+        /* floats, and doubles are returned via XMM0.  */
+        if (len == 4 || len == 8)
           regnum = AMD64_XMM0_REGNUM;
         break;
+      case TYPE_CODE_ARRAY:
+        /* __m128, __m128i and __m128d are returned via XMM0.  */
+	if (TYPE_VECTOR (type) && len == 16)
+	  {
+	    enum type_code code = TYPE_TARGET_TYPE (type)->code ();
+	    if (code == TYPE_CODE_INT || code == TYPE_CODE_FLT)
+	      {
+		regnum = AMD64_XMM0_REGNUM;
+		break;
+	      }
+	  }
+	/* fall through */
       default:
         /* All other values that are 1, 2, 4 or 8 bytes long are returned
            via RAX.  */
         if (len == 1 || len == 2 || len == 4 || len == 8)
           regnum = AMD64_RAX_REGNUM;
+	else if (len == 16 && type->code () == TYPE_CODE_INT)
+	  regnum = AMD64_XMM0_REGNUM;
         break;
     }
 
diff --git a/gdb/testsuite/gdb.base/call-sc.c b/gdb/testsuite/gdb.base/call-sc.c
index ba80576ac3..eb140cd9cf 100644
--- a/gdb/testsuite/gdb.base/call-sc.c
+++ b/gdb/testsuite/gdb.base/call-sc.c
@@ -35,6 +35,7 @@ typedef t T;
 #endif
 
 T foo = '1', L;
+T init = '9';
 
 T fun()
 {
@@ -55,7 +56,10 @@ int main()
 {
   int i;
 
-  Fun(foo);	
+  /* Use a different initial value then is later used in the
+     "value foo returned" test, so in case the struct is then returned
+     on the stack, it doesn't have the correct value by accident.  */
+  Fun(init);
 
   /* An infinite loop that first clears all the variables and then
      calls the function.  This "hack" is to make re-testing easier -
diff --git a/gdb/testsuite/gdb.base/call-sc.exp b/gdb/testsuite/gdb.base/call-sc.exp
index 719000435a..9697c5ac24 100644
--- a/gdb/testsuite/gdb.base/call-sc.exp
+++ b/gdb/testsuite/gdb.base/call-sc.exp
@@ -291,6 +291,19 @@ proc test_scalar_returns { } {
 		fail "${test}"
 	    }
 	}
+	-re " = 57 .*${gdb_prompt} $" {
+	    if $return_value_unknown {
+		# The struct return case.
+		# The return value is stored on the stack, and since GDB
+		# didn't override it, it still has value that was stored
+		# there in the earlier Foo(init) call.
+		pass "${test}"
+	    } else {
+		# This contradicts the above claim that GDB knew
+		# the location of the return-value.
+		fail "${test}"
+	    }
+	}
 	-re ".*${gdb_prompt} $" {
 	    if $return_value_unimplemented {
 		# What a suprize.  The architecture hasn't implemented
-- 
2.26.2

[PATCH 2/2] Handle Windows drives in auto-load script paths

[Hannes Domani]

Fixes this testsuite fail on Windows:
FAIL: gdb.base/auto-load.exp: print $script_loaded

Converts the debugfile path from c:/dir/file to /c/dir/file, so it can be
appended to the auto-load path.

gdb/ChangeLog:

2020-05-29  Hannes Domani  <ssbssa@yahoo.de>

	* auto-load.c (auto_load_objfile_script_1): Convert drive part
	of debugfile path on Windows.

gdb/doc/ChangeLog:

2020-05-29  Hannes Domani  <ssbssa@yahoo.de>

	* gdb.texinfo: Document Windows drive conversion of
	'set auto-load scripts-directory'.
---
v2:
- Document Windows drive conversion of 'set auto-load scripts-directory'.
---
 gdb/auto-load.c     | 7 +++++++
 gdb/doc/gdb.texinfo | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 99bd96b971..88221d9f3d 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -784,6 +784,13 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
 					  "scripts-directory' path \"%s\".\n"),
 			    auto_load_dir);
 
+      /* Convert Windows debugfile path from c:/dir/file to /c/dir/file.  */
+      if (HAS_DRIVE_SPEC (debugfile))
+	{
+	  debugfile_holder = STRIP_DRIVE_SPEC (debugfile);
+	  filename = std::string("/") + debugfile[0] + debugfile_holder;
+	}
+
       for (const gdb::unique_xmalloc_ptr<char> &dir : vec)
 	{
 	  /* FILENAME is absolute, so we don't need a "/" here.  */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 292fd4d14e..f31d7013b9 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -27266,6 +27266,10 @@ script in the specified extension language.
 
 If this file does not exist, then @value{GDBN} will look for
 @var{script-name} file in all of the directories as specified below.
+(On MS-Windows/MS-DOS, the drive letter of the executable's leading
+directories is converted to a one-letter subdirectory, i.e.@:
+@file{d:/usr/bin/} is converted to @file{/d/usr/bin/}, because Windows
+filesystems disallow colons in file names.)
 
 Note that loading of these files requires an accordingly configured
 @code{auto-load safe-path} (@pxref{Auto-loading safe path}).
-- 
2.26.2

Re: [PATCH 2/2] Handle Windows drives in auto-load script paths

[Eli Zaretskii]

> Date: Fri, 29 May 2020 17:08:00 +0200
> From: Hannes Domani via Gdb-patches <gdb-patches@sourceware.org>
> 
> gdb/ChangeLog:
> 
> 2020-05-29  Hannes Domani  <ssbssa@yahoo.de>
> 
> 	* auto-load.c (auto_load_objfile_script_1): Convert drive part
> 	of debugfile path on Windows.
> 
> gdb/doc/ChangeLog:
> 
> 2020-05-29  Hannes Domani  <ssbssa@yahoo.de>
> 
> 	* gdb.texinfo: Document Windows drive conversion of
> 	'set auto-load scripts-directory'.

The documentation part is okay.

Thanks.

Re: [PATCH 2/2] Handle Windows drives in auto-load script paths

[Simon Marchi]

Hi Hannes,

On 2020-05-29 11:08 a.m., Hannes Domani via Gdb-patches wrote:
> Fixes this testsuite fail on Windows:
> FAIL: gdb.base/auto-load.exp: print $script_loaded
>
> Converts the debugfile path from c:/dir/file to /c/dir/file, so it can be
> appended to the auto-load path.
>
> gdb/ChangeLog:
>
> 2020-05-29  Hannes Domani  <ssbssa@yahoo.de>
>
> 	* auto-load.c (auto_load_objfile_script_1): Convert drive part
> 	of debugfile path on Windows.
>
> gdb/doc/ChangeLog:
>
> 2020-05-29  Hannes Domani  <ssbssa@yahoo.de>
>
> 	* gdb.texinfo: Document Windows drive conversion of
> 	'set auto-load scripts-directory'.
> ---
> v2:
> - Document Windows drive conversion of 'set auto-load scripts-directory'.
> ---
>  gdb/auto-load.c     | 7 +++++++
>  gdb/doc/gdb.texinfo | 4 ++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/gdb/auto-load.c b/gdb/auto-load.c
> index 99bd96b971..88221d9f3d 100644
> --- a/gdb/auto-load.c
> +++ b/gdb/auto-load.c
> @@ -784,6 +784,13 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
>  					  "scripts-directory' path \"%s\".\n"),
>  			    auto_load_dir);
>
> +      /* Convert Windows debugfile path from c:/dir/file to /c/dir/file.  */
> +      if (HAS_DRIVE_SPEC (debugfile))
> +	{
> +	  debugfile_holder = STRIP_DRIVE_SPEC (debugfile);

I looked at this patch a bit before realizing it is already merged.  I
was going to make a comment about the line above, so I made it a patch
instead, WDYT?


From 602e996b26d592ecda500ed64adafac30e0e9ce7 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Sun, 4 Oct 2020 22:00:17 -0400
Subject: [PATCH] gdb: avoid unnecessary string copy in
 auto_load_objfile_script_1

Assigning the result of STRIP_DRIVE_SPEC to an std::string creates an
unnecessary copy of the string.  STRIP_DRIVE_SPEC is defined as:

  #define STRIP_DRIVE_SPEC(f) ((f) + 2)

So if it is passed a "const char *", it returns a "const char *".  We
could use a "const char *" intermediary variable instead of an
std::string, or (as implemented in this patch) just use it directly in
the concatenation right after.

gdb/ChangeLog:

	* auto-load.c (auto_load_objfile_script_1): Don't use
	debugfile_holder as temporary variable when stripping drive
	letter.

Change-Id: If2ccc7a156b22100754d9cdf6778ac7eeb93da4c
---
 gdb/auto-load.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 9a51d2f3dc6c..43d007ca5b03 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -777,10 +777,8 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,

       /* Convert Windows file name from c:/dir/file to /c/dir/file.  */
       if (HAS_DRIVE_SPEC (debugfile))
-	{
-	  debugfile_holder = STRIP_DRIVE_SPEC (debugfile);
-	  filename = std::string("\\") + debugfile[0] + debugfile_holder;
-	}
+	filename = (std::string("\\") + debugfile[0]
+		    + STRIP_DRIVE_SPEC (debugfile));

       for (const gdb::unique_xmalloc_ptr<char> &dir : vec)
 	{
-- 
2.28.0

Re: [PATCH 2/2] Handle Windows drives in auto-load script paths

[Hannes Domani]

 Am Montag, 5. Oktober 2020, 04:06:31 MESZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:

> Hi Hannes,
>
> On 2020-05-29 11:08 a.m., Hannes Domani via Gdb-patches wrote:
> > Fixes this testsuite fail on Windows:
> > FAIL: gdb.base/auto-load.exp: print $script_loaded
> >
> > Converts the debugfile path from c:/dir/file to /c/dir/file, so it can be
> > appended to the auto-load path.
> >
> > gdb/ChangeLog:
> >
> > 2020-05-29  Hannes Domani  <ssbssa@yahoo.de>
> >
> >     * auto-load.c (auto_load_objfile_script_1): Convert drive part
> >     of debugfile path on Windows.
> >
> > gdb/doc/ChangeLog:
> >
> > 2020-05-29  Hannes Domani  <ssbssa@yahoo.de>
> >
> >     * gdb.texinfo: Document Windows drive conversion of
> >     'set auto-load scripts-directory'.
> > ---
> > v2:
> > - Document Windows drive conversion of 'set auto-load scripts-directory'.
> > ---
> >  gdb/auto-load.c    | 7 +++++++
> >  gdb/doc/gdb.texinfo | 4 ++++
> >  2 files changed, 11 insertions(+)
> >
> > diff --git a/gdb/auto-load.c b/gdb/auto-load.c
> > index 99bd96b971..88221d9f3d 100644
> > --- a/gdb/auto-load.c
> > +++ b/gdb/auto-load.c
> > @@ -784,6 +784,13 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
> >                        "scripts-directory' path \"%s\".\n"),
> >                  auto_load_dir);
> >
> > +      /* Convert Windows debugfile path from c:/dir/file to /c/dir/file.  */
> > +      if (HAS_DRIVE_SPEC (debugfile))
> > +    {
> > +      debugfile_holder = STRIP_DRIVE_SPEC (debugfile);
>
> I looked at this patch a bit before realizing it is already merged.  I
> was going to make a comment about the line above, so I made it a patch
> instead, WDYT?

Eli later noticed this problem as well, in this thread:
https://sourceware.org/pipermail/gdb-patches/2020-July/170204.html


> From 602e996b26d592ecda500ed64adafac30e0e9ce7 Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Sun, 4 Oct 2020 22:00:17 -0400
> Subject: [PATCH] gdb: avoid unnecessary string copy in
> auto_load_objfile_script_1
>
> Assigning the result of STRIP_DRIVE_SPEC to an std::string creates an
> unnecessary copy of the string.  STRIP_DRIVE_SPEC is defined as:
>
>   #define STRIP_DRIVE_SPEC(f) ((f) + 2)
>
> So if it is passed a "const char *", it returns a "const char *".  We
> could use a "const char *" intermediary variable instead of an
> std::string, or (as implemented in this patch) just use it directly in
> the concatenation right after.
>
> gdb/ChangeLog:
>
>     * auto-load.c (auto_load_objfile_script_1): Don't use
>     debugfile_holder as temporary variable when stripping drive
>     letter.
>
> Change-Id: If2ccc7a156b22100754d9cdf6778ac7eeb93da4c
> ---
> gdb/auto-load.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/auto-load.c b/gdb/auto-load.c
> index 9a51d2f3dc6c..43d007ca5b03 100644
> --- a/gdb/auto-load.c
> +++ b/gdb/auto-load.c
> @@ -777,10 +777,8 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
>
>       /* Convert Windows file name from c:/dir/file to /c/dir/file.  */
>       if (HAS_DRIVE_SPEC (debugfile))
> -    {
> -      debugfile_holder = STRIP_DRIVE_SPEC (debugfile);
> -      filename = std::string("\\") + debugfile[0] + debugfile_holder;
> -    }
> +    filename = (std::string("\\") + debugfile[0]
> +            + STRIP_DRIVE_SPEC (debugfile));
>
>       for (const gdb::unique_xmalloc_ptr<char> &dir : vec)
>
>     {
> --
> 2.28.0

Ummm, LGTM.


Hannes

Re: [PATCH 2/2] Handle Windows drives in auto-load script paths

[Simon Marchi]

On 2020-10-05 7:23 a.m., Hannes Domani wrote:
>  Am Montag, 5. Oktober 2020, 04:06:31 MESZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:
> 
>> Hi Hannes,
>>
>> On 2020-05-29 11:08 a.m., Hannes Domani via Gdb-patches wrote:
>>> Fixes this testsuite fail on Windows:
>>> FAIL: gdb.base/auto-load.exp: print $script_loaded
>>>
>>> Converts the debugfile path from c:/dir/file to /c/dir/file, so it can be
>>> appended to the auto-load path.
>>>
>>> gdb/ChangeLog:
>>>
>>> 2020-05-29  Hannes Domani  <ssbssa@yahoo.de>
>>>
>>>      * auto-load.c (auto_load_objfile_script_1): Convert drive part
>>>      of debugfile path on Windows.
>>>
>>> gdb/doc/ChangeLog:
>>>
>>> 2020-05-29  Hannes Domani  <ssbssa@yahoo.de>
>>>
>>>      * gdb.texinfo: Document Windows drive conversion of
>>>      'set auto-load scripts-directory'.
>>> ---
>>> v2:
>>> - Document Windows drive conversion of 'set auto-load scripts-directory'.
>>> ---
>>>   gdb/auto-load.c    | 7 +++++++
>>>   gdb/doc/gdb.texinfo | 4 ++++
>>>   2 files changed, 11 insertions(+)
>>>
>>> diff --git a/gdb/auto-load.c b/gdb/auto-load.c
>>> index 99bd96b971..88221d9f3d 100644
>>> --- a/gdb/auto-load.c
>>> +++ b/gdb/auto-load.c
>>> @@ -784,6 +784,13 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
>>>                         "scripts-directory' path \"%s\".\n"),
>>>                   auto_load_dir);
>>>
>>> +      /* Convert Windows debugfile path from c:/dir/file to /c/dir/file.  */
>>> +      if (HAS_DRIVE_SPEC (debugfile))
>>> +    {
>>> +      debugfile_holder = STRIP_DRIVE_SPEC (debugfile);
>>
>> I looked at this patch a bit before realizing it is already merged.  I
>> was going to make a comment about the line above, so I made it a patch
>> instead, WDYT?
> 
> Eli later noticed this problem as well, in this thread:
> https://sourceware.org/pipermail/gdb-patches/2020-July/170204.html
> 
> 
>> From 602e996b26d592ecda500ed64adafac30e0e9ce7 Mon Sep 17 00:00:00 2001
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>> Date: Sun, 4 Oct 2020 22:00:17 -0400
>> Subject: [PATCH] gdb: avoid unnecessary string copy in
>> auto_load_objfile_script_1
>>
>> Assigning the result of STRIP_DRIVE_SPEC to an std::string creates an
>> unnecessary copy of the string.  STRIP_DRIVE_SPEC is defined as:
>>
>>    #define STRIP_DRIVE_SPEC(f) ((f) + 2)
>>
>> So if it is passed a "const char *", it returns a "const char *".  We
>> could use a "const char *" intermediary variable instead of an
>> std::string, or (as implemented in this patch) just use it directly in
>> the concatenation right after.
>>
>> gdb/ChangeLog:
>>
>>      * auto-load.c (auto_load_objfile_script_1): Don't use
>>      debugfile_holder as temporary variable when stripping drive
>>      letter.
>>
>> Change-Id: If2ccc7a156b22100754d9cdf6778ac7eeb93da4c
>> ---
>> gdb/auto-load.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/gdb/auto-load.c b/gdb/auto-load.c
>> index 9a51d2f3dc6c..43d007ca5b03 100644
>> --- a/gdb/auto-load.c
>> +++ b/gdb/auto-load.c
>> @@ -777,10 +777,8 @@ auto_load_objfile_script_1 (struct objfile *objfile, const char *realname,
>>
>>        /* Convert Windows file name from c:/dir/file to /c/dir/file.  */
>>        if (HAS_DRIVE_SPEC (debugfile))
>> -    {
>> -      debugfile_holder = STRIP_DRIVE_SPEC (debugfile);
>> -      filename = std::string("\\") + debugfile[0] + debugfile_holder;
>> -    }
>> +    filename = (std::string("\\") + debugfile[0]
>> +            + STRIP_DRIVE_SPEC (debugfile));
>>
>>        for (const gdb::unique_xmalloc_ptr<char> &dir : vec)
>>
>>      {
>> --
>> 2.28.0
> 
> Ummm, LGTM.

Thanks, I pushed it.

Simon

Re: [PATCH v2 1/2] Fix function argument and return value locations

[Hannes Domani]

 Ping.

Am Freitag, 29. Mai 2020, 17:08:40 MESZ hat Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> Folgendes geschrieben:

> Fixes these testsuite fails on Windows:
> FAIL: gdb.base/callfuncs.exp: p t_float_complex_values(fc1, fc2)
> FAIL: gdb.base/callfuncs.exp: p t_float_complex_many_args(fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4)
> FAIL: gdb.base/callfuncs.exp: noproto: p t_float_complex_values(fc1, fc2)
> FAIL: gdb.base/callfuncs.exp: noproto: p t_float_complex_many_args(fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4)
> FAIL: gdb.base/call-sc.exp: p/c fun(); call call-sc-tld
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: zed L for return; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: return foo; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: return foo; synchronize pc to main() for 'call-sc-tld'
> FAIL: gdb.base/call-sc.exp: return foo; synchronize pc to main() for 'call-sc-tld'
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: zed L for finish; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: finish foo; return call-sc-tld (the program is no longer running)
> FAIL: gdb.base/call-sc.exp: value foo finished; return call-sc-tld
>
> For function arguments (callfuncs.exp), only TYPE_CODE_COMPLEX was
> missing in the types passed via integer registers.
>
> For return values, there were a lot more issues:
> - TYPE_CODE_DECFLOAT is NOT returned via XMM0.
> - long double is NOT returned via XMM0.
> - but __int128 IS returned via XMM0.
> - the comments for TYPE_CODE_FLT state that __m128, __m128i and __m128d are
>   returned by XMM0, and this is correct, but it doesn't actually check for
>   them, because they are TYPE_CODE_ARRAY with TYPE_VECTOR
>
> So I had to add TYPE_CODE_DECFLOAT to the arguments passed via XMM register,
> but I had to remove it from the values returned via XMM0 register.
>
> gdb/ChangeLog:
>
> 2020-05-29  Hannes Domani  <ssbssa@yahoo.de>
>
>     * amd64-windows-tdep.c (amd64_windows_passed_by_integer_register):
>     Add TYPE_CODE_COMPLEX.
>     (amd64_windows_return_value): Fix types returned via XMM0.
>
> gdb/testsuite/ChangeLog:
>
> 2020-05-29  Hannes Domani  <ssbssa@yahoo.de>
>
>     * gdb.base/call-sc.c: Fix return struct on stack test case.
>     * gdb.base/call-sc.exp: Likewise.
> ---
> v2:
> - TYPE_CODE_COMPLEX is actually passed via integer register, not XMM
>   register.
> ---
> gdb/amd64-windows-tdep.c          | 21 +++++++++++++++++----
> gdb/testsuite/gdb.base/call-sc.c  |  6 +++++-
> gdb/testsuite/gdb.base/call-sc.exp | 13 +++++++++++++
> 3 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
> index 487dfd45fc..e83d76257f 100644
> --- a/gdb/amd64-windows-tdep.c
> +++ b/gdb/amd64-windows-tdep.c
> @@ -60,6 +60,7 @@ amd64_windows_passed_by_integer_register (struct type *type)
>       case TYPE_CODE_RVALUE_REF:
>       case TYPE_CODE_STRUCT:
>       case TYPE_CODE_UNION:
> +      case TYPE_CODE_COMPLEX:
>     return (TYPE_LENGTH (type) == 1
>         || TYPE_LENGTH (type) == 2
>         || TYPE_LENGTH (type) == 4
> @@ -299,17 +300,29 @@ amd64_windows_return_value (struct gdbarch *gdbarch, struct value *function,
>   switch (type->code ())
>     {
>       case TYPE_CODE_FLT:
> -      case TYPE_CODE_DECFLOAT:
> -        /* __m128, __m128i, __m128d, floats, and doubles are returned
> -          via XMM0.  */
> -        if (len == 4 || len == 8 || len == 16)
> +        /* floats, and doubles are returned via XMM0.  */
> +        if (len == 4 || len == 8)
>           regnum = AMD64_XMM0_REGNUM;
>         break;
> +      case TYPE_CODE_ARRAY:
> +        /* __m128, __m128i and __m128d are returned via XMM0.  */
> +    if (TYPE_VECTOR (type) && len == 16)
> +      {
> +        enum type_code code = TYPE_TARGET_TYPE (type)->code ();
> +        if (code == TYPE_CODE_INT || code == TYPE_CODE_FLT)
> +          {
> +        regnum = AMD64_XMM0_REGNUM;
> +        break;
> +          }
> +      }
> +    /* fall through */
>       default:
>         /* All other values that are 1, 2, 4 or 8 bytes long are returned
>             via RAX.  */
>         if (len == 1 || len == 2 || len == 4 || len == 8)
>           regnum = AMD64_RAX_REGNUM;
> +    else if (len == 16 && type->code () == TYPE_CODE_INT)
> +      regnum = AMD64_XMM0_REGNUM;
>         break;
>     }
>
> diff --git a/gdb/testsuite/gdb.base/call-sc.c b/gdb/testsuite/gdb.base/call-sc.c
> index ba80576ac3..eb140cd9cf 100644
> --- a/gdb/testsuite/gdb.base/call-sc.c
> +++ b/gdb/testsuite/gdb.base/call-sc.c
> @@ -35,6 +35,7 @@ typedef t T;
> #endif
>
> T foo = '1', L;
> +T init = '9';
>
> T fun()
> {
> @@ -55,7 +56,10 @@ int main()
> {
>   int i;
>
> -  Fun(foo);   
> +  /* Use a different initial value then is later used in the
> +    "value foo returned" test, so in case the struct is then returned
> +    on the stack, it doesn't have the correct value by accident.  */
> +  Fun(init);
>
>   /* An infinite loop that first clears all the variables and then
>       calls the function.  This "hack" is to make re-testing easier -
> diff --git a/gdb/testsuite/gdb.base/call-sc.exp b/gdb/testsuite/gdb.base/call-sc.exp
> index 719000435a..9697c5ac24 100644
> --- a/gdb/testsuite/gdb.base/call-sc.exp
> +++ b/gdb/testsuite/gdb.base/call-sc.exp
> @@ -291,6 +291,19 @@ proc test_scalar_returns { } {
>         fail "${test}"
>         }
>     }
> +    -re " = 57 .*${gdb_prompt} $" {
> +        if $return_value_unknown {
> +        # The struct return case.
> +        # The return value is stored on the stack, and since GDB
> +        # didn't override it, it still has value that was stored
> +        # there in the earlier Foo(init) call.
> +        pass "${test}"
> +        } else {
> +        # This contradicts the above claim that GDB knew
> +        # the location of the return-value.
> +        fail "${test}"
> +        }
> +    }
>     -re ".*${gdb_prompt} $" {
>         if $return_value_unimplemented {
>         # What a suprize.  The architecture hasn't implemented
> --
> 2.26.2

Re: [PATCH v2 1/2] Fix function argument and return value locations

[Simon Marchi]

On 2020-05-29 11:07 a.m., Hannes Domani via Gdb-patches wrote:
> Fixes these testsuite fails on Windows:
> FAIL: gdb.base/callfuncs.exp: p t_float_complex_values(fc1, fc2)
> FAIL: gdb.base/callfuncs.exp: p t_float_complex_many_args(fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4)
> FAIL: gdb.base/callfuncs.exp: noproto: p t_float_complex_values(fc1, fc2)
> FAIL: gdb.base/callfuncs.exp: noproto: p t_float_complex_many_args(fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4)
> FAIL: gdb.base/call-sc.exp: p/c fun(); call call-sc-tld
> FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: zed L for return; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: return foo; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: return foo; synchronize pc to main() for 'call-sc-tld'
> FAIL: gdb.base/call-sc.exp: return foo; synchronize pc to main() for 'call-sc-tld'
> FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: zed L for finish; return call-sc-tld
> FAIL: gdb.base/call-sc.exp: finish foo; return call-sc-tld (the program is no longer running)
> FAIL: gdb.base/call-sc.exp: value foo finished; return call-sc-tld
>
> For function arguments (callfuncs.exp), only TYPE_CODE_COMPLEX was
> missing in the types passed via integer registers.
>
> For return values, there were a lot more issues:
> - TYPE_CODE_DECFLOAT is NOT returned via XMM0.
> - long double is NOT returned via XMM0.
> - but __int128 IS returned via XMM0.
> - the comments for TYPE_CODE_FLT state that __m128, __m128i and __m128d are
>   returned by XMM0, and this is correct, but it doesn't actually check for
>   them, because they are TYPE_CODE_ARRAY with TYPE_VECTOR
>
> So I had to add TYPE_CODE_DECFLOAT to the arguments passed via XMM register,
> but I had to remove it from the values returned via XMM0 register.

I won't pretend I cross-checked all of this with the ABI documentation,
but that looks good to me.  If you say the tests now pass, that gives me
enough confidence, so please go ahead and merge.

Simon

Re: [PATCH v2 1/2] Fix function argument and return value locations

[Hannes Domani]

 Am Montag, 5. Oktober 2020, 03:50:20 MESZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben:

> On 2020-05-29 11:07 a.m., Hannes Domani via Gdb-patches wrote:
> > Fixes these testsuite fails on Windows:
> > FAIL: gdb.base/callfuncs.exp: p t_float_complex_values(fc1, fc2)
> > FAIL: gdb.base/callfuncs.exp: p t_float_complex_many_args(fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4)
> > FAIL: gdb.base/callfuncs.exp: noproto: p t_float_complex_values(fc1, fc2)
> > FAIL: gdb.base/callfuncs.exp: noproto: p t_float_complex_many_args(fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4, fc1, fc2, fc3, fc4)
> > FAIL: gdb.base/call-sc.exp: p/c fun(); call call-sc-tld
> > FAIL: gdb.base/call-sc.exp: advance to fun for return; return call-sc-tld
> > FAIL: gdb.base/call-sc.exp: zed L for return; return call-sc-tld
> > FAIL: gdb.base/call-sc.exp: return foo; return call-sc-tld
> > FAIL: gdb.base/call-sc.exp: return foo; synchronize pc to main() for 'call-sc-tld'
> > FAIL: gdb.base/call-sc.exp: return foo; synchronize pc to main() for 'call-sc-tld'
> > FAIL: gdb.base/call-sc.exp: advance to fun for finish; return call-sc-tld
> > FAIL: gdb.base/call-sc.exp: zed L for finish; return call-sc-tld
> > FAIL: gdb.base/call-sc.exp: finish foo; return call-sc-tld (the program is no longer running)
> > FAIL: gdb.base/call-sc.exp: value foo finished; return call-sc-tld
> >
> > For function arguments (callfuncs.exp), only TYPE_CODE_COMPLEX was
> > missing in the types passed via integer registers.
> >
> > For return values, there were a lot more issues:
> > - TYPE_CODE_DECFLOAT is NOT returned via XMM0.
> > - long double is NOT returned via XMM0.
> > - but __int128 IS returned via XMM0.
> > - the comments for TYPE_CODE_FLT state that __m128, __m128i and __m128d are
> >  returned by XMM0, and this is correct, but it doesn't actually check for
> >  them, because they are TYPE_CODE_ARRAY with TYPE_VECTOR
> >
> > So I had to add TYPE_CODE_DECFLOAT to the arguments passed via XMM register,
> > but I had to remove it from the values returned via XMM0 register.
>
>
> I won't pretend I cross-checked all of this with the ABI documentation,
> but that looks good to me.  If you say the tests now pass, that gives me
> enough confidence, so please go ahead and merge.

Pushed, thanks.


Hannes