Merge pull request #6535 from jimingham/backtick-in-parsed

jimingham · web-flow · commit b5f0971f1073 · 2023-03-24T15:53:27.000-07:00
Fix backtick handling in parsed commands.
diff --git 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
@@ -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
@@ -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
@@ -29,4 +29,54 @@ 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")
+        fixed_output = result.GetOutput().rstrip()
+        self.assertEqual("0", fixed_output, "Substitution worked")
+
         

Original file line number	Diff line number	Diff line change
`@@ -727,10 +727,14 @@ bool CommandObjectParsed::Execute(const char *args_string,`
`727`	`727`	`}`
`728`	`728`	`if (!handled) {`
`729`	`729`	`for (auto entry : llvm::enumerate(cmd_args.entries())) {`
`730`		- if (!entry.value().ref().empty() && entry.value().GetQuoteChar() == '`') {
`731`		`- cmd_args.ReplaceArgumentAtIndex(`
`732`		`- entry.index(),`
`733`		`- m_interpreter.ProcessEmbeddedScriptCommands(entry.value().c_str()));`
	`730`	`+ const Args::ArgEntry &value = entry.value();`
	`731`	+ if (!value.ref().empty() && value.GetQuoteChar() == '`') {
	`732`	`+ // We have to put the backtick back in place for PreprocessCommand.`
	`733`	`+ std::string opt_string = value.c_str();`
	`734`	`+ Status error;`
	`735`	`+ error = m_interpreter.PreprocessToken(opt_string);`
	`736`	`+ if (error.Success())`
	`737`	`+ cmd_args.ReplaceArgumentAtIndex(entry.index(), opt_string);`
`734`	`738`	`}`
`735`	`739`	`}`
`736`	`740`