This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [BUG?] Re: [patch]: Fix auto-import for pe targets


"Dave Korn" <dave.korn.cygwin@googlemail.com> wrote on 05.01.2009 
00:22:46:

>   OK, so what you're observing here is that make_head() puts these two 
zeros:
> 
> /*   .section   .idata$2
>     .global      __head_my_dll
>    __head_my_dll:
>     .rva      hname
>     .long      0
>     .long      0
>     .rva      __my_dll_iname
>     .rva      fthunk
> 
>     .section   .idata$5
>     .long      0
>    fthunk:
> 
>     .section   .idata$4
>     .long      0
>    hname:                              */
> 
> before fthunk and hname, whereas make_import_fixup_entry() only emits 
any
> contents for section idata$2 (some local mods to the comment here):
> 
> /*
>    This function generates an IMAGE_IMPORT_DESCRIPTOR:
> 
>    .section   .idata$2
>    ; OriginalFirstThunk
>      .rva      __nm_thnk_SYM (singleton thunk with name of func)
>    ; TimeDateStamp
>     .long      0
>    ; ForwarderChain
>     .long      0
>    ; Name
>     .rva      __my_dll_iname (name of dll)
>    ; FirstThunk
>     .rva      __fuNN_SYM (pointer to reference (address) in text)  */
> 
> and nothing in idata$4 or idata$5, right?  I think I follow so far.

In idata$4 (import lookup table or as I named it import hash table) is 
placed __nm_thnk_SYM. The symbol __fuNN_SYM isn't placed in idata$5.
So far we have common understanding.

> > The issue I saw here before was that for existing import tables the
> > initial zero element let point the fixup to the wrong location. This
> > was the reasone to add wrongly the offset to the auto-fixup import
> > directory.
> 
>   Now I don't understand again.  I cannot see how the presence of this 
gap
> could affect the fixup.  All the pointers in the fixup are relocs in a
> separate instance of idata$2 section in a separate BFD.  They should be
> concatenated by the final link and the relocs should be performed 
correctly
> relative to either one.
> 
> > I sent some days ago a patch to remove these leading zero elements to
> > idata$4 and idata$5. This should fix the auto-import for 32-bit, too.
> > The issue by those leading zero elements is, that IAT and import hash
> > table
> 
>   There is no term "import hash table" in my documentation.  I think you
> mean the "Import Lookup Table", pointed to by the 
OriginalFirstThunkmember of
> IMAGE_IMPORT_DESCRIPTOR, yes?
> 
> > were build in a none coff spec conformance way.
> 
>   But these leading elements are not even part of the IAT or the ILT. 
They
> form gaps between separate individual IATs and ILTs concatenated in the 
output
> .idata section, as far as I can see.

The problem isn't the gap itself (at least not for mingw targets). The 
problem is that the (at the moment by this reason not emitted IAT 
directory entry) isn't ok. The .idata$5 is in fact IAT and is not part of 
the import address directory (at least vs doesn't do this that way).
For some systems it is necessary that the IAT is getting into writeable 
sections (as you mentioned), but the remaining sections of idata$* haven't 
to be putted in write-able sections. IIRC this check was done for 
preventing some sorts of hijacking code.

> > The problem is, that for the first import directory everything is fine
> > (beside that the size of the idata$5 section isn't IAT size),
> 
>   Is it expressed anywhere as a requirement that it should be the same 
size?
> 
> > For more
> > then one import library, we get by this old behavior between each
> > "import hash table" (idata$4) two zero element, like for the import
> > address table (IAT). This isn't as spec tells (I refer here to coff
> > spec 8.1).
> 
>   As far as I can see, the spec makes no requirements that there not be 
gaps
> between different elements concatenated into the idata section.  It 
looks to
> me like fig. 3 at the start of section 6.4 indicates that there is a 
single
> NULL trailing each ILT, but then the separate ILTs with their trailing 
NULLs
> are drawn as disjoint boxes, indicating that they need not be packed or
> concatenated directly end to end but that there may be gaps.  As long as 
the
> pointers in the IMAGE_IMPORT_DESCRIPTORs (the array entries in the 
Import
> Directory Table) point correctly to the start of each IAT/ILT, I cannot 
see
> why it would matter if there are gaps.  Did I miss something in the 
spec?

For sure .idata$4 elements are merged and the .idata$5 elements are merged 
without gaps and without alignment. That no alignment is necessary here is 
easy to understand by the fact, that the element size for each has size of 
address.

The IAT is broken, because it is part of the IMPORT directory and by the 
gap introduced by make_head. By comparing vs and binutils
output and directory entries the following layout differences cab be 
shown:

For Directory entries in pe(+) header:
 - binutils places into PE_IMPORT_TABLE the address to idata and set the 
size to section size of (idata).
 - binutils doesn't handle the the PE_IMPORT_ADDRESS_TABLE at all (and I 
assume the wrong builded size of idata$5)

 - vc places into PE_IMPORT_TABLE the address of idata$2 and set the size 
to of it just to of idata$2 (plus the null entry).
 - vc places into PE_IMPORT_ADDRESS_TABLE to address of idata$5 and sizeof 
merged idata$5.

The layout for idata looks like this in general (beside that idata$5 
should be placed into writable section and for ppc idata$6/idata$7 are 
flipped in meaning)

IMPORT_DIRECTORY_START: .idata$2*
(binutils only) .idata$3 (IIUC this isn't used by any target anymore)
(+zero-idata$2 entry)
IMPORT_DIRECTORY_END:
ILT_START:
.idata$4*
ILT_END:
IAT_START:
.idata$5*
END_START:
HINT_NAME_TABLE_START:
.idata$6*
HINT_NAME_TABLE_END:
DLL_NAME_START:
.idata$7*
DLL_NAME_END:

As I already said all these regions are placed by binutils into one 
section. And PE_IMPORT_TABLE is set as 
(va:IMPORT_DIRECTORY_START,size:DLL_NAME_END-IMPORT_DIRECTORY_START). 
IMPORT_ADDRESS_TABLE isn't set at all.

As I read the 8.1 coff spec and observed by vs (without any section 
merging features enabled) The following places are used:
PE_IMPORT_TABLE is set to 
(va:IMPORT_DIRECTORY_START,size:IMPORT_DIRECTORY_END-IMPORT_DIRECTORY_START) 
and IMPORT_DIRECTORY_START is placed normally in .rdata(.idata)
ILT_START-ILT_END is put into (.rdata/.idata)
DLL_NAME_START-DLL_NAME_END is put into (.rdata/.idata)
HINT_NAME_TABLE_START-HINT_NAME_TABLE_END) is put into (.rdata/.idata) but 
only when used.

IMPORT_ADDRESS_TABLE is set (va:IAT_START,size:IAT_END-IAT_START) is 
placed into .data

So if you want to export via IAT_DIRECTORY_ENTRY in PE(+) image header, 
the IAT begins with a leading zero and is therefore at least one element 
too big.

So I would elminate the writing of the leading zero element in make_one() 
and adjust dlltool for this, too. First there seems to be no need for 
this, because otherwise the fixup import directory element would need it, 
too. Secondly it is just a waste of memory.

Cheers,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]