diff --git a/src/libexpr/eval-error.cc b/src/libexpr/eval-error.cc index 7f01747158c..5da804e7ac4 100644 --- a/src/libexpr/eval-error.cc +++ b/src/libexpr/eval-error.cc @@ -111,6 +111,7 @@ template class EvalErrorBuilder; template class EvalErrorBuilder; template class EvalErrorBuilder; template class EvalErrorBuilder; +template class EvalErrorBuilder; template class EvalErrorBuilder; template class EvalErrorBuilder; diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 0a6b199bf3f..cd8e9890b37 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2403,6 +2403,8 @@ BackedStringView EvalState::coerceToString( bool copyToStore, bool canonicalizePath) { + auto _level = addCallDepth(pos); + forceValue(v, pos); if (v.type() == nString) { @@ -2622,6 +2624,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); @@ -2828,6 +2832,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); diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index c4a2b00af3e..2a7b90057a7 100644 --- a/src/libexpr/get-drvs.cc +++ b/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()) @@ -378,6 +380,8 @@ static void getDerivations( Done & done, bool ignoreAssertionFailures) { + auto _level = state.addCallDepth(vIn.determinePos(noPos)); + Value v; state.autoCallFunction(autoArgs, vIn, v); diff --git a/src/libexpr/include/nix/expr/eval-error.hh b/src/libexpr/include/nix/expr/eval-error.hh index 38db9b7069e..06a7790d5e4 100644 --- a/src/libexpr/include/nix/expr/eval-error.hh +++ b/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 diff --git a/src/libexpr/include/nix/expr/eval-inline.hh b/src/libexpr/include/nix/expr/eval-inline.hh index e8aa380fdb0..561c07f0891 100644 --- a/src/libexpr/include/nix/expr/eval-inline.hh +++ b/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("stack overflow; max-call-depth exceeded").atPos(pos).debugThrow(); + error().atPos(pos).debugThrow(); return CallDepth(callDepth); }; diff --git a/src/libexpr/include/nix/expr/print-ambiguous.hh b/src/libexpr/include/nix/expr/print-ambiguous.hh index c0d811d4b93..7e44a6b66eb 100644 --- a/src/libexpr/include/nix/expr/print-ambiguous.hh +++ b/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 * seen, int depth); +void printAmbiguous(EvalState & state, Value & v, std::ostream & str, std::set * seen, size_t depth = 0); } // namespace nix diff --git a/src/libexpr/print-ambiguous.cc b/src/libexpr/print-ambiguous.cc index 8b80e2a6634..cb1a1ef2d3a 100644 --- a/src/libexpr/print-ambiguous.cc +++ b/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 * seen, int depth) +void printAmbiguous(EvalState & state, Value & v, std::ostream & str, std::set * seen, size_t depth) { checkInterrupt(); - if (depth <= 0) { - str << "«too deep»"; - return; - } + if (depth > state.settings.maxCallDepth) + state.error().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 << " "; diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index 4776be03385..066426ec864 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -537,6 +537,16 @@ class Printer 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().atPos(v.determinePos(noPos)).debugThrow(); + try { if (options.force) { state.forceValue(v, v.determinePos(noPos)); @@ -592,6 +602,11 @@ class Printer 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)) { diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index 0a7a334f41b..2fe24857ce5 100644 --- a/src/libexpr/value-to-xml.cc +++ b/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); diff --git a/src/nix/nix-env/user-env.cc b/src/nix/nix-env/user-env.cc index 21fdf25bc55..6275e37a284 100644 --- a/src/nix/nix-env/user-env.cc +++ b/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::max()); + printAmbiguous(state, manifest, str, nullptr); StringSource source{str.view()}; state.store->addToStoreFromDump( source, diff --git a/src/nix/nix-instantiate/nix-instantiate.cc b/src/nix/nix-instantiate/nix-instantiate.cc index 3d5c3e26a46..e6d7c9186f9 100644 --- a/src/nix/nix-instantiate/nix-instantiate.cc +++ b/src/nix/nix-instantiate/nix-instantiate.cc @@ -68,7 +68,7 @@ void processExpr( if (strict) state.forceValueDeep(vRes); std::set seen; - printAmbiguous(vRes, state.symbols, std::cout, &seen, std::numeric_limits::max()); + printAmbiguous(state, vRes, std::cout, &seen); std::cout << std::endl; } } else { diff --git a/tests/functional/eval.sh b/tests/functional/eval.sh index f876f5ac483..6c19e22c977 100755 --- a/tests/functional/eval.sh +++ b/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" diff --git a/tests/functional/simple.sh b/tests/functional/simple.sh index c1f2eef411e..f6507d74182 100755 --- a/tests/functional/simple.sh +++ b/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"