[PATCH] D11724: COFF: Add test for ld/section created import library

[Martell Malone via llvm-commits]

Okay I will continue with the genlib tool then.
I will abandon this revision and create a new one once I have a library
with the alias coff obj included.

Thanks for the help

On Thu, Aug 6, 2015 at 5:39 PM, Rui Ueyama <ruiu at google.com> wrote:

> On Thu, Aug 6, 2015 at 9:32 AM, Martell Malone <martellmalone at gmail.com>
> wrote:
>
>> Yeah I thought the same thing would b true.
>> Okay I'll start working towards this then for the gendef tool.
>> Did you know that llvm-objdump can't dump implib sections btw ?
>> I started working on a patch for that.
>> I'll add you as a subscriber when I send in the fix
>>
>>
>> Would you be able to send me the patch you have for dll style libs ?
>>
>
> I don't have a patch for that. My idea was identifying dlltool-style
> import library by the existence of ".idata$7" section (because only the GNU
> extension uses that section) and create a ImportFile for that file, but as
> that was too hacky, I didn't actually write code for that.
>
>
>>
>> Thanks again for the help Rui
>>
>> On Thu, Aug 6, 2015 at 5:28 PM, Rui Ueyama <ruiu at google.com> wrote:
>>
>>> On Thu, Aug 6, 2015 at 9:24 AM, Martell Malone <martellmalone at gmail.com>
>>> wrote:
>>>
>>>> On second thought, we don't have to solve that only with the import
>>>>> library. You can create a (regular) COFF object containing a thunk for an
>>>>> aliased function, and include that object file to a .lib, no?
>>>>
>>>> Yes that seems like a good solution to this problem I might be able to
>>>> add something to the genlib tool to support this.
>>>> I might have to mockup one for yaml2obj.
>>>> Can we have a regular coff object in the same lib as an implib ?
>>>>
>>>
>>> I believe so -- the import library is just a regular ar file (.lib file)
>>> after all and there's no special bit or something that distinguishes import
>>> libraries and other .lib files, but you may want to try if in doubt. You
>>> can add a regular COFF file to an existing import library using lib command.
>>>
>>>
>>>>
>>>> On Thu, Aug 6, 2015 at 5:17 PM, Rui Ueyama <ruiu at google.com> wrote:
>>>>
>>>>> On second thought, we don't have to solve that only with the import
>>>>> library. You can create a (regular) COFF object containing a thunk for an
>>>>> aliased function, and include that object file to a .lib, no?
>>>>>
>>>>> On Thu, Aug 6, 2015 at 8:22 AM, Rui Ueyama <ruiu at google.com> wrote:
>>>>>
>>>>>> I don't think the short import library supports that. With that you
>>>>>> can undecorate names, but AFAIK you cannot define arbitrary aliases for
>>>>>> dllexported symbols. Does mingw-w64 heavily relies on that feature?
>>>>>>
>>>>>> On Thu, Aug 6, 2015 at 8:14 AM, Martell Malone <
>>>>>> martellmalone at gmail.com> wrote:
>>>>>>
>>>>>>> Before I let this slip by aswell there is one big issue with
>>>>>>> switching over to the implib format for mingw-w64
>>>>>>>
>>>>>>> LIBRARY "user32.dll"
>>>>>>> EXPORTS
>>>>>>> MessageBoxA == MessageBoxW
>>>>>>>
>>>>>>> dlltool objct format provides us with the option of having an alias.
>>>>>>> Can we do this in implib ?
>>>>>>>
>>>>>>> What the above .def means that where MessagBoxW is called it is
>>>>>>> joined to MessageBoxA.
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Aug 6, 2015 at 3:58 PM, Martell Malone <
>>>>>>> martellmalone at gmail.com> wrote:
>>>>>>>
>>>>>>>> Yeah, I found that too and it (especially the way how it creates a
>>>>>>>>> gap between .idata$3 and .idata$4) looks really hacky. I also found that
>>>>>>>>> GNU ld has a special logic to order .idata$<n> sections.
>>>>>>>>> At first I thought that I could mimic GNU ld and MSVC linker to
>>>>>>>>> generate the .idata section, but seems like it would really mess up the DLL
>>>>>>>>> import table generation code. We probably should keep the existing logic
>>>>>>>>
>>>>>>>> Yeah I don't think copying gnu ld is the way to go, it is a very
>>>>>>>> hacky project to say the least :)
>>>>>>>>
>>>>>>>>
>>>>>>>> It feels to me that it makes more sense to add a new option to
>>>>>>>>> dlltool to generate short import libraries. It shouldn't be that hard.
>>>>>>>>> Martell, what do you think?
>>>>>>>>
>>>>>>>> I already wrote a replacement tool for this called genlib that will
>>>>>>>> go into the mingw-w64 project.
>>>>>>>> I want to remove mingw-w64's dependancy on binutils so that we can
>>>>>>>> have a clang based toolchain without binutils at all.
>>>>>>>> The notion of having dlltool as part of binutils made no sense in
>>>>>>>> the first place, It should have been part of mingw to begin with.
>>>>>>>> The name was chosen to correlate to on of mingw-w64's other tools
>>>>>>>> called gendef which creates the def files from parsing dll's.
>>>>>>>> I'm still finalizing the code in this and doing some tests.
>>>>>>>>
>>>>>>>> I then tried to detect dlltool-style import files and read them as
>>>>>>>>> if they were short import libraries, so that I can keep the existing code.
>>>>>>>>> That didn't work well because it's not easy to detect dlltool-style import
>>>>>>>>> files in a reliable manner without sacrificing performance.
>>>>>>>>
>>>>>>>> While you might not want to merge because of this into the official
>>>>>>>> project because of performance issues it might be something for the
>>>>>>>> mingw-w64 users to avail of until ld supports import style libraries.
>>>>>>>>
>>>>>>>> If you have the patch for this I'd like have a look at it if
>>>>>>>> possible.
>>>>>>>> I'd like to try and apply this over the PECOFF for the clang 3.7
>>>>>>>> package in our msys2 distro.
>>>>>>>>
>>>>>>>> The issue I have is I would have to get approval to switch to using
>>>>>>>> genlib instead of dlltool as mingw-w64's default.
>>>>>>>> This would not be approved until ld supports implibs and the next
>>>>>>>> version of binutils released.
>>>>>>>> As you probably well know how things work that could take months to
>>>>>>>> get changed over.
>>>>>>>> I'd like to use your patch as a base for a temporary stop over
>>>>>>>> until this happens
>>>>>>>>
>>>>>>>> I'm sure the distro users of msys2 won't mind a performance hit
>>>>>>>> until 3.8 rather then having no support at all.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Aug 5, 2015 at 10:51 PM, Rui Ueyama <ruiu at google.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> On Mon, Aug 3, 2015 at 10:36 PM, Martell Malone <
>>>>>>>>> martellmalone at gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>> From my reading on how gnuld handles PE/COFF
>>>>>>>>>> It uses a linker script that describes how it lays out its idata
>>>>>>>>>> section.
>>>>>>>>>> From i386pe.x
>>>>>>>>>>
>>>>>>>>>>   .idata BLOCK(__section_alignment__) :
>>>>>>>>>>   {
>>>>>>>>>>     /* This cannot currently be handled with grouped sections.
>>>>>>>>>> 	See pe.em:sort_sections.  */
>>>>>>>>>>     SORT(*)(.idata$2)
>>>>>>>>>>     SORT(*)(.idata$3)
>>>>>>>>>>     /* These zeroes mark the end of the import list.  */
>>>>>>>>>>     LONG (0); LONG (0); LONG (0); LONG (0); LONG (0);
>>>>>>>>>>     SORT(*)(.idata$4)
>>>>>>>>>>     SORT(*)(.idata$5)
>>>>>>>>>>     SORT(*)(.idata$6)
>>>>>>>>>>     SORT(*)(.idata$7)
>>>>>>>>>>   }
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> Yeah, I found that too and it (especially the way how it creates a
>>>>>>>>> gap between .idata$3 and .idata$4) looks really hacky. I also found that
>>>>>>>>> GNU ld has a special logic to order .idata$<n> sections.
>>>>>>>>>
>>>>>>>>> At first I thought that I could mimic GNU ld and MSVC linker to
>>>>>>>>> generate the .idata section, but seems like it would really mess up the DLL
>>>>>>>>> import table generation code. We probably should keep the existing logic.
>>>>>>>>>
>>>>>>>>> I then tried to detect dlltool-style import files and read them as
>>>>>>>>> if they were short import libraries, so that I can keep the existing code.
>>>>>>>>> That didn't work well because it's not easy to detect dlltool-style import
>>>>>>>>> files in a reliable manner without sacrificing performance.
>>>>>>>>>
>>>>>>>>> It feels to me that it makes more sense to add a new option to
>>>>>>>>> dlltool to generate short import libraries. It shouldn't be that hard.
>>>>>>>>> Martell, what do you think?
>>>>>>>>>
>>>>>>>>>> Under ld/emultempl/pep.em in binutils it describes how it converts the MS import library to its format.
>>>>>>>>>> see here http://github.com/bminor/binutils-gdb/blob/master/ld/emultempl/pep.em#L1625
>>>>>>>>>> From the code in this function and the rest of pep.em we can see how it handles it.
>>>>>>>>>> I'm sure this will make more sense to you however as you know what the MS format is.
>>>>>>>>>>
>>>>>>>>>> I assume the conversion will have to go the other way for us
>>>>>>>>>> I think the most notable thing is the use of idata7 instead of idata6 for the dll name
>>>>>>>>>> We could use that as a check?
>>>>>>>>>> Then hijack it pulling out the function names once we see this being used and insert it into the sections like a MS generated one
>>>>>>>>>>
>>>>>>>>>> You might have a much cleaner solution however. :)
>>>>>>>>>>
>>>>>>>>>> If this isn't enough insight into what you need I can do more digging.
>>>>>>>>>> Just ping me and let me know
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Tue, Aug 4, 2015 at 3:15 AM, Rui Ueyama <ruiu at google.com>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> ruiu added a comment.
>>>>>>>>>>>
>>>>>>>>>>> Do you know anything about how GNU ld handles these import
>>>>>>>>>>> libraries? My
>>>>>>>>>>> linker is able to read it and construct .idata section, but the
>>>>>>>>>>> resulting
>>>>>>>>>>> .idata section is not going to be in correct format. If GNU
>>>>>>>>>>> linker is able
>>>>>>>>>>> to generate a correct .idata section from this type of import
>>>>>>>>>>> libraries,
>>>>>>>>>>> there must be something I'm missing in my linker.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> http://reviews.llvm.org/D11724
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150806/9e9da4a6/attachment.html>