From b54dfb66dd5bbaae430d4883e6cc7589b8d2e98b Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 11 Dec 2025 13:48:04 +0100 Subject: [PATCH 1/3] Fix segfault in getUnitTestData() when env var not set The previous implementation called .value() on std::optional without checking if it had a value. When _NIX_TEST_UNIT_DATA was not set, this would throw std::bad_optional_access or cause a segfault in code that used the raw getenv() result. The new implementation checks the optional first and throws an Error with a helpful message directing users to run tests via meson. The example includes --gdb since this situation may arise when trying to debug tests without knowing about meson's test infrastructure. --- .../include/nix/util/tests/characterization.hh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libutil-test-support/include/nix/util/tests/characterization.hh b/src/libutil-test-support/include/nix/util/tests/characterization.hh index d8fad1df9..67ad8fb51 100644 --- a/src/libutil-test-support/include/nix/util/tests/characterization.hh +++ b/src/libutil-test-support/include/nix/util/tests/characterization.hh @@ -15,7 +15,11 @@ namespace nix { */ static inline std::filesystem::path getUnitTestData() { - return getEnv("_NIX_TEST_UNIT_DATA").value(); + auto data = getEnv("_NIX_TEST_UNIT_DATA"); + if (!data) + throw Error( + "_NIX_TEST_UNIT_DATA environment variable is not set. Recommendation: use meson, example: 'meson test -C build --gdb'"); + return std::filesystem::path(*data); } /** From de6fdb7da5854543e2c6d58241b698f52daab5bd Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 12 Dec 2025 15:44:03 +0100 Subject: [PATCH 2/3] Extract getUnitTestData() to test-data.hh and fix unsafe getenv calls The nix_api_store.cc tests and derivation-parser-bench.cc were using raw getenv() calls or unsafe .value() calls on optional, which would segfault when passed to std::filesystem::path constructor if the _NIX_TEST_UNIT_DATA environment variable was not set. --- src/libstore-tests/derivation-parser-bench.cc | 14 ++++------- src/libstore-tests/nix_api_store.cc | 13 +++++----- .../nix/util/tests/characterization.hh | 15 +----------- .../include/nix/util/tests/meson.build | 1 + .../include/nix/util/tests/test-data.hh | 24 +++++++++++++++++++ 5 files changed, 38 insertions(+), 29 deletions(-) create mode 100644 src/libutil-test-support/include/nix/util/tests/test-data.hh diff --git a/src/libstore-tests/derivation-parser-bench.cc b/src/libstore-tests/derivation-parser-bench.cc index 1709eed1c..f0aa721cb 100644 --- a/src/libstore-tests/derivation-parser-bench.cc +++ b/src/libstore-tests/derivation-parser-bench.cc @@ -2,7 +2,7 @@ #include "nix/store/derivations.hh" #include "nix/store/store-api.hh" #include "nix/util/experimental-features.hh" -#include "nix/util/environment-variables.hh" +#include "nix/util/tests/test-data.hh" #include "nix/store/store-open.hh" #include #include @@ -50,11 +50,7 @@ static void BM_UnparseRealDerivationFile(benchmark::State & state, const std::st } // Register benchmarks for actual test derivation files if they exist -BENCHMARK_CAPTURE( - BM_ParseRealDerivationFile, hello, getEnvNonEmpty("_NIX_TEST_UNIT_DATA").value() + "/derivation/hello.drv"); -BENCHMARK_CAPTURE( - BM_ParseRealDerivationFile, firefox, getEnvNonEmpty("_NIX_TEST_UNIT_DATA").value() + "/derivation/firefox.drv"); -BENCHMARK_CAPTURE( - BM_UnparseRealDerivationFile, hello, getEnvNonEmpty("_NIX_TEST_UNIT_DATA").value() + "/derivation/hello.drv"); -BENCHMARK_CAPTURE( - BM_UnparseRealDerivationFile, firefox, getEnvNonEmpty("_NIX_TEST_UNIT_DATA").value() + "/derivation/firefox.drv"); +BENCHMARK_CAPTURE(BM_ParseRealDerivationFile, hello, (getUnitTestData() / "derivation/hello.drv").string()); +BENCHMARK_CAPTURE(BM_ParseRealDerivationFile, firefox, (getUnitTestData() / "derivation/firefox.drv").string()); +BENCHMARK_CAPTURE(BM_UnparseRealDerivationFile, hello, (getUnitTestData() / "derivation/hello.drv").string()); +BENCHMARK_CAPTURE(BM_UnparseRealDerivationFile, firefox, (getUnitTestData() / "derivation/firefox.drv").string()); diff --git a/src/libstore-tests/nix_api_store.cc b/src/libstore-tests/nix_api_store.cc index ea600f905..e67e5ffef 100644 --- a/src/libstore-tests/nix_api_store.cc +++ b/src/libstore-tests/nix_api_store.cc @@ -8,6 +8,7 @@ #include "nix/store/tests/nix_api_store.hh" #include "nix/store/globals.hh" #include "nix/util/tests/string_callback.hh" +#include "nix/util/tests/test-data.hh" #include "nix/util/url.hh" #include "store-tests-config.hh" @@ -302,7 +303,7 @@ public: store = open_local_store(); - std::filesystem::path unitTestData{getenv("_NIX_TEST_UNIT_DATA")}; + std::filesystem::path unitTestData = nix::getUnitTestData(); std::ifstream t{unitTestData / "derivation/ca/self-contained.json"}; std::stringstream buffer; buffer << t.rdbuf(); @@ -357,7 +358,7 @@ TEST_F(nix_api_store_test_base, build_from_json) auto * store = open_local_store(); - std::filesystem::path unitTestData{getenv("_NIX_TEST_UNIT_DATA")}; + std::filesystem::path unitTestData = nix::getUnitTestData(); std::ifstream t{unitTestData / "derivation/ca/self-contained.json"}; std::stringstream buffer; @@ -404,7 +405,7 @@ TEST_F(nix_api_store_test_base, nix_store_realise_invalid_system) auto * store = open_local_store(); - std::filesystem::path unitTestData{getenv("_NIX_TEST_UNIT_DATA")}; + std::filesystem::path unitTestData = nix::getUnitTestData(); std::ifstream t{unitTestData / "derivation/ca/self-contained.json"}; std::stringstream buffer; buffer << t.rdbuf(); @@ -449,7 +450,7 @@ TEST_F(nix_api_store_test_base, nix_store_realise_builder_fails) auto * store = open_local_store(); - std::filesystem::path unitTestData{getenv("_NIX_TEST_UNIT_DATA")}; + std::filesystem::path unitTestData = nix::getUnitTestData(); std::ifstream t{unitTestData / "derivation/ca/self-contained.json"}; std::stringstream buffer; buffer << t.rdbuf(); @@ -494,7 +495,7 @@ TEST_F(nix_api_store_test_base, nix_store_realise_builder_no_output) auto * store = open_local_store(); - std::filesystem::path unitTestData{getenv("_NIX_TEST_UNIT_DATA")}; + std::filesystem::path unitTestData = nix::getUnitTestData(); std::ifstream t{unitTestData / "derivation/ca/self-contained.json"}; std::stringstream buffer; buffer << t.rdbuf(); @@ -870,7 +871,7 @@ TEST_F(NixApiStoreTestWithRealisedPath, nix_store_get_fs_closure_error_propagati */ static std::string load_json_from_test_data(const char * filename) { - std::filesystem::path unitTestData{getenv("_NIX_TEST_UNIT_DATA")}; + std::filesystem::path unitTestData = nix::getUnitTestData(); std::ifstream t{unitTestData / filename}; std::stringstream buffer; buffer << t.rdbuf(); diff --git a/src/libutil-test-support/include/nix/util/tests/characterization.hh b/src/libutil-test-support/include/nix/util/tests/characterization.hh index 67ad8fb51..52321828f 100644 --- a/src/libutil-test-support/include/nix/util/tests/characterization.hh +++ b/src/libutil-test-support/include/nix/util/tests/characterization.hh @@ -4,24 +4,11 @@ #include #include "nix/util/types.hh" -#include "nix/util/environment-variables.hh" #include "nix/util/file-system.hh" +#include "nix/util/tests/test-data.hh" namespace nix { -/** - * The path to the unit test data directory. See the contributing guide - * in the manual for further details. - */ -static inline std::filesystem::path getUnitTestData() -{ - auto data = getEnv("_NIX_TEST_UNIT_DATA"); - if (!data) - throw Error( - "_NIX_TEST_UNIT_DATA environment variable is not set. Recommendation: use meson, example: 'meson test -C build --gdb'"); - return std::filesystem::path(*data); -} - /** * Whether we should update "golden masters" instead of running tests * against them. See the contributing guide in the manual for further diff --git a/src/libutil-test-support/include/nix/util/tests/meson.build b/src/libutil-test-support/include/nix/util/tests/meson.build index 3be085892..9f09183f3 100644 --- a/src/libutil-test-support/include/nix/util/tests/meson.build +++ b/src/libutil-test-support/include/nix/util/tests/meson.build @@ -10,4 +10,5 @@ headers = files( 'json-characterization.hh', 'nix_api_util.hh', 'string_callback.hh', + 'test-data.hh', ) diff --git a/src/libutil-test-support/include/nix/util/tests/test-data.hh b/src/libutil-test-support/include/nix/util/tests/test-data.hh new file mode 100644 index 000000000..6b965b848 --- /dev/null +++ b/src/libutil-test-support/include/nix/util/tests/test-data.hh @@ -0,0 +1,24 @@ +#pragma once +///@file + +#include +#include "nix/util/environment-variables.hh" +#include "nix/util/error.hh" + +namespace nix { + +/** + * The path to the unit test data directory. See the contributing guide + * in the manual for further details. + */ +static inline std::filesystem::path getUnitTestData() +{ + auto data = getEnv("_NIX_TEST_UNIT_DATA"); + if (!data) + throw Error( + "_NIX_TEST_UNIT_DATA environment variable is not set. " + "Recommendation: use meson, example: 'meson test -C build --gdb'"); + return std::filesystem::path(*data); +} + +} // namespace nix From 76c09bf3d420093d26af7da15e9e16754d8a10a2 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 11 Dec 2025 15:52:34 +0100 Subject: [PATCH 3/3] Fix nix-build.cc double getenv("TZ") race condition This is mostly theoretical, but the code was calling getenv("TZ") twice: once to check if it's non-null, and again to get its value. This creates a potential race condition where the environment could change between calls. --- src/nix/nix-build/nix-build.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/nix/nix-build/nix-build.cc b/src/nix/nix-build/nix-build.cc index a21d1a565..53dc46eaa 100644 --- a/src/nix/nix-build/nix-build.cc +++ b/src/nix/nix-build/nix-build.cc @@ -613,6 +613,8 @@ static void main_nix_build(int argc, char ** argv) environment variables and shell functions. Also don't lose the current $PATH directories. */ auto rcfile = (tmpDir.path() / "rc").string(); + auto tz = getEnv("TZ"); + auto tzExport = tz ? "export TZ=" + escapeShellArgAlways(*tz) + "; " : ""; std::string rc = fmt( (R"(_nix_shell_clean_tmpdir() { command rm -rf %1%; };)"s "trap _nix_shell_clean_tmpdir EXIT; " @@ -646,7 +648,7 @@ static void main_nix_build(int argc, char ** argv) (pure ? "" : "PATH=$PATH:$p; unset p; "), escapeShellArgAlways(dirOf(*shell)), escapeShellArgAlways(*shell), - (getenv("TZ") ? (std::string("export TZ=") + escapeShellArgAlways(getenv("TZ")) + "; ") : ""), + tzExport, envCommand); vomit("Sourcing nix-shell with file %s and contents:\n%s", rcfile, rc); writeFile(rcfile, rc);