fix(Cabal): Consistently translate -Wl, in ld-options when invoking ld directly

Cabal-tests/tests/UnitTests/Distribution/Utils/Structured.hs

@@ -37,4 +37,4 @@ md5CheckGenericPackageDescription proxy = md5Check proxy
 
 md5CheckLocalBuildInfo :: Proxy LocalBuildInfo -> Assertion
 md5CheckLocalBuildInfo proxy = md5Check proxy
-    0x3398bd7f316ecb8f535cbe78498a89a4
+    0x82d22c1d21566e21c9b384cc63bd93bf

Cabal/src/Distribution/Simple/GHC/Build/Link.hs

@@ -127,11 +127,10 @@ linkOrLoadComponent
               [ "-static"
               | withFullyStaticExe lbi
               ]
-                -- Pass extra `ld-options` given
-                -- through to GHC's linker.
+                -- Pass extra `ld-options` given through to GHC's linker.
                 ++ maybe
                   []
-                  programOverrideArgs
+                  programDriverArgs
                   (lookupProgram ldProgram (withPrograms lbi))
           , ghcOptLinkLibs =
               if withFullyStaticExe lbi

Cabal/src/Distribution/Simple/GHC/Internal.hs

@@ -258,7 +258,7 @@ configureToolchain _implInfo ghcProg ghcInfo =
               _ <-
                 getProgramOutput
                   verbosity
-                  ldProg
+                  (suppressOverrideArgs ldProg)
                   ["-x", "-r", testofile, "-o", testofile']
               return True
               `catchIO` (\_ -> return False)

Cabal/src/Distribution/Simple/Program/Builtin.hs

@@ -317,7 +317,7 @@ ldProgram =
         ldHelpOutput <-
           getProgramInvocationOutput
             verbosity
-            (programInvocation ldProg ["--help"])
+            (programInvocation (suppressOverrideArgs ldProg) ["--help"])
             -- In case the linker does not support '--help'. Eg the LLVM linker,
             -- `lld` only accepts `-help`.
             `catchIO` (\_ -> return "")

Cabal/src/Distribution/Simple/Program/Db.hs

@@ -318,10 +318,31 @@ userSpecifyArgs name args' =
               prog
                 { programOverrideArgs =
                     programOverrideArgs prog
+                      ++ interpretUserOptions name args'
+                , programDriverArgs =
+                    programDriverArgs prog
                       ++ args'
                 }
       )
 
+-- | Interpret options written for the linker driver (the form documented
+-- for @ld-options@, see Note [ld-options and the linker driver]) into the
+-- options @ld@ itself accepts (#10789).
+--
+-- A driver option @-Wl,a,b@ means \"pass @a@ and @b@ to the linker\", so it
+-- turns into @[a, b]@. All other options are passed through unchanged.
+interpretUserOptions :: String -> [String] -> [String]
+interpretUserOptions "ld" = concatMap ldOptionToLdFlag
+  where
+    ldOptionToLdFlag opt
+      | "-Wl," `isPrefixOf` opt = splitOnCommas (drop 4 opt)
+      | otherwise = [opt]
+
+    splitOnCommas s = case break (== ',') s of
+      (w, []) -> [w]
+      (w, _ : rest) -> w : splitOnCommas rest
+interpretUserOptions _ = id
+
 -- | Like 'userSpecifyPath' but for a list of progs and their paths.
 userSpecifyPaths
   :: [(String, FilePath)]
@@ -439,7 +460,9 @@ configureUnconfiguredProgram verbosity prog progdb = do
               { programId = name
               , programVersion = version
               , programDefaultArgs = []
-              , programOverrideArgs = userSpecifiedArgs prog progdb
+              , programOverrideArgs =
+                  interpretUserOptions name (userSpecifiedArgs prog progdb)
+              , programDriverArgs = userSpecifiedArgs prog progdb
               , programOverrideEnv = [("PATH", Just newPath)] ++ progOverrideEnv progdb
               , programProperties = Map.empty
               , programLocation = location

Cabal/src/Distribution/Simple/Program/Types.hs

@@ -122,7 +122,21 @@ data ConfiguredProgram = ConfiguredProgram
   , programOverrideArgs :: [String]
   -- ^ Override command-line args for this program.
   -- These flags will appear last on the command line, so they override
-  -- all earlier flags.
+  -- all earlier flags. They hold the user's options for this program
+  -- ('programDriverArgs') interpreted for the program's own command
+  -- line, which for @ld@ differs from the form the user gives them in
+  , programDriverArgs :: [String]
+  -- ^ The user's options for this program, as given by the user (e.g.
+  -- from @--ld-options@), before they are interpreted for the program's
+  -- own command line.
+  --
+  -- For most programs these are the options for the program itself, and
+  -- equal to 'programOverrideArgs'. The exception is @ld@: its options
+  -- are documented as options for GHC's linking phase (GHC receives them
+  -- as @-optl@ arguments, which forwards them to the C compiler driver
+  -- acting as the linker), so options for the linker proper have to be
+  -- given with a @-Wl,@ prefix — see
+  -- They are not passed to @ld@ when Cabal invokes it directly.
   , programOverrideEnv :: [(String, Maybe String)]
   -- ^ Override environment variables for this program.
   -- These env vars will extend\/override the prevailing environment of
@@ -164,7 +178,11 @@ programPath = locationPath . programLocation
 
 -- | Suppress any extra arguments added by the user.
 suppressOverrideArgs :: ConfiguredProgram -> ConfiguredProgram
-suppressOverrideArgs prog = prog{programOverrideArgs = []}
+suppressOverrideArgs prog =
+  prog
+    { programOverrideArgs = []
+    , programDriverArgs = []
+    }
 
 -- | Make a simple 'ConfiguredProgram'.
 --
@@ -176,6 +194,7 @@ simpleConfiguredProgram name loc =
     , programVersion = Nothing
     , programDefaultArgs = []
     , programOverrideArgs = []
+    , programDriverArgs = []
     , programOverrideEnv = []
     , programProperties = Map.empty
     , programLocation = loc

cabal-testsuite/PackageTests/Regression/T10789/app/Main.hs

@@ -0,0 +1,22 @@
+{-# LANGUAGE ForeignFunctionInterface #-}
+
+module Main (main) where
+
+import Foreign.C (CInt (..))
+import Lib (greeting)
+
+-- With `ld-options: -Wl,--wrap=meaning_of_life_ld_real` in cabal.project,
+-- the linker redirects all calls to `meaning_of_life_ld_real` to
+-- `__wrap_meaning_of_life_ld_real`, which returns 55.
+foreign import ccall "ldlib.h meaning_of_life_ld_real"
+  meaning_of_life_ld_real :: IO CInt
+
+main :: IO ()
+main = do
+    secret <- meaning_of_life_ld_real
+    -- The value 55 comes from __wrap_meaning_of_life_ld_real, see
+    -- `ld-options` in cabal.project.
+    if secret == 55
+        then putStrLn ("The secret is " ++ show secret)
+        else error ("Expected value 55, got " ++ show secret)
+    putStrLn greeting

cabal-testsuite/PackageTests/Regression/T10789/cabal.project

@@ -0,0 +1,4 @@
+packages: .
+
+program-options
+    ld-options: -Wl,--allow-multiple-definition -Wl,--wrap=meaning_of_life_ld_real

cabal-testsuite/PackageTests/Regression/T10789/cabal.test.hs

@@ -0,0 +1,28 @@
+import Test.Cabal.Prelude
+
+-- Regression test for #10789: inconsistent use of `ld-options`.
+--
+-- Project-level `ld-options` (`--ld-options`) are documented as flags for
+-- GHC's linking phase: they are passed to GHC as `-optl` arguments, which
+-- forwards them to the C compiler driver acting as the linker. Cabal also
+-- invokes the `ld` program directly (when probing its capabilities and when
+-- combining object files into a library for GHCi); previously the user's
+-- `-Wl,`-prefixed options were passed to `ld` verbatim, which broke the
+-- probes (silently disabling `--enable-library-for-ghci`) and could fail
+-- the build, while the very same options worked fine when passed to GHC.
+main = do
+    -- The assertions below require a linker that reports support for
+    -- relocatable output; `lld` (Windows) and `ld64` (macOS) do not.
+    skipIfWindows "lld does not support relocatable output"
+    skipIfOSX "ld64 does not support relocatable output"
+    cabalTest $ recordMode DoNotRecord $ do
+        -- The `-Wl,`-prefixed `ld-options` must not break the `ld`
+        -- capability probes: the library for GHCi has to be built.
+        cabal "v2-build" ["--enable-library-for-ghci", "all"]
+        _ <- assertGlobMatchesTestDir testDistDir "**/HSt10789-0.1-inplace.o"
+
+        -- The same `ld-options` must still reach the linker through GHC
+        -- (as `-optl` flags).
+        withPlan $ do
+            res <- runPlanExe' "t10789" "wrap-exe" []
+            assertOutputContains "The secret is 55" res

cabal-testsuite/PackageTests/Regression/T10789/cbits/ldlib.c

@@ -0,0 +1,9 @@
+#include "ldlib.h"
+
+int meaning_of_life_ld_real(void) {
+    return 0;
+}
+
+int __wrap_meaning_of_life_ld_real(void) {
+    return 55;
+}

cabal-testsuite/PackageTests/Regression/T10789/cbits/ldlib.h

@@ -0,0 +1,14 @@
+#ifndef LDLIB_H
+#define LDLIB_H
+
+/* The "real" implementation - returns 0, the wrong value.
+ * With `ld-options: -Wl,--wrap=meaning_of_life_ld_real`, the linker
+ * redirects all calls to this function to __wrap_meaning_of_life_ld_real
+ * below. */
+int meaning_of_life_ld_real(void);
+
+/* The wrapper that the linker substitutes in place of the real function.
+ * Returns 55 - see `ld-options` in cabal.project. */
+int __wrap_meaning_of_life_ld_real(void);
+
+#endif

cabal-testsuite/PackageTests/Regression/T10789/src/Lib.hs

@@ -0,0 +1,4 @@
+module Lib (greeting) where
+
+greeting :: String
+greeting = "hello from t10789"

cabal-testsuite/PackageTests/Regression/T10789/t10789.cabal

@@ -0,0 +1,19 @@
+cabal-version:      2.2
+name:               t10789
+version:            0.1
+build-type:         Simple
+
+library
+    exposed-modules:  Lib
+    hs-source-dirs:   src
+    build-depends:    base
+    default-language: Haskell2010
+
+executable wrap-exe
+    main-is:          Main.hs
+    hs-source-dirs:   app
+    build-depends:    base,
+                      t10789
+    default-language: Haskell2010
+    c-sources:        cbits/ldlib.c
+    include-dirs:     cbits

cabal-testsuite/Setup.hs

@@ -14,6 +14,8 @@ import Distribution.Types.UnqualComponentName
 import Distribution.Utils.Path (getSymbolicPath)
 import Distribution.Verbosity
 
+import Data.List (isPrefixOf)
+
 import System.Directory
 import System.FilePath
 
@@ -65,7 +67,15 @@ generateScriptEnvModule lbi verbosity = do
       , "lbiPackages = read " ++ show (show (cabalTestsPackages lbi))
       , ""
       , "lbiProgramDb :: ProgramDb"
-      , "lbiProgramDb = read " ++ show (show (withPrograms lbi))
+      -- The `Show`/`Read` round trip below crosses Cabal versions: this
+      -- setup script is compiled against the released Cabal pinned in the
+      -- `custom-setup` stanza, while the generated module is read by the
+      -- test suite compiled against the in-tree Cabal, whose types may
+      -- have gained fields. Splice the fields added to `ConfiguredProgram`
+      -- since the pinned release, so that the newer `Read` accepts the
+      -- generated value (in the spirit of the `lbiCompiler` workaround
+      -- above).
+      , "lbiProgramDb = read " ++ show (addMissingConfiguredProgramFields (show (withPrograms lbi)))
       , ""
       , "lbiWithSharedLib :: Bool"
       , "lbiWithSharedLib = " ++ show (withSharedLib lbi)
@@ -78,6 +88,23 @@ generateScriptEnvModule lbi verbosity = do
     -- fixme: use component-specific folder
     libAutogenDir = autogenPackageModulesDir lbi
 
+-- | `programDriverArgs` was added to `ConfiguredProgram` after the Cabal
+-- release this setup script is compiled against; its derived `Show`
+-- therefore omits the field that the test suite's newer `Read` expects.
+-- Splice it into every record of the generated `show` output.
+--
+-- The spliced label is assumed not to occur inside any of the shown string
+-- values, which holds for the program arguments occurring in practice.
+addMissingConfiguredProgramFields :: String -> String
+addMissingConfiguredProgramFields = go
+  where
+    needle = ", programOverrideEnv = "
+    replacement = ", programDriverArgs = [], programOverrideEnv = "
+    go [] = []
+    go s@(c : rest)
+      | needle `isPrefixOf` s = replacement ++ go (drop (length needle) s)
+      | otherwise = c : go rest
+
 -- | Convert package database into absolute path, so that
 -- if we change working directories in a subprocess we get the correct database.
 canonicalizePackageDB :: PackageDB -> IO PackageDB

changelog.d/12321.md

@@ -0,0 +1,44 @@
+---
+synopsis: Consistently translate `-Wl,` in `ld-options` when invoking `ld` directly
+packages: [Cabal]
+prs: 12321
+issues: [10789]
+---
+
+`ld-options` (both the package field and the program option) hold options for
+GHC's linking phase: Cabal passes them to GHC as `-optl` arguments, which
+forwards them to the C compiler driver acting as the linker. In that form,
+options destined for the linker itself are written with the `-Wl,` prefix.
+
+However, Cabal also invokes the `ld` program directly, in a few places:
+
+- when probing the linker's capabilities (`ld --help`, `ld -x -r`), and
+- when combining object files into a library for GHCi (`ld -r`).
+
+There, the same options were passed to `ld` verbatim, in a form that `ld`
+does not understand. As a consequence, there was no way to spell an option
+that worked everywhere: options in the documented `-Wl,` form made the
+probes fail (silently disabling `--enable-library-for-ghci`) and failed the
+`ld -r` invocations, while options without the `-Wl,` prefix (the form `ld`
+understands) failed at GHC's linking phase, where the C compiler driver
+requires `-Wl,`.
+
+Now the documented `-Wl,` form works consistently: the options recorded for
+the `ld` program are split into the form `ld` itself understands (with
+`-Wl,a,b` translated into the raw linker options `a b`) for Cabal's direct
+`ld` invocations, and the as-given driver form for GHC's linking phase. The
+linker capability probes no longer include user-specified options at all.
+
+```diff
+  $ cabal build -v3
+  Running: /usr/bin/ld --help
+  Running: /usr/bin/ar -r dist/build/libHSpkg-0.1-inplace.a '@dist/tmp/ar.rsp'
+- Running: /usr/bin/ld -x -r -o dist/build/libHSpkg-0.1-inplace.o '@dist/tmp/ld.rsp' -Wl,--allow-multiple-definition
+- x86_64-linux-gnu-ld.bfd: unrecognized option '-Wl,--allow-multiple-definition'
+- x86_64-linux-gnu-ld.bfd: use the --help option for usage information
+- Error: [Cabal-7125]
+- Failed to build pkg-0.1-inplace.
++ Running: /usr/bin/ld -x -r -o dist/build/libHSpkg-0.1-inplace.o '@dist/tmp/ld.rsp' --allow-multiple-definition
+  Running: ghc --make ... -pgmc /usr/bin/gcc -optl-Wl,--allow-multiple-definition ... -o dist/build/pkg/pkg
++ Linking dist/build/pkg/pkg ...
+```