[lldb] Expand $ when using tcsh

[JDevlieghere]

Summary

Unlike for any of the other shells, we were escaping $ when using tcsh.
There's nothing special about $ in tcsh and this prevents you from
expanding shell variables, one of the main reasons this functionality
exists in the first place.

[jingham]

tcsh is different from the other shells in that a non-existent variable is trapped by the shell, and an error produced BEFORE running the command, whereas in other shells it just gets passed raw. So we should make sure that passing a non-existent shell variable produces a reasonable error. At present it sort of does:

(lldb) process launch --shell=tcsh -- $PWD $NO_SUCH_SHELL_VARIABLE
NO_SUCH_SHELL_VARIABLE: Undefined variable.
error: process exited with status 1
'r' and 'run' are aliases that default to launching through a shell.
Try launching without going through a shell by using 'process launch'.
(lldb)

The advisory part of that message is a little bogus, but that's a separate issue. Erroring out seems reasonable since that's actually what tcsh would have done with these arguments, and we seem in a fine state to try again.

It might be nice to have a test where we pass a bad variable and make sure that the error stays reasonable? But that's on the nice to have side...

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"), " '\"<>()&$;"},
+                                       {ConstString("tcsh"), " '\"<>()&;"},
                                        {ConstString("zsh"), " '\"<>()&;\\|"},
                                        {ConstString("sh"), " '\"<>()&;"}};
 
   // safe minimal set
   llvm::StringRef escapables = " '\"";
 
   if (auto basename = shell.GetFilename()) {
     for (const auto &Shell : g_Shells) {

lldb/test/Shell/Process/Inputs/echo.c

+#include <stdio.h>
+
+int main(int argc, char **argv) {
+  for (int i = 0; i < argc; ++i)
+    printf("%s\n", argv[i]);
+}

lldb/test/Shell/Process/TestShell.test

+REQUIRES: shell, system-darwin
+RUN: %clang_host %p/Inputs/echo.c -o %t.out
+
+RUN: not %lldb %t.out -b -o 'process launch --shell=tcsh -- $NO_SUCH_SHELL_VARIABLE' 2>&1 | FileCheck --check-prefix=FAILURE %s
+RUN: %lldb %t.out -b -o 'process launch --shell=tcsh -- $HOME' 2>&1 | FileCheck --check-prefix=SUCCESS %s
+
+FAILURE: exited with status 1
+SUCCESS: exited with status = 0

lldb/unittests/Utility/ArgsTest.cpp

TEST(ArgsTest, GetShellSafeArgument) {
   // 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*");
 
   // Test escaping tcsh special characters.
   FileSpec tcsh("/bin/tcsh", FileSpec::Style::posix);
-  EXPECT_EQ(Args::GetShellSafeArgument(tcsh, R"( '"<>()&$;)"),
-            R"(\ \'\"\<\>\(\)\&\$\;)");
+  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");
 }