From 4ee54339191a968991cbe89bdfb80659096421b0 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Mon, 5 Feb 2024 13:18:16 -0800 Subject: [PATCH 1/3] Add release note --- doc/manual/rl-next/forbid-nested-debuggers.md | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 doc/manual/rl-next/forbid-nested-debuggers.md diff --git a/doc/manual/rl-next/forbid-nested-debuggers.md b/doc/manual/rl-next/forbid-nested-debuggers.md new file mode 100644 index 000000000..a5924b24f --- /dev/null +++ b/doc/manual/rl-next/forbid-nested-debuggers.md @@ -0,0 +1,32 @@ +--- +synopsis: Nested debuggers are no longer supported +prs: 9920 +--- + +Previously, evaluating an expression that throws an error in the debugger would +enter a second, nested debugger: + +``` +nix-repl> builtins.throw "what" +error: what + + +Starting REPL to allow you to inspect the current state of the evaluator. + +Welcome to Nix 2.18.1. Type :? for help. + +nix-repl> +``` + +Now, it just prints the error message like `nix repl`: + +``` +nix-repl> builtins.throw "what" +error: + … while calling the 'throw' builtin + at «string»:1:1: + 1| builtins.throw "what" + | ^ + + error: what +``` From 14b0356dc5897e4acb02ff18f06c919ffcf8f146 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Fri, 2 Feb 2024 20:07:42 -0800 Subject: [PATCH 2/3] Forbid nested debuggers --- src/libcmd/repl.cc | 8 +------- src/libexpr/eval.cc | 19 +++++++++++++++++-- src/libexpr/eval.hh | 2 ++ src/libutil/fmt.hh | 2 -- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 8b83608fa..75f20d635 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -336,13 +336,7 @@ ReplExitStatus NixRepl::mainLoop() printMsg(lvlError, e.msg()); } } catch (EvalError & e) { - // in debugger mode, an EvalError should trigger another repl session. - // when that session returns the exception will land here. No need to show it again; - // show the error for this repl session instead. - if (state->debugRepl && !state->debugTraces.empty()) - showDebugTrace(std::cout, state->positions, state->debugTraces.front()); - else - printMsg(lvlError, e.msg()); + printMsg(lvlError, e.msg()); } catch (Error & e) { printMsg(lvlError, e.msg()); } catch (Interrupted & e) { diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index f2bbf20bb..722ff6908 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -762,10 +762,24 @@ std::unique_ptr mapStaticEnvBindings(const SymbolTable & st, const Stati return vm; } +/** + * Sets `inDebugger` to true on construction and false on destruction. + */ +class DebuggerGuard { + bool & inDebugger; +public: + DebuggerGuard(bool & inDebugger) : inDebugger(inDebugger) { + inDebugger = true; + } + ~DebuggerGuard() { + inDebugger = false; + } +}; + void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr & expr) { - // double check we've got the debugRepl function pointer. - if (!debugRepl) + // Make sure we have a debugger to run and we're not already in a debugger. + if (!debugRepl || inDebugger) return; auto dts = @@ -792,6 +806,7 @@ void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr & auto se = getStaticEnv(expr); if (se) { auto vm = mapStaticEnvBindings(symbols, *se.get(), env); + DebuggerGuard _guard(inDebugger); auto exitStatus = (debugRepl)(ref(shared_from_this()), *vm); switch (exitStatus) { case ReplExitStatus::QuitAll: diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 01abd4eb1..368bb17b3 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -153,6 +153,7 @@ struct DebugTrace { bool isError; }; + class EvalState : public std::enable_shared_from_this { public: @@ -222,6 +223,7 @@ public: */ ReplExitStatus (* debugRepl)(ref es, const ValMap & extraEnv); bool debugStop; + bool inDebugger = false; int trylevel; std::list debugTraces; std::map> exprEnvs; diff --git a/src/libutil/fmt.hh b/src/libutil/fmt.hh index e996f4ba2..77843f863 100644 --- a/src/libutil/fmt.hh +++ b/src/libutil/fmt.hh @@ -8,7 +8,6 @@ namespace nix { -namespace { /** * A helper for writing `boost::format` expressions. * @@ -42,7 +41,6 @@ void setExceptions(boost::format & fmt) boost::io::too_many_args_bit ^ boost::io::too_few_args_bit); } -} /** * A helper for writing a `boost::format` expression to a string. From 2e8f4faa100101e258b786494bac0996601cb4a1 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Mon, 4 Mar 2024 09:32:02 -0800 Subject: [PATCH 3/3] Fix build Not sure why that was giving a duplicate symbol error, or why marking it inline fixes it. Here it is! --- src/libutil/fmt.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/fmt.hh b/src/libutil/fmt.hh index 77843f863..abbaf95b6 100644 --- a/src/libutil/fmt.hh +++ b/src/libutil/fmt.hh @@ -34,7 +34,7 @@ inline void formatHelper(F & f, const T & x, const Args & ... args) /** * Set the correct exceptions for `fmt`. */ -void setExceptions(boost::format & fmt) +inline void setExceptions(boost::format & fmt) { fmt.exceptions( boost::io::all_error_bits ^