This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [Patch]: ld crash on i386/pe when linking with a dll that has no exports
On Thu, Nov 26, 2009 at 09:24:32PM +0000, Dave Korn wrote:
>Christopher Faylor wrote:
>> On Wed, Nov 25, 2009 at 10:20:54AM +0100, Tristan Gingold wrote:
>>> Hi,
>>>
>>> we got a crash when linking with a dll whose export directory is empty:
>>> ...
>>> NumberOfRvaAndSizes 00000010
>>>
>>> The Data Directory
>>> Entry 0 00000000 00000000 Export Directory [.edata (or where ever we found it)]
>>> Entry 1 00016000 0000003c Import Directory [parts of .idata]
>>> ...
>>>
>>> In fact there is no guard against such values in ld/pe-dll.c(pe_implied_import_dll) and this function
>>> assumes that the export directory entry is not empty if present.
>>>
>>> This patch fixes this crash.
>>
>> Isn't this supposed to be handled by the
>>
>> if (num_entries < 1) /* No exports. */
>> return FALSE;
>>
>> a few lines above it?
>>
>> If this test is inadequate then it seems like num_entries isn't needed
>> and should be deleted.
>
> Nope, that's a misunderstanding of the meaning of num_entries (which I also
>made, until I went back and checked the spec). The NumberOfRvaAndSizes value
>is the count of filled-out entries in the IMAGE_DATA_DIRECTORY array, so this
>is just checking that entry #0 (.edata size and offset) is even present; then
>it goes on to look at the data pointed to by that entry, without noticing that
>despite being filled out, it might be filled out with null values. The order
>of data directory entries is fixed, so you have to have a null export table
>entry if you want to be able to put anything in the subsequent import table entry.
>
> Linking against a DLL with zero exports may be kind of pointless, but it
>should at least work!
Ah, yes, I think that was the point of the patch.
>The patch is OK, thank you Tristan :)
Actually, Tristan, please don't check this in as-is.
Having two comments which say "No exports" is confusing. The comment should
probably be something like:
/* Null export table - nothing to export */
And, maybe the previous comment should say /* No imports or exports */.
cgf