From 5b6a21acc5a4c14e4d5f79bd256e667f8e01ae30 Mon Sep 17 00:00:00 2001 From: Las Safin Date: Tue, 16 Jul 2024 21:05:16 +0000 Subject: [PATCH 1/2] Avoid casting function pointer in libutil test support Casting function pointers seems to be almost always UB. See https://stackoverflow.com/questions/559581/casting-a-function-pointer-to-another-type Fixed by doing the casting of `void*` to `std::string*` inside the function instead. Caught by UBSan. --- tests/unit/libutil-support/tests/string_callback.cc | 5 +++-- tests/unit/libutil-support/tests/string_callback.hh | 5 ++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/libutil-support/tests/string_callback.cc b/tests/unit/libutil-support/tests/string_callback.cc index 2d0e0dad0..7a13bd4ff 100644 --- a/tests/unit/libutil-support/tests/string_callback.cc +++ b/tests/unit/libutil-support/tests/string_callback.cc @@ -2,9 +2,10 @@ namespace nix::testing { -void observe_string_cb(const char * start, unsigned int n, std::string * user_data) +void observe_string_cb(const char * start, unsigned int n, void * user_data) { - *user_data = std::string(start); + auto user_data_casted = reinterpret_cast(user_data); + *user_data_casted = std::string(start); } } diff --git a/tests/unit/libutil-support/tests/string_callback.hh b/tests/unit/libutil-support/tests/string_callback.hh index a02ea3a1b..9a7e8d85d 100644 --- a/tests/unit/libutil-support/tests/string_callback.hh +++ b/tests/unit/libutil-support/tests/string_callback.hh @@ -3,14 +3,13 @@ namespace nix::testing { -void observe_string_cb(const char * start, unsigned int n, std::string * user_data); +void observe_string_cb(const char * start, unsigned int n, void * user_data); inline void * observe_string_cb_data(std::string & out) { return (void *) &out; }; -#define OBSERVE_STRING(str) \ - (nix_get_string_callback) nix::testing::observe_string_cb, nix::testing::observe_string_cb_data(str) +#define OBSERVE_STRING(str) nix::testing::observe_string_cb, nix::testing::observe_string_cb_data(str) } From a1f3f103bc1a618a83b51a97dd0fb0c2f66a6b41 Mon Sep 17 00:00:00 2001 From: Las Safin Date: Tue, 16 Jul 2024 21:38:19 +0000 Subject: [PATCH 2/2] Check if drv is initialized in DerivationGoal::waiteeDone It might not be set, in which case we shouldn't do anything. Surprisingly, this somehow did not cause segfaults before? Caught by UBSan. --- src/libstore/build/derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 010f905d6..b809e3ffe 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1569,7 +1569,7 @@ void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result) { Goal::waiteeDone(waitee, result); - if (!useDerivation) return; + if (!useDerivation || !drv) return; auto & fullDrv = *dynamic_cast(drv.get()); auto * dg = dynamic_cast(&*waitee);