[PATCH] COFFObjectFile imports fix for compressed binaries

Bandzi Michal via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 6 19:07:08 PDT 2016


Cituji David Majnemer <david.majnemer at gmail.com>:

> On Mon, Jul 4, 2016 at 9:52 PM, David Majnemer <david.majnemer at gmail.com>
> wrote:
>
>>
>>
>> On Mon, Jul 4, 2016 at 2:10 PM, Bandzi Michal <xbandz00 at stud.fit.vutbr.cz>
>> wrote:
>>
>>> Unless I made some logical error I can't see, it should work that way,
>>> doesn't it?
>>>
>>> if (lookupTable)
>>>   return func(lookupTable, ...)
>>> return func(addressTable, ...)
>>
>>
>> I was trying to suggest that we ignore the lookup table case and just do:
>>   return func(addressTable, ...)
>>
>
>
> Thinking about it some more... Let's just have two sets of functions, one
> for the lookupTable and one for the addressTable.
> Let's let the caller decide what information they are interested in :)
>

There is a little problem with this though. There is a condition in  
import direcotry iterators "ImportDirectory[0].ImportLookupTableRVA ==  
0 && ImportDirectory[0].ImportAddressTableRVA == 0" which stops  
iteration at empty import table entry. Before using either lookuptable  
or addresstable, one must check if table is not missing and I am not  
sure where to put that control (if such a control is added, mentioned  
condition can be simplified to ImportDirectory[0].Name == 0). This  
complicates code in similar way my previous solution did. Also I am  
not sure if choice is that useful as both tables are identical until  
file is loaded to memory - except for maybe core dump files.

>>
>>
>>>
>>>
>>> I wonder if we should just rely on the ImportAddressTableRVA instead of
>>>> first trying to use the ImportLookupTableRVA.
>>>>
>>>> What are your thoughts?
>>>>
>>>> On Mon, Jul 4, 2016 at 11:05 AM, Bandzi Michal <
>>>> xbandz00 at stud.fit.vutbr.cz>
>>>> wrote:
>>>>
>>>> Cituji David Majnemer <david.majnemer at gmail.com>:
>>>>>
>>>>> Please clang-format the changes in your patch.
>>>>>
>>>>>>
>>>>>>
>>>>> Not sure what exactly this means. Found and used this:
>>>>>
>>>>> http://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting
>>>>> then created a new patch
>>>>>
>>>>> Also, LLVM does not use else after a return:
>>>>>
>>>>>> http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
>>>>>>
>>>>>>
>>>>> Hope this is better. New patch as attachement.
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>
>





More information about the llvm-commits mailing list