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


Kai Tietz wrote:
> 2009/1/4 Kai Tietz:
>> Hi Dave,

  Hi again Kai,

>> 2009/1/3 Dave Korn:
>>>  I wonder if we are having a 32-vs-64-bit clash here, because I know you
>>> mostly work on win64?  I'm sure I don't understand your patch.  At first I
>>> thought maybe it was something to do with the difference between
>>> IMAGE_THUNK_DATA32 and IMAGE_THUNK_DATA64 and you were trying to offset a
>>> 32-bit reloc so it fell into the low part of a 64-bit field, and then I
>>> started thinking about endianness, and then I got confused and decided to ask
>>> you about it instead.  But I'm pretty sure that the right thing for
>>> i686-pc-cygwin is not to have those offsets there; what target were you
>>> working on at the time?
>
> Sorry, it was a bit late yesterday, when I answered your e-mail. I try
> to express myself a bit more understandable.

  Thank you, I acknowledge that you are doing more work to overcome the
language barrier than I am!

> Yes, I think this was a thinko related to the issue that in pe-dll.c
> (make_one ...) the  import sections .idata$4 and .idata$5 are prefixed
> by a zero element. But for auto-imports there are no prefix for
> idata$4 and idata$5 generated.

  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.

> 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 OriginalFirstThunk member 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 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?

> Could you give the idata$4/idata$5 patch of mine a chance to verify,
> that your problem is solved by it, too?

  Your patch looks like it would be a correct way to remove the leading zero
entries, but I have reservations.  First, that I do not understand why the
leading zero elements are present in the first place, so I cannot be sure what
the effect of removing them may be.  Second, that I cannot see why they have
caused a problem, so I am not certain that removing them is the correct
solution to however the problem has arisen.

  Can you show, as I did, with hex dumps, how the leading zero entries cause
a  reloc or fixup somewhere to be miscalculated?  That would really help me to
understand where the problem arises.  What I'm wondering is if the problem
actually comes because of some alignment requirement somewhere that means when
the idata$4/5 sections all get concatenated together, a gap is introduced
between the first and second sections meaning that the 'fthunk' and 'hname'
symbols no longer point at the first byte of the second section but instead
point at some padding that was interposed.  In that case, the answer might be
to add an extra zero word rather than remove them; it depends what the purpose
of the zero words was for in the first place.

    cheers,
      DaveK


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