Merge pull request #14624 from roberth/deepSeq-stack-overflow

Fix most remaining stack overflows

src/libexpr/eval-error.cc

@@ -111,6 +111,7 @@ template class EvalErrorBuilder<TypeError>;
 template class EvalErrorBuilder<UndefinedVarError>;
 template class EvalErrorBuilder<MissingArgumentError>;
 template class EvalErrorBuilder<InfiniteRecursionError>;
+template class EvalErrorBuilder<StackOverflowError>;
 template class EvalErrorBuilder<InvalidPathError>;
 template class EvalErrorBuilder<IFDError>;
 

src/libexpr/eval.cc

@@ -2408,6 +2408,8 @@ BackedStringView EvalState::coerceToString(
     bool copyToStore,
     bool canonicalizePath)
 {
+    auto _level = addCallDepth(pos);
+
     forceValue(v, pos);
 
     if (v.type() == nString) {
@@ -2627,6 +2629,8 @@ SingleDerivedPath EvalState::coerceToSingleDerivedPath(const PosIdx pos, Value &
 // `assert a == b; x` are critical for our users' testing UX.
 void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::string_view errorCtx)
 {
+    auto _level = addCallDepth(pos);
+
     // This implementation must match eqValues.
     forceValue(v1, pos);
     forceValue(v2, pos);
@@ -2833,6 +2837,8 @@ void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::st
 // This implementation must match assertEqValues
 bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_view errorCtx)
 {
+    auto _level = addCallDepth(pos);
+
     forceValue(v1, pos);
     forceValue(v2, pos);
 

src/libexpr/get-drvs.cc

@@ -213,6 +213,8 @@ StringSet PackageInfo::queryMetaNames()
 
 bool PackageInfo::checkMeta(Value & v)
 {
+    auto _level = state->addCallDepth(v.determinePos(noPos));
+
     state->forceValue(v, v.determinePos(noPos));
     if (v.type() == nList) {
         for (auto elem : v.listView())
@@ -397,6 +399,8 @@ static void getDerivations(
     Done & done,
     bool ignoreAssertionFailures)
 {
+    auto _level = state.addCallDepth(vIn.determinePos(noPos));
+
     Value v;
     state.autoCallFunction(autoArgs, vIn, v);
 

src/libexpr/include/nix/expr/eval-error.hh

@@ -54,6 +54,20 @@ MakeError(TypeError, EvalError);
 MakeError(UndefinedVarError, EvalError);
 MakeError(MissingArgumentError, EvalError);
 MakeError(InfiniteRecursionError, EvalError);
+
+/**
+ * Resource exhaustion error when evaluation exceeds max-call-depth.
+ * Inherits from EvalBaseError (not EvalError) because resource exhaustion
+ * should not be cached.
+ */
+struct StackOverflowError : public EvalBaseError
+{
+    StackOverflowError(EvalState & state)
+        : EvalBaseError(state, "stack overflow; max-call-depth exceeded")
+    {
+    }
+};
+
 MakeError(IFDError, EvalBaseError);
 
 struct InvalidPathError : public EvalError

src/libexpr/include/nix/expr/eval-inline.hh

@@ -139,7 +139,7 @@ inline void EvalState::forceList(Value & v, const PosIdx pos, std::string_view e
 inline CallDepth EvalState::addCallDepth(const PosIdx pos)
 {
     if (callDepth > settings.maxCallDepth)
-        error<EvalBaseError>("stack overflow; max-call-depth exceeded").atPos(pos).debugThrow();
+        error<StackOverflowError>().atPos(pos).debugThrow();
 
     return CallDepth(callDepth);
 };

src/libexpr/include/nix/expr/print-ambiguous.hh

@@ -5,6 +5,8 @@
 
 namespace nix {
 
+class EvalState;
+
 /**
  * Print a value in the deprecated format used by `nix-instantiate --eval` and
  * `nix-env` (for manifests).
@@ -15,7 +17,6 @@ namespace nix {
  *
  * See: https://github.com/NixOS/nix/issues/9730
  */
-void printAmbiguous(
-    Value & v, const SymbolTable & symbols, std::ostream & str, std::set<const void *> * seen, int depth);
+void printAmbiguous(EvalState & state, Value & v, std::ostream & str, std::set<const void *> * seen, size_t depth = 0);
 
 } // namespace nix

src/libexpr/print-ambiguous.cc

@@ -2,19 +2,17 @@
 #include "nix/expr/print.hh"
 #include "nix/util/signals.hh"
 #include "nix/expr/eval.hh"
+#include "nix/expr/eval-error.hh"
 
 namespace nix {
 
 // See: https://github.com/NixOS/nix/issues/9730
-void printAmbiguous(
-    Value & v, const SymbolTable & symbols, std::ostream & str, std::set<const void *> * seen, int depth)
+void printAmbiguous(EvalState & state, Value & v, std::ostream & str, std::set<const void *> * seen, size_t depth)
 {
     checkInterrupt();
 
-    if (depth <= 0) {
-        str << "«too deep»";
-        return;
-    }
+    if (depth > state.settings.maxCallDepth)
+        state.error<StackOverflowError>().atPos(v.determinePos(noPos)).debugThrow();
     switch (v.type()) {
     case nInt:
         str << v.integer();
@@ -36,9 +34,9 @@ void printAmbiguous(
             str << "«repeated»";
         else {
             str << "{ ";
-            for (auto & i : v.attrs()->lexicographicOrder(symbols)) {
-                str << symbols[i->name] << " = ";
-                printAmbiguous(*i->value, symbols, str, seen, depth - 1);
+            for (auto & i : v.attrs()->lexicographicOrder(state.symbols)) {
+                str << state.symbols[i->name] << " = ";
+                printAmbiguous(state, *i->value, str, seen, depth + 1);
                 str << "; ";
             }
             str << "}";
@@ -54,7 +52,7 @@ void printAmbiguous(
             str << "[ ";
             for (auto v2 : v.listView()) {
                 if (v2)
-                    printAmbiguous(*v2, symbols, str, seen, depth - 1);
+                    printAmbiguous(state, *v2, str, seen, depth + 1);
                 else
                     str << "(nullptr)";
                 str << " ";

src/libexpr/print.cc

@@ -537,6 +537,16 @@ private:
         output.flush();
         checkInterrupt();
 
+        // Catch infinite recursion before it overflows the C++ stack.
+        // Non-cyclic structures can be infinitely deep when values are
+        // lazily produced (e.g., `let f = n: { inner = f (n + 1); }; in f 0`).
+        // We check print depth against max-call-depth rather than incrementing
+        // the callDepth counter, because accessing an attribute is not a call.
+        // Other places do increment callDepth for simplicity, but that is
+        // technically incorrect.
+        if (depth > state.settings.maxCallDepth)
+            state.error<StackOverflowError>().atPos(v.determinePos(noPos)).debugThrow();
+
         try {
             if (options.force) {
                 state.forceValue(v, v.determinePos(noPos));
@@ -592,6 +602,11 @@ private:
                 printUnknown();
                 break;
             }
+        } catch (StackOverflowError &) {
+            // Always re-throw because stack overflow is a serious condition
+            // that expressions should avoid, unlike say `throw`, which can
+            // be part of legitimate expression patterns.
+            throw;
         } catch (Error & e) {
             if (options.errors == ErrorPrintBehavior::Throw
                 || (options.errors == ErrorPrintBehavior::ThrowTopLevel && depth == 0)) {

src/libexpr/value-to-xml.cc

@@ -66,6 +66,8 @@ static void printValueAsXML(
 {
     checkInterrupt();
 
+    auto _level = state.addCallDepth(pos);
+
     if (strict)
         state.forceValue(v, pos);
 

src/nix/nix-env/user-env.cc

@@ -108,7 +108,7 @@ bool createUserEnv(
        environment. */
     auto manifestFile = ({
         std::ostringstream str;
-        printAmbiguous(manifest, state.symbols, str, nullptr, std::numeric_limits<int>::max());
+        printAmbiguous(state, manifest, str, nullptr);
         StringSource source{str.view()};
         state.store->addToStoreFromDump(
             source,

src/nix/nix-instantiate/nix-instantiate.cc

@@ -68,7 +68,7 @@ void processExpr(
                 if (strict)
                     state.forceValueDeep(vRes);
                 std::set<const void *> seen;
-                printAmbiguous(vRes, state.symbols, std::cout, &seen, std::numeric_limits<int>::max());
+                printAmbiguous(state, vRes, std::cout, &seen);
                 std::cout << std::endl;
             }
         } else {

tests/functional/eval.sh

@@ -39,6 +39,30 @@ nix-instantiate --eval -E 'assert 1 + 2 == 3; true'
 ln -sfn cycle.nix "$TEST_ROOT/cycle.nix"
 (! nix eval --file "$TEST_ROOT/cycle.nix")
 
+# Test that printing deep data structures produces a controlled error.
+# The expression creates a non-cyclic but infinitely deep structure:
+# f returns immediately with a thunk, so Nix call depth stays at 1,
+# but Printer::print recurses on the C++ stack.
+expectStderr 1 nix eval --expr 'let f = n: { inner = f (n + 1); }; in f 0' \
+  | grepQuiet "stack overflow; max-call-depth exceeded"
+
+# Same for builtins.toXML
+expectStderr 1 nix eval --expr 'builtins.toXML (let f = n: { inner = f (n + 1); }; in f 0)' \
+  | grepQuiet "stack overflow; max-call-depth exceeded"
+
+# Same for equality comparison (n is not observable, so structures are equal)
+expectStderr 1 nix eval --expr 'let f = n: { inner = f (n + 1); }; in f 0 == f 1' \
+  | grepQuiet "stack overflow; max-call-depth exceeded"
+
+# Same for assert with equality (uses assertEqValues)
+expectStderr 1 nix eval --expr 'let f = n: { inner = f (n + 1); }; in assert f 0 == f 1; true' \
+  | grepQuiet "stack overflow; max-call-depth exceeded"
+
+# Same for string coercion with __toString
+# shellcheck disable=SC2016
+expectStderr 1 nix eval --expr 'let f = n: { __toString = _: f (n + 1); }; in "${f 0}"' \
+  | grepQuiet "stack overflow; max-call-depth exceeded"
+
 # --file and --pure-eval don't mix.
 expectStderr 1 nix eval --pure-eval --file "$TEST_ROOT/cycle.nix" | grepQuiet "not compatible"
 

tests/functional/simple.sh

@@ -35,3 +35,24 @@ if test "$outPath" != "/foo/xxiwa5zlaajv6xdjynf9yym9g319d6mn-big-derivation-attr
     echo "big-derivation-attr.nix hash appears broken, got $outPath. Memory corruption in large drv attr?"
     exit 1
 fi
+
+# Test that nix-instantiate on a deeply nested recurseForDerivations structure
+# produces a controlled stack overflow error rather than a segfault.
+expectStderr 1 nix-instantiate --expr 'let x = { recurseForDerivations = true; more = x; }; in x' \
+  | grepQuiet "stack overflow; max-call-depth exceeded"
+
+# Test that nix-env -qa --meta on deeply nested meta attributes produces a
+# controlled stack overflow error rather than a segfault.
+echo 'let f = n: { type = "derivation"; name = "test"; system = "x86_64-linux"; meta.nested = f (n + 1); }; in { pkg = f 0; }' > "$TEST_ROOT/deep-meta.nix"
+expectStderr 1 nix-env -qa -f "$TEST_ROOT/deep-meta.nix" --json --meta \
+  | grepQuiet "stack overflow; max-call-depth exceeded"
+
+# Test that nix-instantiate --eval on a pre-forced deep structure (built with
+# foldl' to avoid thunks) produces a controlled stack overflow error rather than
+# a segfault when printAmbiguous traverses the structure.
+# Note: Without the fix, this test may pass if the system stack is large enough.
+# The fix ensures we get a controlled error at max-call-depth (default 10000)
+# rather than relying on the system stack limit.
+# shellcheck disable=SC2016
+expectStderr 1 nix-instantiate --eval --expr 'builtins.foldl'\'' (acc: _: { inner = acc; }) null (builtins.genList (x: x) 20000)' \
+  | grepQuiet "stack overflow; max-call-depth exceeded"