Index: tools/debugserver/source/RNBContext.h =================================================================== --- tools/debugserver/source/RNBContext.h +++ tools/debugserver/source/RNBContext.h @@ -86,6 +86,7 @@ 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()); } Index: tools/debugserver/source/RNBContext.cpp =================================================================== --- tools/debugserver/source/RNBContext.cpp +++ tools/debugserver/source/RNBContext.cpp @@ -42,6 +42,25 @@ 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(); Index: tools/debugserver/source/debugserver.cpp =================================================================== --- tools/debugserver/source/debugserver.cpp +++ tools/debugserver/source/debugserver.cpp @@ -1020,6 +1020,7 @@ 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 @@ 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 @@ 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'}; Index: unittests/tools/lldb-server/CMakeLists.txt =================================================================== --- unittests/tools/lldb-server/CMakeLists.txt +++ 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="$") + add_definitions(-DLLDB_SERVER="$" -DLLDB_SERVER_IS_DEBUGSERVER=1) else() - add_definitions(-DLLDB_SERVER="$") + add_definitions(-DLLDB_SERVER="$" -DLLDB_SERVER_IS_DEBUGSERVER=0) endif() add_definitions( Index: unittests/tools/lldb-server/tests/LLGSTest.cpp =================================================================== --- unittests/tools/lldb-server/tests/LLGSTest.cpp +++ 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("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("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(), + HasValue(testing::Property(&StopReply::getKind, + WaitStatus{WaitStatus::Exit, 0}))); +} Index: unittests/tools/lldb-server/tests/TestClient.h =================================================================== --- unittests/tools/lldb-server/tests/TestClient.h +++ unittests/tools/lldb-server/tests/TestClient.h @@ -21,12 +21,20 @@ #include #include +#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> launch(llvm::StringRef Log, llvm::ArrayRef 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> + launchCustom(llvm::StringRef Log, llvm::ArrayRef ServerArgs, llvm::ArrayRef InferiorArgs); + + ~TestClient() override; llvm::Error SetInferior(llvm::ArrayRef inferior_args); llvm::Error ListThreadsInStopReply(); Index: unittests/tools/lldb-server/tests/TestClient.cpp =================================================================== --- unittests/tools/lldb-server/tests/TestClient.cpp +++ 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 Conn) { SetConnection(Conn.release()); @@ -56,6 +51,10 @@ } Expected> TestClient::launch(StringRef Log, ArrayRef InferiorArgs) { + return launchCustom(Log, {}, InferiorArgs); +} + +Expected> TestClient::launchCustom(StringRef Log, ArrayRef ServerArgs, ArrayRef 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)