From fe6c577edf12cc2510feda89fdb214eda2ae5b4c Mon Sep 17 00:00:00 2001 From: Rodrigo Mesquita Date: Wed, 15 Nov 2023 18:22:28 +0000 Subject: [PATCH] Add extraLibDirs to runtime lib search paths of library 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 #7339 Fixes ghc#19350 --- .../Distribution/Simple/GHC/BuildGeneric.hs | 2 +- .../Distribution/Simple/GHC/BuildOrRepl.hs | 2 +- Cabal/src/Distribution/Simple/Program/GHC.hs | 5 +- .../LinkerOptions/T7339/T19350.script | 3 + .../LinkerOptions/T7339/clib/lib.c | 6 ++ .../LinkerOptions/T7339/lib/Hello.hs | 3 + .../LinkerOptions/T7339/lib/Setup.hs | 2 + .../LinkerOptions/T7339/lib/T7339.cabal | 11 ++++ .../LinkerOptions/T7339/setup.out | 6 ++ .../LinkerOptions/T7339/setup.test.hs | 66 +++++++++++++++++++ 10 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 cabal-testsuite/PackageTests/LinkerOptions/T7339/T19350.script create mode 100644 cabal-testsuite/PackageTests/LinkerOptions/T7339/clib/lib.c create mode 100644 cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/Hello.hs create mode 100644 cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/Setup.hs create mode 100644 cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/T7339.cabal create mode 100644 cabal-testsuite/PackageTests/LinkerOptions/T7339/setup.out create mode 100644 cabal-testsuite/PackageTests/LinkerOptions/T7339/setup.test.hs diff --git a/Cabal/src/Distribution/Simple/GHC/BuildGeneric.hs b/Cabal/src/Distribution/Simple/GHC/BuildGeneric.hs index 7ff326aa9b3..330cc656d04 100644 --- a/Cabal/src/Distribution/Simple/GHC/BuildGeneric.hs +++ b/Cabal/src/Distribution/Simple/GHC/BuildGeneric.hs @@ -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] diff --git a/Cabal/src/Distribution/Simple/GHC/BuildOrRepl.hs b/Cabal/src/Distribution/Simple/GHC/BuildOrRepl.hs index 8ae87642b51..64a8f0a6c40 100644 --- a/Cabal/src/Distribution/Simple/GHC/BuildOrRepl.hs +++ b/Cabal/src/Distribution/Simple/GHC/BuildOrRepl.hs @@ -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 diff --git a/Cabal/src/Distribution/Simple/Program/GHC.hs b/Cabal/src/Distribution/Simple/Program/GHC.hs index 537e008c17f..a3093c80704 100644 --- a/Cabal/src/Distribution/Simple/Program/GHC.hs +++ b/Cabal/src/Distribution/Simple/Program/GHC.hs @@ -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 diff --git a/cabal-testsuite/PackageTests/LinkerOptions/T7339/T19350.script b/cabal-testsuite/PackageTests/LinkerOptions/T7339/T19350.script new file mode 100644 index 00000000000..d5b619f7bff --- /dev/null +++ b/cabal-testsuite/PackageTests/LinkerOptions/T7339/T19350.script @@ -0,0 +1,3 @@ +import Hello +hello +:q diff --git a/cabal-testsuite/PackageTests/LinkerOptions/T7339/clib/lib.c b/cabal-testsuite/PackageTests/LinkerOptions/T7339/clib/lib.c new file mode 100644 index 00000000000..556c1dcb748 --- /dev/null +++ b/cabal-testsuite/PackageTests/LinkerOptions/T7339/clib/lib.c @@ -0,0 +1,6 @@ +#include + +void hello_world(void) { + printf("hello world!"); +} + diff --git a/cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/Hello.hs b/cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/Hello.hs new file mode 100644 index 00000000000..0dd0de66f74 --- /dev/null +++ b/cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/Hello.hs @@ -0,0 +1,3 @@ +module Hello (hello) where + +foreign import ccall "hello_world" hello :: IO () diff --git a/cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/Setup.hs b/cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/Setup.hs new file mode 100644 index 00000000000..9a994af677b --- /dev/null +++ b/cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/Setup.hs @@ -0,0 +1,2 @@ +import Distribution.Simple +main = defaultMain diff --git a/cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/T7339.cabal b/cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/T7339.cabal new file mode 100644 index 00000000000..d4ea571a2c0 --- /dev/null +++ b/cabal-testsuite/PackageTests/LinkerOptions/T7339/lib/T7339.cabal @@ -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 + diff --git a/cabal-testsuite/PackageTests/LinkerOptions/T7339/setup.out b/cabal-testsuite/PackageTests/LinkerOptions/T7339/setup.out new file mode 100644 index 00000000000..45a71c209da --- /dev/null +++ b/cabal-testsuite/PackageTests/LinkerOptions/T7339/setup.out @@ -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... diff --git a/cabal-testsuite/PackageTests/LinkerOptions/T7339/setup.test.hs b/cabal-testsuite/PackageTests/LinkerOptions/T7339/setup.test.hs new file mode 100644 index 00000000000..6b789f2386c --- /dev/null +++ b/cabal-testsuite/PackageTests/LinkerOptions/T7339/setup.test.hs @@ -0,0 +1,66 @@ +-- Test for #19350, #7339 originally 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 + + skipIfWindows + + 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 ()