[lldb/Target] Add custom interpreter option to `platform shell`

[mib]

Summary

This patch adds the ability to use a custom interpreter with the
platform shell command. If the user set the -s|--shell option
with the path to a binary, lldb passes it down to the platform's
RunShellProcess method and set it as the shell to use in
`ProcessLaunchInfo to run commands.

Note that not all the Platforms support running shell commands with
custom interpreters (i.e. RemoteGDBServer is only expected to use the
default shell).

This patch also makes some refactoring and cleanups, like swapping
CString for StringRef when possible and updating SBPlatformShellCommand
with new methods and a new constructor.

rdar://67759256

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

[JDevlieghere]

lldb/source/API/SBPlatform.cpp

59

Given that this pattern repeats a few times in this struct, maybe a small static helper function would be nice:

static bool is_not_empty(cont char* c) { return c && c[0]; }

64

Shouldn't we just bail out if the command is null/empty? Now it will run shell -c, right?

71

m_command is a std::string, why call c_str()?

[kastiglione]

lldb/source/API/SBPlatform.cpp

59

Can these be StringRef, and then check with empty()?

lldb/source/Commands/CommandObjectPlatform.cpp

1627

Is it necessary to check readability? If not done here, at execution time would a non-readable interpreter lead to a useful error message?

If checking for readability is desirable here, is checking that it's executable also desirable?

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

110

[labath]

I am wondering what should the platform classes which do not implement this feature do (although most of them could implemented I don't think you need to implement all of them -- PlatformGdbRemote in particular can be a bit tricky). It seems to me like it would be better for them to bail out if a non-standard shell is requested instead of silently executing a command the user did not intend.

lldb/include/lldb/Target/Platform.h

644

const on a value parameter is useless. Also, default arguments on virtual methods are somewhat shoot-footy: http://www.cplusplus.com/forum/general/69970/. The Non-virtual interface pattern would solve that, and maybe have some other benefits.

lldb/source/API/SBPlatform.cpp

59–68

Rather than duplicating this logic here (and getting it wrong for windows, for nested quotes, etc.) it would be better to just pass the custom shell to the RunShellCommand method. Then it use the existing quoting logic, and all that needs to change is that it now picks up the shell from the argument (if passed) instead of the baked-in default.

lldb/source/Commands/CommandObjectPlatform.cpp

1621–1633

This won't work for remote platforms. The checks should be at least done in the Platform class, but ideally I would really just leave it to the operating system to figure out if it can or cannot run a given command.

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

97

You could build a small executable to serve as the "shell". Then, this test would work everywhere.

[jingham]

Do people really call command-line shells interpreters? I would have thought --shell would be a better name. lldb already has its own command interpreter which is orthogonal to the shells, so it seems confusing to use the same term for both.

[mib]

In D86667#2242566, @jingham wrote:

Do people really call command-line shells interpreters? I would have thought --shell would be a better name. lldb already has its own command interpreter which is orthogonal to the shells, so it seems confusing to use the same term for both.

I think platform shell --shell sounds/looks repetitive so I opted for -i|--interpreter instead, as in Command-line Interpreter.

lldb/source/API/SBPlatform.cpp

59

I think since this method will eventually interact with the Python API, they need to be C strings.

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

110

Since it's a command output, it has a newline character at the end of it.

[mib]

Address most comments:

• Passed down the shell interpreter path down to Host::RunShellCommand to be "assembled" with the rest of the command.
• Wrote a custom test program to simplify testing on multiple platforms.
• Updated code by using llvm::StringRef instead of CString when possible.

[labath]

In D86667#2243781, @mib wrote:

In D86667#2242566, @jingham wrote:

Do people really call command-line shells interpreters? I would have thought --shell would be a better name. lldb already has its own command interpreter which is orthogonal to the shells, so it seems confusing to use the same term for both.

I think platform shell --shell sounds/looks repetitive so I opted for -i|--interpreter instead, as in Command-line Interpreter.

Yes, it is repetitive. OTOH:

• the unix commands that take a shell as an argument (e.g., su, sudo), do so via a --shell/-s argument
• we have a memory load --load command

Overall, I can understand where you're coming from, but I think I'd go with --shell nonetheless...

lldb/include/lldb/Target/Platform.h

654

The run_in_shell argument makes the inferface weird (why do I need to specify a shell, if I don't want to use it). And still suffers from the virtual default argument problem described above. Let's not propagate this Host weirdness into the platform class.

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

2815

These Shouldn't be NULL are out of place now, and probably not needed anymore.

2826

command.data() will do just fine

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

95

We normally set these things in the makefile. The environment tricks are reserved for cases where you need to do something funny (e.g. build multiple things from a single makefile, or similar...).

[jasonmolenda]

In D86667#2243878, @labath wrote:

In D86667#2243781, @mib wrote:

In D86667#2242566, @jingham wrote:

Do people really call command-line shells interpreters? I would have thought --shell would be a better name. lldb already has its own command interpreter which is orthogonal to the shells, so it seems confusing to use the same term for both.

I think platform shell --shell sounds/looks repetitive so I opted for -i|--interpreter instead, as in Command-line Interpreter.

Yes, it is repetitive. OTOH:

• the unix commands that take a shell as an argument (e.g., su, sudo), do so via a --shell/-s argument
• we have a memory load --load command

Overall, I can understand where you're coming from, but I think I'd go with --shell nonetheless...

I'd agree, process launch already takes a -c|--shell argument so it would be consistent to do that. (I assume -c was used because it's kind of like sh(1)'s -c option but not really. maybe more likely that -s was already used ;)

[mib]

lldb/include/lldb/Target/Platform.h

654

I started by removing this argument, then I noticed that some code calls Host::RunShellCommand with the run_in_shell argument set to false (i.e. PlatformDarwin::GetXcodeSelectPath). This means that Host::RunShellCommand will run the command without prepending the "sh -c" part.

That's why I kept it, and renamed it to run_in_shell instead of run_in_default_shell.

[mib]

• Changed CommandObject option from -i|--interpreter to -s|--shell.
• Updated test program build settings.

[labath]

lldb/include/lldb/Target/Platform.h

654

Right, but that code only calls the Host version. I'm only saying we should delete it (or, not add it, to be precise) to the Platform version. (I'd like to remove it from the Host version too, but that's a different story.)

[mib]

Removed bool run_in_shell argument from Platform classes.

[mib]

Replaced all the interpreter occurrences with shell in the code.

[labath]

Thanks, this looks good to me now.

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

16

If you add this, then there's no need for @no_debug_info_test on individual test methods.

[max-kudr]

Hi @mib, Windows buildbot is broken after this commit. Please fix or revert.

Failed Tests (2):

lldb-api :: commands/platform/basic/TestPlatformCommand.py
lldb-api :: commands/platform/basic/TestPlatformPython.py

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/18671/steps/test/logs/stdio

http://lab.llvm.org:8011/buildslaves/win-py3-buildbot

[mib]

@max-kudr Looking, thanks!

[mib]

lldb/test/API/commands/platform/basic/myshell.c

11

@max-kudr It seems I'm missing the leading _ (correct me if I'm wrong but IIUC the macro should be _WIN32), explaining the failure, I'll send a fix now.

[max-kudr]

lldb/test/API/commands/platform/basic/myshell.c

11

Yes, at least _WIN32 is used in clang. Thank you!

[mib]

Should be fixed with 0e86f390457a2b4dd1f2d1770db912963a36f240. @max-kudr I'll keep an eye on the Windows bot to make sure it worked :)

lldb/bindings/interface/SBPlatform.i

public:
 
     void
     SetLocalCacheDirectory(const char *path);
 };
 
 class SBPlatformShellCommand
 {
 public:
+    SBPlatformShellCommand (const char *shell, const char *shell_command);
     SBPlatformShellCommand (const char *shell_command);
 
     SBPlatformShellCommand (const SBPlatformShellCommand &rhs);
 
     ~SBPlatformShellCommand();
 
     void
     Clear();
 
     const char *
+    GetShell();
+
+    void
+    SetShell(const char *shell_interpreter);
+
+    const char *
     GetCommand();
 
     void
     SetCommand(const char *shell_command);
 
     const char *
     GetWorkingDirectory ();
 

lldb/include/lldb/API/SBPlatform.h

public:
   void SetLocalCacheDirectory(const char *path);
 
 protected:
   PlatformConnectOptions *m_opaque_ptr;
 };
 
 class LLDB_API SBPlatformShellCommand {
 public:
+  SBPlatformShellCommand(const char *shell, const char *shell_command);
   SBPlatformShellCommand(const char *shell_command);
 
   SBPlatformShellCommand(const SBPlatformShellCommand &rhs);
 
   SBPlatformShellCommand &operator=(const SBPlatformShellCommand &rhs);
 
   ~SBPlatformShellCommand();
 
   void Clear();
 
+  const char *GetShell();
+
+  void SetShell(const char *shell);
+
   const char *GetCommand();
 
   void SetCommand(const char *shell_command);
 
   const char *GetWorkingDirectory();
 
   void SetWorkingDirectory(const char *path);
 

lldb/include/lldb/Host/Host.h

public:
   /// Perform expansion of the command-line for this launch info This can
   /// potentially involve wildcard expansion
   /// environment variable replacement, and whatever other
   /// argument magic the platform defines as part of its typical
   /// user experience
   static Status ShellExpandArguments(ProcessLaunchInfo &launch_info);
 
   /// Run a shell command.
-  /// \arg command  shouldn't be NULL
+  /// \arg command  shouldn't be empty
   /// \arg working_dir Pass empty FileSpec to use the current working directory
   /// \arg status_ptr  Pass NULL if you don't want the process exit status
   /// \arg signo_ptr   Pass NULL if you don't want the signal that caused the
   ///                  process to exit
   /// \arg command_output  Pass NULL if you don't want the command output
   /// \arg hide_stderr if this is false, redirect stderr to stdout
-  /// TODO: Convert this function to take a StringRef.
-  static Status RunShellCommand(const char *command,
+  static Status RunShellCommand(llvm::StringRef command,
                                 const FileSpec &working_dir, int *status_ptr,
                                 int *signo_ptr, std::string *command_output,
                                 const Timeout<std::micro> &timeout,
-                                bool run_in_default_shell = true,
+                                bool run_in_shell = true,
+                                bool hide_stderr = false);
+
+  /// Run a shell command.
+  /// \arg shell  Pass an empty string if you want to use the default shell
+  /// interpreter \arg command \arg working_dir  Pass empty FileSpec to use the
+  /// current working directory \arg status_ptr   Pass NULL if you don't want
+  /// the process exit status \arg signo_ptr    Pass NULL if you don't want the
+  /// signal that caused
+  ///                   the process to exit
+  /// \arg command_output  Pass NULL if you don't want the command output
+  /// \arg hide_stderr  If this is \b false, redirect stderr to stdout
+  static Status RunShellCommand(llvm::StringRef shell, llvm::StringRef command,
+                                const FileSpec &working_dir, int *status_ptr,
+                                int *signo_ptr, std::string *command_output,
+                                const Timeout<std::micro> &timeout,
+                                bool run_in_shell = true,
                                 bool hide_stderr = false);
 
   /// Run a shell command.
   /// \arg working_dir Pass empty FileSpec to use the current working directory
   /// \arg status_ptr  Pass NULL if you don't want the process exit status
   /// \arg signo_ptr   Pass NULL if you don't want the signal that caused the
   ///                  process to exit
   /// \arg command_output  Pass NULL if you don't want the command output
   /// \arg hide_stderr if this is false, redirect stderr to stdout
   static Status RunShellCommand(const Args &args, const FileSpec &working_dir,
                                 int *status_ptr, int *signo_ptr,
                                 std::string *command_output,
                                 const Timeout<std::micro> &timeout,
-                                bool run_in_default_shell = true,
+                                bool run_in_shell = true,
+                                bool hide_stderr = false);
+
+  /// Run a shell command.
+  /// \arg shell            Pass an empty string if you want to use the default
+  /// shell interpreter \arg command \arg working_dir Pass empty FileSpec to use
+  /// the current working directory \arg status_ptr    Pass NULL if you don't
+  /// want the process exit status \arg signo_ptr     Pass NULL if you don't
+  /// want the signal that caused the
+  ///               process to exit
+  /// \arg command_output  Pass NULL if you don't want the command output
+  /// \arg hide_stderr If this is \b false, redirect stderr to stdout
+  static Status RunShellCommand(llvm::StringRef shell, const Args &args,
+                                const FileSpec &working_dir, int *status_ptr,
+                                int *signo_ptr, std::string *command_output,
+                                const Timeout<std::micro> &timeout,
+                                bool run_in_shell = true,
                                 bool hide_stderr = false);
 
   static bool OpenFileInExternalEditor(const FileSpec &file_spec,
                                        uint32_t line_no);
 
   static Environment GetEnvironment();
 
   static std::unique_ptr<Connection>

lldb/include/lldb/Target/Platform.h

public:
   }
 
   virtual lldb_private::OptionGroupOptions *
   GetConnectionOptions(CommandInterpreter &interpreter) {
     return nullptr;
   }
 
   virtual lldb_private::Status RunShellCommand(
-      const char *command,         // Shouldn't be nullptr
+      llvm::StringRef command,
+      const FileSpec &working_dir, // Pass empty FileSpec to use the current
+                                   // working directory
+      int *status_ptr, // Pass nullptr if you don't want the process exit status
+      int *signo_ptr,  // Pass nullptr if you don't want the signal that caused
+                       // the process to exit
+      std::string
+          *command_output, // Pass nullptr if you don't want the command output
+      const Timeout<std::micro> &timeout);
+
+  virtual lldb_private::Status RunShellCommand(
+      llvm::StringRef shell, llvm::StringRef command,
       const FileSpec &working_dir, // Pass empty FileSpec to use the current
                                    // working directory
       int *status_ptr, // Pass nullptr if you don't want the process exit status
       int *signo_ptr,  // Pass nullptr if you don't want the signal that caused
                        // the process to exit
       std::string
           *command_output, // Pass nullptr if you don't want the command output
       const Timeout<std::micro> &timeout);
 
   virtual void SetLocalCacheDirectory(const char *local);
 
   virtual const char *GetLocalCacheDirectory();
 
   virtual std::string GetPlatformSpecificConnectionInformation() { return ""; }
 
   virtual bool CalculateMD5(const FileSpec &file_spec, uint64_t &low,
                             uint64_t &high);
 
   virtual int32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) {
     return 1;
   }
 
   virtual const lldb::UnixSignalsSP &GetRemoteUnixSignals();
 
   lldb::UnixSignalsSP GetUnixSignals();
 
   /// Locate a queue name given a thread's qaddr

lldb/include/lldb/Target/RemoteAwarePlatform.h

public:
   Status GetFileWithUUID(const FileSpec &platform_file, const UUID *uuid,
                          FileSpec &local_file) override;
 
   bool GetRemoteOSVersion() override;
   bool GetRemoteOSBuildString(std::string &s) override;
   bool GetRemoteOSKernelDescription(std::string &s) override;
   ArchSpec GetRemoteSystemArchitecture() override;
 
-  Status RunShellCommand(const char *command, const FileSpec &working_dir,
+  Status RunShellCommand(llvm::StringRef command, const FileSpec &working_dir,
                          int *status_ptr, int *signo_ptr,
                          std::string *command_output,
                          const Timeout<std::micro> &timeout) override;
 
+  Status RunShellCommand(llvm::StringRef interpreter, llvm::StringRef command,
+                         const FileSpec &working_dir, int *status_ptr,
+                         int *signo_ptr, std::string *command_output,
+                         const Timeout<std::micro> &timeout) override;
+
   const char *GetHostname() override;
   UserIDResolver &GetUserIDResolver() override;
   lldb_private::Environment GetEnvironment() override;
 
   bool IsConnected() const override;
 
   bool GetProcessInfo(lldb::pid_t pid, ProcessInstanceInfo &proc_info) override;
   uint32_t FindProcesses(const ProcessInstanceInfoMatch &match_info,

lldb/source/API/SBPlatform.cpp

struct PlatformConnectOptions {
   std::string m_rsync_remote_path_prefix;
   bool m_rsync_enabled;
   bool m_rsync_omit_hostname_from_remote_path;
   ConstString m_local_cache_directory;
 };
 
 // PlatformShellCommand
 struct PlatformShellCommand {
-  PlatformShellCommand(const char *shell_command = nullptr)
+  PlatformShellCommand(llvm::StringRef shell_interpreter,
+                       llvm::StringRef shell_command)
       : m_command(), m_working_dir(), m_status(0), m_signo(0) {
-    if (shell_command && shell_command[0])
-      m_command = shell_command;
+    if (!shell_interpreter.empty())
+      m_shell = shell_interpreter.str();
+
+    if (!m_shell.empty() && !shell_command.empty())
+      m_command = shell_command.str();
+  }
+
+  PlatformShellCommand(llvm::StringRef shell_command = llvm::StringRef())
+      : m_shell(), m_command(), m_working_dir(), m_status(0), m_signo(0) {
+    if (!shell_command.empty())
+      m_command = shell_command.str();
   }
 
   ~PlatformShellCommand() = default;
 
+  std::string m_shell;
   std::string m_command;
   std::string m_working_dir;
   std::string m_output;
   int m_status;
   int m_signo;
   Timeout<std::ratio<1>> m_timeout = llvm::None;
 };
 // SBPlatformConnectOptions
void SBPlatformConnectOptions::SetLocalCacheDirectory(const char *path) {
 
   if (path && path[0])
     m_opaque_ptr->m_local_cache_directory.SetCString(path);
   else
     m_opaque_ptr->m_local_cache_directory = ConstString();
 }
 
 // SBPlatformShellCommand
+SBPlatformShellCommand::SBPlatformShellCommand(const char *shell_interpreter,
+                                               const char *shell_command)
+    : m_opaque_ptr(new PlatformShellCommand(shell_interpreter, shell_command)) {
+  LLDB_RECORD_CONSTRUCTOR(SBPlatformShellCommand, (const char *, const char *),
+                          shell_interpreter, shell_command);
+}
+
 SBPlatformShellCommand::SBPlatformShellCommand(const char *shell_command)
     : m_opaque_ptr(new PlatformShellCommand(shell_command)) {
   LLDB_RECORD_CONSTRUCTOR(SBPlatformShellCommand, (const char *),
                           shell_command);
 }
 
 SBPlatformShellCommand::SBPlatformShellCommand(
     const SBPlatformShellCommand &rhs)

 void SBPlatformShellCommand::Clear() {
   LLDB_RECORD_METHOD_NO_ARGS(void, SBPlatformShellCommand, Clear);
 
   m_opaque_ptr->m_output = std::string();
   m_opaque_ptr->m_status = 0;
   m_opaque_ptr->m_signo = 0;
 }
 
+const char *SBPlatformShellCommand::GetShell() {
+  LLDB_RECORD_METHOD_NO_ARGS(const char *, SBPlatformShellCommand, GetShell);
+
+  if (m_opaque_ptr->m_shell.empty())
+    return nullptr;
+  return m_opaque_ptr->m_shell.c_str();
+}
+
+void SBPlatformShellCommand::SetShell(const char *shell_interpreter) {
+  LLDB_RECORD_METHOD(void, SBPlatformShellCommand, SetShell, (const char *),
+                     shell_interpreter);
+
+  if (shell_interpreter && shell_interpreter[0])
+    m_opaque_ptr->m_shell = shell_interpreter;
+  else
+    m_opaque_ptr->m_shell.clear();
+}
+
 const char *SBPlatformShellCommand::GetCommand() {
   LLDB_RECORD_METHOD_NO_ARGS(const char *, SBPlatformShellCommand, GetCommand);
 
   if (m_opaque_ptr->m_command.empty())
     return nullptr;
   return m_opaque_ptr->m_command.c_str();
 }
 
if (!command)
       return Status("invalid shell command (empty)");
 
     const char *working_dir = shell_command.GetWorkingDirectory();
     if (working_dir == nullptr) {
       working_dir = platform_sp->GetWorkingDirectory().GetCString();
       if (working_dir)
         shell_command.SetWorkingDirectory(working_dir);
     }
-    return platform_sp->RunShellCommand(command, FileSpec(working_dir),
+    return platform_sp->RunShellCommand(shell_command.m_opaque_ptr->m_shell,
+                                        command, FileSpec(working_dir),
                                         &shell_command.m_opaque_ptr->m_status,
                                         &shell_command.m_opaque_ptr->m_signo,
                                         &shell_command.m_opaque_ptr->m_output,
                                         shell_command.m_opaque_ptr->m_timeout);
   }));
 }
 
 SBError SBPlatform::Launch(SBLaunchInfo &launch_info) {

 void RegisterMethods<SBPlatformShellCommand>(Registry &R) {
   LLDB_REGISTER_CONSTRUCTOR(SBPlatformShellCommand, (const char *));
   LLDB_REGISTER_CONSTRUCTOR(SBPlatformShellCommand,
                             (const lldb::SBPlatformShellCommand &));
   LLDB_REGISTER_METHOD(
       SBPlatformShellCommand &,
       SBPlatformShellCommand, operator=,(const lldb::SBPlatformShellCommand &));
   LLDB_REGISTER_METHOD(void, SBPlatformShellCommand, Clear, ());
+  LLDB_REGISTER_METHOD(const char *, SBPlatformShellCommand, GetShell, ());
+  LLDB_REGISTER_METHOD(void, SBPlatformShellCommand, SetShell, (const char *));
   LLDB_REGISTER_METHOD(const char *, SBPlatformShellCommand, GetCommand, ());
   LLDB_REGISTER_METHOD(void, SBPlatformShellCommand, SetCommand,
                        (const char *));
   LLDB_REGISTER_METHOD(const char *, SBPlatformShellCommand,
                        GetWorkingDirectory, ());
   LLDB_REGISTER_METHOD(void, SBPlatformShellCommand, SetWorkingDirectory,
                        (const char *));
   LLDB_REGISTER_METHOD(uint32_t, SBPlatformShellCommand, GetTimeoutSeconds,

lldb/source/Commands/CommandObjectPlatform.cpp

Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
         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
           m_timeout = std::chrono::seconds(timeout_sec);
         break;
+      case 's': {
+        if (option_arg.empty()) {
+          error.SetErrorStringWithFormat(
+              "missing shell interpreter path for option -i|--interpreter.");
+          return error;
+        }
+
+        m_shell_interpreter = option_arg.str();
+        break;
+      }
       default:
         llvm_unreachable("Unimplemented option");
       }
 
       return error;
     }
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
       m_timeout.reset();
       m_use_host_platform = false;
+      m_shell_interpreter.clear();
     }
 
     Timeout<std::micro> m_timeout = std::chrono::seconds(10);
     bool m_use_host_platform;
+    std::string m_shell_interpreter;
   };
 
   CommandObjectPlatformShell(CommandInterpreter &interpreter)
       : CommandObjectRaw(interpreter, "platform shell",
                          "Run a shell command on the current platform.",
                          "platform shell <shell-command>", 0),
         m_options() {}
 
bool DoExecute(llvm::StringRef raw_command_line,
     // 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;
     }
 
+    llvm::StringRef cmd = args.GetRawPart();
+
     PlatformSP platform_sp(
         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,
+      error = (platform_sp->RunShellCommand(m_options.m_shell_interpreter, cmd,
+                                            working_dir, &status, &signo,
                                             &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(

lldb/source/Commands/Options.td

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.">;
+  def platform_shell_interpreter : Option<"shell", "s">, Arg<"Path">,
+    Desc<"Shell interpreter path. This is the binary used to run 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">,
     Desc<"Name of the process plugin you want to use.">;
   def process_attach_pid : Option<"pid", "p">, Group<1>, Arg<"Pid">,

lldb/source/Host/common/Host.cpp

MonitorShellCommand(std::shared_ptr<ShellInfo> shell_info, lldb::pid_t pid,
   shell_info->signo = signo;
   shell_info->status = status;
   // Let the thread running Host::RunShellCommand() know that the process
   // exited and that ShellInfo has been filled in by broadcasting to it
   shell_info->process_reaped.SetValue(true, eBroadcastAlways);
   return true;
 }
 
-Status Host::RunShellCommand(const char *command, const FileSpec &working_dir,
-                             int *status_ptr, int *signo_ptr,
-                             std::string *command_output_ptr,
+Status Host::RunShellCommand(llvm::StringRef command,
+                             const FileSpec &working_dir, int *status_ptr,
+                             int *signo_ptr, std::string *command_output_ptr,
+                             const Timeout<std::micro> &timeout,
+                             bool run_in_shell, bool hide_stderr) {
+  return RunShellCommand(llvm::StringRef(), Args(command), working_dir,
+                         status_ptr, signo_ptr, command_output_ptr, timeout,
+                         run_in_shell, hide_stderr);
+}
+
+Status Host::RunShellCommand(llvm::StringRef shell_path,
+                             llvm::StringRef command,
+                             const FileSpec &working_dir, int *status_ptr,
+                             int *signo_ptr, std::string *command_output_ptr,
                              const Timeout<std::micro> &timeout,
-                             bool run_in_default_shell,
-                             bool hide_stderr) {
-  return RunShellCommand(Args(command), working_dir, status_ptr, signo_ptr,
-                         command_output_ptr, timeout, run_in_default_shell,
+                             bool run_in_shell, bool hide_stderr) {
+  return RunShellCommand(shell_path, Args(command), working_dir, status_ptr,
+                         signo_ptr, command_output_ptr, timeout, run_in_shell,
                          hide_stderr);
 }
 
 Status Host::RunShellCommand(const Args &args, const FileSpec &working_dir,
                              int *status_ptr, int *signo_ptr,
                              std::string *command_output_ptr,
                              const Timeout<std::micro> &timeout,
-                             bool run_in_default_shell,
-                             bool hide_stderr) {
+                             bool run_in_shell, bool hide_stderr) {
+  return RunShellCommand(llvm::StringRef(), args, working_dir, status_ptr,
+                         signo_ptr, command_output_ptr, timeout, run_in_shell,
+                         hide_stderr);
+}
+
+Status Host::RunShellCommand(llvm::StringRef shell_path, const Args &args,
+                             const FileSpec &working_dir, int *status_ptr,
+                             int *signo_ptr, std::string *command_output_ptr,
+                             const Timeout<std::micro> &timeout,
+                             bool run_in_shell, bool hide_stderr) {
   Status error;
   ProcessLaunchInfo launch_info;
   launch_info.SetArchitecture(HostInfo::GetArchitecture());
-  if (run_in_default_shell) {
+  if (run_in_shell) {
     // Run the command in a shell
-    launch_info.SetShell(HostInfo::GetDefaultShell());
+    FileSpec shell = HostInfo::GetDefaultShell();
+    if (!shell_path.empty())
+      shell.SetPath(shell_path);
+
+    launch_info.SetShell(shell);
     launch_info.GetArguments().AppendArguments(args);
     const bool localhost = true;
     const bool will_debug = false;
     const bool first_arg_is_full_shell_command = false;
     launch_info.ConvertArgumentsForLaunchingInShell(
         error, localhost, will_debug, first_arg_is_full_shell_command, 0);
   } else {
     // No shell, just run it

lldb/source/Host/macosx/objcxx/Host.mm

if (!FileSystem::Instance().Exists(cwd)) {
             "cwd does not exist; cannot launch with shell argument expansion");
         return error;
       } else {
         FileSpec working_dir(wd);
         free(wd);
         launch_info.SetWorkingDirectory(working_dir);
       }
     }
-    bool run_in_default_shell = true;
+    bool run_in_shell = true;
     bool hide_stderr = true;
-    Status e = RunShellCommand(expand_command, cwd, &status, nullptr, &output,
-                               std::chrono::seconds(10), run_in_default_shell,
-                               hide_stderr);
+    Status e =
+        RunShellCommand(expand_command, cwd, &status, nullptr, &output,
+                        std::chrono::seconds(10), run_in_shell, hide_stderr);
 
     if (e.Fail())
       return e;
 
     if (status != 0) {
       error.SetErrorStringWithFormat("lldb-argdumper exited with error %d",
                                      status);
       return error;

lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h

public:
 
   Status CreateSymlink(const FileSpec &src, const FileSpec &dst) override;
 
   bool GetFileExists(const FileSpec &file_spec) override;
 
   Status Unlink(const FileSpec &path) override;
 
   Status RunShellCommand(
-      const char *command,         // Shouldn't be NULL
+      llvm::StringRef shell, llvm::StringRef command,
       const FileSpec &working_dir, // Pass empty FileSpec to use the current
                                    // working directory
       int *status_ptr, // Pass NULL if you don't want the process exit status
       int *signo_ptr,  // Pass NULL if you don't want the signal that caused the
                        // process to exit
       std::string
           *command_output, // Pass NULL if you don't want the command output
       const lldb_private::Timeout<std::micro> &timeout) override;

lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp

Status PlatformRemoteGDBServer::Unlink(const FileSpec &file_spec) {
   return error;
 }
 
 bool PlatformRemoteGDBServer::GetFileExists(const FileSpec &file_spec) {
   return m_gdb_client.GetFileExists(file_spec);
 }
 
 Status PlatformRemoteGDBServer::RunShellCommand(
-    const char *command, // Shouldn't be NULL
+    llvm::StringRef shell, llvm::StringRef command,
     const FileSpec &
         working_dir, // Pass empty FileSpec to use the current working directory
     int *status_ptr, // Pass NULL if you don't want the process exit status
     int *signo_ptr,  // Pass NULL if you don't want the signal that caused the
                      // process to exit
     std::string
         *command_output, // Pass NULL if you don't want the command output
     const Timeout<std::micro> &timeout) {

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

public:
 
   Status Unlink(const FileSpec &file_spec);
 
   Status MakeDirectory(const FileSpec &file_spec, uint32_t mode);
 
   bool GetFileExists(const FileSpec &file_spec);
 
   Status RunShellCommand(
-      const char *command,         // Shouldn't be nullptr
+      llvm::StringRef command,
       const FileSpec &working_dir, // Pass empty FileSpec to use the current
                                    // working directory
       int *status_ptr, // Pass nullptr if you don't want the process exit status
       int *signo_ptr,  // Pass nullptr if you don't want the signal that caused
                        // the process to exit
       std::string
           *command_output, // Pass nullptr if you don't want the command output
       const Timeout<std::micro> &timeout);

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

lldb::addr_t GDBRemoteCommunicationClient::GetShlibInfoAddr() {
   if (SendPacketAndWaitForResponse("qShlibInfoAddr", response, false) !=
           PacketResult::Success ||
       !response.IsNormalResponse())
     return LLDB_INVALID_ADDRESS;
   return response.GetHexMaxU64(false, LLDB_INVALID_ADDRESS);
 }
 
 lldb_private::Status GDBRemoteCommunicationClient::RunShellCommand(
-    const char *command, // Shouldn't be NULL
+    llvm::StringRef command,
     const FileSpec &
         working_dir, // Pass empty FileSpec to use the current working directory
     int *status_ptr, // Pass NULL if you don't want the process exit status
     int *signo_ptr,  // Pass NULL if you don't want the signal that caused the
                      // process to exit
     std::string
         *command_output, // Pass NULL if you don't want the command output
     const Timeout<std::micro> &timeout) {
   lldb_private::StreamString stream;
   stream.PutCString("qPlatform_shell:");
-  stream.PutBytesAsRawHex8(command, strlen(command));
+  stream.PutBytesAsRawHex8(command.data(), command.size());
   stream.PutChar(',');
   uint32_t timeout_sec = UINT32_MAX;
   if (timeout) {
     // TODO: Use chrono version of std::ceil once c++17 is available.
     timeout_sec = std::ceil(std::chrono::duration<double>(*timeout).count());
   }
   stream.PutHex32(timeout_sec);
   if (working_dir) {

lldb/source/Target/Platform.cpp

MmapArgList Platform::GetMmapArgumentList(const ArchSpec &arch, addr_t addr,
   if (flags & eMmapFlagsAnon)
     flags_platform |= MAP_ANON;
 
   MmapArgList args({addr, length, prot, flags_platform, fd, offset});
   return args;
 }
 
 lldb_private::Status Platform::RunShellCommand(
-    const char *command, // Shouldn't be nullptr
+    llvm::StringRef command,
+    const FileSpec &
+        working_dir, // Pass empty FileSpec to use the current working directory
+    int *status_ptr, // Pass nullptr if you don't want the process exit status
+    int *signo_ptr, // Pass nullptr if you don't want the signal that caused the
+                    // process to exit
+    std::string
+        *command_output, // Pass nullptr if you don't want the command output
+    const Timeout<std::micro> &timeout) {
+  return RunShellCommand(llvm::StringRef(), command, working_dir, status_ptr,
+                         signo_ptr, command_output, timeout);
+}
+
+lldb_private::Status Platform::RunShellCommand(
+    llvm::StringRef shell,   // Pass empty if you want to use the default
+                             // shell interpreter
+    llvm::StringRef command, // Shouldn't be empty
     const FileSpec &
         working_dir, // Pass empty FileSpec to use the current working directory
     int *status_ptr, // Pass nullptr if you don't want the process exit status
     int *signo_ptr, // Pass nullptr if you don't want the signal that caused the
                     // process to exit
     std::string
         *command_output, // Pass nullptr if you don't want the command output
     const Timeout<std::micro> &timeout) {
   if (IsHost())
-    return Host::RunShellCommand(command, working_dir, status_ptr, signo_ptr,
-                                 command_output, timeout);
+    return Host::RunShellCommand(shell, command, working_dir, status_ptr,
+                                 signo_ptr, command_output, timeout);
   else
     return Status("unimplemented");
 }
 
 bool Platform::CalculateMD5(const FileSpec &file_spec, uint64_t &low,
                             uint64_t &high) {
   if (!IsHost())
     return false;

lldb/source/Target/RemoteAwarePlatform.cpp

// it another try.
       }
     }
   }
 
   return error;
 }
 
 Status RemoteAwarePlatform::RunShellCommand(
-    const char *command, const FileSpec &working_dir, int *status_ptr,
+    llvm::StringRef command, const FileSpec &working_dir, int *status_ptr,
     int *signo_ptr, std::string *command_output,
     const Timeout<std::micro> &timeout) {
+  return RunShellCommand(llvm::StringRef(), command, working_dir, status_ptr,
+                         signo_ptr, command_output, timeout);
+}
+
+Status RemoteAwarePlatform::RunShellCommand(
+    llvm::StringRef shell, llvm::StringRef command, const FileSpec &working_dir,
+    int *status_ptr, int *signo_ptr, std::string *command_output,
+    const Timeout<std::micro> &timeout) {
   if (IsHost())
-    return Host::RunShellCommand(command, working_dir, status_ptr, signo_ptr,
-                                 command_output, timeout);
+    return Host::RunShellCommand(shell, command, working_dir, status_ptr,
+                                 signo_ptr, command_output, timeout);
   if (m_remote_platform_sp)
-    return m_remote_platform_sp->RunShellCommand(
-        command, working_dir, status_ptr, signo_ptr, command_output, timeout);
+    return m_remote_platform_sp->RunShellCommand(shell, command, working_dir,
+                                                 status_ptr, signo_ptr,
+                                                 command_output, timeout);
   return Status("unable to run a remote command without a platform");
 }
 
 Status RemoteAwarePlatform::MakeDirectory(const FileSpec &file_spec,
                                           uint32_t file_permissions) {
   if (m_remote_platform_sp)
     return m_remote_platform_sp->MakeDirectory(file_spec, file_permissions);
   return Platform::MakeDirectory(file_spec, file_permissions);

lldb/test/API/commands/platform/basic/Makefile

+C_SOURCES := myshell.c
+CFLAGS_EXTRAS := -g0 # No debug info.
+MAKE_DSYM := NO
+
+include Makefile.rules

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

 """
 Test some lldb platform commands.
 """
 
 
 
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
 class PlatformCommandTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
+    NO_DEBUG_INFO_TESTCASE = True
 
     @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.",
class PlatformCommandTestCase(TestBase):
     @no_debug_info_test
     def test_shell_timeout(self):
         """ Test a shell built-in command (sleep) that times out """
         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"])
+
+    @no_debug_info_test
+    def test_host_shell_interpreter(self):
+        """ Test the host platform shell with a different interpreter """
+        self.build()
+        exe = self.getBuildArtifact('a.out')
+        self.expect("platform shell -h -s " + exe + " -- 'echo $0'",
+                    substrs=['SUCCESS', 'a.out'])

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

 """
 Test the lldb platform Python API.
 """
 
 
 
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
 class PlatformPythonTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @add_test_categories(['pyapi'])
     @no_debug_info_test
     def test_platform_list(self):
         """Test SBDebugger::GetNumPlatforms() & GetPlatformAtIndex() API"""
         # Verify the host platform is present by default.
         initial_num_platforms = self.dbg.GetNumPlatforms()
         self.assertGreater(initial_num_platforms, 0)
         host_platform = self.dbg.GetPlatformAtIndex(0)
def test_available_platform_list(self):
                 name_data.GetType(), lldb.eStructuredDataTypeString,
                 'Platform name is a string')
             self.assertTrue(
                 desc_data and desc_data.IsValid(),
                 'Platform has a description')
             self.assertEqual(
                 desc_data.GetType(), lldb.eStructuredDataTypeString,
                 'Platform description is a string')
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_shell_interpreter(self):
+        """ Test a shell with a custom interpreter """
+        platform = self.dbg.GetSelectedPlatform()
+        self.assertTrue(platform.IsValid())
+
+        sh_cmd = lldb.SBPlatformShellCommand('/bin/zsh', 'echo $0')
+        self.assertIn('/bin/zsh', sh_cmd.GetShell())
+        self.assertIn('echo $0', sh_cmd.GetCommand())
+
+        self.build()
+        sh_cmd.SetShell(self.getBuildArtifact('a.out'))
+        err = platform.Run(sh_cmd)
+        self.assertTrue(err.Success())
+        self.assertIn("SUCCESS", sh_cmd.GetOutput())

lldb/test/API/commands/platform/basic/myshell.c

+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+int main(int argc, char *argv[]) {
+  if (argc < 3) {
+    fprintf(stderr, "ERROR: Too few arguments (count: %d).\n", argc - 1);
+    exit(1);
+  }
+
+#ifdef WIN32
+  char *cmd_opt = "/C";
+#else
+  char *cmd_opt = "-c";
+#endif
+
+  if (strncmp(argv[1], cmd_opt, 2)) {
+    fprintf(stderr, "ERROR: Missing shell command option ('%s').\n", cmd_opt);
+    exit(1);
+  }
+
+  printf("SUCCESS: %s\n", argv[0]);
+  return 0;
+}