llgs: Propagate the environment when launching the inferior from command line

Summary:
We were failing to propagate the environment when lldb-server was
started with a pre-loaded process
(e.g.: lldb-server gdbserver -- inferior --inferior_args)

This patch makes sure the environment is propagated. Instead of adding a
new GDBRemoteCommunicationServerLLGS::SetLaunchEnvironment function to
complement SetLaunchArgs and SetLaunchFlags, I replace these with a
more generic SetLaunchInfo, which can be used to set any launch-related
property.

The accompanying test also verifies that the server correctly terminates
the connection after sending the exit packet (specifically, that it does
not send the exit packet twice).

Reviewers: clayborg, eugene

Subscribers: lldb-commits, mgorny

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

llvm-svn: 320984

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

@@ -204,21 +204,8 @@ void GDBRemoteCommunicationServerLLGS::RegisterPacketHandlers() {
                         });
 }
 
-Status
-GDBRemoteCommunicationServerLLGS::SetLaunchArguments(const char *const args[],
-                                                     int argc) {
-  if ((argc < 1) || !args || !args[0] || !args[0][0])
-    return Status("%s: no process command line specified to launch",
-                  __FUNCTION__);
-
-  m_process_launch_info.SetArguments(const_cast<const char **>(args), true);
-  return Status();
-}
-
-Status
-GDBRemoteCommunicationServerLLGS::SetLaunchFlags(unsigned int launch_flags) {
-  m_process_launch_info.GetFlags().Set(launch_flags);
-  return Status();
+void GDBRemoteCommunicationServerLLGS::SetLaunchInfo(const ProcessLaunchInfo &info) {
+  m_process_launch_info = info;
 }
 
 Status GDBRemoteCommunicationServerLLGS::LaunchProcess() {

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

@@ -43,32 +43,7 @@ public:
       MainLoop &mainloop,
       const NativeProcessProtocol::Factory &process_factory);
 
-  //------------------------------------------------------------------
-  /// Specify the program to launch and its arguments.
-  ///
-  /// @param[in] args
-  ///     The command line to launch.
-  ///
-  /// @param[in] argc
-  ///     The number of elements in the args array of cstring pointers.
-  ///
-  /// @return
-  ///     An Status object indicating the success or failure of making
-  ///     the setting.
-  //------------------------------------------------------------------
-  Status SetLaunchArguments(const char *const args[], int argc);
-
-  //------------------------------------------------------------------
-  /// Specify the launch flags for the process.
-  ///
-  /// @param[in] launch_flags
-  ///     The launch flags to use when launching this process.
-  ///
-  /// @return
-  ///     An Status object indicating the success or failure of making
-  ///     the setting.
-  //------------------------------------------------------------------
-  Status SetLaunchFlags(unsigned int launch_flags);
+  void SetLaunchInfo(const ProcessLaunchInfo &info);
 
   //------------------------------------------------------------------
   /// Launch a process with the current launch settings.

lldb/tools/lldb-server/lldb-gdbserver.cpp

@@ -177,27 +177,28 @@ void handle_attach(GDBRemoteCommunicationServerLLGS &gdb_server,
 
 void handle_launch(GDBRemoteCommunicationServerLLGS &gdb_server, int argc,
                    const char *const argv[]) {
-  Status error;
-  error = gdb_server.SetLaunchArguments(argv, argc);
-  if (error.Fail()) {
-    fprintf(stderr, "error: failed to set launch args for '%s': %s\n", argv[0],
-            error.AsCString());
+  ProcessLaunchInfo info;
+  info.GetFlags().Set(eLaunchFlagStopAtEntry | eLaunchFlagDebug |
+                      eLaunchFlagDisableASLR);
+  info.SetArguments(const_cast<const char **>(argv), true);
+
+  llvm::SmallString<64> cwd;
+  if (std::error_code ec = llvm::sys::fs::current_path(cwd)) {
+    llvm::errs() << "Error getting current directory: " << ec.message() << "\n";
     exit(1);
   }
+  info.SetWorkingDirectory(FileSpec(cwd, true));
 
-  unsigned int launch_flags = eLaunchFlagStopAtEntry | eLaunchFlagDebug;
+  StringList env;
+  Host::GetEnvironment(env);
+  info.GetEnvironmentEntries() = Args(env);
 
-  error = gdb_server.SetLaunchFlags(launch_flags);
+  gdb_server.SetLaunchInfo(info);
+
+  Status error = gdb_server.LaunchProcess();
   if (error.Fail()) {
-    fprintf(stderr, "error: failed to set launch flags for '%s': %s\n", argv[0],
-            error.AsCString());
-    exit(1);
-  }
-
-  error = gdb_server.LaunchProcess();
-  if (error.Fail()) {
-    fprintf(stderr, "error: failed to launch '%s': %s\n", argv[0],
-            error.AsCString());
+    llvm::errs() << llvm::formatv("error: failed to launch '{0}': {1}\n",
+                                  argv[0], error);
     exit(1);
   }
 }

lldb/unittests/tools/lldb-server/CMakeLists.txt

@@ -10,6 +10,7 @@ function(add_lldb_test_executable test_name)
 endfunction()
 
 add_lldb_test_executable(thread_inferior inferior/thread_inferior.cpp)
+add_lldb_test_executable(environment_check inferior/environment_check.cpp)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_definitions(-DLLDB_SERVER="$<TARGET_FILE:debugserver>")

lldb/unittests/tools/lldb-server/inferior/environment_check.cpp

@@ -0,0 +1,20 @@
+//===-- thread_inferior.cpp -------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include <string>
+#include <cstdlib>
+
+int main() {
+  const char *value = std::getenv("LLDB_TEST_MAGIC_VARIABLE");
+  if (!value)
+    return 1;
+  if (std::string(value) != "LLDB_TEST_MAGIC_VALUE")
+    return 2;
+  return 0;
+}

lldb/unittests/tools/lldb-server/tests/CMakeLists.txt

@@ -1,7 +1,8 @@
 add_lldb_unittest(LLDBServerTests
+  LLGSTest.cpp
+  MessageObjects.cpp
   TestBase.cpp
   TestClient.cpp
-  MessageObjects.cpp
   ThreadIdsInJstopinfoTest.cpp
 
   LINK_LIBS

lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp

@@ -0,0 +1,36 @@
+//===-- LLGSTest.cpp --------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "TestBase.h"
+#include "lldb/Host/Host.h"
+#include "llvm/Testing/Support/Error.h"
+
+using namespace llgs_tests;
+using namespace lldb_private;
+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(),
+                                     {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})));
+}

lldb/unittests/tools/lldb-server/tests/TestClient.cpp

@@ -89,6 +89,11 @@ Expected<std::unique_ptr<TestClient>> TestClient::launch(StringRef Log, ArrayRef
   ProcessLaunchInfo Info;
   Info.SetArchitecture(arch_spec);
   Info.SetArguments(args, true);
+
+  StringList Env;
+  Host::GetEnvironment(Env);
+  Info.GetEnvironmentEntries() = Args(Env);
+
   status = Host::LaunchProcess(Info);
   if (status.Fail())
     return status.ToError();
@@ -96,7 +101,14 @@ Expected<std::unique_ptr<TestClient>> TestClient::launch(StringRef Log, ArrayRef
   Socket *accept_socket;
   listen_socket.Accept(accept_socket);
   auto Conn = llvm::make_unique<ConnectionFileDescriptor>(accept_socket);
-  return std::unique_ptr<TestClient>(new TestClient(std::move(Conn)));
+  auto Client = std::unique_ptr<TestClient>(new TestClient(std::move(Conn)));
+
+  if (!InferiorArgs.empty()) {
+    if (Error E = Client->QueryProcessInfo())
+      return std::move(E);
+  }
+
+  return std::move(Client);
 }
 
 Error TestClient::SetInferior(llvm::ArrayRef<std::string> inferior_args) {