Add Windows import library support to the Runtime Linker
ClosedPublic

Authored by Phyx on Dec 26 2015, 12:25 PM.

Details

Summary

Import libraries are files ending in .dll.a and .lib depending on which
compiler creates them (GCC, vs MSVC).

Import Libraries are standard archive files that contain object files.
These object files can have two different formats:

  1. The normal COFF Object format for object files (contains all ascii data and very little program code, so do not try to execute.)
  2. "short import" format which just contains a symbol name and the dll in which the symbol can be found.

Import Libraries are useful for two things:

  1. Allowing applications that don't support dynamic linking to link against the import lib (non-short format) which then makes calls into the DLL by loading it at runtime.
  1. Allow linking of mutually recursive dlls. if A.DLL requires B.DLL and vice versa, import libs can be used to break the cycle as they can be created from the expected exports of the DLLs.

A side effect of having these two capabilities is that Import libs are often
used to hide specific versions of DLLs behind a non-versioned import lib.

e.g. GCC_S.a (non-conventional import lib) will point to the correct
libGCC DLL. With this support Windows Haskell files can now just link
to -lGCC_S and not have to worry about what the actual name of libGCC is.

Also third party libraries such as icuuc use import libs to forward to
versioned DLLs. e.g. icuuc.lib points to icuuc51.dll etc.

Test Plan

./validate

Two new tests added T11072gcc T11072msvc

Two binary files have been added to the test folder because the "short"
import library format doesn't seem to be creatable via dlltool
and requires Microsoft's lib.exe.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
There are a very large number of changes, so older changes are hidden. Show Older Changes
Phyx updated the Trac tickets for this revision.Dec 26 2015, 12:25 PM
Phyx updated this revision to Diff 5973.Dec 26 2015, 12:40 PM
Phyx edited edge metadata.
  • T11072: updated layout
Phyx added a comment.Dec 26 2015, 12:40 PM

This is an alternate implementation for D1564

Phyx edited edge metadata.Dec 26 2015, 12:58 PM
Phyx changed the visibility from "Public (No Login Required)" to "Phyx (Tamar Christina)".
Phyx updated this revision to Diff 5981.Dec 26 2015, 4:27 PM
  • T11072: fix linux tests
  • T11072: use isImpLib so linux builds.
  • T11072: change printf for pos_t
  • T11072: replaced calls to fgetpost with ftell
Phyx changed the visibility from "Phyx (Tamar Christina)" to "Public (No Login Required)".Dec 26 2015, 4:31 PM
RyanGlScott edited edge metadata.Dec 28 2015, 9:49 PM

Thanks for looking into this, @Phyx.

My original motivation for filing Trac #11072 was to make text-icu work in Windows GHCi. Unfortunately, I can't build text-icu on GHC HEAD at the moment due to Trac #11303, but I made a stripped-down version of it called T11072-test here.

Unfortunately, I can't seem to run T11072-test with cabal repl using your changes. It just fails as follows:

$ cabal repl
Preprocessing executable 'T11072-test' for T11072-test-0.1...
GHCi, version 7.11.20151227: http://www.haskell.org/ghc/  :? for help
GHC runtime linker: fatal error: I found a duplicate definition for symbol
   __imp__ZTSN6icu_568ByteSinkE
whilst processing object file
   C:/msys64/mingw64/lib/../lib/libicuin.dll.a
This could be caused by:
   * Loading two different object files which export the same symbol
   * Specifying the same object file twice on the GHCi command line
   * An incorrect `package.conf' entry, causing some object to be
     loaded twice.
ghc-stage2.exe: panic! (the 'impossible' happened)
  (GHC version 7.11.20151227 for x86_64-unknown-mingw32):
        loadArchive "C:/msys64/mingw64/lib/../lib/libicuin.dll.a": failed
CallStack (from ImplicitParams):
  error, called at libraries\ghci\GHCi\ObjLink.hs:91:21 in ghci-0:GHCi.ObjLink

Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug

I think T11072-test just needs icuuc to work, so I also tried removing the icuin and icudt extra-libraries from T11072-test.cabal to see what would happen. That caused an even scarier error. On MSYS2:

$ cabal repl
Preprocessing executable 'T11072-test' for T11072-test-0.1...
GHCi, version 7.11.20151227: http://www.haskell.org/ghc/  :? for help
Ok, modules loaded: Main.
> main
Illegal instruction

On cmd.exe, invoking main causes a segfault.

Oops, I should amend my last claim. I didn't realize that at the moment, the text-icu library doesn't even work fully with cabal repl even on Linux with GHC 7.10.3, so that's a bad metric for whether this patch is successful.

I changed T11072-test to a library, installed it with cabal install, loaded the T11072 module in GHCi, and tried printing c_countAvailable using the two different combinations of extra-libraries mentioned above and got the same error messages.

Phyx added a comment.Dec 29 2015, 2:39 AM

Thanks for the test @RyanGlScott ,

I think T11072-test just needs icuuc to work, so I also tried removing the icuin and icudt extra-libraries from T11072-test.cabal

icuuc56.dll has an import to icudt56.dll so that needs to be there as well.

The extra-lib-dirs doesn't seem to be set when the call to load icuuc56.dll is made. So it can't find it. At least not here. Must be on your PATH somewhere.

In any case, the first option was correct. The error you got I think is from Trac #11223 .
On my current dev branch where I have multiple other linker related changes merged in I get:

$ ghc --interactive
GHCi, version 7.11.20151226: http://www.haskell.org/ghc/  :? for help
Prelude> :m +T11072
Prelude T11072> c_countAvailable
231

I will rebuild using only the changes here to confirm. If so, I will fix the extra-lib-dirs problem but the duplicate symbols will be fixed by the Trac #11223 changes.

Which I am still trying to test properly. If you want I can push that branch to my GitHub if you want to take a look.

Phyx added a comment.Dec 29 2015, 4:48 AM

Hmm even with just the changes here it did work.

Tamar@BoxyBox MINGW64 /r/T11072-test
$ cabal install --extra-lib-dirs=/r/icu/lib64/ --extra-lib-dirs=/r/icu/bin64
Resolving dependencies...
In order, the following will be installed:
T11072-test-0.1 (reinstall)
Warning: Note that reinstalls are always dangerous. Continuing anyway...
Configuring T11072-test-0.1...
Building T11072-test-0.1...
Preprocessing library T11072-test-0.1...
Creating package registration file:
E:\msys64\tmp\pkgConf-T11072-test-048275436.1
Installing library in
C:\Users\Tamar\AppData\Roaming\cabal\x86_64-windows-ghc-7.11.20151226\T11072-test-0.1-61fuPe0Jwri0JKfTjjsJ1E
Registering T11072-test-0.1...
Installed T11072-test-0.1

Tamar@BoxyBox MINGW64 /r/T11072-test
$ ghc --interactive
GHCi, version 7.11.20151226: http://www.haskell.org/ghc/  :? for help
Prelude> :m +T11072
Prelude T11072> c_countAvailable
231
Prelude T11072> :q
Leaving GHCi.

And just to double check that it doesn't contain my other linker changes:

$ ghc --interactive -lmingwex
GHCi, version 7.11.20151226: http://www.haskell.org/ghc/  :? for help
GHC runtime linker: fatal error: I found a duplicate definition for symbol
   coshf
whilst processing object file

It does (until I pass along the extra-libs-dir) require you to put he icuu dlls in the same folder as you're starting ghci in..

Oh, I realized we might be discussing two different problems. I had installed icu via the mingw-w64-x86_64-icu package in MSYS2, which was giving me the above errors. I just tried reinstalling T11072-test by using ICU4C (which I presume is compiled with MSVC, not MinGW-w64), and everything links correctly in GHCi. (I even tried compiling it with only icuuc as an extra library, and it also worked.)

I also forgot to mention that I put /mingw64/lib on my LIBRARY_PATH, /mingw64/include on my C_INCLUDE_PATH, and /mingw64/bin on my PATH, which is why I didn't specify them manually with extra-library-dirs and extra-include-dirs during cabal install. Sorry for the confusion!

Would MinGW-w64 vs. MSVC make a difference here?

Phyx updated this revision to Diff 6038.Dec 30 2015, 4:09 AM
Phyx edited edge metadata.
  • T11072: Making sure the extra-lib-dirs are available for archive loading as well
  • T11072: Make dll-no-loadable exception show only once.
Phyx added a comment.Dec 30 2015, 4:14 AM

Yes there is a difference. the MSVC toolchains produce a short import library. which basically is a list consisting of a symbol name and the DLL in which you can find it.
The import libs produced by dlltool on the other hand are using the long variant. which are essentially stubs that you should be able to link against and call to get access to the function in the DLL.

I managed to reproduce the illegal instruction error using the msys2 variant. I'll try and get that sorted.

bgamari requested changes to this revision.Jan 5 2016, 3:14 PM
bgamari edited edge metadata.

Punting out of review queue while remaining issues are sorted.

This revision now requires changes to proceed.Jan 5 2016, 3:14 PM
Phyx added a comment.Mar 9 2016, 5:19 PM

Right, so I know why this segfaults. The gcc format import libraries .dll.a don't contain ready to use stubs as I originally thought they would.
I had figured that they contained actual calls to LoadLibrary so you could just simply link to them or be done with.

Instead they're closer to the .lib import libraries in that they provide just redirections to functions. An initializer is used to make a call to a symbol ending with _iname which
points to text instead of executable code. This text is the name of the dll to load. Contrary to .lib import libs, the symbol entries can only be pointing at one dll.

Actual exported symbols all point to import sections. According to the GCC docs these idata section will always contain specific information. One of the sections will
instead of containing executable code contain the name of the function to look up in the dll mentioned by the _iname.

TL;DR:

I believe I can solve this by once the object file is needed, to load whatever is pointed to by the _iname symbol and unload the archive. This would forward all calls to the DLL.

Alternatively I can also add code to not just execute idata sections of .dll.a files but to perform lookups in the dll functions.

Phyx updated this revision to Diff 7181.Apr 4 2016, 4:44 AM
Phyx edited edge metadata.
  • T11072: updated layout
  • T11072: fix linux tests
  • T11072: use isImpLib so linux builds.
  • T11072: change printf for pos_t
  • T11072: replaced calls to fgetpost with ftell
  • T11072: Making sure the extra-lib-dirs are available for archive loading as well
Phyx updated this object.Apr 4 2016, 4:50 AM
Phyx edited edge metadata.
erikd requested changes to this revision.Apr 4 2016, 6:03 PM
erikd edited edge metadata.
erikd added inline comments.
rts/Linker.c
2393

Would it not be better to use something like endsWithPath from below or even strrchr to find the last . character and then do a strcmp with ".dll" ?

This revision now requires changes to proceed.Apr 4 2016, 6:03 PM
erikd resigned from this revision.Apr 4 2016, 6:03 PM
erikd removed a reviewer: erikd.
erikd added a reviewer: erikd.
Phyx added inline comments.Apr 5 2016, 3:11 AM
rts/Linker.c
2393

I just copied the code from what's being done already, but you're right, could be cleaned up a bit. it's stored little endian though but I could just compare against "lld."

Phyx added inline comments.Apr 5 2016, 3:58 AM
rts/Linker.c
2393

whoops, no need to reverse the string. It's early :)

Phyx updated this revision to Diff 7188.Apr 5 2016, 4:22 AM
  • T11072: used strncmp instead of manually comparing characters.
erikd requested changes to this revision.EditedApr 5 2016, 5:14 AM
erikd edited edge metadata.

Testing on Linux I get:

rts/Linker.c: In function ‘loadArchive_’:

rts/Linker.c:2449:21: error:
     error: implicit declaration of function ‘findAndLoadImportLibrary’ [-Werror=implicit-function-declaration]
                         findAndLoadImportLibrary(oc);

rts/Linker.c:2449:21: error:
     error: nested extern declaration of ‘findAndLoadImportLibrary’ [-Werror=nested-externs]
rts/Linker.c: At top level:

rts/Linker.c:284:15: error:
     error: ‘endsWithPath’ defined but not used [-Werror=unused-function]
     static HsBool endsWithPath(pathchar* base, pathchar* str) {
This revision now requires changes to proceed.Apr 5 2016, 5:14 AM
Phyx added a comment.Apr 5 2016, 5:53 AM

Ah, thanks! Forgot that harbormaster is still down.
I'll correct them.

Phyx updated this revision to Diff 7204.Apr 6 2016, 4:47 PM
Phyx edited edge metadata.
  • T11072: Refactor to fix linux
erikd added a comment.Apr 6 2016, 8:19 PM

I'm a little concerned about the two libAS.lib binaries we are adding to the git repo.

A couple of questions:

  • Where did these binaries come from?
  • How often are they going to change?
  • Is it not possible to generate these when the testsuite is run?
Phyx updated this revision to Diff 7214.Apr 7 2016, 2:02 PM
Phyx edited edge metadata.
  • T11072: Trimmed def file
  • T11072: Removed unneeded binary file and reduced lib size
Phyx added a comment.Apr 7 2016, 2:02 PM

The binaries come from running Microsoft's lib.exe tool on the libAS.def file checked into the repository.
the PE format specifies a short format for import libraries., but GCC re-uses the normal (long) object file format for this purpose.
Microsoft and Intel compilers thus always produce the short one (using the suffix .lib) and GCC seems to always produce the long one (using suffix .dll.a).

These files are unlikely to ever change. The support here doesn't look at the symbols in the .lib only at the
library to load (this does mean you can't do symbol aliasing. e.g saying calls to a point to b, but haven't find a library that actually uses this, so for now I
kept it simple and didn't add support for it for now). So even if new symbols are added to A.c you don't need to change the .lib.

libtool doesn't seem to be able to generate these (or I haven't found out how). So the only way to make them is to have the Microsoft compilers
on your path. I can of course omit the tests for them, but .lib are on Windows by far the most common format.

I don't however need the .exp files. So those I'll remove those. I was exporting too many symbols before, as the .def file was created from a dll made with GHC.
I'm now only exporting the symbol I need foo, so these files are tiny now ~2kb each.

erikd accepted this revision.Apr 7 2016, 4:37 PM
erikd edited edge metadata.

Thanks @Phyx ! LGTM.

RyanGlScott accepted this revision.Apr 7 2016, 6:20 PM
RyanGlScott edited edge metadata.

I successfully linked text-icu and luajit on Windows with these changes, so I'm happy! Thanks for the patch, @Phyx.

Phyx added a comment.Apr 8 2016, 3:25 PM

Cheers, Thanks for the review @RyanGlScott and @erikd :)

Phyx updated this revision to Diff 7255.Apr 12 2016, 1:26 AM
Phyx edited edge metadata.

rebased with master

Phyx updated this revision to Diff 7256.Apr 12 2016, 1:39 AM
Phyx edited edge metadata.

rebase again since arc got confused about the base

Phyx added a comment.Apr 12 2016, 1:40 AM

*sigh* I'll correct this tonight. it's not seeing that it already has the commits in master.

Phyx updated this revision to Diff 7259.Apr 13 2016, 4:18 AM
Phyx edited edge metadata.

Interactively rebased on new squashed commits in master

Phyx updated this revision to Diff 7260.Apr 13 2016, 4:31 AM
Phyx edited edge metadata.

*sigh* rebase again with updated master so diff excludes other changes.

Phyx updated this revision to Diff 7301.Apr 17 2016, 6:00 AM
Phyx edited edge metadata.
  • T11072: removed trailing whitespaces
Phyx updated this revision to Diff 7302.Apr 17 2016, 6:04 AM
Phyx edited edge metadata.
  • T11072: correct EOL and added ending newline
This revision was automatically updated to reflect the committed changes.