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(); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
