From 6e1e76f10f3cc900764c5f43ca7bcd97aa408278 Mon Sep 17 00:00:00 2001 From: blokyk Date: Tue, 17 Mar 2026 19:43:34 +0100 Subject: [PATCH] libcmd/repl: print error for invalid :st argument currently, the `:st ` command in the debugger will simply silently fail if the argument cannot be converted to an integer, or if it falls outside the range of valid stack indices. this isn't too big of problem, but it can be nicer to tell the user something went wrong, rather than not give them any output and having them guess (esp. in the second case). this commit adds two errors, one for each case: 1. the argument is not actually an integer, or is outside INT_MIN/MAX -> "argument '%arg' is not a valid integer" 2. the argument is an integer outside the range of stack traces -> "stack index must be between 0 and %max_frame, but was %arg" Change-Id: I8109feeede79a9ad3db9ee7dc95d37e7dd19741a --- doc/manual/change-authors.yml | 4 + doc/manual/rl-next/repl-st-errors.md | 14 ++++ lix/libcmd/repl.cc | 29 ++++++- .../data/stacktrace_invalid_arg.test | 73 ++++++++++++++++++ .../data/stacktrace_oob.test | 77 +++++++++++++++++++ .../repl_characterization.cc | 2 + 6 files changed, 195 insertions(+), 4 deletions(-) create mode 100644 doc/manual/rl-next/repl-st-errors.md create mode 100644 tests/functional/repl_characterization/data/stacktrace_invalid_arg.test create mode 100644 tests/functional/repl_characterization/data/stacktrace_oob.test diff --git a/doc/manual/change-authors.yml b/doc/manual/change-authors.yml index 084e9807e..56048e930 100644 --- a/doc/manual/change-authors.yml +++ b/doc/manual/change-authors.yml @@ -57,6 +57,10 @@ blitz: display_name: Julian Stecklina github: blitz +blokyk: + display_name: blokyk + github: blokyk + cole-h: display_name: Cole Helbling github: cole-h diff --git a/doc/manual/rl-next/repl-st-errors.md b/doc/manual/rl-next/repl-st-errors.md new file mode 100644 index 000000000..9549927c5 --- /dev/null +++ b/doc/manual/rl-next/repl-st-errors.md @@ -0,0 +1,14 @@ +--- +synopsis: "invalid arguments to :st now print an error" +cls: [5386] +category: "Improvements" +credits: [blokyk] +--- + +When using the debugger, the `:st` command used to traverse the call stack would +silently fail and put the debugger in an invalid state if the argument given to +it wasn't a valid stack frame index. + +This change adds an error message warning the user if the given index wasn't a +valid frame (telling them the range of valid indices), as well as if it wasn't +even a valid integer to begin with. diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index 50ff1ddeb..0194dc6ca 100644 --- a/lix/libcmd/repl.cc +++ b/lix/libcmd/repl.cc @@ -712,14 +712,21 @@ void NixRepl::initDebugBuiltinCommands() addCommand( "show-trace", [](NixRepl & repl, const std::string & arg) { - try { - repl.debugTraceIndex = stoi(arg); - } catch (...) { + int requestedTraceIdx = repl.debugTraceIndex; + if (arg.length() != 0) { + std::optional maybeIdx = string2Int(arg); + if (!maybeIdx) { + throw Error("argument '%s' is not a valid integer", arg); + } + requestedTraceIdx = maybeIdx.value(); } + size_t traceCount = 0; auto traces = repl.evaluator.debug->traces(); for (const auto & [idx, i] : enumerate(traces)) { - if (idx == repl.debugTraceIndex) { + traceCount++; + if (idx == (size_t) requestedTraceIdx) { + repl.debugTraceIndex = requestedTraceIdx; std::cout << "\n" << ANSI_BLUE << idx << ANSI_NORMAL << ": "; showDebugTrace(std::cout, repl.evaluator.positions, *i); std::cout << std::endl; @@ -728,6 +735,20 @@ void NixRepl::initDebugBuiltinCommands() break; } } + + // if we didn't find any trace matching the user's request + if (repl.debugTraceIndex != (size_t) requestedTraceIdx) { + // note: if we get here, then the loop ran fully without matching anything, + // so `traceCount` is the total number of traces + throw Error( + "stack index must be between %ld and %ld (inclusive), but was %ld", + 0, + // decrease it by one to get the max *index*, since stacks are indexed by 0 + traceCount - 1, + requestedTraceIdx + ); + } + return ProcessLineResult::PromptAgain; }, {.aliases = {"st"}, diff --git a/tests/functional/repl_characterization/data/stacktrace_invalid_arg.test b/tests/functional/repl_characterization/data/stacktrace_invalid_arg.test new file mode 100644 index 000000000..39c3350e2 --- /dev/null +++ b/tests/functional/repl_characterization/data/stacktrace_invalid_arg.test @@ -0,0 +1,73 @@ +@args --debugger + nix-repl> throw "(forever?????????)" + error: (forever?????????) + +argument-less :st works fine + nix-repl> :st + + 0: error: (forever?????????) + «string»:1:1 + + 1| throw "(forever?????????)" + | ^ + + Env level 0 + static: + + Env level 1 + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true + +a non-numeric strings produces an error + nix-repl> :st chat + error: argument 'chat' is not a valid integer + + nix-repl> :st bedroom community + error: argument 'bedroom community' is not a valid integer + +...even when they start with a digit + nix-repl> :st 6up + error: argument '6up' is not a valid integer + +...or when they're floats + nix-repl> :st 4.50 + error: argument '4.50' is not a valid integer + +an integer outside the range produces an error + nix-repl> :st 23571113171923 + error: argument '23571113171923' is not a valid integer + +argument-less :st is still at the same index after errors + nix-repl> :st 1 + + 1: while calling a function + «string»:1:1 + + 1| throw "(forever?????????)" + | ^ + + Env level 0 + static: + + Env level 1 + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true + + nix-repl> :st foo + error: argument 'foo' is not a valid integer + + nix-repl> :st + + 1: while calling a function + «string»:1:1 + + 1| throw "(forever?????????)" + | ^ + + Env level 0 + static: + + Env level 1 + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true + +quit + nix-repl> :quit + error: (forever?????????) diff --git a/tests/functional/repl_characterization/data/stacktrace_oob.test b/tests/functional/repl_characterization/data/stacktrace_oob.test new file mode 100644 index 000000000..a282ea8e0 --- /dev/null +++ b/tests/functional/repl_characterization/data/stacktrace_oob.test @@ -0,0 +1,77 @@ +@args --debugger + nix-repl> let f = _: throw "x_x"; x = f 5; in x + error: x_x + +frames from 0 up to 4 work fine + nix-repl> :st 0 + + 0: error: x_x + «string»:1:12 + + 1| let f = _: throw "x_x"; x = f 5; in x + | ^ + + Env level 0 + static: _ + + Env level 1 + static: f x + + Env level 2 + static: + + Env level 3 + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true + + nix-repl> :st 4 + + 4: while evaluating a 'let' expression + «string»:1:1 + + 1| let f = _: throw "x_x"; x = f 5; in x + | ^ + + Env level 0 + static: f x + + Env level 1 + static: + + Env level 2 + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true + +negative frames print an error + nix-repl> :st -1 + error: stack index must be between 0 and 4, but was -1 + + nix-repl> :st -100 + error: stack index must be between 0 and 4, but was -100 + +positives frames out of bounds also print an error + nix-repl> :st 5 + error: stack index must be between 0 and 4, but was 5 + + nix-repl> :st 100 + error: stack index must be between 0 and 4, but was 100 + +argument-less :st is still at the same after oob + nix-repl> :st + + 4: while evaluating a 'let' expression + «string»:1:1 + + 1| let f = _: throw "x_x"; x = f 5; in x + | ^ + + Env level 0 + static: f x + + Env level 1 + static: + + Env level 2 + abort baseNameOf break builtins derivation derivationStrict dirOf false fetchGit fetchMercurial fetchTarball fetchTree fromTOML import isNull map null placeholder removeAttrs scopedImport throw toString true + +quit + nix-repl> :quit + error: x_x diff --git a/tests/functional/repl_characterization/repl_characterization.cc b/tests/functional/repl_characterization/repl_characterization.cc index 2ba529b7b..f81367dad 100644 --- a/tests/functional/repl_characterization/repl_characterization.cc +++ b/tests/functional/repl_characterization/repl_characterization.cc @@ -197,5 +197,7 @@ REPL_TEST(idempotent); REPL_TEST(debug_frames); REPL_TEST(debug_ignore_try); REPL_TEST(debug_ignore_try_defaults); +REPL_TEST(stacktrace_invalid_arg); +REPL_TEST(stacktrace_oob); }; // namespace nix