labath updated this revision to Diff 127681.
labath added a comment.
Herald added a subscriber: mgorny.
New version: Make sure we respect variables set by --env and that they are not
overridden by --forward-env (the last part relies on the fact that in the
presence of multiply-defined environment variables, getenv() will pick the
first one).
https://reviews.llvm.org/D41352
Files:
tools/debugserver/source/debugserver.cpp
unittests/tools/lldb-server/CMakeLists.txt
unittests/tools/lldb-server/tests/LLGSTest.cpp
unittests/tools/lldb-server/tests/TestClient.cpp
unittests/tools/lldb-server/tests/TestClient.h
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 <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();
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<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)
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<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})));
+}
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="$<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(
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().PushEnvironment(env_entry);
+ }
+
RNBRunLoopMode mode = start_mode;
char err_str[1024] = {'\0'};
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
