[Lldb-commits] [lldb] bb1d702 - [lldb][NFC] Make GetShellSafeArgument return std::string and unittest it.

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-10-22T14:47:10+02:00
New Revision: bb1d702e25f5f23e8d5a755295f2921caaea2abb

URL: https://github.com/llvm/llvm-project/commit/bb1d702e25f5f23e8d5a755295f2921caaea2abb
DIFF: https://github.com/llvm/llvm-project/commit/bb1d702e25f5f23e8d5a755295f2921caaea2abb.diff

LOG: [lldb][NFC] Make GetShellSafeArgument return std::string and unittest it.

Added: 
    

Modified: 
    lldb/include/lldb/Utility/Args.h
    lldb/source/Host/common/ProcessLaunchInfo.cpp
    lldb/source/Utility/Args.cpp
    lldb/unittests/Utility/ArgsTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Utility/Args.h b/lldb/include/lldb/Utility/Args.h
index 82e6d147ae56..b93677b53935 100644
--- a/lldb/include/lldb/Utility/Args.h
+++ b/lldb/include/lldb/Utility/Args.h
@@ -263,9 +263,8 @@ class Args {
 
   static uint32_t StringToGenericRegister(llvm::StringRef s);
 
-  static const char *GetShellSafeArgument(const FileSpec &shell,
-                                          const char *unsafe_arg,
-                                          std::string &safe_arg);
+  static std::string GetShellSafeArgument(const FileSpec &shell,
+                                          llvm::StringRef unsafe_arg);
 
   // EncodeEscapeSequences will change the textual representation of common
   // escape sequences like "\n" (two characters) into a single '\n'. It does

diff  --git a/lldb/source/Host/common/ProcessLaunchInfo.cpp b/lldb/source/Host/common/ProcessLaunchInfo.cpp
index 851069d0f20f..01cf3d76314e 100644
--- a/lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ b/lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -253,7 +253,6 @@ bool ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell(
       if (argv == nullptr || argv[0] == nullptr)
         return false;
       Args shell_arguments;
-      std::string safe_arg;
       shell_arguments.AppendArgument(shell_executable);
       const llvm::Triple &triple = GetArchitecture().GetTriple();
       if (triple.getOS() == llvm::Triple::Win32 &&
@@ -330,9 +329,10 @@ bool ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell(
           return false;
       } else {
         for (size_t i = 0; argv[i] != nullptr; ++i) {
-          const char *arg =
-              Args::GetShellSafeArgument(m_shell, argv[i], safe_arg);
-          shell_command.Printf(" %s", arg);
+          std::string safe_arg = Args::GetShellSafeArgument(m_shell, argv[i]);
+          // Add a space to separate this arg from the previous one.
+          shell_command.PutCString(" ");
+          shell_command.PutCString(safe_arg);
         }
       }
       shell_arguments.AppendArgument(shell_command.GetString());

diff  --git a/lldb/source/Utility/Args.cpp b/lldb/source/Utility/Args.cpp
index 2cbe727ed240..152e73db08ea 100644
--- a/lldb/source/Utility/Args.cpp
+++ b/lldb/source/Utility/Args.cpp
@@ -379,9 +379,8 @@ void Args::Clear() {
   m_argv.push_back(nullptr);
 }
 
-const char *Args::GetShellSafeArgument(const FileSpec &shell,
-                                       const char *unsafe_arg,
-                                       std::string &safe_arg) {
+std::string Args::GetShellSafeArgument(const FileSpec &shell,
+                                       llvm::StringRef unsafe_arg) {
   struct ShellDescriptor {
     ConstString m_basename;
     const char *m_escapables;
@@ -403,7 +402,7 @@ const char *Args::GetShellSafeArgument(const FileSpec &shell,
     }
   }
 
-  safe_arg.assign(unsafe_arg);
+  std::string safe_arg(unsafe_arg);
   size_t prev_pos = 0;
   while (prev_pos < safe_arg.size()) {
     // Escape spaces and quotes
@@ -414,7 +413,7 @@ const char *Args::GetShellSafeArgument(const FileSpec &shell,
     } else
       break;
   }
-  return safe_arg.c_str();
+  return safe_arg;
 }
 
 lldb::Encoding Args::StringToEncoding(llvm::StringRef s,

diff  --git a/lldb/unittests/Utility/ArgsTest.cpp b/lldb/unittests/Utility/ArgsTest.cpp
index ed1235f99c71..cd62c08df31e 100644
--- a/lldb/unittests/Utility/ArgsTest.cpp
+++ b/lldb/unittests/Utility/ArgsTest.cpp
@@ -9,6 +9,7 @@
 #include "gtest/gtest.h"
 
 #include "lldb/Utility/Args.h"
+#include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/StringList.h"
 
 #include <limits>
@@ -313,3 +314,25 @@ TEST(ArgsTest, Yaml) {
   EXPECT_EQ(entries[2].GetQuoteChar(), '"');
   EXPECT_EQ(entries[3].GetQuoteChar(), '\0');
 }
+
+TEST(ArgsTest, GetShellSafeArgument) {
+  // Try escaping with bash at start/middle/end of the argument.
+  FileSpec bash("/bin/bash", FileSpec::Style::posix);
+  EXPECT_EQ(Args::GetShellSafeArgument(bash, "\"b"), "\\\"b");
+  EXPECT_EQ(Args::GetShellSafeArgument(bash, "a\""), "a\\\"");
+  EXPECT_EQ(Args::GetShellSafeArgument(bash, "a\"b"), "a\\\"b");
+
+  // String that doesn't need to be escaped
+  EXPECT_EQ(Args::GetShellSafeArgument(bash, "a"), "a");
+
+  // Try escaping with tcsh and the tcsh-specific "$" escape.
+  FileSpec tcsh("/bin/tcsh", FileSpec::Style::posix);
+  EXPECT_EQ(Args::GetShellSafeArgument(tcsh, "a$b"), "a\\$b");
+  // Bash however doesn't need escaping for "$".
+  EXPECT_EQ(Args::GetShellSafeArgument(bash, "a$b"), "a$b");
+
+  // Try escaping with an unknown shell.
+  FileSpec unknown_shell("/bin/unknown_shell", FileSpec::Style::posix);
+  EXPECT_EQ(Args::GetShellSafeArgument(unknown_shell, "a'b"), "a\\'b");
+  EXPECT_EQ(Args::GetShellSafeArgument(unknown_shell, "a\"b"), "a\\\"b");
+}