[Lldb-commits] [lldb] r321355 - debugserver: Propagate environment in launch-mode (pr35671)

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Fri Dec 22 03:09:22 PST 2017


Author: labath
Date: Fri Dec 22 03:09:21 2017
New Revision: 321355

URL: http://llvm.org/viewvc/llvm-project?rev=321355&view=rev
Log:
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

Modified:
    lldb/trunk/tools/debugserver/source/RNBContext.cpp
    lldb/trunk/tools/debugserver/source/RNBContext.h
    lldb/trunk/tools/debugserver/source/debugserver.cpp
    lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt
    lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp
    lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
    lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h

Modified: lldb/trunk/tools/debugserver/source/RNBContext.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBContext.cpp?rev=321355&r1=321354&r2=321355&view=diff
==============================================================================
--- lldb/trunk/tools/debugserver/source/RNBContext.cpp (original)
+++ lldb/trunk/tools/debugserver/source/RNBContext.cpp Fri Dec 22 03:09:21 2017
@@ -42,6 +42,25 @@ const char *RNBContext::EnvironmentAtInd
     return NULL;
 }
 
+static std::string GetEnvironmentKey(const std::string &env) {
+  std::string key = env.substr(0, env.find('='));
+  if (!key.empty() && key.back() == '=')
+    key.pop_back();
+  return key;
+}
+
+void RNBContext::PushEnvironmentIfNeeded(const char *arg) {
+  if (!arg)
+    return;
+  std::string arg_key = GetEnvironmentKey(arg);
+
+  for (const std::string &entry: m_env_vec) {
+    if (arg_key == GetEnvironmentKey(entry))
+      return;
+  }
+  m_env_vec.push_back(arg);
+}
+
 const char *RNBContext::ArgumentAtIndex(size_t index) {
   if (index < m_arg_vec.size())
     return m_arg_vec[index].c_str();

Modified: lldb/trunk/tools/debugserver/source/RNBContext.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBContext.h?rev=321355&r1=321354&r2=321355&view=diff
==============================================================================
--- lldb/trunk/tools/debugserver/source/RNBContext.h (original)
+++ lldb/trunk/tools/debugserver/source/RNBContext.h Fri Dec 22 03:09:21 2017
@@ -86,6 +86,7 @@ public:
     if (arg)
       m_env_vec.push_back(arg);
   }
+  void PushEnvironmentIfNeeded(const char *arg);
   void ClearEnvironment() {
     m_env_vec.erase(m_env_vec.begin(), m_env_vec.end());
   }

Modified: lldb/trunk/tools/debugserver/source/debugserver.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/debugserver.cpp?rev=321355&r1=321354&r2=321355&view=diff
==============================================================================
--- lldb/trunk/tools/debugserver/source/debugserver.cpp (original)
+++ lldb/trunk/tools/debugserver/source/debugserver.cpp Fri Dec 22 03:09:21 2017
@@ -1020,6 +1020,7 @@ int main(int argc, char *argv[]) {
   optind = 1;
 #endif
 
+  bool forward_env = false;
   while ((ch = getopt_long_only(argc, argv, short_options, g_long_options,
                                 &long_option_index)) != -1) {
     DNBLogDebug("option: ch == %c (0x%2.2x) --%s%c%s\n", ch, (uint8_t)ch,
@@ -1251,14 +1252,7 @@ int main(int argc, char *argv[]) {
       break;
 
     case 'F':
-      // Pass the current environment down to the process that gets launched
-      {
-        char **host_env = *_NSGetEnviron();
-        char *env_entry;
-        size_t i;
-        for (i = 0; (env_entry = host_env[i]) != NULL; ++i)
-          remote->Context().PushEnvironment(env_entry);
-      }
+      forward_env = true;
       break;
 
     case '2':
@@ -1420,6 +1414,18 @@ int main(int argc, char *argv[]) {
   if (start_mode == eRNBRunLoopModeExit)
     return -1;
 
+  if (forward_env || start_mode == eRNBRunLoopModeInferiorLaunching) {
+    // Pass the current environment down to the process that gets launched
+    // This happens automatically in the "launching" mode. For the rest, we
+    // only do that if the user explicitly requested this via --forward-env
+    // argument.
+    char **host_env = *_NSGetEnviron();
+    char *env_entry;
+    size_t i;
+    for (i = 0; (env_entry = host_env[i]) != NULL; ++i)
+      remote->Context().PushEnvironmentIfNeeded(env_entry);
+  }
+
   RNBRunLoopMode mode = start_mode;
   char err_str[1024] = {'\0'};
 

Modified: lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt?rev=321355&r1=321354&r2=321355&view=diff
==============================================================================
--- lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt (original)
+++ lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt Fri Dec 22 03:09:21 2017
@@ -13,9 +13,9 @@ add_lldb_test_executable(thread_inferior
 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(

Modified: lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp?rev=321355&r1=321354&r2=321355&view=diff
==============================================================================
--- lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp (original)
+++ lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp Fri Dec 22 03:09:21 2017
@@ -16,17 +16,29 @@ 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})));
+}
+
+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(

Modified: lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp?rev=321355&r1=321354&r2=321355&view=diff
==============================================================================
--- lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp (original)
+++ lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp Fri Dec 22 03:09:21 2017
@@ -27,11 +27,6 @@ using namespace lldb_private;
 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>> Te
 }
 
 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 @@ Expected<std::unique_ptr<TestClient>> Te
   args.AppendArgument(
       ("localhost:" + Twine(listen_socket.GetLocalPortNumber())).str());
 
+  for (StringRef arg : ServerArgs)
+    args.AppendArgument(arg);
+
   if (!InferiorArgs.empty()) {
     args.AppendArgument("--");
     for (StringRef arg : InferiorArgs)

Modified: lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h?rev=321355&r1=321354&r2=321355&view=diff
==============================================================================
--- lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h (original)
+++ lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h Fri Dec 22 03:09:21 2017
@@ -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 @@ public:
   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();




More information about the lldb-commits mailing list