From 0d85875c3a4284dabad79069758a9056898c42dc Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Mon, 11 Mar 2024 00:50:57 -0700 Subject: [PATCH] Allow dlopen of plugins to fail It happens with some frequency that plugins that might be unimportant to the evaluation at hand mismatch with the nix version, leading to spurious load failures. Let's make these non fatal. Change-Id: Iba10e951d171725ccf1a121bcd9be1e1d6ad69eb --- src/libstore/globals.cc | 2 +- tests/functional/local.mk | 3 ++- tests/functional/plugins.sh | 16 +++++++++++++++- tests/functional/plugins/local.mk | 18 +++++++++++++++++- tests/functional/plugins/plugintest.cc | 12 ++++++++++++ tests/functional/plugins/plugintestfail.cc | 1 + 6 files changed, 48 insertions(+), 4 deletions(-) create mode 120000 tests/functional/plugins/plugintestfail.cc diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 0aecd2b6a..2929bd6e6 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -342,7 +342,7 @@ void initPlugins() void *handle = dlopen(file.c_str(), RTLD_LAZY | RTLD_LOCAL); if (!handle) - throw Error("could not dynamically open plugin file '%s': %s", file, dlerror()); + warn("could not dynamically open plugin file '%s': %s", file, dlerror()); } } diff --git a/tests/functional/local.mk b/tests/functional/local.mk index af2a55e58..417c78509 100644 --- a/tests/functional/local.mk +++ b/tests/functional/local.mk @@ -141,7 +141,8 @@ endif $(d)/test-libstoreconsumer.sh.test $(d)/test-libstoreconsumer.sh.test-debug: \ $(buildprefix)$(d)/test-libstoreconsumer/test-libstoreconsumer $(d)/plugins.sh.test $(d)/plugins.sh.test-debug: \ - $(buildprefix)$(d)/plugins/libplugintest.$(SO_EXT) + $(buildprefix)$(d)/plugins/libplugintest.$(SO_EXT) \ + $(buildprefix)$(d)/plugins/libplugintestfail.$(SO_EXT) $(d)/test-repl-characterization.sh.test $(d)/test-repl-characterization.sh.test-debug: \ $(buildprefix)$(d)/repl_characterization/test-repl-characterization diff --git a/tests/functional/plugins.sh b/tests/functional/plugins.sh index baf71a362..491b933b7 100644 --- a/tests/functional/plugins.sh +++ b/tests/functional/plugins.sh @@ -4,6 +4,20 @@ if [[ $BUILD_SHARED_LIBS != 1 ]]; then skipTest "Plugins are not supported" fi -res=$(nix --option setting-set true --option plugin-files $PWD/plugins/libplugintest* eval --expr builtins.anotherNull) +res=$(nix --option setting-set true --option plugin-files $PWD/plugins/libplugintest.* eval --expr builtins.anotherNull) [ "$res"x = "nullx" ] + +# Plugin load failing due to missing symbols +res=$(nix --option plugin-files $PWD/plugins/libplugintestfail.* eval --expr '1234 + 5' 2>&1) +# We expect this to succeed evaluating +echo "$res" | grep 1239 >/dev/null +# On Linux, we expect this to print some failure of dlopen. +# Only on Linux do we expect for sure that -z now is set on the .so file, so it +# will definitely fail to load instead of lazy loading (and thus not hitting +# the missing symbol). +# FIXME(jade): does there exist an equivalent of -z now on macOS that eluded us +# in search? +if [[ "$(uname -s)" == Linux ]]; then + echo "$res" | grep "could not dynamically open plugin file" >/dev/null +fi diff --git a/tests/functional/plugins/local.mk b/tests/functional/plugins/local.mk index 40350aa96..77339d4c6 100644 --- a/tests/functional/plugins/local.mk +++ b/tests/functional/plugins/local.mk @@ -1,4 +1,4 @@ -libraries += libplugintest +libraries += libplugintest libplugintestfail libplugintest_DIR := $(d) @@ -9,3 +9,19 @@ libplugintest_ALLOW_UNDEFINED := 1 libplugintest_EXCLUDE_FROM_LIBRARY_LIST := 1 libplugintest_CXXFLAGS := -I src/libutil -I src/libstore -I src/libexpr -I src/libfetchers + +libplugintestfail_DIR := $(d) + +libplugintestfail_SOURCES := $(d)/plugintestfail.cc + +libplugintestfail_ALLOW_UNDEFINED := 1 + +libplugintestfail_EXCLUDE_FROM_LIBRARY_LIST := 1 + +libplugintestfail_CXXFLAGS := -I src/libutil -I src/libstore -I src/libexpr -I src/libfetchers -DMISSING_REFERENCE + +# Make sure that the linker strictly evaluates all symbols on .so load on Linux +# so it will definitely fail to load as expected. +ifdef HOST_LINUX + libplugintestfail_LDFLAGS += -z now +endif diff --git a/tests/functional/plugins/plugintest.cc b/tests/functional/plugins/plugintest.cc index e02fd68d5..9c20f2241 100644 --- a/tests/functional/plugins/plugintest.cc +++ b/tests/functional/plugins/plugintest.cc @@ -1,8 +1,15 @@ #include "config.hh" #include "primops.hh" +#include using namespace nix; +#ifdef MISSING_REFERENCE +extern void meow(); +#else +#define meow() {} +#endif + struct MySettings : Config { Setting settingSet{this, false, "setting-set", @@ -13,6 +20,11 @@ MySettings mySettings; static GlobalConfig::Register rs(&mySettings); +[[gnu::used, gnu::unused, gnu::retain]] +static void maybeRequireMeowForDlopen() { + meow(); +} + static void prim_anotherNull (EvalState & state, const PosIdx pos, Value ** args, Value & v) { if (mySettings.settingSet) diff --git a/tests/functional/plugins/plugintestfail.cc b/tests/functional/plugins/plugintestfail.cc new file mode 120000 index 000000000..df60f3222 --- /dev/null +++ b/tests/functional/plugins/plugintestfail.cc @@ -0,0 +1 @@ +plugintest.cc \ No newline at end of file