From 5fa27057b2e257753439dff525ab3bbfd84fa05b Mon Sep 17 00:00:00 2001
From: Dusk Banks <me@bb010g.com>
Date: Wed, 29 Jan 2025 03:16:58 -0800
Subject: [PATCH] libexpr: fix `--debugger --ignore-try`

a65e9e58282f3809abcbc4c264db7256ef8874e2 did not inform `tryEval` that
(as far as it's concerned) `state.debug` moved to `state.errors.debug`
and changed types. this resulted in the REPL erroneously coming up, that
REPL having a non-debug state, and segfaulting after that REPL exited.

it's probably good that `state.debug` isn't mutated by `--ignore-try`
anymore.

Change-Id: I1918e93edacd626452aa423fc2eb825080738835
Fixes: a65e9e58282f ("libexpr: extract eval error creation into new type")
Signed-off-by: Dusk Banks <me@bb010g.com>
(cherry picked from commit 6a583136b7bde900b158d78606b9fe5f623ced18)
---
 doc/manual/rl-next/libexpr-fix-ignore-try.md  |  9 ++++
 lix/libexpr/primops.cc                        | 45 ++++++++++---------
 .../data/debug_ignore_try.test                |  8 ++++
 .../repl_characterization.cc                  |  1 +
 4 files changed, 43 insertions(+), 20 deletions(-)
 create mode 100644 doc/manual/rl-next/libexpr-fix-ignore-try.md
 create mode 100644 tests/functional/repl_characterization/data/debug_ignore_try.test

diff --git a/doc/manual/rl-next/libexpr-fix-ignore-try.md b/doc/manual/rl-next/libexpr-fix-ignore-try.md
new file mode 100644
index 000000000..a7935c3a0
--- /dev/null
+++ b/doc/manual/rl-next/libexpr-fix-ignore-try.md
@@ -0,0 +1,9 @@
+---
+synopsis: "Fix `--debugger --ignore-try`"
+issues: []
+cls: [2440]
+category: "Fixes"
+credits: ["bb010g"]
+---
+
+When in debug mode (e.g. from using the `--debugger` flag), enabling [`ignore-try`](@docroot@/command-ref/conf-file.md#conf-ignore-try) once again properly disables debug REPLs within [`builtins.tryEval`](@docroot@/language/builtins.md#builtins-tryEval) calls. Previously, a debug REPL would be started as if `ignore-try` was disabled, but that REPL wouldn't actually be in debug mode, and upon exiting the REPL the evaluating process would segfault.
diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc
index cfbf86ced..17b9e9d58 100644
--- a/lix/libexpr/primops.cc
+++ b/lix/libexpr/primops.cc
@@ -10,6 +10,7 @@
 #include "lix/libexpr/json-to-value.hh"
 #include "lix/libstore/names.hh"
 #include "lix/libstore/path-references.hh"
+#include "lix/libutil/finally.hh"
 #include "lix/libutil/processes.hh"
 #include "lix/libstore/store-api.hh"
 #include "lix/libexpr/value-to-json.hh"
@@ -640,29 +641,33 @@ static void prim_tryEval(EvalState & state, const PosIdx pos, Value * * args, Va
 {
     auto attrs = state.ctx.buildBindings(2);
 
-    std::optional<MaintainCount<int>> trylevel;
-    std::unique_ptr<DebugState> savedDebug;
-    if (state.ctx.debug) {
-        trylevel.emplace(state.ctx.debug->trylevel);
-        if (evalSettings.ignoreExceptionsDuringTry) {
-            /* to prevent starting the repl from exceptions withing a tryEval, null it. */
-            savedDebug = std::move(state.ctx.debug);
+    {
+        std::optional<MaintainCount<int>> trylevel;
+        DebugState * savedDebug = nullptr;
+        Finally resetDebug([&] {
+            if (savedDebug) {
+                state.ctx.errors.debug = savedDebug;
+            }
+        });
+        if (state.ctx.errors.debug != nullptr) {
+            trylevel.emplace(state.ctx.errors.debug->trylevel);
+            if (evalSettings.ignoreExceptionsDuringTry) {
+                /* to prevent starting the repl from exceptions within a tryEval, null it. */
+                savedDebug = state.ctx.errors.debug;
+                state.ctx.errors.debug = nullptr;
+            }
+        }
+
+        try {
+            state.forceValue(*args[0], pos);
+            attrs.insert(state.ctx.s.value, args[0]);
+            attrs.alloc("success").mkBool(true);
+        } catch (AssertionError & e) {
+            attrs.alloc(state.ctx.s.value).mkBool(false);
+            attrs.alloc("success").mkBool(false);
         }
     }
 
-    try {
-        state.forceValue(*args[0], pos);
-        attrs.insert(state.ctx.s.value, args[0]);
-        attrs.alloc("success").mkBool(true);
-    } catch (AssertionError & e) {
-        attrs.alloc(state.ctx.s.value).mkBool(false);
-        attrs.alloc("success").mkBool(false);
-    }
-
-    // restore the debugRepl pointer if we saved it earlier.
-    if (savedDebug)
-        state.ctx.debug = std::move(savedDebug);
-
     v.mkAttrs(attrs);
 }
 
diff --git a/tests/functional/repl_characterization/data/debug_ignore_try.test b/tests/functional/repl_characterization/data/debug_ignore_try.test
new file mode 100644
index 000000000..2c326a401
--- /dev/null
+++ b/tests/functional/repl_characterization/data/debug_ignore_try.test
@@ -0,0 +1,8 @@
+@args --debugger --ignore-try
+
+we don't enter a debug repl through tryEval
+  nix-repl> (builtins.tryEval ((x: throw "foo") 1)).success
+  false
+
+no segfault either
+  nix-repl> :quit
diff --git a/tests/functional/repl_characterization/repl_characterization.cc b/tests/functional/repl_characterization/repl_characterization.cc
index 7e33ecec0..eccaeaf8e 100644
--- a/tests/functional/repl_characterization/repl_characterization.cc
+++ b/tests/functional/repl_characterization/repl_characterization.cc
@@ -187,5 +187,6 @@ REPL_TEST(stack_vars);
 REPL_TEST(errors);
 REPL_TEST(idempotent);
 REPL_TEST(debug_frames);
+REPL_TEST(debug_ignore_try);
 
 }; // namespace nix