Filter plugin dylib locations

Summary:
Previously we just created a cartesian product of the library
paths of the plugin package and the libraries of the package.
Of course, some of these combinations result in a filepath of
a file doesn't exists, leading to #15475.

Instead of making `haskFile` return Nothing in case a file
doesn't exist (which would hide errors), we look at all the
possible dylib locations and ensure that at least one of those
locations is an existing file. If the list turns out to be
empty however, we panic.

Reviewers: mpickering, bgamari

Reviewed By: mpickering

Subscribers: monoidal, rwbarton, carter

GHC Trac Issues: #15475

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

5 files changed, 59 insertions, 8 deletions

diff --git a/compiler/deSugar/DsUsage.hs b/compiler/deSugar/DsUsage.hs
index 45d4dcfb48..58c31eee44 100644
--- a/compiler/deSugar/DsUsage.hs
+++ b/compiler/deSugar/DsUsage.hs
@@ -25,6 +25,7 @@ import Maybes
 import Packages
 import Finder
 
+import Control.Monad (filterM)
 import Data.List
 import Data.IORef
 import Data.Map (Map)
@@ -166,14 +167,19 @@ mkPluginUsage hsc_env pluginModule
   = case lookupPluginModuleWithSuggestions dflags pNm Nothing of
     -- The plug is from an external package, we just look up the dylib that
     -- contains the plugin
-    LookupFound _ pkg ->
+    LookupFound _ pkg -> do
       let searchPaths = collectLibraryPaths dflags [pkg]
           libs        = packageHsLibs dflags pkg
-          dynlibs     = [ searchPath </> mkHsSOName platform lib
+          dynlibLocs  = [ searchPath </> mkHsSOName platform lib
                         | searchPath <- searchPaths
                         , lib <- libs
                         ]
-      in  mapM hashFile (nub dynlibs)
+      dynlibs <- filterM doesFileExist dynlibLocs
+      case dynlibs of
+        [] -> pprPanic
+                ("mkPluginUsage: no dylibs, tried:\n" ++ unlines dynlibLocs)
+                (ppr pNm)
+        _  -> mapM hashFile (nub dynlibs)
     _ -> do
       foundM <- findPluginModule hsc_env pNm
       case foundM of
@@ -186,10 +192,25 @@ mkPluginUsage hsc_env pluginModule
           return (nub (pluginObject : depObjects))
         _ -> pprPanic "mkPluginUsage: no object or dylib" (ppr pNm)
   where
-    -- plugins are shared libraries, so add WayDyn to the dflags in order to get
-    -- the correct filenames and library paths; just in case the object that is
-    -- currently being build is not going to be linked dynamically
-    dflags   = addWay' WayDyn (hsc_dflags hsc_env)
+    -- plugins are shared libraries, so WayDyn should be part of the dflags in
+    -- order to get the correct filenames and library paths.
+    --
+    -- We can distinguish two scenarios:
+    --
+    -- 1. The dflags do not contain WayDyn, in this case we need to remove
+    --    all other ways and only add WayDyn. Why? Because other ways change
+    --    the library tags, i.e. WayProf adds `_p`, and we would end up looking
+    --    for a profiled plugin which might not be installed. See #15492
+    --
+    -- 2. The dflags do contain WayDyn, in this case we can leave the ways as
+    --    is, because the plugin must be compiled with the same ways as the
+    --    module that is currently being build, e.g., if the module is
+    --    build with WayDyn and WayProf, then the plugin that was used
+    --    would've also had to been build with WayProf (and WayDyn).
+    dflags1  = hsc_dflags hsc_env
+    dflags   = if WayDyn `elem` ways dflags1
+                 then dflags1
+                 else updateWays (addWay' WayDyn (dflags1 {ways = []}))
     platform = targetPlatform dflags
     pNm      = moduleName (mi_module pluginModule)
     pPkg     = moduleUnitId (mi_module pluginModule)
diff --git a/testsuite/tests/plugins/Makefile b/testsuite/tests/plugins/Makefile
index 688ac04fb5..8a6af5b118 100644
--- a/testsuite/tests/plugins/Makefile
+++ b/testsuite/tests/plugins/Makefile
@@ -112,3 +112,12 @@ plugin-recomp-change:
 	"$(TEST_HC)" $(TEST_HC_OPTS) $(ghcPluginWayFlags) -v0 plugin-recomp-test.hs -package-db plugin-recomp/pkg.plugins01/local.package.conf -fplugin PurePlugin
 	"$(MAKE)" -s --no-print-directory -C plugin-recomp package.plugins01 TOP=$(TOP) RUN=-DRUN2
 	"$(TEST_HC)" $(TEST_HC_OPTS) $(ghcPluginWayFlags) -v0 plugin-recomp-test.hs -package-db plugin-recomp/pkg.plugins01/local.package.conf -fplugin PurePlugin
+
+# Should recompile the module because the plugin changed, test for 15492
+.PHONY: plugin-recomp-change-prof
+plugin-recomp-change-prof:
+	"$(TEST_HC)" $(TEST_HC_OPTS) $(ghcPluginWayFlags) -v0 plugin-recomp-test.hs -package-db plugin-recomp/pkg.plugins01/local.package.conf -fplugin PurePlugin
+	"$(TEST_HC)" $(TEST_HC_OPTS) -prof -osuf p_o -hisuf p_hi -v0 plugin-recomp-test.hs -package-db plugin-recomp/pkg.plugins01/local.package.conf -fplugin PurePlugin
+	"$(MAKE)" -s --no-print-directory -C plugin-recomp package.plugins01 TOP=$(TOP) RUN=-DRUN2
+	"$(TEST_HC)" $(TEST_HC_OPTS) $(ghcPluginWayFlags) -v0 plugin-recomp-test.hs -package-db plugin-recomp/pkg.plugins01/local.package.conf -fplugin PurePlugin
+	"$(TEST_HC)" $(TEST_HC_OPTS) -prof -osuf p_o -hisuf p_hi -v0 plugin-recomp-test.hs -package-db plugin-recomp/pkg.plugins01/local.package.conf -fplugin PurePlugin
diff --git a/testsuite/tests/plugins/all.T b/testsuite/tests/plugins/all.T
index 48efb05f81..339b9bab76 100644
--- a/testsuite/tests/plugins/all.T
+++ b/testsuite/tests/plugins/all.T
@@ -161,3 +161,10 @@ test('plugin-recomp-change',
       pre_cmd('$MAKE -s --no-print-directory -C plugin-recomp package.plugins01 TOP={top}')
       ],
      run_command, ['$MAKE -s --no-print-directory plugin-recomp-change'])
+
+test('plugin-recomp-change-prof',
+     [extra_files(['plugin-recomp/', 'plugin-recomp-test.hs']),
+      pre_cmd('$MAKE -s --no-print-directory -C plugin-recomp package.plugins01 TOP={top}'),
+      when(not config.have_profiling,skip)
+      ],
+     run_command, ['$MAKE -s --no-print-directory plugin-recomp-change-prof'])
diff --git a/testsuite/tests/plugins/plugin-recomp-change-prof.stderr b/testsuite/tests/plugins/plugin-recomp-change-prof.stderr
new file mode 100644
index 0000000000..b8018055db
--- /dev/null
+++ b/testsuite/tests/plugins/plugin-recomp-change-prof.stderr
@@ -0,0 +1,12 @@
+Simple Plugin Passes Queried
+Got options:
+Simple Plugin Pass Run
+Simple Plugin Passes Queried
+Got options:
+Simple Plugin Pass Run
+Simple Plugin Passes Queried
+Got options:
+Simple Plugin Pass Run 2
+Simple Plugin Passes Queried
+Got options:
+Simple Plugin Pass Run 2
diff --git a/testsuite/tests/plugins/plugin-recomp/Makefile b/testsuite/tests/plugins/plugin-recomp/Makefile
index db2df8dbc8..9ee7737339 100644
--- a/testsuite/tests/plugins/plugin-recomp/Makefile
+++ b/testsuite/tests/plugins/plugin-recomp/Makefile
@@ -16,6 +16,8 @@ package.%:
 
 	"$(GHC_PKG)" init pkg.$*/local.package.conf
 
-	pkg.$*/setup configure --distdir pkg.$*/dist -v0 $(CABAL_PLUGIN_BUILD) --ghc-option="$(RUN)" --prefix="$(HERE)/pkg.$*/install" --with-compiler="$(TEST_HC)" --with-hc-pkg="$(GHC_PKG)" --package-db=pkg.$*/local.package.conf $(if $(findstring YES,$(HAVE_PROFILING)), --enable-library-profiling)
+  # The bogus extra-lib-dirs ensures the package is registered with multiple
+  # dynamic-library-directories which tests that the fix for #15475 works
+	pkg.$*/setup configure --distdir pkg.$*/dist -v0 $(CABAL_PLUGIN_BUILD) --ghc-option="$(RUN)" --prefix="$(HERE)/pkg.$*/install" --with-compiler="$(TEST_HC)" --with-hc-pkg="$(GHC_PKG)" --extra-lib-dirs="$(HERE)" --package-db=pkg.$*/local.package.conf $(if $(findstring YES,$(HAVE_PROFILING)), --enable-library-profiling)
 	pkg.$*/setup build     --distdir pkg.$*/dist -v0
 	pkg.$*/setup install   --distdir pkg.$*/dist -v0