jingham created this revision.
jingham added reviewers: JDevlieghere, bulbazord, kastiglione.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

A while back I reworked the way that backticks are handled to make backticks in 
aliases work, but I made two bad assumptions and broke the handling for parsed 
commands.  The bad assumptions were:

1. There was testing for backticks in regular commands (turns out the extant 
tests were all for raw commands)
2. That the extant function that was supposed to handle backtick options did 
the thing you do with backtick options

2 is me not reading closely enough.  The function called was 
m_interpreter.ProcessEmbeddedScriptCommands so I really should have seen this 
wasn't doing the right thing.  Apparently at some point we were going to add 
the ability to invoke a script command and insert its text in place of the 
option.  That's not a bad idea, but it was never actually implemented, and 
anyway, you can't use backticks as the demarcation - that's already taken for 
expression substitution.

This patch repairs that breakage, and adds some more tests for backticks in raw 
commands, in the arguments of parsed commands and in the option values for 
parsed commands.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146779

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/test/API/commands/command/backticks/TestBackticksInAlias.py

Index: lldb/test/API/commands/command/backticks/TestBackticksInAlias.py
===================================================================
--- lldb/test/API/commands/command/backticks/TestBackticksInAlias.py
+++ lldb/test/API/commands/command/backticks/TestBackticksInAlias.py
@@ -29,4 +29,53 @@
         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")
+
         
Index: lldb/source/Interpreter/CommandObject.cpp
===================================================================
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -728,9 +728,12 @@
   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()));
+        // We have to put the backtick back in place for PreprocessCommand.
+        std::string opt_string = entry.value().c_str();
+        Status error;
+        error = m_interpreter.PreprocessToken(opt_string);
+        if (error.Success())
+          cmd_args.ReplaceArgumentAtIndex(entry.index(), opt_string);
       }
     }
 
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===================================================================
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1750,112 +1750,124 @@
 
     std::string expr_str(command, expr_content_start,
                          end_backtick - expr_content_start);
+    error = PreprocessToken(expr_str);
+    if (error.Success()) {
+      command.erase(start_backtick, end_backtick - start_backtick + 1);
+      command.insert(start_backtick, std::string(expr_str));
+      pos = start_backtick + expr_str.size();
+      continue;
+    } else
+      break;
+  }
+  return error;                        
+}
 
-    ExecutionContext exe_ctx(GetExecutionContext());
+Status
+CommandInterpreter::PreprocessToken(std::string &expr_str) {
+  Status error;
+  ExecutionContext exe_ctx(GetExecutionContext());
 
-    // 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(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)) {
-        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;
-      }
+  // 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();
 
-      continue;
-    }
+  ValueObjectSP expr_result_valobj_sp;
 
-    if (expr_result_valobj_sp)
-      error = expr_result_valobj_sp->GetError();
+  EvaluateExpressionOptions options;
+  options.SetCoerceToId(false);
+  options.SetUnwindOnError(true);
+  options.SetIgnoreBreakpoints(true);
+  options.SetKeepInMemory(false);
+  options.SetTryAllThreads(true);
+  options.SetTimeout(std::nullopt);
 
-    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 "
+  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;
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===================================================================
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -637,6 +637,9 @@
 
   bool IOHandlerInterrupt(IOHandler &io_handler) override;
 
+  Status PreprocessCommand(std::string &command);
+  Status PreprocessToken(std::string &token);
+
 protected:
   friend class Debugger;
 
@@ -671,8 +674,6 @@
 
   void RestoreExecutionContext();
 
-  Status PreprocessCommand(std::string &command);
-
   void SourceInitFile(FileSpec file, CommandReturnObject &result);
 
   // Completely resolves aliases and abbreviations, returning a pointer to the
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to