[PATCHv5,3/3] gdbserver: pass osabi to GDB in more target descriptions

Message ID eafb21ac2e8b0bfdc34e0a254746d5812462ff6e.1730059609.git.aburgess@redhat.com
State New
Headers
Series Returning osabi from gdbserer in more cases |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Andrew Burgess Oct. 27, 2024, 8:07 p.m. UTC
  Problem Description
-------------------

On a Windows machine I built gdbserver, configured for the target
'x86_64-w64-mingw32', then on a GNU/Linux machine I built GDB with
support for all target (--enable-targets=all).

On the Windows machine I start gdbserver with a small test binary:

  $ gdbserver 192.168.129.25:54321 C:\some\directory\executable.exe

On the GNU/Linux machine I start GDB without the test binary, and
connect to gdbserver.

As I have not given GDB the test binary, my expectation is that GDB
would connect to gdbserver and then download the file over the remote
protocol, but instead I was presented with this message:

  (gdb) target remote 192.168.129.25:54321
  Remote debugging using 192.168.129.25:54321
  warning: C:\some\directory\executable.exe: No such file or directory.
  0x00007ffa3e1e1741 in ?? ()
  (gdb)

What I found is that if I told GDB where to find the binary, like
this:

  (gdb) file target:C:/some/directory/executable.exe
  A program is being debugged already.
  Are you sure you want to change the file? (y or n) y
  Reading C:/some/directory/executable.exe from remote target...
  warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
  Reading C:/some/directory/executable.exe from remote target...
  Reading symbols from target:C:/some/directory/executable.exe...
  (gdb)

then GDB would download the executable.

The Actual Issue
----------------

I tracked the problem down to exec_file_find (solib.c).  The remote
target was passing an absolute Windows filename (beginning with "C:/"
in this case), but in exec_file_find GDB was failing the
IS_TARGET_ABSOLUTE_PATH call, and so was treating the filename as
relative.

The IS_TARGET_ABSOLUTE_PATH call was failing because GDB thought that
the file system kind was "unix", and as the filename didn't start with
a "/" it assumed the filename was not absolute.

But I'm connecting to a Windows target and 'target-file-system-kind'
was set to "auto", so GDB should be figuring out that the target
file-system is "dos-based".

Looking in effective_target_file_system_kind (filesystem.c), we find
that the logic of "auto" is delegated to the current gdbarch.  However
in windows-tdep.c we see:

  set_gdbarch_has_dos_based_file_system (gdbarch, 1);

So if we are using a Windows gdbarch we should have "dos-based"
filesystems.  What this means is that after connecting to the remote
target GDB has selected the wrong gdbarch.

What's happening is that the target description sent back by the
remote target only includes the x86-64 registers.  There's no
information about which OS we're on.  As a consequence, GDB picks the
first x86-64 gdbarch which can handle the provided register set, which
happens to be a GNU/Linux gdbarch.

And indeed, there doesn't appear to be anywhere in gdbserver that sets
the osabi on the target descriptions. Some target descriptions do have
their osabi set when the description is created, e.g. in:

  gdb/arch/amd64.c	- Sets GNU/Linux osabi when appropriate.
  gdb/arch/i386.c	- Likewise.
  gdb/arch/tic6x.c	- Always set GNU/Linux osabi.

There are also some cases in gdb/features/*.c where the tdesc is set,
but these locations are only called from GDB, not from gdbserver.

This means that many target descriptions are created without an osabi,
gdbserver does nothing to fix this, and the description is returned to
GDB without an osabi included.  This leaves GDB having to guess what
the target osabi is, and in some cases, GDB can get this wrong.

Proposed Solution
-----------------

I propose to change init_target_desc so that it requires an gdb_osabi
to be passed in, this will then be used to set the target_desc osabi
field.

I believe that within gdbserver init_target_desc is called for every
target_desc, so this should mean that every target_desc has an
opportunity to set the osabi to something sane.

I did consider passing the osabi into the code which creates the
target_desc objects, but that would require updating far more code, as
each target has its own code for creating target descriptions.
The approach taken here requires minimal changes and forces every
user of init_target_desc to think about what the correct osabi is.

In some cases, e.g. amd64, where the osabi is already set when the
target_desc is created, the init_target_desc call will override the
current value, however, we should always be replacing it with the same
actual value.  i.e. if the target_desc is created with the osabi set
to GNU/Linux, then this should only happen when gdbserver is built for
GNU/Linux, in which case the init_target_desc should also be setting
the osabi to GNU/Linux.

The Tricky Bits
---------------

Some targets, like amd64, use a features based approach for creating
target_desc objects, there's a function in arch/amd64.c which creates
a target_desc, adds features too it, and returns the new target_desc.
This target_desc is then passed to an init_target_desc call within
gdbserver.  This is the easy case to handle.

Then there are other targets which instead have a fixed set of xml
files, each of which is converted into a .dat file, which is then used
to generate a .cc file, which is compiled into gdbserver.  The
generated .cc file creates the target_desc object and calls
init_target_desc on it.  In this case though the target description
that is sent to GDB isn't generated from the target_desc object, but
is instead the contents of the fixed xml file.  For this case the
osabi which we pass to init_target_desc should match the osabi that
exists in the fixed xml file.

Luckily, in the previous commit I copied the osabi information from
the fixed xml files into the .dat files.  So in this commit I have
extended regdat.sh to read the osabi from the .dat file and use it in
the generated init_target_desc call.

The problem with some of these .dat base targets is that their fixed
xml files don't currently contain any osabi information, and the file
names don't indicate that they are Linux only (despite them currently
only being used from gdbserver for Linux targets), so I don't
currently feel confident adding any osabi information to these files.
An example would be features/rs6000/powerpc-64.xml.  For now I've just
ignored these cases.  The init_target_desc will use GDB_OSABI_UNKNOWN
which is the default.  This means that for these targets nothing
changes from the current behaviour.  But many other targets do now
pass the osabi back.  Targets that do pass the osabi back are
improved with this commit.

Conclusion
----------

Now when I connect to the Windows remote the target description
returned includes the osabi name.  With this extra information GDB
selects the correct gdbarch object, which means that GDB understands
the target has a "dos-based" file-system.  With that correct GDB
understands that the filename it was given is absolute, and so fetches
the file from the remote as we'd like.
---
 gdb/regformats/regdat.sh         | 14 ++++++++++----
 gdbserver/Makefile.in            |  9 +++++----
 gdbserver/linux-aarch32-tdesc.cc |  2 +-
 gdbserver/linux-aarch64-tdesc.cc |  3 ++-
 gdbserver/linux-arc-low.cc       |  2 +-
 gdbserver/linux-arm-tdesc.cc     |  2 +-
 gdbserver/linux-csky-low.cc      |  2 +-
 gdbserver/linux-loongarch-low.cc |  2 +-
 gdbserver/linux-riscv-low.cc     |  2 +-
 gdbserver/linux-x86-tdesc.cc     | 15 +++++++++++++--
 gdbserver/netbsd-aarch64-low.cc  |  2 +-
 gdbserver/netbsd-amd64-low.cc    |  2 +-
 gdbserver/netbsd-i386-low.cc     |  2 +-
 gdbserver/tdesc.cc               |  5 ++++-
 gdbserver/tdesc.h                |  6 ++++--
 gdbserver/win32-i386-low.cc      |  4 ++--
 gdbserver/win32-low.h            |  7 +++++++
 17 files changed, 56 insertions(+), 25 deletions(-)
  

Comments

Kevin Buettner Oct. 31, 2024, 2:28 a.m. UTC | #1
On Sun, 27 Oct 2024 20:07:42 +0000
Andrew Burgess <aburgess@redhat.com> wrote:

> Problem Description
> -------------------
> 
> On a Windows machine I built gdbserver, configured for the target
> 'x86_64-w64-mingw32', then on a GNU/Linux machine I built GDB with
> support for all target (--enable-targets=all).
> 
> On the Windows machine I start gdbserver with a small test binary:
> 
>   $ gdbserver 192.168.129.25:54321 C:\some\directory\executable.exe
> 
> On the GNU/Linux machine I start GDB without the test binary, and
> connect to gdbserver.
> 
> As I have not given GDB the test binary, my expectation is that GDB
> would connect to gdbserver and then download the file over the remote
> protocol, but instead I was presented with this message:
> 
>   (gdb) target remote 192.168.129.25:54321
>   Remote debugging using 192.168.129.25:54321
>   warning: C:\some\directory\executable.exe: No such file or directory.
>   0x00007ffa3e1e1741 in ?? ()
>   (gdb)
> 
> What I found is that if I told GDB where to find the binary, like
> this:
> 
>   (gdb) file target:C:/some/directory/executable.exe
>   A program is being debugged already.
>   Are you sure you want to change the file? (y or n) y
>   Reading C:/some/directory/executable.exe from remote target...
>   warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
>   Reading C:/some/directory/executable.exe from remote target...
>   Reading symbols from target:C:/some/directory/executable.exe...
>   (gdb)
> 
> then GDB would download the executable.
> 
> The Actual Issue
> ----------------
> 
> I tracked the problem down to exec_file_find (solib.c).  The remote
> target was passing an absolute Windows filename (beginning with "C:/"
> in this case), but in exec_file_find GDB was failing the
> IS_TARGET_ABSOLUTE_PATH call, and so was treating the filename as
> relative.
> 
> The IS_TARGET_ABSOLUTE_PATH call was failing because GDB thought that
> the file system kind was "unix", and as the filename didn't start with
> a "/" it assumed the filename was not absolute.
> 
> But I'm connecting to a Windows target and 'target-file-system-kind'
> was set to "auto", so GDB should be figuring out that the target
> file-system is "dos-based".
> 
> Looking in effective_target_file_system_kind (filesystem.c), we find
> that the logic of "auto" is delegated to the current gdbarch.  However
> in windows-tdep.c we see:
> 
>   set_gdbarch_has_dos_based_file_system (gdbarch, 1);
> 
> So if we are using a Windows gdbarch we should have "dos-based"
> filesystems.  What this means is that after connecting to the remote
> target GDB has selected the wrong gdbarch.
> 
> What's happening is that the target description sent back by the
> remote target only includes the x86-64 registers.  There's no
> information about which OS we're on.  As a consequence, GDB picks the
> first x86-64 gdbarch which can handle the provided register set, which
> happens to be a GNU/Linux gdbarch.
> 
> And indeed, there doesn't appear to be anywhere in gdbserver that sets
> the osabi on the target descriptions. Some target descriptions do have
> their osabi set when the description is created, e.g. in:
> 
>   gdb/arch/amd64.c	- Sets GNU/Linux osabi when appropriate.
>   gdb/arch/i386.c	- Likewise.
>   gdb/arch/tic6x.c	- Always set GNU/Linux osabi.
> 
> There are also some cases in gdb/features/*.c where the tdesc is set,
> but these locations are only called from GDB, not from gdbserver.
> 
> This means that many target descriptions are created without an osabi,
> gdbserver does nothing to fix this, and the description is returned to
> GDB without an osabi included.  This leaves GDB having to guess what
> the target osabi is, and in some cases, GDB can get this wrong.
> 
> Proposed Solution
> -----------------
> 
> I propose to change init_target_desc so that it requires an gdb_osabi
> to be passed in, this will then be used to set the target_desc osabi
> field.
> 
> I believe that within gdbserver init_target_desc is called for every
> target_desc, so this should mean that every target_desc has an
> opportunity to set the osabi to something sane.
> 
> I did consider passing the osabi into the code which creates the
> target_desc objects, but that would require updating far more code, as
> each target has its own code for creating target descriptions.
> The approach taken here requires minimal changes and forces every
> user of init_target_desc to think about what the correct osabi is.
> 
> In some cases, e.g. amd64, where the osabi is already set when the
> target_desc is created, the init_target_desc call will override the
> current value, however, we should always be replacing it with the same
> actual value.  i.e. if the target_desc is created with the osabi set
> to GNU/Linux, then this should only happen when gdbserver is built for
> GNU/Linux, in which case the init_target_desc should also be setting
> the osabi to GNU/Linux.
> 
> The Tricky Bits
> ---------------
> 
> Some targets, like amd64, use a features based approach for creating
> target_desc objects, there's a function in arch/amd64.c which creates
> a target_desc, adds features too it, and returns the new target_desc.
> This target_desc is then passed to an init_target_desc call within
> gdbserver.  This is the easy case to handle.
> 
> Then there are other targets which instead have a fixed set of xml
> files, each of which is converted into a .dat file, which is then used
> to generate a .cc file, which is compiled into gdbserver.  The
> generated .cc file creates the target_desc object and calls
> init_target_desc on it.  In this case though the target description
> that is sent to GDB isn't generated from the target_desc object, but
> is instead the contents of the fixed xml file.  For this case the
> osabi which we pass to init_target_desc should match the osabi that
> exists in the fixed xml file.
> 
> Luckily, in the previous commit I copied the osabi information from
> the fixed xml files into the .dat files.  So in this commit I have
> extended regdat.sh to read the osabi from the .dat file and use it in
> the generated init_target_desc call.
> 
> The problem with some of these .dat base targets is that their fixed
> xml files don't currently contain any osabi information, and the file
> names don't indicate that they are Linux only (despite them currently
> only being used from gdbserver for Linux targets), so I don't
> currently feel confident adding any osabi information to these files.
> An example would be features/rs6000/powerpc-64.xml.  For now I've just
> ignored these cases.  The init_target_desc will use GDB_OSABI_UNKNOWN
> which is the default.  This means that for these targets nothing
> changes from the current behaviour.  But many other targets do now
> pass the osabi back.  Targets that do pass the osabi back are
> improved with this commit.
> 
> Conclusion
> ----------
> 
> Now when I connect to the Windows remote the target description
> returned includes the osabi name.  With this extra information GDB
> selects the correct gdbarch object, which means that GDB understands
> the target has a "dos-based" file-system.  With that correct GDB
> understands that the filename it was given is absolute, and so fetches
> the file from the remote as we'd like.

The approach that you've taken makes sense to me, but I'm not an
expert in this area.  Therefore, I don't feel comfortable giving an
"Approved-by" for this particular patch.  That said, I don't think it
makes sense for it to languish on the mailing list, so I recommend
that you approve it if there are no other comments within a week or
so.

Reviewed-by: Kevin Buettner <kevinb@redhat.com>
  
Andrew Burgess Oct. 31, 2024, 9:43 a.m. UTC | #2
Kevin Buettner <kevinb@redhat.com> writes:

> On Sun, 27 Oct 2024 20:07:42 +0000
> Andrew Burgess <aburgess@redhat.com> wrote:
>
>> Problem Description
>> -------------------
>> 
>> On a Windows machine I built gdbserver, configured for the target
>> 'x86_64-w64-mingw32', then on a GNU/Linux machine I built GDB with
>> support for all target (--enable-targets=all).
>> 
>> On the Windows machine I start gdbserver with a small test binary:
>> 
>>   $ gdbserver 192.168.129.25:54321 C:\some\directory\executable.exe
>> 
>> On the GNU/Linux machine I start GDB without the test binary, and
>> connect to gdbserver.
>> 
>> As I have not given GDB the test binary, my expectation is that GDB
>> would connect to gdbserver and then download the file over the remote
>> protocol, but instead I was presented with this message:
>> 
>>   (gdb) target remote 192.168.129.25:54321
>>   Remote debugging using 192.168.129.25:54321
>>   warning: C:\some\directory\executable.exe: No such file or directory.
>>   0x00007ffa3e1e1741 in ?? ()
>>   (gdb)
>> 
>> What I found is that if I told GDB where to find the binary, like
>> this:
>> 
>>   (gdb) file target:C:/some/directory/executable.exe
>>   A program is being debugged already.
>>   Are you sure you want to change the file? (y or n) y
>>   Reading C:/some/directory/executable.exe from remote target...
>>   warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
>>   Reading C:/some/directory/executable.exe from remote target...
>>   Reading symbols from target:C:/some/directory/executable.exe...
>>   (gdb)
>> 
>> then GDB would download the executable.
>> 
>> The Actual Issue
>> ----------------
>> 
>> I tracked the problem down to exec_file_find (solib.c).  The remote
>> target was passing an absolute Windows filename (beginning with "C:/"
>> in this case), but in exec_file_find GDB was failing the
>> IS_TARGET_ABSOLUTE_PATH call, and so was treating the filename as
>> relative.
>> 
>> The IS_TARGET_ABSOLUTE_PATH call was failing because GDB thought that
>> the file system kind was "unix", and as the filename didn't start with
>> a "/" it assumed the filename was not absolute.
>> 
>> But I'm connecting to a Windows target and 'target-file-system-kind'
>> was set to "auto", so GDB should be figuring out that the target
>> file-system is "dos-based".
>> 
>> Looking in effective_target_file_system_kind (filesystem.c), we find
>> that the logic of "auto" is delegated to the current gdbarch.  However
>> in windows-tdep.c we see:
>> 
>>   set_gdbarch_has_dos_based_file_system (gdbarch, 1);
>> 
>> So if we are using a Windows gdbarch we should have "dos-based"
>> filesystems.  What this means is that after connecting to the remote
>> target GDB has selected the wrong gdbarch.
>> 
>> What's happening is that the target description sent back by the
>> remote target only includes the x86-64 registers.  There's no
>> information about which OS we're on.  As a consequence, GDB picks the
>> first x86-64 gdbarch which can handle the provided register set, which
>> happens to be a GNU/Linux gdbarch.
>> 
>> And indeed, there doesn't appear to be anywhere in gdbserver that sets
>> the osabi on the target descriptions. Some target descriptions do have
>> their osabi set when the description is created, e.g. in:
>> 
>>   gdb/arch/amd64.c	- Sets GNU/Linux osabi when appropriate.
>>   gdb/arch/i386.c	- Likewise.
>>   gdb/arch/tic6x.c	- Always set GNU/Linux osabi.
>> 
>> There are also some cases in gdb/features/*.c where the tdesc is set,
>> but these locations are only called from GDB, not from gdbserver.
>> 
>> This means that many target descriptions are created without an osabi,
>> gdbserver does nothing to fix this, and the description is returned to
>> GDB without an osabi included.  This leaves GDB having to guess what
>> the target osabi is, and in some cases, GDB can get this wrong.
>> 
>> Proposed Solution
>> -----------------
>> 
>> I propose to change init_target_desc so that it requires an gdb_osabi
>> to be passed in, this will then be used to set the target_desc osabi
>> field.
>> 
>> I believe that within gdbserver init_target_desc is called for every
>> target_desc, so this should mean that every target_desc has an
>> opportunity to set the osabi to something sane.
>> 
>> I did consider passing the osabi into the code which creates the
>> target_desc objects, but that would require updating far more code, as
>> each target has its own code for creating target descriptions.
>> The approach taken here requires minimal changes and forces every
>> user of init_target_desc to think about what the correct osabi is.
>> 
>> In some cases, e.g. amd64, where the osabi is already set when the
>> target_desc is created, the init_target_desc call will override the
>> current value, however, we should always be replacing it with the same
>> actual value.  i.e. if the target_desc is created with the osabi set
>> to GNU/Linux, then this should only happen when gdbserver is built for
>> GNU/Linux, in which case the init_target_desc should also be setting
>> the osabi to GNU/Linux.
>> 
>> The Tricky Bits
>> ---------------
>> 
>> Some targets, like amd64, use a features based approach for creating
>> target_desc objects, there's a function in arch/amd64.c which creates
>> a target_desc, adds features too it, and returns the new target_desc.
>> This target_desc is then passed to an init_target_desc call within
>> gdbserver.  This is the easy case to handle.
>> 
>> Then there are other targets which instead have a fixed set of xml
>> files, each of which is converted into a .dat file, which is then used
>> to generate a .cc file, which is compiled into gdbserver.  The
>> generated .cc file creates the target_desc object and calls
>> init_target_desc on it.  In this case though the target description
>> that is sent to GDB isn't generated from the target_desc object, but
>> is instead the contents of the fixed xml file.  For this case the
>> osabi which we pass to init_target_desc should match the osabi that
>> exists in the fixed xml file.
>> 
>> Luckily, in the previous commit I copied the osabi information from
>> the fixed xml files into the .dat files.  So in this commit I have
>> extended regdat.sh to read the osabi from the .dat file and use it in
>> the generated init_target_desc call.
>> 
>> The problem with some of these .dat base targets is that their fixed
>> xml files don't currently contain any osabi information, and the file
>> names don't indicate that they are Linux only (despite them currently
>> only being used from gdbserver for Linux targets), so I don't
>> currently feel confident adding any osabi information to these files.
>> An example would be features/rs6000/powerpc-64.xml.  For now I've just
>> ignored these cases.  The init_target_desc will use GDB_OSABI_UNKNOWN
>> which is the default.  This means that for these targets nothing
>> changes from the current behaviour.  But many other targets do now
>> pass the osabi back.  Targets that do pass the osabi back are
>> improved with this commit.
>> 
>> Conclusion
>> ----------
>> 
>> Now when I connect to the Windows remote the target description
>> returned includes the osabi name.  With this extra information GDB
>> selects the correct gdbarch object, which means that GDB understands
>> the target has a "dos-based" file-system.  With that correct GDB
>> understands that the filename it was given is absolute, and so fetches
>> the file from the remote as we'd like.
>
> The approach that you've taken makes sense to me, but I'm not an
> expert in this area.  Therefore, I don't feel comfortable giving an
> "Approved-by" for this particular patch.  That said, I don't think it
> makes sense for it to languish on the mailing list, so I recommend
> that you approve it if there are no other comments within a week or
> so.
>
> Reviewed-by: Kevin Buettner <kevinb@redhat.com>

Thanks for the reviews.  I'll give this some additional time to see if
anyone else wants to comment.

Thanks,
Andrew
  
Andrew Burgess Nov. 12, 2024, 1:46 p.m. UTC | #3
Andrew Burgess <aburgess@redhat.com> writes:

> Kevin Buettner <kevinb@redhat.com> writes:
>
>> On Sun, 27 Oct 2024 20:07:42 +0000
>> Andrew Burgess <aburgess@redhat.com> wrote:
>>
>>> Problem Description
>>> -------------------
>>> 
>>> On a Windows machine I built gdbserver, configured for the target
>>> 'x86_64-w64-mingw32', then on a GNU/Linux machine I built GDB with
>>> support for all target (--enable-targets=all).
>>> 
>>> On the Windows machine I start gdbserver with a small test binary:
>>> 
>>>   $ gdbserver 192.168.129.25:54321 C:\some\directory\executable.exe
>>> 
>>> On the GNU/Linux machine I start GDB without the test binary, and
>>> connect to gdbserver.
>>> 
>>> As I have not given GDB the test binary, my expectation is that GDB
>>> would connect to gdbserver and then download the file over the remote
>>> protocol, but instead I was presented with this message:
>>> 
>>>   (gdb) target remote 192.168.129.25:54321
>>>   Remote debugging using 192.168.129.25:54321
>>>   warning: C:\some\directory\executable.exe: No such file or directory.
>>>   0x00007ffa3e1e1741 in ?? ()
>>>   (gdb)
>>> 
>>> What I found is that if I told GDB where to find the binary, like
>>> this:
>>> 
>>>   (gdb) file target:C:/some/directory/executable.exe
>>>   A program is being debugged already.
>>>   Are you sure you want to change the file? (y or n) y
>>>   Reading C:/some/directory/executable.exe from remote target...
>>>   warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
>>>   Reading C:/some/directory/executable.exe from remote target...
>>>   Reading symbols from target:C:/some/directory/executable.exe...
>>>   (gdb)
>>> 
>>> then GDB would download the executable.
>>> 
>>> The Actual Issue
>>> ----------------
>>> 
>>> I tracked the problem down to exec_file_find (solib.c).  The remote
>>> target was passing an absolute Windows filename (beginning with "C:/"
>>> in this case), but in exec_file_find GDB was failing the
>>> IS_TARGET_ABSOLUTE_PATH call, and so was treating the filename as
>>> relative.
>>> 
>>> The IS_TARGET_ABSOLUTE_PATH call was failing because GDB thought that
>>> the file system kind was "unix", and as the filename didn't start with
>>> a "/" it assumed the filename was not absolute.
>>> 
>>> But I'm connecting to a Windows target and 'target-file-system-kind'
>>> was set to "auto", so GDB should be figuring out that the target
>>> file-system is "dos-based".
>>> 
>>> Looking in effective_target_file_system_kind (filesystem.c), we find
>>> that the logic of "auto" is delegated to the current gdbarch.  However
>>> in windows-tdep.c we see:
>>> 
>>>   set_gdbarch_has_dos_based_file_system (gdbarch, 1);
>>> 
>>> So if we are using a Windows gdbarch we should have "dos-based"
>>> filesystems.  What this means is that after connecting to the remote
>>> target GDB has selected the wrong gdbarch.
>>> 
>>> What's happening is that the target description sent back by the
>>> remote target only includes the x86-64 registers.  There's no
>>> information about which OS we're on.  As a consequence, GDB picks the
>>> first x86-64 gdbarch which can handle the provided register set, which
>>> happens to be a GNU/Linux gdbarch.
>>> 
>>> And indeed, there doesn't appear to be anywhere in gdbserver that sets
>>> the osabi on the target descriptions. Some target descriptions do have
>>> their osabi set when the description is created, e.g. in:
>>> 
>>>   gdb/arch/amd64.c	- Sets GNU/Linux osabi when appropriate.
>>>   gdb/arch/i386.c	- Likewise.
>>>   gdb/arch/tic6x.c	- Always set GNU/Linux osabi.
>>> 
>>> There are also some cases in gdb/features/*.c where the tdesc is set,
>>> but these locations are only called from GDB, not from gdbserver.
>>> 
>>> This means that many target descriptions are created without an osabi,
>>> gdbserver does nothing to fix this, and the description is returned to
>>> GDB without an osabi included.  This leaves GDB having to guess what
>>> the target osabi is, and in some cases, GDB can get this wrong.
>>> 
>>> Proposed Solution
>>> -----------------
>>> 
>>> I propose to change init_target_desc so that it requires an gdb_osabi
>>> to be passed in, this will then be used to set the target_desc osabi
>>> field.
>>> 
>>> I believe that within gdbserver init_target_desc is called for every
>>> target_desc, so this should mean that every target_desc has an
>>> opportunity to set the osabi to something sane.
>>> 
>>> I did consider passing the osabi into the code which creates the
>>> target_desc objects, but that would require updating far more code, as
>>> each target has its own code for creating target descriptions.
>>> The approach taken here requires minimal changes and forces every
>>> user of init_target_desc to think about what the correct osabi is.
>>> 
>>> In some cases, e.g. amd64, where the osabi is already set when the
>>> target_desc is created, the init_target_desc call will override the
>>> current value, however, we should always be replacing it with the same
>>> actual value.  i.e. if the target_desc is created with the osabi set
>>> to GNU/Linux, then this should only happen when gdbserver is built for
>>> GNU/Linux, in which case the init_target_desc should also be setting
>>> the osabi to GNU/Linux.
>>> 
>>> The Tricky Bits
>>> ---------------
>>> 
>>> Some targets, like amd64, use a features based approach for creating
>>> target_desc objects, there's a function in arch/amd64.c which creates
>>> a target_desc, adds features too it, and returns the new target_desc.
>>> This target_desc is then passed to an init_target_desc call within
>>> gdbserver.  This is the easy case to handle.
>>> 
>>> Then there are other targets which instead have a fixed set of xml
>>> files, each of which is converted into a .dat file, which is then used
>>> to generate a .cc file, which is compiled into gdbserver.  The
>>> generated .cc file creates the target_desc object and calls
>>> init_target_desc on it.  In this case though the target description
>>> that is sent to GDB isn't generated from the target_desc object, but
>>> is instead the contents of the fixed xml file.  For this case the
>>> osabi which we pass to init_target_desc should match the osabi that
>>> exists in the fixed xml file.
>>> 
>>> Luckily, in the previous commit I copied the osabi information from
>>> the fixed xml files into the .dat files.  So in this commit I have
>>> extended regdat.sh to read the osabi from the .dat file and use it in
>>> the generated init_target_desc call.
>>> 
>>> The problem with some of these .dat base targets is that their fixed
>>> xml files don't currently contain any osabi information, and the file
>>> names don't indicate that they are Linux only (despite them currently
>>> only being used from gdbserver for Linux targets), so I don't
>>> currently feel confident adding any osabi information to these files.
>>> An example would be features/rs6000/powerpc-64.xml.  For now I've just
>>> ignored these cases.  The init_target_desc will use GDB_OSABI_UNKNOWN
>>> which is the default.  This means that for these targets nothing
>>> changes from the current behaviour.  But many other targets do now
>>> pass the osabi back.  Targets that do pass the osabi back are
>>> improved with this commit.
>>> 
>>> Conclusion
>>> ----------
>>> 
>>> Now when I connect to the Windows remote the target description
>>> returned includes the osabi name.  With this extra information GDB
>>> selects the correct gdbarch object, which means that GDB understands
>>> the target has a "dos-based" file-system.  With that correct GDB
>>> understands that the filename it was given is absolute, and so fetches
>>> the file from the remote as we'd like.
>>
>> The approach that you've taken makes sense to me, but I'm not an
>> expert in this area.  Therefore, I don't feel comfortable giving an
>> "Approved-by" for this particular patch.  That said, I don't think it
>> makes sense for it to languish on the mailing list, so I recommend
>> that you approve it if there are no other comments within a week or
>> so.
>>
>> Reviewed-by: Kevin Buettner <kevinb@redhat.com>
>
> Thanks for the reviews.  I'll give this some additional time to see if
> anyone else wants to comment.

I've gone ahead and pushed this series now.

Thanks,
Andrew
  

Patch

diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
index 38464c65b06..49aa02a466d 100755
--- a/gdb/regformats/regdat.sh
+++ b/gdb/regformats/regdat.sh
@@ -105,7 +105,7 @@  EOF
 }
 
 
-exec > new-$2
+exec > new-$3
 copyright $1
 echo '#include "regdef.h"'
 echo '#include "tdesc.h"'
@@ -118,6 +118,7 @@  xmlarch=x
 xmlosabi=x
 expedite=x
 feature=x
+osabi=unknown
 exec < $1
 while do_read
 do
@@ -143,7 +144,7 @@  do
   elif test "${type}" = "xmlarch"; then
     xmlarch="${entry}"
     continue
-  elif test "${type}" = "osabi"; then
+  elif test "${type}" = "xmlosabi"; then
     xmlosabi="${entry}"
     continue
   elif test "${type}" = "expedite"; then
@@ -152,6 +153,9 @@  do
   elif test "${type}" = "feature"; then
     feature="${entry}"
     continue
+  elif test "${type}" = "osabi"; then
+    osabi="${entry}"
+    continue
   elif test "${name}" = x; then
     echo "$0: $1 does not specify \`\`name''." 1>&2
     exit 1
@@ -188,11 +192,13 @@  else
 fi
 echo
 
+osabi_enum=$(grep "${osabi}" "$2" | sed 's/.*(\([^,]\+\),.*/GDB_OSABI_\1/')
+
 cat <<EOF
   result->xmltarget = xmltarget_${name};
 #endif
 
-  init_target_desc (result, expedite_regs_${name});
+  init_target_desc (result, expedite_regs_${name}, ${osabi_enum});
 
   tdesc_${name} = result;
 }
@@ -200,4 +206,4 @@  EOF
 
 # close things off
 exec 1>&2
-mv -- "new-$2" "$2"
+mv -- "new-$3" "$3"
diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in
index 6148ccf9121..f555ff439c4 100644
--- a/gdbserver/Makefile.in
+++ b/gdbserver/Makefile.in
@@ -488,6 +488,7 @@  stamp-xml: $(XML_DIR)/feature_to_c.sh Makefile $(XML_FILES)
 MAKEOVERRIDES =
 
 regdat_sh = $(srcdir)/../gdb/regformats/regdat.sh
+osabi_def = $(srcdir)/../gdbsupport/osabi.def
 
 UST_CFLAGS = \
 	$(ustinc) \
@@ -588,11 +589,11 @@  target/%.o: ../gdb/target/%.c
 # Rules for register format descriptions.  Suffix destination files with
 # -generated to identify and clean them easily.
 
-%-generated.cc: ../gdb/regformats/%.dat $(regdat_sh)
-	$(ECHO_REGDAT) $(SHELL) $(regdat_sh) $< $@
+%-generated.cc: ../gdb/regformats/%.dat $(osabi_def) $(regdat_sh)
+	$(ECHO_REGDAT) $(SHELL) $(regdat_sh) $< $(osabi_def) $@
 
-%-generated.cc: ../gdb/regformats/rs6000/%.dat $(regdat_sh)
-	$(ECHO_REGDAT) $(SHELL) $(regdat_sh) $< $@
+%-generated.cc: ../gdb/regformats/rs6000/%.dat $(osabi_def) $(regdat_sh)
+	$(ECHO_REGDAT) $(SHELL) $(regdat_sh) $< $(osabi_def) $@
 
 # Rule for gdbreplay.o.  This is the same as COMPILE, but includes common-defs.h
 # instead of server.h.
diff --git a/gdbserver/linux-aarch32-tdesc.cc b/gdbserver/linux-aarch32-tdesc.cc
index b8987752b9f..441fe668e6a 100644
--- a/gdbserver/linux-aarch32-tdesc.cc
+++ b/gdbserver/linux-aarch32-tdesc.cc
@@ -34,7 +34,7 @@  aarch32_linux_read_description ()
       tdesc_aarch32 = aarch32_create_target_description (false);
 
       static const char *expedite_regs[] = { "r11", "sp", "pc", 0 };
-      init_target_desc (tdesc_aarch32, expedite_regs);
+      init_target_desc (tdesc_aarch32, expedite_regs, GDB_OSABI_LINUX);
     }
   return tdesc_aarch32;
 }
diff --git a/gdbserver/linux-aarch64-tdesc.cc b/gdbserver/linux-aarch64-tdesc.cc
index 31ec7854cc0..39d5bccdce1 100644
--- a/gdbserver/linux-aarch64-tdesc.cc
+++ b/gdbserver/linux-aarch64-tdesc.cc
@@ -67,7 +67,8 @@  aarch64_linux_read_description (const aarch64_features &features)
 
       expedited_registers.push_back (nullptr);
 
-      init_target_desc (tdesc, (const char **) expedited_registers.data ());
+      init_target_desc (tdesc, (const char **) expedited_registers.data (),
+			GDB_OSABI_LINUX);
 
       tdesc_aarch64_map[features] = tdesc;
     }
diff --git a/gdbserver/linux-arc-low.cc b/gdbserver/linux-arc-low.cc
index 1bcaf6c3f91..16d8d5824aa 100644
--- a/gdbserver/linux-arc-low.cc
+++ b/gdbserver/linux-arc-low.cc
@@ -114,7 +114,7 @@  arc_linux_read_description (void)
   target_desc_up tdesc = arc_create_target_description (features);
 
   static const char *expedite_regs[] = { "sp", "status32", nullptr };
-  init_target_desc (tdesc.get (), expedite_regs);
+  init_target_desc (tdesc.get (), expedite_regs, GDB_OSABI_LINUX);
 
   return tdesc.release ();
 }
diff --git a/gdbserver/linux-arm-tdesc.cc b/gdbserver/linux-arm-tdesc.cc
index 559f9b0f3dc..fff2e948f81 100644
--- a/gdbserver/linux-arm-tdesc.cc
+++ b/gdbserver/linux-arm-tdesc.cc
@@ -37,7 +37,7 @@  arm_linux_read_description (arm_fp_type fp_type)
       tdesc = arm_create_target_description (fp_type, false);
 
       static const char *expedite_regs[] = { "r11", "sp", "pc", 0 };
-      init_target_desc (tdesc, expedite_regs);
+      init_target_desc (tdesc, expedite_regs, GDB_OSABI_LINUX);
 
       tdesc_arm_list[fp_type] = tdesc;
     }
diff --git a/gdbserver/linux-csky-low.cc b/gdbserver/linux-csky-low.cc
index 2eb5a2df17b..18a0d152b5a 100644
--- a/gdbserver/linux-csky-low.cc
+++ b/gdbserver/linux-csky-low.cc
@@ -133,7 +133,7 @@  csky_target::low_arch_setup ()
 
   if (tdesc->expedite_regs.empty ())
     {
-      init_target_desc (tdesc.get (), expedite_regs);
+      init_target_desc (tdesc.get (), expedite_regs, GDB_OSABI_LINUX);
       gdb_assert (!tdesc->expedite_regs.empty ());
     }
 
diff --git a/gdbserver/linux-loongarch-low.cc b/gdbserver/linux-loongarch-low.cc
index 584ea64a7d9..cf7d6c0743c 100644
--- a/gdbserver/linux-loongarch-low.cc
+++ b/gdbserver/linux-loongarch-low.cc
@@ -85,7 +85,7 @@  loongarch_target::low_arch_setup ()
 
   if (tdesc->expedite_regs.empty ())
     {
-      init_target_desc (tdesc.get (), expedite_regs);
+      init_target_desc (tdesc.get (), expedite_regs, GDB_OSABI_LINUX);
       gdb_assert (!tdesc->expedite_regs.empty ());
     }
   current_process ()->tdesc = tdesc.release ();
diff --git a/gdbserver/linux-riscv-low.cc b/gdbserver/linux-riscv-low.cc
index c4554c507a8..7170ad9922e 100644
--- a/gdbserver/linux-riscv-low.cc
+++ b/gdbserver/linux-riscv-low.cc
@@ -91,7 +91,7 @@  riscv_target::low_arch_setup ()
 
   if (tdesc->expedite_regs.empty ())
     {
-      init_target_desc (tdesc.get (), expedite_regs);
+      init_target_desc (tdesc.get (), expedite_regs, GDB_OSABI_LINUX);
       gdb_assert (!tdesc->expedite_regs.empty ());
     }
 
diff --git a/gdbserver/linux-x86-tdesc.cc b/gdbserver/linux-x86-tdesc.cc
index 13c80762605..6aa5c4ab970 100644
--- a/gdbserver/linux-x86-tdesc.cc
+++ b/gdbserver/linux-x86-tdesc.cc
@@ -26,10 +26,21 @@ 
 void
 x86_linux_post_init_tdesc (target_desc *tdesc, bool is_64bit)
 {
+  enum gdb_osabi osabi = GDB_OSABI_LINUX;
+
+#ifndef IN_PROCESS_AGENT
+  /* x86 target descriptions are created with the osabi already set.
+     However, init_target_desc requires us to override the already set
+     value.  That's fine, out new string should match the old one.  */
+  gdb_assert (tdesc_osabi_name (tdesc) != nullptr);
+  gdb_assert (strcmp (tdesc_osabi_name (tdesc),
+		      gdbarch_osabi_name (osabi)) == 0);
+#endif /* ! IN_PROCESS_AGENT */
+
 #ifdef __x86_64__
   if (is_64bit)
-    init_target_desc (tdesc, amd64_expedite_regs);
+    init_target_desc (tdesc, amd64_expedite_regs, osabi);
   else
 #endif
-    init_target_desc (tdesc, i386_expedite_regs);
+    init_target_desc (tdesc, i386_expedite_regs, osabi);
 }
diff --git a/gdbserver/netbsd-aarch64-low.cc b/gdbserver/netbsd-aarch64-low.cc
index f20a1a71773..8834e0ad894 100644
--- a/gdbserver/netbsd-aarch64-low.cc
+++ b/gdbserver/netbsd-aarch64-low.cc
@@ -98,7 +98,7 @@  netbsd_aarch64_target::low_arch_setup ()
     = aarch64_create_target_description ({});
 
   static const char *expedite_regs_aarch64[] = { "x29", "sp", "pc", NULL };
-  init_target_desc (tdesc, expedite_regs_aarch64);
+  init_target_desc (tdesc, expedite_regs_aarch64, GDB_OSABI_NETBSD);
 
   current_process ()->tdesc = tdesc;
 }
diff --git a/gdbserver/netbsd-amd64-low.cc b/gdbserver/netbsd-amd64-low.cc
index b3f3aab5ec3..ad7cb430b92 100644
--- a/gdbserver/netbsd-amd64-low.cc
+++ b/gdbserver/netbsd-amd64-low.cc
@@ -193,7 +193,7 @@  netbsd_amd64_target::low_arch_setup ()
   target_desc *tdesc
     = amd64_create_target_description (X86_XSTATE_SSE_MASK, false, false, false);
 
-  init_target_desc (tdesc, amd64_expedite_regs);
+  init_target_desc (tdesc, amd64_expedite_regs, GDB_OSABI_NETBSD);
 
   current_process ()->tdesc = tdesc;
 }
diff --git a/gdbserver/netbsd-i386-low.cc b/gdbserver/netbsd-i386-low.cc
index 247a39797c4..ea6fce4c6f9 100644
--- a/gdbserver/netbsd-i386-low.cc
+++ b/gdbserver/netbsd-i386-low.cc
@@ -142,7 +142,7 @@  netbsd_i386_target::low_arch_setup ()
   target_desc *tdesc
     = i386_create_target_description (X86_XSTATE_SSE_MASK, false, false);
 
-  init_target_desc (tdesc, i386_expedite_regs);
+  init_target_desc (tdesc, i386_expedite_regs, GDB_OSABI_NETBSD);
 
   current_process ()->tdesc = tdesc;
 }
diff --git a/gdbserver/tdesc.cc b/gdbserver/tdesc.cc
index d052f43c76e..da1287abbbe 100644
--- a/gdbserver/tdesc.cc
+++ b/gdbserver/tdesc.cc
@@ -53,7 +53,8 @@  void target_desc::accept (tdesc_element_visitor &v) const
 
 void
 init_target_desc (struct target_desc *tdesc,
-		  const char **expedite_regs)
+		  const char **expedite_regs,
+		  enum gdb_osabi osabi)
 {
   int offset = 0;
 
@@ -88,6 +89,8 @@  init_target_desc (struct target_desc *tdesc,
   int expedite_count = 0;
   while (expedite_regs[expedite_count] != nullptr)
     tdesc->expedite_regs.push_back (expedite_regs[expedite_count++]);
+
+  set_tdesc_osabi (tdesc, osabi);
 #endif
 }
 
diff --git a/gdbserver/tdesc.h b/gdbserver/tdesc.h
index 4796b50b4d1..9264786de51 100644
--- a/gdbserver/tdesc.h
+++ b/gdbserver/tdesc.h
@@ -20,6 +20,7 @@ 
 #define GDBSERVER_TDESC_H
 
 #include "gdbsupport/tdesc.h"
+#include "gdbsupport/osabi.h"
 
 #include "regdef.h"
 #include <vector>
@@ -81,10 +82,11 @@  void copy_target_description (struct target_desc *dest,
 			      const struct target_desc *src);
 
 /* Initialize TDESC, and then set its expedite_regs field to
-   EXPEDITE_REGS.  */
+   EXPEDITE_REGS and its osabi to OSABI.  */
 
 void init_target_desc (struct target_desc *tdesc,
-		       const char **expedite_regs);
+		       const char **expedite_regs,
+		       enum gdb_osabi osabi);
 
 /* Return the current inferior's target description.  Never returns
    NULL.  */
diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
index 0a761ca58ef..13f9aca99b1 100644
--- a/gdbserver/win32-i386-low.cc
+++ b/gdbserver/win32-i386-low.cc
@@ -596,12 +596,12 @@  i386_arch_setup (void)
 #ifdef __x86_64__
   tdesc = amd64_create_target_description (X86_XSTATE_SSE_MASK, false,
 					   false, false);
-  init_target_desc (tdesc, amd64_expedite_regs);
+  init_target_desc (tdesc, amd64_expedite_regs, WINDOWS_OSABI);
   win32_tdesc = tdesc;
 #endif
 
   tdesc = i386_create_target_description (X86_XSTATE_SSE_MASK, false, false);
-  init_target_desc (tdesc, i386_expedite_regs);
+  init_target_desc (tdesc, i386_expedite_regs, WINDOWS_OSABI);
 #ifdef __x86_64__
   wow64_win32_tdesc = tdesc;
 #else
diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h
index ff997df0a66..daed16a6ae6 100644
--- a/gdbserver/win32-low.h
+++ b/gdbserver/win32-low.h
@@ -21,6 +21,7 @@ 
 
 #include <windows.h>
 #include "nat/windows-nat.h"
+#include "gdbsupport/osabi.h"
 
 struct target_desc;
 
@@ -31,6 +32,12 @@  extern const struct target_desc *win32_tdesc;
 extern const struct target_desc *wow64_win32_tdesc;
 #endif
 
+#ifdef __CYGWIN__
+constexpr enum gdb_osabi WINDOWS_OSABI = GDB_OSABI_CYGWIN;
+#else
+constexpr enum gdb_osabi WINDOWS_OSABI = GDB_OSABI_WINDOWS;
+#endif
+
 struct win32_target_ops
 {
   /* Architecture-specific setup.  */