Merge pull request #14623 from Radvendii/exprcall-alloc-shvach

libexpr: plug ExprCall memory leak

Author: Ericson2314

src/libexpr/eval.cc

@@ -1751,9 +1751,9 @@ void ExprCall::eval(EvalState & state, Env & env, Value & v)
     // 4: about 60
     // 5: under 10
     // This excluded attrset lambdas (`{...}:`). Contributions of mixed lambdas appears insignificant at ~150 total.
-    SmallValueVector<4> vArgs(args.size());
-    for (size_t i = 0; i < args.size(); ++i)
-        vArgs[i] = args[i]->maybeThunk(state, env);
+    SmallValueVector<4> vArgs(args->size());
+    for (size_t i = 0; i < args->size(); ++i)
+        vArgs[i] = (*args)[i]->maybeThunk(state, env);
 
     state.callFunction(vFun, vArgs, v, pos);
 }

src/libexpr/include/nix/expr/nixexpr.hh

@@ -592,19 +592,22 @@ public:
 struct ExprCall : Expr
 {
     Expr * fun;
-    std::vector<Expr *> args;
+    /**
+     * args will never be null. See comment on ExprAttrs::AttrDefs below.
+     */
+    std::optional<std::pmr::vector<Expr *>> args;
     PosIdx pos;
     std::optional<PosIdx> cursedOrEndPos; // used during parsing to warn about https://github.com/NixOS/nix/issues/11118
 
-    ExprCall(const PosIdx & pos, Expr * fun, std::vector<Expr *> && args)
+    ExprCall(const PosIdx & pos, Expr * fun, std::pmr::vector<Expr *> && args)
         : fun(fun)
         , args(args)
         , pos(pos)
         , cursedOrEndPos({})
     {
     }
 
-    ExprCall(const PosIdx & pos, Expr * fun, std::vector<Expr *> && args, PosIdx && cursedOrEndPos)
+    ExprCall(const PosIdx & pos, Expr * fun, std::pmr::vector<Expr *> && args, PosIdx && cursedOrEndPos)
         : fun(fun)
         , args(args)
         , pos(pos)
@@ -836,15 +839,15 @@ public:
     // we define some calls to add explicitly so that the argument can be passed in as initializer lists
     template<class C>
     [[gnu::always_inline]]
-    C * add(const PosIdx & pos, Expr * fun, std::vector<Expr *> && args)
+    C * add(const PosIdx & pos, Expr * fun, std::pmr::vector<Expr *> && args)
         requires(std::same_as<C, ExprCall>)
     {
         return alloc.new_object<C>(pos, fun, std::move(args));
     }
 
     template<class C>
     [[gnu::always_inline]]
-    C * add(const PosIdx & pos, Expr * fun, std::vector<Expr *> && args, PosIdx && cursedOrEndPos)
+    C * add(const PosIdx & pos, Expr * fun, std::pmr::vector<Expr *> && args, PosIdx && cursedOrEndPos)
         requires(std::same_as<C, ExprCall>)
     {
         return alloc.new_object<C>(pos, fun, std::move(args), std::move(cursedOrEndPos));

src/libexpr/nixexpr.cc

@@ -191,7 +191,7 @@ void ExprCall::show(const SymbolTable & symbols, std::ostream & str) const
 {
     str << '(';
     fun->show(symbols, str);
-    for (auto e : args) {
+    for (auto e : *args) {
         str << ' ';
         e->show(symbols, str);
     }
@@ -486,11 +486,17 @@ void ExprLambda::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv>
 
 void ExprCall::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv> & env)
 {
+    // Move storage into the Exprs arena
+    {
+        auto arena = es.mem.exprs.alloc;
+        std::pmr::vector<Expr *> newArgs{std::move(*args), arena};
+        args.emplace(std::move(newArgs), arena);
+    }
     if (es.debugRepl)
         es.exprEnvs.insert(std::make_pair(this, env));
 
     fun->bindVars(es, env);
-    for (auto e : args)
+    for (auto e : *args)
         e->bindVars(es, env);
 }
 

src/libexpr/parser.y

@@ -115,7 +115,7 @@ static void setDocPosition(const LexerState & lexerState, ExprLambda * lambda, P
 
 static Expr * makeCall(Exprs & exprs, PosIdx pos, Expr * fn, Expr * arg) {
     if (auto e2 = dynamic_cast<ExprCall *>(fn)) {
-        e2->args.push_back(arg);
+        e2->args->push_back(arg);
         return fn;
     }
     return exprs.add<ExprCall>(pos, fn, {arg});
@@ -129,7 +129,7 @@ static Expr * makeCall(Exprs & exprs, PosIdx pos, Expr * fn, Expr * arg) {
 %type <Expr *> start expr expr_function expr_if expr_op
 %type <Expr *> expr_select expr_simple expr_app
 %type <Expr *> expr_pipe_from expr_pipe_into
-%type <std::vector<Expr *>> list
+%type <std::pmr::vector<Expr *>> list
 %type <ExprAttrs *> binds binds1
 %type <FormalsBuilder> formals formal_set
 %type <Formal> formal