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