From 351d898c43202ca9c3f94daf7d727b3b7bbd3daa Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Sun, 15 Jun 2025 16:51:40 +0000 Subject: [PATCH] libexpr: Switch builtins.sort primop to use peeksort This prevents C++ level undefined behavior from affecting the evaluator. Stdlib implementation details should not affect eval, regardless of the build platform. Even erroneous usage of `builtins.sort` should not make it possible to crash the evaluator or produce results that depend on the host platform. --- src/libexpr/primops.cc | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 60f44ca62..75d7465dd 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -14,6 +14,7 @@ #include "nix/expr/value-to-xml.hh" #include "nix/expr/primops.hh" #include "nix/fetchers/fetch-to-store.hh" +#include "nix/util/sort.hh" #include #include @@ -3695,10 +3696,14 @@ static void prim_sort(EvalState & state, const PosIdx pos, Value * * args, Value return state.forceBool(vBool, pos, "while evaluating the return value of the sorting function passed to builtins.sort"); }; - /* FIXME: std::sort can segfault if the comparator is not a strict - weak ordering. What to do? std::stable_sort() seems more - resilient, but no guarantees... */ - std::stable_sort(list.begin(), list.end(), comparator); + /* NOTE: Using custom implementation because std::sort and std::stable_sort + are not resilient to comparators that violate strict weak ordering. Diagnosing + incorrect implementations is a O(n^3) problem, so doing the checks is much more + expensive that doing the sorting. For this reason we choose to use sorting algorithms + that are can't be broken by invalid comprators. peeksort (mergesort) + doesn't misbehave when any of the strict weak order properties is + violated - output is always a reordering of the input. */ + peeksort(list.begin(), list.end(), comparator); v.mkList(list); }