debugserver: Propagate environment in launch-mode (pr35671)

Summary:
Make sure we propagate environment when starting debugserver with a pre-loaded
inferior. AFAIK, RNBRunLoopLaunchInferior is only called in pre-loaded inferior
scenario, so we can just pick up the debugserver environment instead of trying
to construct an envp from the (empty) context.

This makes debugserver pass an test added for an equivalent lldb-server fix.

Reviewers: jasonmolenda, clayborg

Subscribers: JDevlieghere, lldb-commits

Differential Revision: https://reviews.llvm.org/D41352

llvm-svn: 321355
diff --git a/lldb/unittests/tools/lldb-server/CMakeLists.txt b/lldb/unittests/tools/lldb-server/CMakeLists.txt
index 8641e8d..14df6e6 100644
--- a/lldb/unittests/tools/lldb-server/CMakeLists.txt
+++ b/lldb/unittests/tools/lldb-server/CMakeLists.txt
@@ -13,9 +13,9 @@
 add_lldb_test_executable(environment_check inferior/environment_check.cpp)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
-  add_definitions(-DLLDB_SERVER="$<TARGET_FILE:debugserver>")
+  add_definitions(-DLLDB_SERVER="$<TARGET_FILE:debugserver>" -DLLDB_SERVER_IS_DEBUGSERVER=1)
 else()
-  add_definitions(-DLLDB_SERVER="$<TARGET_FILE:lldb-server>")
+  add_definitions(-DLLDB_SERVER="$<TARGET_FILE:lldb-server>" -DLLDB_SERVER_IS_DEBUGSERVER=0)
 endif()
 
 add_definitions(
diff --git a/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp b/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
index 75d964e..8733ade 100644
--- a/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
+++ b/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
@@ -16,11 +16,6 @@
 using namespace llvm;
 
 TEST_F(TestBase, LaunchModePreservesEnvironment) {
-  if (TestClient::IsDebugServer()) {
-    GTEST_LOG_(WARNING) << "Test fails with debugserver: llvm.org/pr35671";
-    return;
-  }
-
   putenv(const_cast<char *>("LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE"));
 
   auto ClientOr = TestClient::launch(getLogFileName(),
@@ -34,3 +29,20 @@
       HasValue(testing::Property(&StopReply::getKind,
                                  WaitStatus{WaitStatus::Exit, 0})));
 }
+
+TEST_F(TestBase, DS_TEST(DebugserverEnv)) {
+  // Test that --env takes precedence over inherited environment variables.
+  putenv(const_cast<char *>("LLDB_TEST_MAGIC_VARIABLE=foobar"));
+
+  auto ClientOr = TestClient::launchCustom(getLogFileName(),
+      { "--env", "LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE" },
+                                     {getInferiorPath("environment_check")});
+  ASSERT_THAT_EXPECTED(ClientOr, Succeeded());
+  auto &Client = **ClientOr;
+
+  ASSERT_THAT_ERROR(Client.ContinueAll(), Succeeded());
+  ASSERT_THAT_EXPECTED(
+      Client.GetLatestStopReplyAs<StopReplyExit>(),
+      HasValue(testing::Property(&StopReply::getKind,
+                                 WaitStatus{WaitStatus::Exit, 0})));
+}
diff --git a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
index 773466ab..4653c2d 100644
--- a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
+++ b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -27,11 +27,6 @@
 using namespace llvm;
 
 namespace llgs_tests {
-bool TestClient::IsDebugServer() {
-  return sys::path::filename(LLDB_SERVER).contains("debugserver");
-}
-
-bool TestClient::IsLldbServer() { return !IsDebugServer(); }
 
 TestClient::TestClient(std::unique_ptr<Connection> Conn) {
   SetConnection(Conn.release());
@@ -56,6 +51,10 @@
 }
 
 Expected<std::unique_ptr<TestClient>> TestClient::launch(StringRef Log, ArrayRef<StringRef> InferiorArgs) {
+  return launchCustom(Log, {}, InferiorArgs);
+}
+
+Expected<std::unique_ptr<TestClient>> TestClient::launchCustom(StringRef Log, ArrayRef<StringRef> ServerArgs, ArrayRef<StringRef> InferiorArgs) {
   const ArchSpec &arch_spec = HostInfo::GetArchitecture();
   Args args;
   args.AppendArgument(LLDB_SERVER);
@@ -80,6 +79,9 @@
   args.AppendArgument(
       ("localhost:" + Twine(listen_socket.GetLocalPortNumber())).str());
 
+  for (StringRef arg : ServerArgs)
+    args.AppendArgument(arg);
+
   if (!InferiorArgs.empty()) {
     args.AppendArgument("--");
     for (StringRef arg : InferiorArgs)
diff --git a/lldb/unittests/tools/lldb-server/tests/TestClient.h b/lldb/unittests/tools/lldb-server/tests/TestClient.h
index 4fa1bbb..b519530 100644
--- a/lldb/unittests/tools/lldb-server/tests/TestClient.h
+++ b/lldb/unittests/tools/lldb-server/tests/TestClient.h
@@ -21,12 +21,20 @@
 #include <memory>
 #include <string>
 
+#if LLDB_SERVER_IS_DEBUGSERVER
+#define LLGS_TEST(x) DISABLED_ ## x
+#define DS_TEST(x) x
+#else
+#define LLGS_TEST(x) x
+#define DS_TEST(x) DISABLED_ ## x
+#endif
+
 namespace llgs_tests {
 class TestClient
     : public lldb_private::process_gdb_remote::GDBRemoteCommunicationClient {
 public:
-  static bool IsDebugServer();
-  static bool IsLldbServer();
+  static bool IsDebugServer() { return LLDB_SERVER_IS_DEBUGSERVER; }
+  static bool IsLldbServer() { return !IsDebugServer(); }
 
   /// Launches the server, connects it to the client and returns the client. If
   /// Log is non-empty, the server will write it's log to this file.
@@ -37,6 +45,13 @@
   static llvm::Expected<std::unique_ptr<TestClient>>
   launch(llvm::StringRef Log, llvm::ArrayRef<llvm::StringRef> InferiorArgs);
 
+  /// Allows user to pass additional arguments to the server. Be careful when
+  /// using this for generic tests, as the two stubs have different
+  /// command-line interfaces.
+  static llvm::Expected<std::unique_ptr<TestClient>>
+  launchCustom(llvm::StringRef Log, llvm::ArrayRef<llvm::StringRef> ServerArgs, llvm::ArrayRef<llvm::StringRef> InferiorArgs);
+
+
   ~TestClient() override;
   llvm::Error SetInferior(llvm::ArrayRef<std::string> inferior_args);
   llvm::Error ListThreadsInStopReply();