[lldb] Escape semicolons for all shells

[teemperor]

Summary

LLDB supports having globbing regexes in the process launch arguments that will be resolved
using the user's shell. This requires that we pass the launch args to the shell and then read
back the expanded arguments using LLDB's argdumper utility.

As the shell will not just expand the globbing regexes but all special characters, we need to
escape all non-globbing charcters such as $, &, <, >, etc. as those otherwise are interpreted and
removed in the step where we expand the globbing characters. Also because the special characters
are shell-specific, LLDB needs to maintain a list of all the characters that need to be escaped for
each specific shell.

This patch adds the missing semicolon character to the escape list for all currently supported
shells. Without this having a semicolon in the binary path or having a semicolon in the launch
arguments will cause the argdumping process to fail. E.g., lldb -- ./calc "a;b" was failing before
but is working now.

Fixes rdar://55776943

[JDevlieghere]

LGTM

[jingham]

This is good in general, but I'm not sure we want to escape $. One of the uses of shell expansion is to get shell variables expanded.

lldb/source/Utility/Args.cpp

 
 std::string Args::GetShellSafeArgument(const FileSpec &shell,
                                        llvm::StringRef unsafe_arg) {
   struct ShellDescriptor {
     ConstString m_basename;
     llvm::StringRef m_escapables;
   };
 
-  static ShellDescriptor g_Shells[] = {{ConstString("bash"), " '\"<>()&"},
-                                       {ConstString("tcsh"), " '\"<>()&$"},
+  static ShellDescriptor g_Shells[] = {{ConstString("bash"), " '\"<>()&;"},
+                                       {ConstString("tcsh"), " '\"<>()&$;"},
                                        {ConstString("zsh"), " '\"<>()&;\\|"},
-                                       {ConstString("sh"), " '\"<>()&"}};
+                                       {ConstString("sh"), " '\"<>()&;"}};
 
   // safe minimal set
   llvm::StringRef escapables = " '\"";
 
   if (auto basename = shell.GetFilename()) {
     for (const auto &Shell : g_Shells) {
       if (Shell.m_basename == basename) {
         escapables = Shell.m_escapables;

lldb/unittests/Utility/ArgsTest.cpp

TEST(ArgsTest, GetShellSafeArgument) {
   EXPECT_EQ(Args::GetShellSafeArgument(bash, "a\"b"), "a\\\"b");
 
   FileSpec zsh("/bin/zsh", FileSpec::Style::posix);
   EXPECT_EQ(Args::GetShellSafeArgument(zsh, R"('";()<>&|\)"),
             R"(\'\"\;\(\)\<\>\&\|\\)");
   // Normal characters and expressions that shouldn't be escaped.
   EXPECT_EQ(Args::GetShellSafeArgument(zsh, "aA$1*"), "aA$1*");
 
-  // String that doesn't need to be escaped
-  EXPECT_EQ(Args::GetShellSafeArgument(bash, "a"), "a");
+  // Test escaping bash special characters.
+  EXPECT_EQ(Args::GetShellSafeArgument(bash, R"( '"<>()&;)"),
+            R"(\ \'\"\<\>\(\)\&\;)");
+  // Normal characters and globbing expressions that shouldn't be escaped.
+  EXPECT_EQ(Args::GetShellSafeArgument(bash, "aA$1*"), "aA$1*");
 
-  // Try escaping with tcsh and the tcsh-specific "$" escape.
+  // Test escaping tcsh special characters.
   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");
+  EXPECT_EQ(Args::GetShellSafeArgument(tcsh, R"( '"<>()&$;)"),
+            R"(\ \'\"\<\>\(\)\&\$\;)");
+  // Normal characters and globbing expressions that shouldn't be escaped.
+  EXPECT_EQ(Args::GetShellSafeArgument(tcsh, "aA1*"), "aA1*");
+
+  // Test escaping sh special characters.
+  FileSpec sh("/bin/sh", FileSpec::Style::posix);
+  EXPECT_EQ(Args::GetShellSafeArgument(sh, R"( '"<>()&;)"),
+            R"(\ \'\"\<\>\(\)\&\;)");
+  // Normal characters and globbing expressions that shouldn't be escaped.
+  EXPECT_EQ(Args::GetShellSafeArgument(sh, "aA$1*"), "aA$1*");
 
   // 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");
 }