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

[Kevin Buettner <kevinb@redhat.com>]

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>