Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#11223 closed task (fixed)

Runtime linker performs eager loading of all object files

Reported by: Phyx- Owned by: Phyx-
Priority: normal Milestone: 8.0.1
Component: Runtime System (Linker) Version: 7.10.3
Keywords: Cc: lukexi, Elieux, RyanGlScott
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Compile-time crash Test Case:
Blocked By: Blocking:
Related Tickets: #10726 #11317 #11748 Differential Rev(s): Phab:D1805 Phab:D2102
Wiki Page:

Description (last modified by bgamari)

The runtime linker seems to be re-exporting some of the symbols of libmingwex from the rts archive (using SymI_HasProto). Only a very small subset of symbols are re-exporting.

If a symbol is needed that isn't re-exported (e.g. log1p) then this code can't be run in GHCi because it will result in a duplicate symbols error.

A workaround

The rts seems to be a special case again. The linker seems to ignore the extra-libraries from the package.conf, which explains why you can put anything you want in there and it'll still compile.

128 emptyPLS :: DynFlags -> PersistentLinkerState 
129 emptyPLS _ = PersistentLinkerState { 
130                         closure_env = emptyNameEnv, 
131                         itbl_env    = emptyNameEnv, 
132                         pkgs_loaded = init_pkgs, 
133                         bcos_loaded = [], 
134                         objs_loaded = [], 
135                         temp_sos = [] } 
136 
 
137   -- Packages that don't need loading, because the compiler 
138   -- shares them with the interpreted program. 
139   -- 
140   -- The linker's symbol table is populated with RTS symbols using an 
141   -- explicit list.  See rts/Linker.c for details. 
142   where init_pkgs = [rtsUnitId] 

I've tried 2 approaches which haven't worked completely:

  1. I tried removing the symbols from the export list and adding libmingwex to the rts's package.confand have it just link against it. But turns out the rts's package.conf is ignored on all platforms. I didn't want to have to make an exception for Windows here and I don't know why the other platforms also ignore it so I abandoned this approach.
  1. I tried marking the symbols we're re-exporting as weak symbols, so there wouldn't be a change to existing code and would allow you to link against libmingwex. But unfortunately because of when the other libraries specified by -l are linked in, some of the symbols have already been used and thus aren't weak anymore. So I still get duplicate link errors.

What I want to try now is leaving them as weak symbols, but loading libmingwex.a at rts initialization time. Much like how kernel32 is loaded. This is hopefully early enough that the symbols haven't been used yet.

Example

-- LogFloat.hs
module Main (main) where

import Data.Number.LogFloat (log1p)

main :: IO ()
main = print $ log1p 1.0

runGhc LogFloat.hs will fail:

Loading package logfloat-0.13.3.3 ... 
linking ... 
LogFloat.hs: ...\x86_64-windows-ghc-7.11.20151123\logfloat-0.13.3.3-4JZYNCXKwghOD60rvMUAcn\HSlogfloat-0.13.3.3-4JZYNCXKwghOD60rvMUAcn.o: unknown symbol `log1p' 

LogFloat.hs: LogFloat.hs: unable to load package `logfloat-0.13.3.3' 

Change History (21)

comment:1 Changed 4 years ago by Phyx-

Owner: set to Phyx-

comment:2 Changed 4 years ago by bgamari

Description: modified (diff)

comment:3 Changed 4 years ago by Phyx-

Loading the symbols earlier seems like it will work. The issue now is that a horrible design decision with libmingwex is causing us to have to link in far more than expected. libmingwex has a depencency on the symbol __mingw_raise_matherr. This symbol is defined in libmingw32 which is where the fun begins..

We also end up needing the ld symbol __image_base__ because of these dependencies and ultimately libmingw32 also requires wWinMain which is the only symbol I don't know what to do about.

The list of needed libraries:

  • libgcc.a
  • libws2_32.a
  • libmingw32.a
  • user32.dll
  • Advapi32.dll

I'll keep looking, but we may want to consider re-distributing msvcrt120.dll which would also have all the symbols we need. It seems like the license for Visual Studio 2013 C++ Redistributables might allow this now.

comment:4 Changed 4 years ago by Elieux

I don't know much real-world code in GHC on Windows, so this may be a moot point, but please consider compatibility with existing libraries before going for the numbered CRTs. See https://msdn.microsoft.com/en-us/library/ms235460(v=vs.100).aspx for possible issues.

comment:5 Changed 4 years ago by Phyx-

Yeah, It's not on the top of the list of possible solutions for sure.

I have another thing I want to try next, instead of linking in libmingw32.a to get __mingw_raise_matherr I can just inline the definition of __mingw_raise_matherr in the rts and export the symbol as a weak symbol. That way you can still link to libmingw32 if you want to and thus override it, but I don't need to link in everything else including the kitchen sink to use libmingwex.

comment:6 Changed 4 years ago by Phyx-

It seems the RTS already has an import for __mingw_raise_matherr so just re-exporting it works.

I also need to satisfy a few other symbols:

      SymI_HasProto_redirect(_fputwc_nolock, fputwc)     \
      SymI_HasProto_redirect(_fgetwc_nolock, fgetwc)     \
      SymI_HasProto_redirect(fileno        , _fileno)    \
      SymI_HasProto_redirect(strdup        , _strdup)    \

Along with

      SymI_HasProto(__mingw_raise_matherr)               \
      SymI_NeedsProto(mingw_app_type)                    \

Though I need to find a better solution for redirecting the nolock version of fputwc and fgetwc to the locking versions.

Now I've reached a fork in the road: Some libraries like base require a few symbols in mingwex, however they don't seem to specify the libraries in their package.conf.

Because packages are pre-loaded by the rts then the symbols aren't weak anything. Which means that again, libmingwex can't be linked.

This leaves me with 3 options:

  1. Fix the entire dependency chain from packages like base on up. This is a large change but would be consistent.
  1. Add all symbols to the RTS as weak symbols in ocGetNames_PEi386. Which means every library we load will accept duplicate symbols. So long the symbols haven't been used yet. This would also "fix" #6107 . Doing this will allow you to link in libmingwex by just simply adding -lmingwex -lgcc to the compiler. This would also make libmingw32 work.
Loading object (static archive) E:/msys64/home/Tamar/ghc2/inplace/mingw/bin/../lib/gcc/x86_64-w64-mingw32/5.2.0/../../../../x86_64-w64-mingw32/lib/../lib/libmingwex.a ... done
Loading object (static archive) E:/msys64/home/Tamar/ghc2/inplace/mingw/bin/../lib/gcc/x86_64-w64-mingw32/5.2.0/libgcc.a ... done
final link ... done
  1. Pre-load libmingwex and libgcc during the initLinker calls for GHCi as weak symbols. This will ignore the symbols we already have loaded (they come from mingwex anyway). This will fix GHCi as in, there's no change needed to use all symbols in libmingwex, but libmingw32 won't work as it will fail on a duplicate symbols. This will also diverge GHCi and GHC a bit more.

These are the three options I currently see.

Last edited 4 years ago by Phyx- (previous) (diff)

comment:7 Changed 4 years ago by Phyx-

I have a repository with an implementation of no# 2 here https://github.com/Mistuke/ghc/tree/trac-11223-fix-symbols-re-exported-from-rts

Though I think no# 1 is the proper solution.

comment:8 Changed 4 years ago by Matt

I did some testing on how GCC linker and GHC runtime linker resolve symobols and I found 2 major differences on how they treat symbols from static archives.

Let me illustrate them on some examples:

Given two C source files which both have the same function defined:

$ cat test1.c
int test() {
        return 111;
}

$ cat test2.c
int test() {
        return 555;
}

C and Haskell program that call our test fuction:

$ cat prog1.c
#include <stdio.h>

int test();

int main(int argc, char **argv) {
        printf("%d\n", test());
        return 0;
}

$ cat prog1.hs
module Main (main) where

foreign import ccall test :: Int

main = print test

Build static library from two source files

$ gcc -c test1.c test2.c
$ ar rcs libtest1.a test1.o
$ ar rcs libtest2.a test2.o

Now this is where it gets interesting:

$ gcc prog1.c test1.o -L. -ltest2
$ ./a.exe
111

$ gcc prog1.c -L. -ltest1 -ltest2
$ ./a.exe
111

As you can see gcc will happily link both programs. Test function was resolved from first object file/static library while other one is ignored.

However GHC runtime linker fails to link both programs with duplicate symbols error:

$ ghci prog1.hs test1.o -L. -ltest2
GHC runtime linker: fatal error: I found a duplicate definition for symbol
   _test
whilst processing object file
   .\libtest2.a
...

$ ghci prog1.hs -L. -ltest1 -ltest2
GHC runtime linker: fatal error: I found a duplicate definition for symbol
   _test
whilst processing object file
   .\libtest2.a
...

So this is 1st major difference: duplicate symbols from static archives seem to be ignored by GCC, while GHC runtime linker errors out on them.

Now the 2nd point. In new 3rd file we define function that calls function which isn't defined anywhere.

$ cat test3.c

/* this function is nowhere defined */
void bla_bla();

/* this function is not called at all */
void test_uncalled() {
        bla_bla();
}

Make new static library with first object file and this 3rd one that contains call to undefined function.

$ gcc -c test3.c
$ ar rcs libtest3.a test1.o test3.o

And try to run both C and Haskell example:

$ gcc prog1.c -L. -ltest3
$ ./a.exe
111

$ ghci prog1.hs -L. -ltest3
linking extra libraries/objects failed
ghc.exe: .\libtest3.a: unknown symbol `_bla_bla'

As you can see GHCi errors out with unknown symbol to function in object file we do not call, while GCC seems to ignore it.

So based on this 2nd major difference: GCC seems to ignores symbols from object files in static archive which are not used while GHC runtime linker tries to eagerly resolve all object files, even those that are not called into.

I think because of this 2nd difference we end up with all kinda strange unresolved symbols like __image_base and similar which cause so much problems.

comment:9 Changed 4 years ago by Phyx-

Thanks for the experiment. I was attempting to get an interim version out for GHC 8.0 hence the attempt to re-use weak-symbols to get specifically libmingwex to link.

As I have missed that Window anyway I will instead target 8.2 and do a proper fix of the link behavior along with other changes that are required for other tickets.

The behavior you described is correct, ld is using libBFD for symbol resolving. So the resolution behavior is described there. http://ftp.gnu.org/old-gnu/Manuals/bfd-2.9.1/html_mono/bfd.html#SEC149 Describes how the different kind of symbols are resolved.

Also you can just ask ld directly. --print-map will print out how it's resolving symbols. (If called via GCC use -Wl,--print-map).

I think because of this 2nd difference we end up with all kinda strange unresolved symbols like image_base and similar which cause so much problems.

We'll have to resolve this symbol in any case. __image_base is an ld symbol that it inserts during linking, if we want to load mingw object files we have to deal with it. This is easy to resolve anyway. It's just IMAGE_DOS_HEADER in the final link.

comment:10 Changed 4 years ago by RyanGlScott

Cc: RyanGlScott added; RyanGIScott removed

comment:11 Changed 4 years ago by Phyx-

Operating System: WindowsUnknown/Multiple
Summary: Dynamic linker on Windows is unable to link against libmingwexRuntime linker performs eager loading of all object files

The Runtime Linker is currently eagerly loading all object files on all platforms which do not use the system linker for GHCi.

After talking with @rwbarton I have taken the approach of loading entire object files when a symbol is needed instead of doing the dependency tracking on a per symbol basis. This is a lot less fragile and a lot less complicated to implement.

The changes come down to the following steps:

1) modify the linker to keep a list of required symbols. Required symbols are any symbols that are passed in from .o arguments to GHCi.

2) Change ObjectCode's to be indexed but not initialized or resolved. This means we know where we would load the symbols, but haven't actually done so.

3) Mark any ObjectCode belonging to .o passed as argument as required loadObject.

4) During Resolve object calls, mark all ObjectCode containing the required symbols as loadObject

5) During lookupSymbol lookups, (which is called from linkExpr and linkDecl in GHCI.hs) is the symbol is in a not-yet-loaded ObjectCode then load the ObjectCode on demand and return the address of the symbol. Otherwise produce an unresolved symbols error as expected.

This change affects all platforms and OSes which use the runtime linker.

comment:12 Changed 4 years ago by Phyx-

Owner: Phyx- deleted

comment:13 Changed 4 years ago by Phyx-

Differential Rev(s): Phab:D1805
Owner: set to Phyx-

comment:14 Changed 4 years ago by Phyx-

Status: newpatch

comment:15 Changed 4 years ago by Phyx-

comment:16 Changed 4 years ago by Ben Gamari <ben@…>

In 90538d86/ghc:

Change runtime linker to perform lazy loading of symbols/sections

The Runtime Linker is currently eagerly loading all object files on all
platforms which do not use the system linker for `GHCi`.

The problem with this approach is that it requires all symbols to be
found.  Even those of functions never used/called. This makes the number
of libraries required to link things like `mingwex` quite high.

To work around this the `rts` was relying on a trick. It itself was
compiled with `MingW64-w`'s `GCC`. So it was already linked against
`mingwex`.  As such, it re-exported the symbols from itself.

While this worked it made it impossible to link against `mingwex` in
user libraries. And with this means no `C99` code could ever run in
`GHCi` on Windows without having the required symbols re-exported from
the rts.

Consequently this rules out a large number of packages on Windows.
SDL2, HMatrix etc.

After talking with @rwbarton I have taken the approach of loading entire
object files when a symbol is needed instead of doing the dependency
tracking on a per symbol basis. This is a lot less fragile and a lot
less complicated to implement.

The changes come down to the following steps:

1) modify the linker to and introduce a new state for ObjectCode:
   `Needed`.  A Needed object is one that is required for the linking to
   succeed.  The initial set consists of all Object files passed as
   arguments to the link.

2) Change `ObjectCode`'s to be indexed but not initialized or resolved.
   This means we know where we would load the symbols,
   but haven't actually done so.

3) Mark any `ObjectCode` belonging to `.o` passed as argument
   as required: ObjectState `NEEDED`.

4) During `Resolve` object calls, mark all `ObjectCode`
   containing the required symbols as `NEEDED`

5) During `lookupSymbol` lookups, (which is called from `linkExpr`
   and `linkDecl` in `GHCI.hs`) is the symbol is in a not-yet-loaded
   `ObjectCode` then load the `ObjectCode` on demand and return the
   address of the symbol. Otherwise produce an unresolved symbols error
   as expected.

6) On `unloadObj` we then change the state of the object and remove
   it's symbols from the `reqSymHash` table so it can be reloaded.

This change affects all platforms and OSes which use the runtime linker.
It seems there are no real perf tests for `GHCi`, but performance
shouldn't be impacted much. We gain a lot of time not loading all `obj`
files, and we lose some time in `lookupSymbol` when we're finding
sections that have to be loaded. The actual finding itself is O(1)
(Assuming the hashtnl is perfect)

It also consumes slighly more memory as instead of storing just the
address of a symbol I also store some other information, like if the
symbol is weak or not.

This change will break any packages relying on renamed POSIX functions
that were re-named and re-exported by the rts. Any packages following
the proper naming for functions as found on MSDN will work fine.

Test Plan: ./validate on all platforms which use the Runtime linker.

Reviewers: thomie, rwbarton, simonmar, erikd, bgamari, austin, hvr

Reviewed By: erikd

Subscribers: kgardas, gridaphobe, RyanGlScott, simonmar,
             rwbarton, #ghc_windows_task_force

Differential Revision: https://phabricator.haskell.org/D1805

GHC Trac Issues: #11223

comment:17 Changed 4 years ago by bgamari

Milestone: 8.2.18.0.1
Resolution: fixed
Status: patchclosed

Merged to ghc-8.0 as 068d9273f0a427cbab4ea95cfca211ec127dc785.

Thanks Phyx!

comment:18 Changed 4 years ago by Tamar Christina <tamar@…>

In c6e579b/ghc:

Add linker notes

Summary: Add linker notes following #11223 and D1805

Reviewers: austin, bgamari, erikd

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D2102

GHC Trac Issues: #11223

comment:19 Changed 4 years ago by Phyx-

Differential Rev(s): Phab:D1805Phab:D1805 Phab:D2102

Cheers!

comment:20 Changed 4 years ago by bgamari

This introduced a minor regression; see #11828.

comment:21 Changed 3 years ago by Ben Gamari <ben@…>

In 37473722/ghc:

Refactored SymbolInfo to lower memory usage in RTS

Previously as part of #11223 a new struct `SymbolInfo` was introduced to
keep track it the weak symbol status of a symbol.

This structure also kept a copy of the calculated address of the symbol
which turns out was useful in ignoring non-weak zero-valued symbols.

The information was kept in an array so it means for every symbol two
extra bytes were kept even though the vast majority of symbols are
non-weak and non-zero valued.

This changes the array into a sparse map keeping this information only
for the symbols that are weak or zero-valued. This allows for a
reduction in the amount of information needed to be kept while giving up
a small (negligable) hit in performance as this information now has to
be looked up in hashmaps.

Test Plan: ./validate on all platforms that use the runtime linker.

For unix platforms please ensure `DYNAMIC_GHC_PROGRAMS=NO` is added to
your validate file.

Reviewers: simonmar, austin, erikd, bgamari

Reviewed By: simonmar, bgamari

Subscribers: thomie, #ghc_windows_task_force

Differential Revision: https://phabricator.haskell.org/D2184

GHC Trac Issues: #11816
Note: See TracTickets for help on using tickets.