Merge pull request #9430 from hercules-ci/remove-vlas

Fix stack overflow in `filter`
This commit is contained in:
Eelco Dolstra 2023-11-30 15:31:44 +01:00 committed by GitHub
commit cb7f25869d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 70 additions and 9 deletions

View file

@ -0,0 +1,12 @@
diff --git a/include/gc_allocator.h b/include/gc_allocator.h
index 597c7f13..587286be 100644
--- a/include/gc_allocator.h
+++ b/include/gc_allocator.h
@@ -312,6 +312,7 @@ public:
template<>
class traceable_allocator<void> {
+public:
typedef size_t size_type;
typedef ptrdiff_t difference_type;
typedef void* pointer;

View file

@ -238,6 +238,9 @@
}).overrideAttrs(o: { }).overrideAttrs(o: {
patches = (o.patches or []) ++ [ patches = (o.patches or []) ++ [
./boehmgc-coroutine-sp-fallback.diff ./boehmgc-coroutine-sp-fallback.diff
# https://github.com/ivmai/bdwgc/pull/586
./boehmgc-traceable_allocator-public.diff
]; ];
}) })
) )

View file

@ -16,6 +16,7 @@
#include "fs-input-accessor.hh" #include "fs-input-accessor.hh"
#include "memory-input-accessor.hh" #include "memory-input-accessor.hh"
#include "signals.hh" #include "signals.hh"
#include "gc-small-vector.hh"
#include <algorithm> #include <algorithm>
#include <chrono> #include <chrono>
@ -31,6 +32,7 @@
#include <sys/resource.h> #include <sys/resource.h>
#include <nlohmann/json.hpp> #include <nlohmann/json.hpp>
#include <boost/container/small_vector.hpp>
#if HAVE_BOEHMGC #if HAVE_BOEHMGC
@ -1712,7 +1714,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value &
/* We have all the arguments, so call the primop with /* We have all the arguments, so call the primop with
the previous and new arguments. */ the previous and new arguments. */
Value * vArgs[arity]; Value * vArgs[maxPrimOpArity];
auto n = argsDone; auto n = argsDone;
for (Value * arg = &vCur; arg->isPrimOpApp(); arg = arg->primOpApp.left) for (Value * arg = &vCur; arg->isPrimOpApp(); arg = arg->primOpApp.left)
vArgs[--n] = arg->primOpApp.right; vArgs[--n] = arg->primOpApp.right;
@ -1775,11 +1777,11 @@ void ExprCall::eval(EvalState & state, Env & env, Value & v)
// 4: about 60 // 4: about 60
// 5: under 10 // 5: under 10
// This excluded attrset lambdas (`{...}:`). Contributions of mixed lambdas appears insignificant at ~150 total. // This excluded attrset lambdas (`{...}:`). Contributions of mixed lambdas appears insignificant at ~150 total.
Value * vArgs[args.size()]; SmallValueVector<4> vArgs(args.size());
for (size_t i = 0; i < args.size(); ++i) for (size_t i = 0; i < args.size(); ++i)
vArgs[i] = args[i]->maybeThunk(state, env); vArgs[i] = args[i]->maybeThunk(state, env);
state.callFunction(vFun, args.size(), vArgs, v, pos); state.callFunction(vFun, args.size(), vArgs.data(), v, pos);
} }
@ -2018,8 +2020,9 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
return result; return result;
}; };
Value values[es->size()]; // List of returned strings. References to these Values must NOT be persisted.
Value * vTmpP = values; SmallTemporaryValueVector<conservativeStackReservation> values(es->size());
Value * vTmpP = values.data();
for (auto & [i_pos, i] : *es) { for (auto & [i_pos, i] : *es) {
Value & vTmp = *vTmpP++; Value & vTmp = *vTmpP++;

View file

@ -0,0 +1,42 @@
#pragma once
#include <boost/container/small_vector.hpp>
#if HAVE_BOEHMGC
#include <gc/gc.h>
#include <gc/gc_cpp.h>
#include <gc/gc_allocator.h>
#endif
namespace nix {
struct Value;
/**
* A GC compatible vector that may used a reserved portion of `nItems` on the stack instead of allocating on the heap.
*/
#if HAVE_BOEHMGC
template <typename T, size_t nItems>
using SmallVector = boost::container::small_vector<T, nItems, traceable_allocator<T>>;
#else
template <typename T, size_t nItems>
using SmallVector = boost::container::small_vector<T, nItems>;
#endif
/**
* A vector of value pointers. See `SmallVector`.
*/
template <size_t nItems>
using SmallValueVector = SmallVector<Value *, nItems>;
/**
* A vector of values that must not be referenced after the vector is destroyed.
*
* See also `SmallValueVector`.
*/
template <size_t nItems>
using SmallTemporaryValueVector = SmallVector<Value, nItems>;
}

View file

@ -4,6 +4,7 @@
#include "eval-inline.hh" #include "eval-inline.hh"
#include "eval.hh" #include "eval.hh"
#include "eval-settings.hh" #include "eval-settings.hh"
#include "gc-small-vector.hh"
#include "globals.hh" #include "globals.hh"
#include "json-to-value.hh" #include "json-to-value.hh"
#include "names.hh" #include "names.hh"
@ -2729,7 +2730,7 @@ static void prim_catAttrs(EvalState & state, const PosIdx pos, Value * * args, V
auto attrName = state.symbols.create(state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.catAttrs")); auto attrName = state.symbols.create(state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.catAttrs"));
state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.catAttrs"); state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.catAttrs");
Value * res[args[1]->listSize()]; SmallValueVector<nonRecursiveStackReservation> res(args[1]->listSize());
size_t found = 0; size_t found = 0;
for (auto v2 : args[1]->listItems()) { for (auto v2 : args[1]->listItems()) {
@ -3064,8 +3065,7 @@ static void prim_filter(EvalState & state, const PosIdx pos, Value * * args, Val
state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.filter"); state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.filter");
// FIXME: putting this on the stack is risky. SmallValueVector<nonRecursiveStackReservation> vs(args[1]->listSize());
Value * vs[args[1]->listSize()];
size_t k = 0; size_t k = 0;
bool same = true; bool same = true;
@ -3461,7 +3461,8 @@ static void prim_concatMap(EvalState & state, const PosIdx pos, Value * * args,
state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.concatMap"); state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.concatMap");
auto nrLists = args[1]->listSize(); auto nrLists = args[1]->listSize();
Value lists[nrLists]; // List of returned lists before concatenation. References to these Values must NOT be persisted.
SmallTemporaryValueVector<conservativeStackReservation> lists(nrLists);
size_t len = 0; size_t len = 0;
for (unsigned int n = 0; n < nrLists; ++n) { for (unsigned int n = 0; n < nrLists; ++n) {