BFD interface for getting PE import section data

BFD interface for getting PE import section data

[Simon Marchi]

Hi,

In order for GDB to be able to differentiate Cygwin executables from non-Cygwin
Windows executables (in this context [1]), I'd like to access the data of the
import section of the PE executables, more specifically the DLL names.  We would
check if the cygwin DLL is present in this list.  I was able to look up and parse
enough of the .idata section to get the imported DLL names, using the official doc:

  https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#the-idata-section

as well as this page:

  https://blog.kowalczyk.info/articles/pefileformat.html#9ccef823-67e7-4372-9172-045d7b1fb006

However, this sounds like some facility BFD should provide.  Is there a way to get
this information that I'm missing?  I found this code, that's used by "objdump -p"
to print the content of the import tables:

  https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=bfd/peXXigen.c;h=e42d646552a0ca1e856e082256cd3d943b54ddf0;hb=HEAD#l1261

However, that code is tightly coupled with the printed, so it's not practical to
use it from GDB.  In order to avoid code duplication, I suppose that this code could
be refactored to provide this information through some programmatic interface, which
both GDB and this printing code would use.  However, that's a bit too much for me to
chew, since I'm not familiar with BFD development.

Any pointers would be appreciated.  Thanks!

Simon

[1] https://sourceware.org/ml/gdb-patches/2020-03/msg00195.html

Re: BFD interface for getting PE import section data

[Nick Clifton]

Hi Simon,

  [Sorry for the long delay in replying to your email].

> However, this sounds like some facility BFD should provide.  Is there a way to get
> this information that I'm missing?

Hmm - only if you have access to the BFD library's internal headers.  (Which
I assume you do not want to have to do).

Would a new BFD library function help ?  One that took a BFD and returned a
pointer to its internal_extra_pe_aouthdr structure (as defined in include/coff/internal.h)
or NULL if the BFD does not have one ?  Providing a function like that should
be fairly simple to implement.

Cheers
  Nick

Re: BFD interface for getting PE import section data

[Simon Marchi]

On 2020-03-23 1:18 p.m., Nick Clifton wrote:
> Hi Simon,
> 
>   [Sorry for the long delay in replying to your email].
> 
>> However, this sounds like some facility BFD should provide.  Is there a way to get
>> this information that I'm missing?
> 
> Hmm - only if you have access to the BFD library's internal headers.  (Which
> I assume you do not want to have to do).

Hi Nick,

In fact, since this is in GDB, we do have access to BFD's internal header.  It's
not ideal, but it works.  Even before my work on this, there was an instance of
accessing this internal structure:

  https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/amd64-windows-tdep.c;h=6d5076d1c4371becf4b122a5393ede780cd0f37a;hb=HEAD#l954

I got inspiration from that code and I also implemented what I wanted by accessing
that internal structure, this is what I ended up doing:

  https://sourceware.org/git/?p=binutils-gdb.git;a=blobdiff;f=gdb/windows-tdep.c;h=31b7b57005df67f5b02e98e92b568a0cfaea97d0;hp=e02b1ceed3873e3d4906a8df62964258a777f2a4;hb=8db52437243e251c01e352cdb325bc9ace578e7c;hpb=5982a56ab9d161923e75712fcb358824748ea4ba

> Would a new BFD library function help ?  One that took a BFD and returned a
> pointer to its internal_extra_pe_aouthdr structure (as defined in include/coff/internal.h)
> or NULL if the BFD does not have one ?  Providing a function like that should
> be fairly simple to implement.

I think it would help a bit, in that GDB wouldn't have to import a BFD internal header
just for this.

Simon

Re: BFD interface for getting PE import section data

[Nick Clifton]

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

Hi Simon,

>> Would a new BFD library function help ?  One that took a BFD and returned a
>> pointer to its internal_extra_pe_aouthdr structure (as defined in include/coff/internal.h)
>> or NULL if the BFD does not have one ?  Providing a function like that should
>> be fairly simple to implement.
> 
> I think it would help a bit, in that GDB wouldn't have to import a BFD internal header
> just for this.

Fair enough.  In which case I have now applied this patch.

Cheers
  Nick

bfd/ChangeLog
2020-03-25  Nick Clifton  <nickc@redhat.com>

	* cofflink.c (bfd_coff_get_internal_extra_pe_aouthdr): New
	function.
	* libbfd-in.h (bfd_coff_get_internal_extra_pe_aouthdr): Prototype.
	* libbfd.h: Regenerate.

[-- Attachment #2: bfd_coff_get_internal_extra_pe_aouthdr.patch --]
[-- Type: text/x-patch, Size: 1612 bytes --]

diff --git a/bfd/cofflink.c b/bfd/cofflink.c
index e52f543ee6..845d1e5846 100644
--- a/bfd/cofflink.c
+++ b/bfd/cofflink.c
@@ -3157,3 +3157,12 @@ _bfd_coff_generic_relocate_section (bfd *output_bfd,
     }
   return TRUE;
 }
+
+
+struct internal_extra_pe_aouthdr *
+bfd_coff_get_internal_extra_pe_aouthdr (bfd* abfd)
+{
+  if (abfd == NULL || bfd_get_flavour (abfd) != bfd_target_coff_flavour)
+    return NULL;
+  return & pe_data (abfd)->pe_opthdr;
+}
diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h
index 5d24efbeb2..2f1f88644e 100644
--- a/bfd/libbfd-in.h
+++ b/bfd/libbfd-in.h
@@ -854,6 +854,11 @@ extern bfd_vma _bfd_get_gp_value
 extern void _bfd_set_gp_value
   (bfd *, bfd_vma) ATTRIBUTE_HIDDEN;
 
+/* Provide access to the internal_extra_pe_aouthdr structure which
+   contains interesting information for PE format binaries.  */
+extern struct internal_extra_pe_aouthdr *
+  bfd_coff_get_internal_extra_pe_aouthdr (bfd *);
+
 /* Function shared by the COFF and ELF SH backends, which have no
    other common header files.  */
 
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index 348ccfd4b5..fc8b81c45f 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -859,6 +859,11 @@ extern bfd_vma _bfd_get_gp_value
 extern void _bfd_set_gp_value
   (bfd *, bfd_vma) ATTRIBUTE_HIDDEN;
 
+/* Provide access to the internal_extra_pe_aouthdr structure which
+   contains interesting information for PE format binaries.  */
+extern struct internal_extra_pe_aouthdr *
+  bfd_coff_get_internal_extra_pe_aouthdr (bfd *);
+
 /* Function shared by the COFF and ELF SH backends, which have no
    other common header files.  */
 

Re: BFD interface for getting PE import section data

[Simon Marchi]

On 2020-03-25 7:58 a.m., Nick Clifton wrote:
> Hi Simon,
> 
>>> Would a new BFD library function help ?  One that took a BFD and returned a
>>> pointer to its internal_extra_pe_aouthdr structure (as defined in include/coff/internal.h)
>>> or NULL if the BFD does not have one ?  Providing a function like that should
>>> be fairly simple to implement.
>>
>> I think it would help a bit, in that GDB wouldn't have to import a BFD internal header
>> just for this.
> 
> Fair enough.  In which case I have now applied this patch.
> 
> Cheers
>   Nick
> 
> bfd/ChangeLog
> 2020-03-25  Nick Clifton  <nickc@redhat.com>
> 
> 	* cofflink.c (bfd_coff_get_internal_extra_pe_aouthdr): New
> 	function.
> 	* libbfd-in.h (bfd_coff_get_internal_extra_pe_aouthdr): Prototype.
> 	* libbfd.h: Regenerate.
> 

Thanks.  I am not sure how we are meant to use it though.

If we take a look at windows-tdep.c in GDB for example, we currently include:

  #include "coff/internal.h"
  #include "libcoff.h"

and access the structure with

  bfd_vma idata_addr =
    pe_data (abfd)->pe_opthdr.DataDirectory[PE_IMPORT_TABLE].VirtualAddress;

With the new function, it would become this I suppose:

  bfd_vma idata_addr =
    bfd_coff_get_internal_extra_pe_aouthdr (abfd)->DataDirectory[PE_IMPORT_TABLE].VirtualAddress;

To use it, are we expected to include "libbfd.h" instead of the two other files?  Since "libbfd.h"
is also a BFD-internal file, what's the advantage of using that over the other internal headers?

I noticed that GDB used to include libbfd.h, but Alan removed all includes of that files in this commit:

  https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=293acfae4e3c9aad417e262edc9847c79bbbbb11

I don't really know what was the rationale of this commit, what was the motivation for remove all
these includes?  If there was a good reason to remove all these includes, there must be a good
reason not to add one back.

Anyhow, when I try to include "libbfd.h" from GDB's "windows-tdep.c", I get a bunch of "invalid
conversion" errors (which could be easily fixed), since GDB is compiled as C++.

But even after that, we need to access the structure itself,the PE_IMPORT_TABLE define as well
as the IMAGE_DATA_DIRECTORY structure, all defined in coff/internal.h.

I don't really mind if GDB includes some BFD internal header, since we live in the same repo.  If
a BFD change breaks GDB, we'll know quickly.

What I think BFD could provide here (or maybe this is out of the scope of BFD?) is a more high
level function that lets the user iterate on all DLLs imported by a given PE.  I wrote some code
to parse the section that describes that, which is fine, but if some other user needed the same
information, it would be a shame if they had to re-write the same code.  In fact, this code is
already written in function `pe_print_idata`, it's just not made to be consumed in a programmatic
interface.

Simon

Re: BFD interface for getting PE import section data

[Nick Clifton]

Hi Simon,

> I don't really know what was the rationale of this commit, what was the motivation for remove all
> these includes?  If there was a good reason to remove all these includes, there must be a good
> reason not to add one back.

You have a good point.  I did not really think this through.
I will remove the new function and consider how/if the BFD
library can provide a more high level interface along the
lines that you have suggested.
 
Cheers
  Nick