Skip to content

Commit

Permalink
Add extraLibDirs to runtime lib search paths of library
Browse files Browse the repository at this point in the history
Runtime search paths are hard. Here's the current picture to understand
why this patch exists:

* When linking a shared library, GHC will include in the rpath entries
  of the shared library all the paths listed in the library dirs section
  of the installed package info of all packages the shared library
  depends on.
    * On darwin, GHC has special logic to inject the library dirs listed
      in the installed dependent packages info into the rpath section
      instead of passing the dirs as -rpath flags to the linker.
      However, only the dirs where used libraries are found are
      actually injected. The others are ignored. This works around
      limitations of the darwin loader.

* Cabal, in addition, passes directly to the linker (via
  -optl-Wl,-rpath,...) the library dirs of packages the
  shared library for the package being built depends on.
    * In a vanilla cabal installation, this will typically only be the
      path to the cabal store and the path to the installed GHC's boot
      libraries store.
    * When using nix there will a different library dir per installed
      package. Since these lib dirs are passed directly to the linker as
      rpaths, we bypass the darwin loader logic and, for very big
      packages, on darwin, we could end up reaching the load command
      limit and fail linking. We don't address this situation in this MR.

When we specify `extra-lib-dirs` in Cabal, these extra-lib-dirs will be
added to the library dirs listed in the installed package info of the
library they were specified for. Furthermore, when building a shared
library, extra-lib-dirs will be passed as `-L` flags to the linker
invocation. However, the same extra-lib-dirs will not be passed as
`-rpath` to the linker.

The end situation is as follows:

    1. The shared library `libA` built for a package `A` will be linked
       against some libraries `libExtra` found in extra-lib-dirs
       `extraA`.

    2. The RPATH section of `A` will NOT contain `extraA`, because we
       don't pass -rpath extra-lib-dirs when linking the library, but it
       will depend on `libExtra`.

    3. The installed package info of that package `A` will contain, in
       the library dirs section, the extra-lib-dirs `extraA` and the
       path to `libA`.

    4. When a package `B` depends on package `A`, it will include in the
       RPATH section of the shared library `libB` the lib dirs from the
       installed package info of `A`, i.e. `/path/to/libA` and `extraA`,
       and depends on `libA` and, transitively, on `libExtra`.

The conclusion is:

    5. When we load `libB`, we will load `libA`, which is found in
       `/path/to/libA`, and, transitively, load `libExtra` which is
       found in `extraA` -- they are both found because both
       `/path/to/libA` and `extraA` are listed in the RPATH entries.

    6. However, if we load `libA` directly we will /NOT/ find
       `libExtra`, because `extraA` is not included in the RPATH
       entries.

So, ultimately, what this commit fixes, is the failure described in (6),
caused by the incorrect behaviour of (2), by specifying `-rpath
extra-lib-dirs` when linking the shared library of a package, to include
the extra lib dirs in the RPATH entries of that shared library (even
though dependents of this library would already get the extra-lib-dirs
in their RPATH, the library itself didn't, resulting in cabal#7339 and
ghc#19350)

Fixes haskell#7339
Fixes ghc#19350
  • Loading branch information
alt-romes committed Dec 22, 2023
1 parent 5b44c05 commit 318b8e1
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 6 deletions.
2 changes: 1 addition & 1 deletion Cabal/src/Distribution/Simple/GHC/BuildGeneric.hs
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ gbuild verbosity numJobs pkg_descr lbi bm clbi = do
}
dynLinkerOpts =
mempty
{ ghcOptRPaths = rpaths
{ ghcOptRPaths = rpaths <> toNubListR (extraLibDirs bnfo)
, ghcOptInputFiles =
toNubListR
[tmpDir </> x | x <- cLikeObjs ++ cxxObjs ++ cmmObjs ++ asmObjs]
Expand Down
2 changes: 1 addition & 1 deletion Cabal/src/Distribution/Simple/GHC/BuildOrRepl.hs
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ buildOrReplLib mReplFlags verbosity numJobs pkg_descr lbi lib clbi = do
, ghcOptLinkFrameworks = toNubListR $ PD.frameworks libBi
, ghcOptLinkFrameworkDirs =
toNubListR $ PD.extraFrameworkDirs libBi
, ghcOptRPaths = rpaths
, ghcOptRPaths = rpaths <> toNubListR (extraLibDirs libBi)
}
ghcStaticLinkArgs =
mempty
Expand Down
5 changes: 1 addition & 4 deletions Cabal/src/Distribution/Simple/Program/GHC.hs
Original file line number Diff line number Diff line change
Expand Up @@ -773,10 +773,7 @@ renderGhcOptions comp _platform@(Platform _arch os) opts
else []
, ["-no-hs-main" | flagBool ghcOptLinkNoHsMain]
, ["-dynload deploy" | not (null (flags ghcOptRPaths))]
, concat
[ ["-optl-Wl,-rpath," ++ dir]
| dir <- flags ghcOptRPaths
]
, ["-optl-Wl,-rpath," ++ dir | dir <- flags ghcOptRPaths]
, flags ghcOptLinkModDefFiles
, -------------
-- Packages
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import Hello
hello
:q
6 changes: 6 additions & 0 deletions cabal-testsuite/PackageTests/LinkerOptions/T7339/clib/lib.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include <stdio.h>

void hello_world(void) {
printf("hello world!");
}

3 changes: 3 additions & 0 deletions cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/Hello.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module Hello (hello) where

foreign import ccall "hello_world" hello :: IO ()
2 changes: 2 additions & 0 deletions cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/Setup.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import Distribution.Simple
main = defaultMain
11 changes: 11 additions & 0 deletions cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/T7339.cabal
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
cabal-version: >=1.10
name: T7339
version: 1.0
build-type: Simple

library
build-depends: base
exposed-modules: Hello
default-language: Haskell2010
extra-libraries: hello

6 changes: 6 additions & 0 deletions cabal-testsuite/PackageTests/LinkerOptions/T7339/setup.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Setup configure
# Setup build
Preprocessing library for T7339-1.0...
Building library for T7339-1.0...
# Setup register
Registering library for T7339-1.0...
63 changes: 63 additions & 0 deletions cabal-testsuite/PackageTests/LinkerOptions/T7339/setup.test.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
-- Test for #19350, #7339 by @bgamari
-- ==================================
--
-- The plan
-- ---------
-- We build a C shared library (`libhello`, contained in ./clib) and then build
-- a Haskell library (`T19350-lib`, in ./lib) which depends upon it via `foreign
-- import`. We make sure that the libhello shared object can only be found via
-- the extra-lib-dirs from the package database registration (which we do by
-- moving libhello.so from its original place).
--
-- Finally, we enter GHCi, load the Haskell library, and try to use it to call
-- into libhello.

import System.Directory
import Distribution.System
import Test.Cabal.Prelude


main = setupTest $ do
withPackageDb $ do
cwd <- takeDirectory . testCurrentDir <$> getTestEnv
plat <- testPlatform <$> getTestEnv
let libExt = case plat of
Platform _ OSX -> "dylib"
Platform _ Windows -> "dll"
Platform _ _other -> "so"


-- Link a C program against the library
_ <- runProgramM ghcProgram
[ "-fPIC", "-c", "clib/lib.c"
, "-o", "clib/lib.o" ]
Nothing
_ <- runProgramM ghcProgram
[ "-shared", "-no-hs-main", "clib/lib.o"
, "-o", "clib/libhello" <.> libExt ]
Nothing

withDirectory "lib" $ do
setup "configure" ["-v0"
, "--extra-lib-dirs=" ++ (cwd </> "clib")
, "--extra-lib-dirs=" ++ (cwd </> "clib-install")
, "--disable-library-vanilla"
, "--enable-shared"]
setup "build" []
setup "register" ["--inplace"]

-- Move libhello from its original place to ensure it isn't found via RPATH
liftIO $ do
createDirectoryIfMissing False (cwd </> "clib-install")
copyFile (cwd </> "clib/libhello" <.> libExt) ( cwd </> "clib-install/libhello" <.> libExt)
removeFile (cwd </> "clib/libhello" <.> libExt)

pkgDb <- testPackageDbDir <$> getTestEnv
ghciScript <- liftIO $ readFile (cwd </> "T19350.script")
_ <- runProgramM ghcProgram
[ "--interactive"
, "-package", "T7339"
, "-package-db", pkgDb
] (Just ghciScript)

return ()

0 comments on commit 318b8e1

Please sign in to comment.