From: Christian Biesinger <cbiesinger@google.com>
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: gdb-patches <gdb-patches@sourceware.org>,
Jon Turney <jon.turney@dronecode.org.uk>,
Simon Marchi <simon.marchi@efficios.com>
Subject: Re: [PATCH 6/7] gdb: select "Cygwin" OS ABI for Cygwin binaries
Date: Mon, 16 Mar 2020 13:16:39 -0500 [thread overview]
Message-ID: <CAPTJ0XEg+eWuQ7xSw4KzhvMCj8O0zMtrf-x2L1VP=WB8m=Wfew@mail.gmail.com> (raw)
In-Reply-To: <20200316170845.184386-7-simon.marchi@polymtl.ca>
On Mon, Mar 16, 2020 at 12:09 PM Simon Marchi via Gdb-patches
<gdb-patches@sourceware.org> wrote:
>
> From: Simon Marchi <simon.marchi@efficios.com>
>
> Before this patch, the "Windows" OS ABI is selected for all Windows
> executables, including Cygwin ones. This patch makes GDB differentiate
> Cygwin binaries from non-Cygwin ones, and selects the "Cygwin" OS ABI
> for the Cygwin ones.
>
> To check whether a Windows PE executable is a Cygwin one, we check the
> library list in the .idata section, see if it contains "cygwin1.dll".
>
> I had to add code to parse the .idata section, because BFD doesn't seem
> to expose this information. BFD does parse this information, but only
> to print it in textual form (function pe_print_idata):
>
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=bfd/peXXigen.c;h=e42d646552a0ca1e856e082256cd3d943b54ddf0;hb=HEAD#l1261
>
> Here's the relevant portion of the PE format documentation:
>
> https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#the-idata-section
>
> This page was also useful:
>
> https://blog.kowalczyk.info/articles/pefileformat.html#9ccef823-67e7-4372-9172-045d7b1fb006
>
> With this patch applied, this is what I get:
>
> (gdb) file some_mingw_x86_64_binary.exe
> Reading symbols from some_mingw_x86_64_binary.exe...
> (gdb) show osabi
> The current OS ABI is "auto" (currently "Windows").
> The default OS ABI is "GNU/Linux".
>
> (gdb) file some_mingw_i386_binary.exe
> Reading symbols from some_mingw_i386_binary.exe...
> (gdb) show osabi
> The current OS ABI is "auto" (currently "Windows").
> The default OS ABI is "GNU/Linux".
>
> (gdb) file some_cygwin_x86_64_binary.exe
> Reading symbols from some_cygwin_x86_64_binary.exe...
> (gdb) show osabi
> The current OS ABI is "auto" (currently "Cygwin").
> The default OS ABI is "GNU/Linux".
>
> gdb/ChangeLog:
>
> * windows-tdep.h (is_linked_with_cygwin_dll): New declaration.
> * windows-tdep.c (CYGWIN_DLL_NAME): New.
> (pe_import_directory_entry): New struct type.
> (is_linked_with_cygwin_dll): New function.
> * amd64-windows-tdep.c (amd64_windows_osabi_sniffer): Select
> GDB_OSABI_CYGWIN if the BFD is linked with the Cygwin DLL.
> * i386-windows-tdep.c (i386_windows_osabi_sniffer): Likewise.
> ---
> gdb/amd64-windows-tdep.c | 9 ++--
> gdb/i386-windows-tdep.c | 9 ++--
> gdb/windows-tdep.c | 101 +++++++++++++++++++++++++++++++++++++++
> gdb/windows-tdep.h | 6 +++
> 4 files changed, 119 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
> index 88ff794abcb6..e0346f8628fe 100644
> --- a/gdb/amd64-windows-tdep.c
> +++ b/gdb/amd64-windows-tdep.c
> @@ -1249,10 +1249,13 @@ amd64_windows_osabi_sniffer (bfd *abfd)
> {
> const char *target_name = bfd_get_target (abfd);
>
> - if (strcmp (target_name, "pei-x86-64") == 0)
> - return GDB_OSABI_WINDOWS;
> + if (!streq (target_name, "pei-x86-64"))
> + return GDB_OSABI_UNKNOWN;
>
> - return GDB_OSABI_UNKNOWN;
> + if (is_linked_with_cygwin_dll (abfd))
> + return GDB_OSABI_CYGWIN;
> +
> + return GDB_OSABI_WINDOWS;
> }
>
> void _initialize_amd64_windows_tdep ();
> diff --git a/gdb/i386-windows-tdep.c b/gdb/i386-windows-tdep.c
> index a71ceda781f8..bd6107b02f1f 100644
> --- a/gdb/i386-windows-tdep.c
> +++ b/gdb/i386-windows-tdep.c
> @@ -232,10 +232,13 @@ i386_windows_osabi_sniffer (bfd *abfd)
> {
> const char *target_name = bfd_get_target (abfd);
>
> - if (strcmp (target_name, "pei-i386") == 0)
> - return GDB_OSABI_WINDOWS;
> + if (!streq (target_name, "pei-i386"))
> + return GDB_OSABI_UNKNOWN;
>
> - return GDB_OSABI_UNKNOWN;
> + if (is_linked_with_cygwin_dll (abfd))
> + return GDB_OSABI_CYGWIN;
> +
> + return GDB_OSABI_WINDOWS;
> }
>
> static enum gdb_osabi
> diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
> index e02b1ceed387..32e551bcb175 100644
> --- a/gdb/windows-tdep.c
> +++ b/gdb/windows-tdep.c
> @@ -38,6 +38,8 @@
> #include "libcoff.h"
> #include "solist.h"
>
> +#define CYGWIN_DLL_NAME "cygwin1.dll"
> +
> /* Windows signal numbers differ between MinGW flavors and between
> those and Cygwin. The below enumeration was gleaned from the
> respective headers; the ones marked with MinGW64/Cygwin are defined
> @@ -898,6 +900,105 @@ static const struct internalvar_funcs tlb_funcs =
> NULL
> };
>
> +/* Layout of an element of a PE's Import Directory Table. Based on:
> +
> + https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#import-directory-table
> + */
> +
> +struct pe_import_directory_entry
> +{
> + uint32_t import_lookup_table_rva;
> + uint32_t timestamp;
> + uint32_t forwarder_chain;
> + uint32_t name_rva;
> + uint32_t import_address_table_rva;
> +};
> +
> +gdb_static_assert (sizeof (pe_import_directory_entry) == 20);
> +
> +/* Return true if the Portable Executable behind ABFD uses the Cygwin dll
> + (cygwin1.dll). */
> +/* See windows-tdep.h. */
Remove the first of these two comments?
> +
> +bool
> +is_linked_with_cygwin_dll (bfd *abfd)
> +{
> + /* The list of DLLs a PE is linked to is in the .idata section. See:
> +
> + https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#the-idata-section
> + */
> + asection *idata_section = bfd_get_section_by_name (abfd, ".idata");
> + if (idata_section == nullptr)
> + return false;
> +
> + /* Find the virtual address of the .idata section. We must subtract this
> + from the RVAs (relative virtual addresses) to obtain an offset in the
> + section. */
> + bfd_vma idata_addr =
> + pe_data (abfd)->pe_opthdr.DataDirectory[PE_IMPORT_TABLE].VirtualAddress;
> +
> + /* Map the section's data. */
> + bfd_size_type idata_size;
> + const gdb_byte *const idata_contents
> + = gdb_bfd_map_section (idata_section, &idata_size);
> + if (idata_contents == nullptr)
> + {
> + warning (_("Failed to get content of .idata section."));
> + return false;
> + }
> +
> + const gdb_byte *iter = idata_contents;
> + const gdb_byte *end = idata_contents + idata_size;
> + const pe_import_directory_entry null_dir_entry = { 0 };
> +
> + /* Iterate through all directory entries. */
> + while (true)
> + {
> + /* Is there enough space left in the section for another entry? */
> + if (iter + sizeof (pe_import_directory_entry) > end)
> + {
> + warning (_("Failed to parse .idata section: unexpected end of "
> + ".idata section."));
> + break;
> + }
> +
> + pe_import_directory_entry *dir_entry = (pe_import_directory_entry *) iter;
> +
> + /* Is it the end of list marker? */
> + if (memcmp (dir_entry, &null_dir_entry,
> + sizeof (pe_import_directory_entry)) == 0)
> + break;
> +
> + bfd_vma name_addr = dir_entry->name_rva;
> +
> + /* If the name's virtual address is smaller than the section's virtual
> + address, there's a problem. */
> + if (name_addr < idata_addr
> + || name_addr >= (idata_addr + idata_size))
> + {
> + warning (_("\
> +Failed to parse .idata section: name's virtual address (0x%" BFD_VMA_FMT "x) \
> +is outside .idata section's range [0x%" BFD_VMA_FMT "x, 0x%" BFD_VMA_FMT "x[."),
> + name_addr, idata_addr, idata_addr + idata_size);
> + break;
> + }
> +
> + const gdb_byte *name = &idata_contents[name_addr - idata_addr];
> +
> + /* Make sure we don't overshoot the end of the section with the streq. */
> + if (name + sizeof(CYGWIN_DLL_NAME) > end)
> + continue;
> +
> + /* Finally, check if this is the dll name we are looking for. */
> + if (streq ((const char *) name, CYGWIN_DLL_NAME))
> + return true;
> +
> + iter += sizeof(pe_import_directory_entry);
> + }
> +
> + return false;
> +}
> +
> void _initialize_windows_tdep ();
> void
> _initialize_windows_tdep ()
> diff --git a/gdb/windows-tdep.h b/gdb/windows-tdep.h
> index 34474f259c2a..f2dc4260469d 100644
> --- a/gdb/windows-tdep.h
> +++ b/gdb/windows-tdep.h
> @@ -33,4 +33,10 @@ extern void windows_xfer_shared_library (const char* so_name,
>
> extern void windows_init_abi (struct gdbarch_info info,
> struct gdbarch *gdbarch);
> +
> +/* Return true if the Portable Executable behind ABFD uses the Cygwin dll
> + (cygwin1.dll). */
> +
> +extern bool is_linked_with_cygwin_dll (bfd *abfd);
> +
> #endif
> --
> 2.25.1
>
next prev parent reply other threads:[~2020-03-16 18:17 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-16 17:08 [PATCH 0/7] Add "Windows" OS ABI Simon Marchi
2020-03-16 17:08 ` [PATCH 1/7] gdb: recognize 64 bits Windows executables as Cygwin osabi Simon Marchi
2020-03-16 17:08 ` [PATCH 2/7] gdb: move enum gdb_osabi to osabi.h Simon Marchi
2020-03-16 17:08 ` [PATCH 3/7] gdb: add Windows OS ABI Simon Marchi
2020-03-16 17:08 ` [PATCH 4/7] gdb: rename i386-cygwin-tdep.c to i386-windows-tdep.c Simon Marchi
2020-03-16 17:08 ` [PATCH 5/7] gdb: rename content of i386-windows-tdep.c, cygwin to windows Simon Marchi
2020-03-16 17:08 ` [PATCH 6/7] gdb: select "Cygwin" OS ABI for Cygwin binaries Simon Marchi
2020-03-16 18:16 ` Christian Biesinger [this message]
2020-03-16 18:18 ` Simon Marchi
2020-03-16 19:03 ` Jon Turney
2020-03-16 21:00 ` Simon Marchi
2020-04-01 19:05 ` Tom Tromey
2020-04-01 19:25 ` Simon Marchi
2020-04-01 21:36 ` Pedro Alves
2020-04-01 21:53 ` Simon Marchi
2020-04-02 13:56 ` Pedro Alves
2020-04-02 14:01 ` Simon Marchi
2020-04-02 14:03 ` Pedro Alves
2020-04-02 14:08 ` Simon Marchi
2020-04-02 14:17 ` Simon Marchi
2020-04-02 13:22 ` Tom Tromey
2020-04-02 14:55 ` [PATCH] gdb: use bfd_get_section_contents to read section contents in, is_linked_with_cygwin_dll (was: Re: [PATCH 6/7] gdb: select "Cygwin" OS ABI for Cygwin binaries) Simon Marchi
2020-04-02 14:57 ` [PATCH] gdb: use bfd_get_section_contents to read section contents in, is_linked_with_cygwin_dll Simon Marchi
2020-04-02 19:01 ` Tom Tromey
2020-04-02 19:42 ` Simon Marchi
2020-04-02 19:45 ` Tom Tromey
2020-04-02 19:47 ` Simon Marchi
2020-03-16 17:08 ` [PATCH 7/7] gdb: define builtin long type to be 64 bits on amd64 Cygwin Simon Marchi
2020-03-16 17:46 ` [PATCH 0/7] Add "Windows" OS ABI Eli Zaretskii
2020-03-16 17:48 ` Simon Marchi
2020-03-16 19:04 ` Jon Turney
2020-04-01 21:42 ` Pedro Alves
2020-04-01 21:56 ` Simon Marchi
2020-04-02 3:06 ` [PATCH] gdb: stop using host-dependent signal numbers in, windows-tdep.c (was: Re: [PATCH 0/7] Add "Windows" OS ABI) Simon Marchi
2020-04-02 14:00 ` [PATCH] gdb: stop using host-dependent signal numbers in, windows-tdep.c Pedro Alves
2020-04-02 14:02 ` Simon Marchi
2020-04-02 15:12 ` Eli Zaretskii
2020-04-08 12:45 ` Jon Turney
2020-04-08 18:16 ` Simon Marchi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPTJ0XEg+eWuQ7xSw4KzhvMCj8O0zMtrf-x2L1VP=WB8m=Wfew@mail.gmail.com' \
--to=cbiesinger@google.com \
--cc=gdb-patches@sourceware.org \
--cc=jon.turney@dronecode.org.uk \
--cc=simon.marchi@efficios.com \
--cc=simon.marchi@polymtl.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).