[lldb/Commands] Add ability to run shell command on the host.

[mib]

Summary

This patch introduces the (-h|--host) option to the platform shell
command. It allows the user to run shell commands from the host platform
(always available) without putting lldb in the background.

Since the default behaviour of platform shell is to run the command of
the selected platform, having such a choice can be quite handy when
debugging remote targets, for instances.

This patch also introduces a shell alias, to improve the command
discoverability and make it more convenient to use for the user.

rdar://62856024

Signed-off-by: Med Ismail Bennani <[e-mail]>

[jingham]

We certainly shouldn't duplicate the code for CommandObjectPlatformShell.

Why don't you just add a "m_use_host_platform" ivar to CommandObjectPlatformShell, and make two instances of CommandObjectPlatformShell, one with m_use_host_platform set to false and added as a sub-command of platform, and another instance with m_use_host_platform to true, and add that as a base command "shell"? Then you'd just have to make CommandObjectPlatformShell switch on m_use_host_platform and either get the selected platform or the host platform.

You'd also have to adjust the help string for one or the other of the commands, but CommandObject::SetHelp and SetHelpLong will allow you to do that.

[jingham]

I do something similar with CommandObjectThreadStepWithTypeAndScope so that I can share most of that command code, if you want to see an example of doing that.

[mib]

With the first implementation, I thought it'd be a good idea to have separate Command Objects for platform shell and shell because I was imagining the latter could be more "powerful": shell could have an interactive mode like the script command, which might not be as relevant for platform shell when debugging a remote target.

That was my original idea, but I agree we should avoid code duplication, so here is a different implementation following @jingham's suggestions. shell is an alias to platform shell -h -- in this case.

[mib]

It looks like command options are not supported on aliases that already have options:

(lldb) shell -t 1 -- sleep 5
zsh:1: command not found: -t
error: command returned with status 127

In this case, shell -t 1 -- sleep 5 expands to platform shell --host -- -t 1 -- sleep 5, and the option argument is passed as shell command instead of being parsed by lldb.

I might have done something wrong when creating the alias, so I'll skip the test_shell_timeout test until I come up with with a solution.

[JDevlieghere]

lldb/source/Commands/CommandObjectPlatform.cpp

1627

Is this only true for the alias?

lldb/test/API/commands/shell/TestShellCommand.py

27 ↗

(On Diff #263005)

I assume you copied this from TestPlatformCommand.py but if the test is skipped on Windows we can probably remove this line here.

On that note, I think we could avoid some code duplication by merging these tests and have the same tests, once called with shell and once with platform shell.

[mib]

lldb/source/Commands/CommandObjectPlatform.cpp

1627

I added this check because when using shell with no command, instead of showing the command usage, lldb tries running an empty command (if that makes sense) and showed the following error message:

(lldb) shell
error: executable doesn't exist: '(empty)'

[mib]

Merged the shell and platform shell test as @JDevlieghere suggested it.

[JDevlieghere]

LGTM

lldb/source/Commands/CommandObjectPlatform.cpp

1590

Should we reset the timeout too?

lldb/source/Commands/CommandObjectPlatform.cpp

public:
 
     Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
                           ExecutionContext *execution_context) override {
       Status error;
 
       const char short_option = (char)GetDefinitions()[option_idx].short_option;
 
       switch (short_option) {
+      case 'h':
+        m_use_host_platform = true;
+        break;
       case 't':
         uint32_t timeout_sec;
         if (option_arg.getAsInteger(10, timeout_sec))
           error.SetErrorStringWithFormat(
               "could not convert \"%s\" to a numeric value.",
               option_arg.str().c_str());
         else
-          timeout = std::chrono::seconds(timeout_sec);
+          m_timeout = std::chrono::seconds(timeout_sec);
         break;
       default:
         llvm_unreachable("Unimplemented option");
       }
 
       return error;
     }
 
-    void OptionParsingStarting(ExecutionContext *execution_context) override {}
+    void OptionParsingStarting(ExecutionContext *execution_context) override {
+      m_timeout.reset();
+      m_use_host_platform = false;
+    }
 
-    Timeout<std::micro> timeout = std::chrono::seconds(10);
+    Timeout<std::micro> m_timeout = std::chrono::seconds(10);
+    bool m_use_host_platform;
   };
 
   CommandObjectPlatformShell(CommandInterpreter &interpreter)
       : CommandObjectRaw(interpreter, "platform shell",
                          "Run a shell command on the current platform.",
                          "platform shell <shell-command>", 0),
         m_options() {}
 
   ~CommandObjectPlatformShell() override = default;
 
   Options *GetOptions() override { return &m_options; }
 
   bool DoExecute(llvm::StringRef raw_command_line,
                  CommandReturnObject &result) override {
     ExecutionContext exe_ctx = GetCommandInterpreter().GetExecutionContext();
     m_options.NotifyOptionParsingStarting(&exe_ctx);
 
     // Print out an usage syntax on an empty command line.
     if (raw_command_line.empty()) {
       result.GetOutputStream().Printf("%s\n", this->GetSyntax().str().c_str());
       return true;
     }
 
+    const bool is_alias = !raw_command_line.contains("platform");
     OptionsWithRaw args(raw_command_line);
     const char *expr = args.GetRawPart().c_str();
 
     if (args.HasArgs())
       if (!ParseOptions(args.GetArgs(), result))
         return false;
 
+    if (args.GetRawPart().empty()) {
+      result.GetOutputStream().Printf("%s <shell-command>\n",
+                                      is_alias ? "shell" : "platform shell");
+      return false;
+    }
+
     PlatformSP platform_sp(
-        GetDebugger().GetPlatformList().GetSelectedPlatform());
+        m_options.m_use_host_platform
+            ? Platform::GetHostPlatform()
+            : GetDebugger().GetPlatformList().GetSelectedPlatform());
     Status error;
     if (platform_sp) {
       FileSpec working_dir{};
       std::string output;
       int status = -1;
       int signo = -1;
       error = (platform_sp->RunShellCommand(expr, working_dir, &status, &signo,
-                                            &output, m_options.timeout));
+                                            &output, m_options.m_timeout));
       if (!output.empty())
         result.GetOutputStream().PutCString(output);
       if (status > 0) {
         if (signo > 0) {
           const char *signo_cstr = Host::GetSignalAsCString(signo);
           if (signo_cstr)
             result.GetOutputStream().Printf(
                 "error: command returned with status %i and signal %s\n",

lldb/source/Commands/Options.td

def platform_process_attach_pid : Option<"pid", "p">, Group<1>, Arg<"Pid">,
     Desc<"The process ID of an existing process to attach to.">;
   def platform_process_attach_name : Option<"name", "n">, Group<2>,
     Arg<"ProcessName">, Desc<"The name of the process to attach to.">;
   def platform_process_attach_waitfor : Option<"waitfor", "w">, Group<2>,
     Desc<"Wait for the process with <process-name> to launch.">;
 }
 
 let Command = "platform shell" in {
+  def platform_shell_host : Option<"host", "h">,
+    Desc<"Run the commands on the host shell when enabled.">;
   def platform_shell_timeout : Option<"timeout", "t">, Arg<"Value">,
     Desc<"Seconds to wait for the remote host to finish running the command.">;
 }
 
 let Command = "process attach" in {
   def process_attach_continue : Option<"continue", "c">,
     Desc<"Immediately continue the process once attached.">;
   def process_attach_plugin : Option<"plugin", "P">, Arg<"Plugin">,
let Command = "script add" in {
   Desc<"Name of the Python class to bind to this command name.">;
   def script_add_help : Option<"help", "h">, Group<1>, Arg<"HelpText">,
   Desc<"The help text to display for this command.">;
   def script_add_synchronicity : Option<"synchronicity", "s">,
     EnumArg<"ScriptedCommandSynchronicity", "ScriptSynchroType()">,
     Desc<"Set the synchronicity of this command's executions with regard to "
     "LLDB event system.">;
 }
+
 let Command = "source info" in {
   def source_info_count : Option<"count", "c">, Arg<"Count">,
     Desc<"The number of line entries to display.">;
   def source_info_shlib : Option<"shlib", "s">, Groups<[1,2]>, Arg<"ShlibName">,
     Completion<"Module">, Desc<"Look up the source in the given module or "
     "shared library (can be specified more than once).">;
   def source_info_file : Option<"file", "f">, Group<1>, Arg<"Filename">,
     Completion<"SourceFile">, Desc<"The file from which to display source.">;

lldb/source/Interpreter/CommandInterpreter.cpp

if (cmd_obj_sp) {
     if (poarray_alias) {
       poarray_alias->SetHelp("poarray <COUNT> <EXPRESSION> -- lldb will "
           "evaluate EXPRESSION to get the address of an array of COUNT "
           "objects in memory, and will call po on them.");
       poarray_alias->SetHelpLong("");
     }
   }
 
+  cmd_obj_sp = GetCommandSPExact("platform shell", false);
+  if (cmd_obj_sp) {
+    CommandAlias *shell_alias = AddAlias("shell", cmd_obj_sp, " --host --");
+    if (shell_alias) {
+      shell_alias->SetHelp("Run a shell command on the host.");
+      shell_alias->SetHelpLong("");
+      shell_alias->SetSyntax("shell <shell-command>");
+    }
+  }
+
   cmd_obj_sp = GetCommandSPExact("process kill", false);
   if (cmd_obj_sp) {
     AddAlias("kill", cmd_obj_sp);
   }
 
   cmd_obj_sp = GetCommandSPExact("process launch", false);
   if (cmd_obj_sp) {
     alias_arguments_vector_sp = std::make_shared<OptionArgVector>();

lldb/test/API/commands/platform/basic/TestPlatformCommand.py

class PlatformCommandTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @no_debug_info_test
     def test_help_platform(self):
         self.runCmd("help platform")
 
     @no_debug_info_test
+    def test_help_platform(self):
+        self.expect("help shell", substrs=["Run a shell command on the host.",
+                                           "shell <shell-command>"])
+
+
+    @no_debug_info_test
     def test_list(self):
         self.expect("platform list",
                     patterns=['^Available platforms:'])
 
     @no_debug_info_test
     def test_process_list(self):
         self.expect("platform process list",
                     substrs=['PID', 'TRIPLE', 'NAME'])
class PlatformCommandTestCase(TestBase):
     @no_debug_info_test
     def test_shell(self):
         """ Test that the platform shell command can invoke ls. """
         triple = self.dbg.GetSelectedPlatform().GetTriple()
         if re.match(".*-.*-windows", triple):
             self.expect(
                 "platform shell dir c:\\", substrs=[
                     "Windows", "Program Files"])
+            self.expect("shell dir c:\\", substrs=["Windows", "Program Files"])
         elif re.match(".*-.*-.*-android", triple):
             self.expect(
                 "platform shell ls /",
                 substrs=[
                     "cache",
                     "dev",
                     "system"])
+            self.expect("shell ls /",
+                substrs=["cache", "dev", "system"])
         else:
             self.expect("platform shell ls /", substrs=["dev", "tmp", "usr"])
+            self.expect("shell ls /", substrs=["dev", "tmp", "usr"])
 
     @no_debug_info_test
     def test_shell_builtin(self):
         """ Test a shell built-in command (echo) """
         self.expect("platform shell echo hello lldb",
                     substrs=["hello lldb"])
+        self.expect("shell echo hello lldb",
+                    substrs=["hello lldb"])
+
 
-    # FIXME: re-enable once platform shell -t can specify the desired timeout
     @no_debug_info_test
     def test_shell_timeout(self):
         """ Test a shell built-in command (sleep) that times out """
-        self.skipTest("due to taking too long to complete.")
-        self.expect("platform shell sleep 15", error=True, substrs=[
+        self.skipTest("Alias with option not supported by the command interpreter.")
+        self.expect("platform shell -t 1 -- sleep 15", error=True, substrs=[
+                    "error: timed out waiting for shell command to complete"])
+        self.expect("shell -t 1 --  sleep 3", error=True, substrs=[
                     "error: timed out waiting for shell command to complete"])