From 3600f9fa9f92bd6609c40950578b5fe5d2cf9879 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Fri, 24 Mar 2023 10:38:56 -0700 Subject: [PATCH 1/2] Fix backtick handling in parsed commands. https://reviews.llvm.org/D146779 (cherry picked from commit 75ca15fcbb2e1b3671e41f971a000c6d59f5e5ae) --- .../lldb/Interpreter/CommandInterpreter.h | 5 +- .../source/Interpreter/CommandInterpreter.cpp | 208 +++++++++--------- lldb/source/Interpreter/CommandObject.cpp | 12 +- .../command/backticks/TestBackticksInAlias.py | 49 +++++ 4 files changed, 170 insertions(+), 104 deletions(-) diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h index 091a384af0f8e..9e03f51e75de2 100644 --- a/lldb/include/lldb/Interpreter/CommandInterpreter.h +++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h @@ -636,6 +636,9 @@ class CommandInterpreter : public Broadcaster, bool IOHandlerInterrupt(IOHandler &io_handler) override; + Status PreprocessCommand(std::string &command); + Status PreprocessToken(std::string &token); + protected: friend class Debugger; @@ -670,8 +673,6 @@ class CommandInterpreter : public Broadcaster, void RestoreExecutionContext(); - Status PreprocessCommand(std::string &command); - void SourceInitFile(FileSpec file, CommandReturnObject &result); // Completely resolves aliases and abbreviations, returning a pointer to the diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 8c5b359417510..3382fc6b31d3f 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -1761,112 +1761,124 @@ Status CommandInterpreter::PreprocessCommand(std::string &command) { std::string expr_str(command, expr_content_start, end_backtick - expr_content_start); + error = PreprocessToken(expr_str); + // We always stop at the first error: + if (error.Fail()) + break; - ExecutionContext exe_ctx(GetExecutionContext()); + command.erase(start_backtick, end_backtick - start_backtick + 1); + command.insert(start_backtick, std::string(expr_str)); + pos = start_backtick + expr_str.size(); + } + return error; +} - // Get a dummy target to allow for calculator mode while processing - // backticks. This also helps break the infinite loop caused when target is - // null. - Target *exe_target = exe_ctx.GetTargetPtr(); - Target &target = exe_target ? *exe_target : m_debugger.GetDummyTarget(); - - ValueObjectSP expr_result_valobj_sp; - - EvaluateExpressionOptions options; - options.SetCoerceToId(false); - options.SetUnwindOnError(true); - options.SetIgnoreBreakpoints(true); - options.SetKeepInMemory(false); - options.SetTryAllThreads(true); - options.SetTimeout(llvm::None); - - ExpressionResults expr_result = - target.EvaluateExpression(expr_str.c_str(), exe_ctx.GetFramePtr(), - expr_result_valobj_sp, options); - - if (expr_result == eExpressionCompleted) { - Scalar scalar; - if (expr_result_valobj_sp) - expr_result_valobj_sp = - expr_result_valobj_sp->GetQualifiedRepresentationIfAvailable( - expr_result_valobj_sp->GetDynamicValueType(), true); - if (expr_result_valobj_sp->ResolveValue(scalar)) { - command.erase(start_backtick, end_backtick - start_backtick + 1); - StreamString value_strm; - const bool show_type = false; - scalar.GetValue(&value_strm, show_type); - size_t value_string_size = value_strm.GetSize(); - if (value_string_size) { - command.insert(start_backtick, std::string(value_strm.GetString())); - pos = start_backtick + value_string_size; - continue; - } else { - error.SetErrorStringWithFormat("expression value didn't result " - "in a scalar value for the " - "expression '%s'", - expr_str.c_str()); - break; - } - } else { - error.SetErrorStringWithFormat("expression value didn't result " - "in a scalar value for the " - "expression '%s'", - expr_str.c_str()); - break; - } +Status +CommandInterpreter::PreprocessToken(std::string &expr_str) { + Status error; + ExecutionContext exe_ctx(GetExecutionContext()); - continue; - } + // Get a dummy target to allow for calculator mode while processing + // backticks. This also helps break the infinite loop caused when target is + // null. + Target *exe_target = exe_ctx.GetTargetPtr(); + Target &target = exe_target ? *exe_target : m_debugger.GetDummyTarget(); - if (expr_result_valobj_sp) - error = expr_result_valobj_sp->GetError(); + ValueObjectSP expr_result_valobj_sp; - if (error.Success()) { - switch (expr_result) { - case eExpressionSetupError: - error.SetErrorStringWithFormat( - "expression setup error for the expression '%s'", expr_str.c_str()); - break; - case eExpressionParseError: - error.SetErrorStringWithFormat( - "expression parse error for the expression '%s'", expr_str.c_str()); - break; - case eExpressionResultUnavailable: - error.SetErrorStringWithFormat( - "expression error fetching result for the expression '%s'", - expr_str.c_str()); - break; - case eExpressionCompleted: - break; - case eExpressionDiscarded: - error.SetErrorStringWithFormat( - "expression discarded for the expression '%s'", expr_str.c_str()); - break; - case eExpressionInterrupted: - error.SetErrorStringWithFormat( - "expression interrupted for the expression '%s'", expr_str.c_str()); - break; - case eExpressionHitBreakpoint: - error.SetErrorStringWithFormat( - "expression hit breakpoint for the expression '%s'", - expr_str.c_str()); - break; - case eExpressionTimedOut: - error.SetErrorStringWithFormat( - "expression timed out for the expression '%s'", expr_str.c_str()); - break; - case eExpressionStoppedForDebug: - error.SetErrorStringWithFormat("expression stop at entry point " - "for debugging for the " + EvaluateExpressionOptions options; + options.SetCoerceToId(false); + options.SetUnwindOnError(true); + options.SetIgnoreBreakpoints(true); + options.SetKeepInMemory(false); + options.SetTryAllThreads(true); + options.SetTimeout(std::nullopt); + + ExpressionResults expr_result = + target.EvaluateExpression(expr_str.c_str(), exe_ctx.GetFramePtr(), + expr_result_valobj_sp, options); + + if (expr_result == eExpressionCompleted) { + Scalar scalar; + if (expr_result_valobj_sp) + expr_result_valobj_sp = + expr_result_valobj_sp->GetQualifiedRepresentationIfAvailable( + expr_result_valobj_sp->GetDynamicValueType(), true); + if (expr_result_valobj_sp->ResolveValue(scalar)) { + + StreamString value_strm; + const bool show_type = false; + scalar.GetValue(&value_strm, show_type); + size_t value_string_size = value_strm.GetSize(); + if (value_string_size) { + expr_str = value_strm.GetData(); + } else { + error.SetErrorStringWithFormat("expression value didn't result " + "in a scalar value for the " "expression '%s'", expr_str.c_str()); - break; - case eExpressionThreadVanished: - error.SetErrorStringWithFormat( - "expression thread vanished for the expression '%s'", - expr_str.c_str()); - break; } + } else { + error.SetErrorStringWithFormat("expression value didn't result " + "in a scalar value for the " + "expression '%s'", + expr_str.c_str()); + } + return error; + } + + // If we have an error from the expression evaluation it will be in the + // ValueObject error, which won't be success and we will just report it. + // But if for some reason we didn't get a value object at all, then we will + // make up some helpful errors from the expression result. + if (expr_result_valobj_sp) + error = expr_result_valobj_sp->GetError(); + + if (error.Success()) { + switch (expr_result) { + case eExpressionSetupError: + error.SetErrorStringWithFormat( + "expression setup error for the expression '%s'", expr_str.c_str()); + break; + case eExpressionParseError: + error.SetErrorStringWithFormat( + "expression parse error for the expression '%s'", expr_str.c_str()); + break; + case eExpressionResultUnavailable: + error.SetErrorStringWithFormat( + "expression error fetching result for the expression '%s'", + expr_str.c_str()); + break; + case eExpressionCompleted: + break; + case eExpressionDiscarded: + error.SetErrorStringWithFormat( + "expression discarded for the expression '%s'", expr_str.c_str()); + break; + case eExpressionInterrupted: + error.SetErrorStringWithFormat( + "expression interrupted for the expression '%s'", expr_str.c_str()); + break; + case eExpressionHitBreakpoint: + error.SetErrorStringWithFormat( + "expression hit breakpoint for the expression '%s'", + expr_str.c_str()); + break; + case eExpressionTimedOut: + error.SetErrorStringWithFormat( + "expression timed out for the expression '%s'", expr_str.c_str()); + break; + case eExpressionStoppedForDebug: + error.SetErrorStringWithFormat("expression stop at entry point " + "for debugging for the " + "expression '%s'", + expr_str.c_str()); + break; + case eExpressionThreadVanished: + error.SetErrorStringWithFormat( + "expression thread vanished for the expression '%s'", + expr_str.c_str()); + break; } } return error; diff --git a/lldb/source/Interpreter/CommandObject.cpp b/lldb/source/Interpreter/CommandObject.cpp index 4500378c62298..c13ce4ece4297 100644 --- a/lldb/source/Interpreter/CommandObject.cpp +++ b/lldb/source/Interpreter/CommandObject.cpp @@ -727,10 +727,14 @@ bool CommandObjectParsed::Execute(const char *args_string, } if (!handled) { for (auto entry : llvm::enumerate(cmd_args.entries())) { - if (!entry.value().ref().empty() && entry.value().GetQuoteChar() == '`') { - cmd_args.ReplaceArgumentAtIndex( - entry.index(), - m_interpreter.ProcessEmbeddedScriptCommands(entry.value().c_str())); + const Args::ArgEntry &value = entry.value(); + if (!value.ref().empty() && value.GetQuoteChar() == '`') { + // We have to put the backtick back in place for PreprocessCommand. + std::string opt_string = value.c_str(); + Status error; + error = m_interpreter.PreprocessToken(opt_string); + if (error.Success()) + cmd_args.ReplaceArgumentAtIndex(entry.index(), opt_string); } } diff --git a/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py b/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py index 495e52db53aa3..4a95cff21e713 100644 --- a/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py +++ b/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py @@ -29,4 +29,53 @@ def test_backticks_in_alias(self): interp.HandleCommand("_test-argv-parray-cmd", result) self.assertFalse(result.Succeeded(), "CommandAlias::Desugar currently fails if a alias substitutes %N arguments in another alias") + def test_backticks_in_parsed_cmd_argument(self): + """ break list is a parsed command, use a variable for the breakpoint number + and make sure that and the direct use of the ID get the same result. """ + self.build() + target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c")) + # Make a second breakpoint so that if the backtick part -> nothing we'll print too much: + # It doesn't need to resolve to anything. + dummy_bkpt = target.BreakpointCreateByName("dont_really_care_if_this_exists") + + bkpt_id = bkpt.GetID() + self.runCmd(f"expr int $number = {bkpt_id}") + direct_result = lldb.SBCommandReturnObject() + backtick_result = lldb.SBCommandReturnObject() + interp = self.dbg.GetCommandInterpreter() + interp.HandleCommand(f"break list {bkpt_id}", direct_result) + self.assertTrue(direct_result.Succeeded(), "Break list with id works") + interp.HandleCommand("break list `$number`", backtick_result) + self.assertTrue(direct_result.Succeeded(), "Break list with backtick works") + self.assertEqual(direct_result.GetOutput(), backtick_result.GetOutput(), "Output is the same") + + def test_backticks_in_parsed_cmd_option(self): + # The script interpreter is a raw command, so try that one: + self.build() + target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c")) + + self.runCmd(f"expr int $number = 2") + direct_result = lldb.SBCommandReturnObject() + backtick_result = lldb.SBCommandReturnObject() + interp = self.dbg.GetCommandInterpreter() + interp.HandleCommand(f"memory read --count 2 argv", direct_result) + self.assertTrue(direct_result.Succeeded(), "memory read with direct count works") + interp.HandleCommand("memory read --count `$number` argv", backtick_result) + self.assertTrue(direct_result.Succeeded(), "memory read with backtick works") + self.assertEqual(direct_result.GetOutput(), backtick_result.GetOutput(), "Output is the same") + + def test_backticks_in_raw_cmd(self): + # The script interpreter is a raw command, so try that one: + self.build() + target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c")) + argc_valobj = thread.frames[0].FindVariable("argc") + self.assertTrue(argc_valobj.GetError().Success(), "Made argc valobj") + argc_value = argc_valobj.GetValueAsUnsigned(0) + self.assertNotEqual(argc_value, 0, "Got a value for argc") + result = lldb.SBCommandReturnObject() + interp = self.dbg.GetCommandInterpreter() + interp.HandleCommand(f"script {argc_value} - `argc`", result) + self.assertTrue(result.Succeeded(), "Command succeeded") + self.assertEqual("0\n", result.GetOutput(), "Substitution worked") + From 8800893f9f7729a711278f70ce75de85caeaa602 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Fri, 24 Mar 2023 12:27:33 -0700 Subject: [PATCH 2/2] Don't expect what newlines look like - never works on Windows. (cherry picked from commit 0999996f68186183eaf065e4a05a3e73170a19ae) --- .../API/commands/command/backticks/TestBackticksInAlias.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py b/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py index 4a95cff21e713..74c33c66cdd08 100644 --- a/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py +++ b/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py @@ -76,6 +76,7 @@ def test_backticks_in_raw_cmd(self): interp = self.dbg.GetCommandInterpreter() interp.HandleCommand(f"script {argc_value} - `argc`", result) self.assertTrue(result.Succeeded(), "Command succeeded") - self.assertEqual("0\n", result.GetOutput(), "Substitution worked") + fixed_output = result.GetOutput().rstrip() + self.assertEqual("0", fixed_output, "Substitution worked")