aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/libexpr/eval.cc35
-rw-r--r--src/libexpr/eval.hh12
-rw-r--r--src/libexpr/primops.cc19
-rw-r--r--src/libexpr/primops.hh16
-rw-r--r--src/libexpr/value.hh8
-rw-r--r--src/libstore/derivations.cc9
-rw-r--r--src/libstore/gc.cc9
-rw-r--r--tests/unit/libexpr/value/print.cc3
8 files changed, 78 insertions, 33 deletions
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index b1268bb12..e98280f33 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -1,6 +1,7 @@
#include "eval.hh"
#include "eval-settings.hh"
#include "hash.hh"
+#include "primops.hh"
#include "types.hh"
#include "util.hh"
#include "store-api.hh"
@@ -38,6 +39,7 @@
#include <boost/coroutine2/coroutine.hpp>
#include <boost/coroutine2/protected_fixedsize_stack.hpp>
#include <boost/context/stack_context.hpp>
+#include <boost/container/small_vector.hpp>
#endif
@@ -703,6 +705,23 @@ void EvalState::addConstant(const std::string & name, Value * v, Constant info)
}
+void PrimOp::check()
+{
+ if (arity > maxPrimOpArity) {
+ throw Error("primop arity must not exceed %1%", maxPrimOpArity);
+ }
+}
+
+
+void Value::mkPrimOp(PrimOp * p)
+{
+ p->check();
+ clearValue();
+ internalType = tPrimOp;
+ primOp = p;
+}
+
+
Value * EvalState::addPrimOp(PrimOp && primOp)
{
/* Hack to make constants lazy: turn them into a application of
@@ -1683,7 +1702,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value &
/* We have all the arguments, so call the primop with
the previous and new arguments. */
- Value * vArgs[arity];
+ Value * vArgs[maxPrimOpArity];
auto n = argsDone;
for (Value * arg = &vCur; arg->isPrimOpApp(); arg = arg->primOpApp.left)
vArgs[--n] = arg->primOpApp.right;
@@ -1740,11 +1759,17 @@ void ExprCall::eval(EvalState & state, Env & env, Value & v)
Value vFun;
fun->eval(state, env, vFun);
- Value * vArgs[args.size()];
+ // Empirical arity of Nixpkgs lambdas by regex e.g. ([a-zA-Z]+:(\s|(/\*.*\/)|(#.*\n))*){5}
+ // 2: over 4000
+ // 3: about 300
+ // 4: about 60
+ // 5: under 10
+ // This excluded attrset lambdas (`{...}:`). Contributions of mixed lambdas appears insignificant at ~150 total.
+ boost::container::small_vector<Value *, 4> vArgs(args.size());
for (size_t i = 0; i < args.size(); ++i)
vArgs[i] = args[i]->maybeThunk(state, env);
- state.callFunction(vFun, args.size(), vArgs, v, pos);
+ state.callFunction(vFun, args.size(), vArgs.data(), v, pos);
}
@@ -1983,8 +2008,8 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
return result;
};
- Value values[es->size()];
- Value * vTmpP = values;
+ boost::container::small_vector<Value, conservativeStackReservation> values(es->size());
+ Value * vTmpP = values.data();
for (auto & [i_pos, i] : *es) {
Value & vTmp = *vTmpP++;
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 42dd73d11..4425efbca 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -18,6 +18,12 @@
namespace nix {
+/**
+ * We put a limit on primop arity because it lets us use a fixed size array on
+ * the stack. 8 is already an impractical number of arguments. Use an attrset
+ * argument for such overly complicated functions.
+ */
+constexpr size_t maxPrimOpArity = 8;
class Store;
class EvalState;
@@ -69,6 +75,12 @@ struct PrimOp
* Optional experimental for this to be gated on.
*/
std::optional<ExperimentalFeature> experimentalFeature;
+
+ /**
+ * Validity check to be performed by functions that introduce primops,
+ * such as RegisterPrimOp() and Value::mkPrimOp().
+ */
+ void check();
};
/**
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index fca20358a..44a97461a 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -28,7 +28,6 @@
#include <cmath>
-
namespace nix {
@@ -2547,6 +2546,7 @@ static void prim_removeAttrs(EvalState & state, const PosIdx pos, Value * * args
/* Get the attribute names to be removed.
We keep them as Attrs instead of Symbols so std::set_difference
can be used to remove them from attrs[0]. */
+ // 64: large enough to fit the attributes of a derivation
boost::container::small_vector<Attr, 64> names;
names.reserve(args[1]->listSize());
for (auto elem : args[1]->listItems()) {
@@ -2726,8 +2726,8 @@ 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"));
state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.catAttrs");
- Value * res[args[1]->listSize()];
- unsigned int found = 0;
+ boost::container::small_vector<Value *, nonRecursiveStackReservation> res(args[1]->listSize());
+ size_t found = 0;
for (auto v2 : args[1]->listItems()) {
state.forceAttrs(*v2, pos, "while evaluating an element in the list passed as second argument to builtins.catAttrs");
@@ -3061,9 +3061,8 @@ 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");
- // FIXME: putting this on the stack is risky.
- Value * vs[args[1]->listSize()];
- unsigned int k = 0;
+ boost::container::small_vector<Value *, nonRecursiveStackReservation> vs(args[1]->listSize());
+ size_t k = 0;
bool same = true;
for (unsigned int n = 0; n < args[1]->listSize(); ++n) {
@@ -3188,10 +3187,14 @@ static void anyOrAll(bool any, EvalState & state, const PosIdx pos, Value * * ar
state.forceFunction(*args[0], pos, std::string("while evaluating the first argument passed to builtins.") + (any ? "any" : "all"));
state.forceList(*args[1], pos, std::string("while evaluating the second argument passed to builtins.") + (any ? "any" : "all"));
+ std::string_view errorCtx = any
+ ? "while evaluating the return value of the function passed to builtins.any"
+ : "while evaluating the return value of the function passed to builtins.all";
+
Value vTmp;
for (auto elem : args[1]->listItems()) {
state.callFunction(*args[0], *elem, vTmp, pos);
- bool res = state.forceBool(vTmp, pos, std::string("while evaluating the return value of the function passed to builtins.") + (any ? "any" : "all"));
+ bool res = state.forceBool(vTmp, pos, errorCtx);
if (res == any) {
v.mkBool(any);
return;
@@ -3447,7 +3450,7 @@ 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");
auto nrLists = args[1]->listSize();
- Value lists[nrLists];
+ boost::container::small_vector<Value, conservativeStackReservation> lists(nrLists);
size_t len = 0;
for (unsigned int n = 0; n < nrLists; ++n) {
diff --git a/src/libexpr/primops.hh b/src/libexpr/primops.hh
index 930e7f32a..45486608f 100644
--- a/src/libexpr/primops.hh
+++ b/src/libexpr/primops.hh
@@ -8,6 +8,22 @@
namespace nix {
+/**
+ * For functions where we do not expect deep recursion, we can use a sizable
+ * part of the stack a free allocation space.
+ *
+ * Note: this is expected to be multiplied by sizeof(Value), or about 24 bytes.
+ */
+constexpr size_t nonRecursiveStackReservation = 128;
+
+/**
+ * Functions that maybe applied to self-similar inputs, such as concatMap on a
+ * tree, should reserve a smaller part of the stack for allocation.
+ *
+ * Note: this is expected to be multiplied by sizeof(Value), or about 24 bytes.
+ */
+constexpr size_t conservativeStackReservation = 16;
+
struct RegisterPrimOp
{
typedef std::vector<PrimOp> PrimOps;
diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index c44683e50..ec7d82a64 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -349,13 +349,7 @@ public:
// Value will be overridden anyways
}
- inline void mkPrimOp(PrimOp * p)
- {
- clearValue();
- internalType = tPrimOp;
- primOp = p;
- }
-
+ void mkPrimOp(PrimOp * p);
inline void mkPrimOpApp(Value * l, Value * r)
{
diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
index 7ea4d50e6..6678227fe 100644
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@@ -152,11 +152,10 @@ StorePath writeDerivation(Store & store,
/* Read string `s' from stream `str'. */
static void expect(std::istream & str, std::string_view s)
{
- char s2[s.size()];
- str.read(s2, s.size());
- std::string_view s2View { s2, s.size() };
- if (s2View != s)
- throw FormatError("expected string '%s', got '%s'", s, s2View);
+ for (auto & c : s) {
+ if (str.get() != c)
+ throw FormatError("expected string '%1%'", s);
+ }
}
diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index 516cbef83..7c7273012 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -323,9 +323,7 @@ typedef std::unordered_map<Path, std::unordered_set<std::string>> UncheckedRoots
static void readProcLink(const std::string & file, UncheckedRoots & roots)
{
- /* 64 is the starting buffer size gnu readlink uses... */
- auto bufsiz = ssize_t{64};
-try_again:
+ constexpr auto bufsiz = PATH_MAX;
char buf[bufsiz];
auto res = readlink(file.c_str(), buf, bufsiz);
if (res == -1) {
@@ -334,10 +332,7 @@ try_again:
throw SysError("reading symlink");
}
if (res == bufsiz) {
- if (SSIZE_MAX / 2 < bufsiz)
- throw Error("stupidly long symlink");
- bufsiz *= 2;
- goto try_again;
+ throw Error("overly long symlink starting with '%1%'", std::string_view(buf, bufsiz));
}
if (res > 0 && buf[0] == '/')
roots[std::string(static_cast<char *>(buf), res)]
diff --git a/tests/unit/libexpr/value/print.cc b/tests/unit/libexpr/value/print.cc
index 5e96e12ec..a4f6fc014 100644
--- a/tests/unit/libexpr/value/print.cc
+++ b/tests/unit/libexpr/value/print.cc
@@ -114,7 +114,8 @@ TEST_F(ValuePrintingTests, vLambda)
TEST_F(ValuePrintingTests, vPrimOp)
{
Value vPrimOp;
- vPrimOp.mkPrimOp(nullptr);
+ PrimOp primOp{};
+ vPrimOp.mkPrimOp(&primOp);
test(vPrimOp, "<PRIMOP>");
}
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-22 12:00:52 +0000