labath created this revision. labath added reviewers: zturner, davide, jingham, clayborg. Herald added subscribers: mgorny, emaste.
There was some confusion in the code about how to represent process environment. Most of the code (ab)used the Args class for this purpose, but some of it used a more basic StringList class instead. In either case, the fact that the underlying abstraction did not provide primitive operations for the typical environment operations meant that even a simple operation like checking for an environment variable value was several lines of code. This patch adds a separate Environment class, which is essentialy a llvm::StringMap<std::string> in disguise. To standard StringMap functionality, it adds a couple of new functions, which are specific to the environment use case: - (most important) envp conversion for passing into execve() and likes. Instead of trying to maintain a constantly up-to-date envp view, it provides a function which creates a envp view on demand, with the expectation that this will be called as the very last thing before handing the value to the system function. - insert(StringRef KeyEqValue) - splits KeyEqValue into (key, value) pair and inserts it into the environment map. - compose(value_type KeyValue) - takes a map entry and converts in back into "KEY=VALUE" representation. With this interface most of the environment-manipulating code becomes one-liners. The only tricky part was maintaining compatibility in SBLaunchInfo, which expects that the environment entries are accessible by index and that the returned const char* is backed by the launch info object (random access into maps is hard and the map stores the entry in a deconstructed form, so we cannot just return a .c_str() value). To solve this, I have the SBLaunchInfo convert the environment into the "envp" form, and use it to answer the environment queries. Extra code is added to make sure the envp version is always in sync. (This also improves the layering situation as Args was in the Interpreter module whereas Environment is in Utility.) https://reviews.llvm.org/D41359 Files: include/lldb/API/SBLaunchInfo.h include/lldb/Host/Host.h include/lldb/Interpreter/Args.h include/lldb/Target/Platform.h include/lldb/Target/ProcessInfo.h include/lldb/Target/Target.h include/lldb/Utility/Environment.h packages/Python/lldbsuite/test/python_api/sblaunchinfo/TestSBLaunchInfo.py source/API/SBLaunchInfo.cpp source/API/SBPlatform.cpp source/API/SBProcess.cpp source/API/SBTarget.cpp source/Commands/CommandObjectProcess.cpp source/Host/freebsd/Host.cpp source/Host/linux/Host.cpp source/Host/macosx/Host.mm source/Host/netbsd/Host.cpp source/Host/openbsd/Host.cpp source/Host/posix/ProcessLauncherPosixFork.cpp source/Host/windows/Host.cpp source/Interpreter/Args.cpp source/Plugins/Platform/MacOSX/PlatformDarwin.cpp source/Plugins/Platform/MacOSX/PlatformiOSSimulatorCoreSimulatorSupport.mm source/Plugins/Platform/POSIX/PlatformPOSIX.cpp source/Plugins/Platform/POSIX/PlatformPOSIX.h source/Plugins/Platform/Windows/PlatformWindows.cpp source/Plugins/Platform/Windows/PlatformWindows.h source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp source/Target/Platform.cpp source/Target/Process.cpp source/Target/ProcessInfo.cpp source/Target/Target.cpp source/Utility/CMakeLists.txt source/Utility/Environment.cpp tools/lldb-server/lldb-gdbserver.cpp unittests/Host/HostTest.cpp unittests/Interpreter/TestArgs.cpp unittests/Utility/CMakeLists.txt unittests/Utility/EnvironmentTest.cpp unittests/tools/lldb-server/tests/TestClient.cpp
Index: unittests/tools/lldb-server/tests/TestClient.cpp =================================================================== --- unittests/tools/lldb-server/tests/TestClient.cpp +++ unittests/tools/lldb-server/tests/TestClient.cpp @@ -89,10 +89,7 @@ ProcessLaunchInfo Info; Info.SetArchitecture(arch_spec); Info.SetArguments(args, true); - - StringList Env; - Host::GetEnvironment(Env); - Info.GetEnvironmentEntries() = Args(Env); + Info.GetEnvironment() = Host::GetEnvironment(); status = Host::LaunchProcess(Info); if (status.Fail()) @@ -112,14 +109,9 @@ } Error TestClient::SetInferior(llvm::ArrayRef<std::string> inferior_args) { - StringList env; - Host::GetEnvironment(env); - for (size_t i = 0; i < env.GetSize(); ++i) { - if (SendEnvironmentPacket(env[i].c_str()) != 0) { - return make_error<StringError>( - formatv("Failed to set environment variable: {0}", env[i]).str(), - inconvertibleErrorCode()); - } + if (SendEnvironment(Host::GetEnvironment()) != 0) { + return make_error<StringError>("Failed to set launch environment", + inconvertibleErrorCode()); } std::stringstream command; command << "A"; Index: unittests/Utility/EnvironmentTest.cpp =================================================================== --- /dev/null +++ unittests/Utility/EnvironmentTest.cpp @@ -0,0 +1,49 @@ +//===-- EnvironmentTest.cpp -------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "gtest/gtest.h" + +#include "lldb/Utility/Environment.h" + +using namespace lldb_private; + +TEST(EnvironmentTest, EnvpConstruction) { + const char **Envp1 = nullptr; + EXPECT_EQ(0u, Environment(Envp1).size()); + + const char *Envp2[] = {"FOO=BAR", nullptr}; + EXPECT_EQ("BAR", Environment(Envp2).lookup("FOO")); + + const char *Envp3[] = {"FOO=BAR", "FOO=BAZ", nullptr}; + EXPECT_EQ("BAR", Environment(Envp3).lookup("FOO")); + + const char *Envp4[] = {"FOO=", "BAR", nullptr}; + Environment Env4(Envp4); + ASSERT_EQ(2u, Env4.size()); + EXPECT_EQ("", Environment(Envp4).find("FOO")->second); + EXPECT_EQ("", Environment(Envp4).find("BAR")->second); + + const char *Envp5[] = {"FOO=BAR=BAZ"}; + EXPECT_EQ("BAR=BAZ", Environment(Envp5).lookup("FOO")); +} + +TEST(EnvironmentTest, EnvpConversion) { + std::string FOO_EQ_BAR("FOO=BAR"); + std::string BAR_EQ_BAZ("BAR=BAZ"); + + Environment Env; + Env.insert(FOO_EQ_BAR); + Env.insert(BAR_EQ_BAZ); + Environment::Envp Envp = Env.getEnvp(); + const char * const *Envp_ = Envp; + + EXPECT_TRUE(FOO_EQ_BAR == Envp_[0] || FOO_EQ_BAR == Envp_[1]); + EXPECT_TRUE(BAR_EQ_BAZ == Envp_[0] || BAR_EQ_BAZ == Envp_[1]); + EXPECT_EQ(nullptr, Envp_[2]); +} Index: unittests/Utility/CMakeLists.txt =================================================================== --- unittests/Utility/CMakeLists.txt +++ unittests/Utility/CMakeLists.txt @@ -1,6 +1,7 @@ add_lldb_unittest(UtilityTests ArchSpecTest.cpp ConstStringTest.cpp + EnvironmentTest.cpp JSONTest.cpp LogTest.cpp NameMatchesTest.cpp Index: unittests/Interpreter/TestArgs.cpp =================================================================== --- unittests/Interpreter/TestArgs.cpp +++ unittests/Interpreter/TestArgs.cpp @@ -276,87 +276,3 @@ &success)); EXPECT_FALSE(success); } - -TEST(ArgsTest, StringToVersion) {} - -// Environment Variable Tests - -class EnvVarFixture: public ::testing::Test { -protected: - - void SetUp() { - args.AppendArgument(llvm::StringRef("Arg1=foo")); - args.AppendArgument(llvm::StringRef("Arg2")); - args.AppendArgument(llvm::StringRef("Arg3=bar")); - } - - size_t GetIndexForEnvVar(llvm::StringRef envvar_name) { - size_t argument_index = std::numeric_limits<size_t>::max(); - EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name, - &argument_index)); - EXPECT_LT(argument_index, args.GetArgumentCount()); - return argument_index; - } - - Args args; -}; - - -TEST_F(EnvVarFixture, TestContainsEnvironmentVariableNoValue) { - EXPECT_TRUE(args.ContainsEnvironmentVariable(llvm::StringRef("Arg2"))); -} - -TEST_F(EnvVarFixture, TestContainsEnvironmentVariableWithValue) { - EXPECT_TRUE(args.ContainsEnvironmentVariable(llvm::StringRef("Arg3"))); -} - -TEST_F(EnvVarFixture, TestContainsEnvironmentVariableNonExistentVariable) { - auto nonexistent_envvar = llvm::StringRef("ThisEnvVarShouldNotExist"); - EXPECT_FALSE(args.ContainsEnvironmentVariable(nonexistent_envvar)); -} - -TEST_F(EnvVarFixture, TestReplaceEnvironmentVariableInitialNoValueWithNoValue) { - auto envvar_name = llvm::StringRef("Arg2"); - auto argument_index = GetIndexForEnvVar(envvar_name); - - args.AddOrReplaceEnvironmentVariable(envvar_name, llvm::StringRef("")); - EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name)); - EXPECT_EQ(envvar_name, args.GetArgumentAtIndex(argument_index)); -} - -TEST_F(EnvVarFixture, TestReplaceEnvironmentVariableInitialNoValueWithValue) { - auto envvar_name = llvm::StringRef("Arg2"); - auto argument_index = GetIndexForEnvVar(envvar_name); - - auto new_value = llvm::StringRef("NewValue"); - args.AddOrReplaceEnvironmentVariable(envvar_name, new_value); - EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name)); - - std::stringstream stream; - stream << envvar_name.str() << '=' << new_value.str(); - EXPECT_EQ(llvm::StringRef(stream.str()), - args.GetArgumentAtIndex(argument_index)); -} - -TEST_F(EnvVarFixture, TestReplaceEnvironmentVariableInitialValueWithNoValue) { - auto envvar_name = llvm::StringRef("Arg1"); - auto argument_index = GetIndexForEnvVar(envvar_name); - - args.AddOrReplaceEnvironmentVariable(envvar_name, llvm::StringRef("")); - EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name)); - EXPECT_EQ(envvar_name, args.GetArgumentAtIndex(argument_index)); -} - -TEST_F(EnvVarFixture, TestReplaceEnvironmentVariableInitialValueWithValue) { - auto envvar_name = llvm::StringRef("Arg1"); - auto argument_index = GetIndexForEnvVar(envvar_name); - - auto new_value = llvm::StringRef("NewValue"); - args.AddOrReplaceEnvironmentVariable(envvar_name, new_value); - EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name)); - - std::stringstream stream; - stream << envvar_name.str() << '=' << new_value.str(); - EXPECT_EQ(llvm::StringRef(stream.str()), - args.GetArgumentAtIndex(argument_index)); -} Index: unittests/Host/HostTest.cpp =================================================================== --- unittests/Host/HostTest.cpp +++ unittests/Host/HostTest.cpp @@ -20,3 +20,9 @@ EXPECT_EQ("Exited with status 4", formatv("{0}", WaitStatus{WaitStatus::Exit, 4}).str()); } + +TEST(Host, GetEnvironment) { + putenv(const_cast<char *>("LLDB_TEST_ENVIRONMENT_VAR=Host::GetEnvironment")); + ASSERT_EQ("Host::GetEnvironment", + Host::GetEnvironment().lookup("LLDB_TEST_ENVIRONMENT_VAR")); +} Index: tools/lldb-server/lldb-gdbserver.cpp =================================================================== --- tools/lldb-server/lldb-gdbserver.cpp +++ tools/lldb-server/lldb-gdbserver.cpp @@ -188,10 +188,7 @@ exit(1); } info.SetWorkingDirectory(FileSpec(cwd, true)); - - StringList env; - Host::GetEnvironment(env); - info.GetEnvironmentEntries() = Args(env); + info.GetEnvironment() = Host::GetEnvironment(); gdb_server.SetLaunchInfo(info); Index: source/Utility/Environment.cpp =================================================================== --- /dev/null +++ source/Utility/Environment.cpp @@ -0,0 +1,49 @@ +//===-- Environment.cpp -----------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "lldb/Utility/Environment.h" + +using namespace lldb_private; + +char *Environment::Envp::make_entry(llvm::StringRef Key, llvm::StringRef Value) { + const size_t size = Key.size() + 1 /*=*/ + Value.size() + 1 /*\0*/; + char *Result = reinterpret_cast<char *>( + Allocator.Allocate(sizeof(char) * size, alignof(char))); + char *Next = Result; + + Next = std::copy(Key.begin(), Key.end(), Next); + *Next++ = '='; + Next = std::copy(Value.begin(), Value.end(), Next); + *Next++ = '\0'; + + return Result; +} + +Environment::Envp::Envp(const Environment &Env) { + Data = reinterpret_cast<char **>( + Allocator.Allocate(sizeof(char *) * (Env.size() + 1), alignof(char *))); + char **Next = Data; + for (const auto &KV : Env) + *Next++ = make_entry(KV.first(), KV.second); + *Next++ = nullptr; +} + +Environment::Environment(const char *const *Env) { + if (!Env) + return; + while(*Env) + insert(*Env++); +} + +void Environment::insert(const_iterator first, const_iterator last) { + while (first != last) { + try_emplace(first->first(), first->second); + ++first; + } +} Index: source/Utility/CMakeLists.txt =================================================================== --- source/Utility/CMakeLists.txt +++ source/Utility/CMakeLists.txt @@ -48,6 +48,7 @@ DataBufferLLVM.cpp DataEncoder.cpp DataExtractor.cpp + Environment.cpp FastDemangle.cpp FileSpec.cpp History.cpp Index: source/Target/Target.cpp =================================================================== --- source/Target/Target.cpp +++ source/Target/Target.cpp @@ -3611,36 +3611,19 @@ nullptr, idx, g_properties[idx].default_uint_value != 0)) { PlatformSP platform_sp(m_target->GetPlatform()); if (platform_sp) { - StringList env; - if (platform_sp->GetEnvironment(env)) { - OptionValueDictionary *env_dict = - GetPropertyAtIndexAsOptionValueDictionary(nullptr, - ePropertyEnvVars); - if (env_dict) { - const bool can_replace = false; - const size_t envc = env.GetSize(); - for (size_t idx = 0; idx < envc; idx++) { - const char *env_entry = env.GetStringAtIndex(idx); - if (env_entry) { - const char *equal_pos = ::strchr(env_entry, '='); - ConstString key; - // It is ok to have environment variables with no values - const char *value = nullptr; - if (equal_pos) { - key.SetCStringWithLength(env_entry, - equal_pos - env_entry); - if (equal_pos[1]) - value = equal_pos + 1; - } else { - key.SetCString(env_entry); - } - // Don't allow existing keys to be replaced with ones we get - // from the platform environment - env_dict->SetValueForKey( - key, OptionValueSP(new OptionValueString(value)), - can_replace); - } - } + Environment env = platform_sp->GetEnvironment(); + OptionValueDictionary *env_dict = + GetPropertyAtIndexAsOptionValueDictionary(nullptr, + ePropertyEnvVars); + if (env_dict) { + const bool can_replace = false; + for (const auto &KV : env) { + // Don't allow existing keys to be replaced with ones we get + // from the platform environment + env_dict->SetValueForKey( + ConstString(KV.first()), + OptionValueSP(new OptionValueString(KV.second.c_str())), + can_replace); } } } @@ -3906,15 +3889,19 @@ m_launch_info.GetArguments() = args; } -size_t TargetProperties::GetEnvironmentAsArgs(Args &env) const { +Environment TargetProperties::GetEnvironment() const { + // TODO: Get rid of the Args intermediate step + Args env; const uint32_t idx = ePropertyEnvVars; - return m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, idx, env); + m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, idx, env); + return Environment(env); } -void TargetProperties::SetEnvironmentFromArgs(const Args &env) { +void TargetProperties::SetEnvironment(Environment env) { + // TODO: Get rid of the Args intermediate step const uint32_t idx = ePropertyEnvVars; - m_collection_sp->SetPropertyAtIndexFromArgs(nullptr, idx, env); - m_launch_info.GetEnvironmentEntries() = env; + m_collection_sp->SetPropertyAtIndexFromArgs(nullptr, idx, Args(env)); + m_launch_info.GetEnvironment() = std::move(env); } bool TargetProperties::GetSkipPrologue() const { @@ -4151,7 +4138,7 @@ m_launch_info = launch_info; SetArg0(launch_info.GetArg0()); SetRunArguments(launch_info.GetArguments()); - SetEnvironmentFromArgs(launch_info.GetEnvironmentEntries()); + SetEnvironment(launch_info.GetEnvironment()); const FileAction *input_file_action = launch_info.GetFileActionForFD(STDIN_FILENO); if (input_file_action) { @@ -4192,9 +4179,7 @@ OptionValue *) { TargetProperties *this_ = reinterpret_cast<TargetProperties *>(target_property_ptr); - Args args; - if (this_->GetEnvironmentAsArgs(args)) - this_->m_launch_info.GetEnvironmentEntries() = args; + this_->m_launch_info.GetEnvironment() = this_->GetEnvironment(); } void TargetProperties::InputPathValueChangedCallback(void *target_property_ptr, Index: source/Target/ProcessInfo.cpp =================================================================== --- source/Target/ProcessInfo.cpp +++ source/Target/ProcessInfo.cpp @@ -35,7 +35,7 @@ void ProcessInfo::Clear() { m_executable.Clear(); m_arguments.Clear(); - m_environment.Clear(); + m_environment.clear(); m_uid = UINT32_MAX; m_gid = UINT32_MAX; m_arch.Clear(); @@ -59,8 +59,7 @@ s << "Arguments:\n"; m_arguments.Dump(s); - s << "Environment:\n"; - m_environment.Dump(s, "env"); + s.Format("Environment:\n{0}", m_environment); } void ProcessInfo::SetExecutableFile(const FileSpec &exe_file, Index: source/Target/Process.cpp =================================================================== --- source/Target/Process.cpp +++ source/Target/Process.cpp @@ -307,16 +307,7 @@ } } - const uint32_t envc = m_environment.GetArgumentCount(); - if (envc > 0) { - for (uint32_t i = 0; i < envc; i++) { - const char *env = m_environment.GetArgumentAtIndex(i); - if (i < 10) - s.Printf(" env[%u] = %s\n", i, env); - else - s.Printf("env[%u] = %s\n", i, env); - } - } + s.Format("{0}", m_environment); if (m_arch.IsValid()) { s.Printf(" arch = "); @@ -529,7 +520,7 @@ break; case 'v': - launch_info.GetEnvironmentEntries().AppendArgument(option_arg); + launch_info.GetEnvironment().insert(option_arg); break; default: Index: source/Target/Platform.cpp =================================================================== --- source/Target/Platform.cpp +++ source/Target/Platform.cpp @@ -1539,10 +1539,7 @@ return error; } -size_t Platform::GetEnvironment(StringList &environment) { - environment.Clear(); - return false; -} +Environment Platform::GetEnvironment() { return Environment(); } const std::vector<ConstString> &Platform::GetTrapHandlerSymbolNames() { if (!m_calculated_trap_handlers) { Index: source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp =================================================================== --- source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp +++ source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp @@ -1514,25 +1514,19 @@ SetGlobalEnableOptions(debugger_sp, options_sp); } - auto &env_vars = launch_info.GetEnvironmentEntries(); if (!options_sp->GetEchoToStdErr()) { // The user doesn't want to see os_log/NSLog messages echo to stderr. // That mechanism is entirely separate from the DarwinLog support. // By default we don't want to get it via stderr, because that would // be in duplicate of the explicit log support here. // Here we need to strip out any OS_ACTIVITY_DT_MODE setting to prevent // echoing of os_log()/NSLog() to stderr in the target program. - size_t argument_index; - if (env_vars.ContainsEnvironmentVariable( - llvm::StringRef("OS_ACTIVITY_DT_MODE"), &argument_index)) - env_vars.DeleteArgumentAtIndex(argument_index); + launch_info.GetEnvironment().erase("OS_ACTIVITY_DT_MODE"); // We will also set the env var that tells any downstream launcher // from adding OS_ACTIVITY_DT_MODE. - env_vars.AddOrReplaceEnvironmentVariable( - llvm::StringRef("IDE_DISABLED_OS_ACTIVITY_DT_MODE"), - llvm::StringRef("1")); + launch_info.GetEnvironment()["IDE_DISABLED_OS_ACTIVITY_DT_MODE"] = "1"; } // Set the OS_ACTIVITY_MODE env var appropriately to enable/disable @@ -1545,10 +1539,7 @@ else env_var_value = "default"; - if (env_var_value) { - launch_info.GetEnvironmentEntries().AddOrReplaceEnvironmentVariable( - llvm::StringRef("OS_ACTIVITY_MODE"), llvm::StringRef(env_var_value)); - } + launch_info.GetEnvironment()["OS_ACTIVITY_MODE"] = env_var_value; return error; } Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp =================================================================== --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -888,16 +888,7 @@ } // Send the environment and the program + arguments after we connect - const Args &environment = launch_info.GetEnvironmentEntries(); - if (environment.GetArgumentCount()) { - size_t num_environment_entries = environment.GetArgumentCount(); - for (size_t i = 0; i < num_environment_entries; ++i) { - const char *env_entry = environment.GetArgumentAtIndex(i); - if (env_entry == NULL || - m_gdb_comm.SendEnvironmentPacket(env_entry) != 0) - break; - } - } + m_gdb_comm.SendEnvironment(launch_info.GetEnvironment()); { // Scope for the scoped timeout object Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -945,8 +945,7 @@ packet.SetFilePos(::strlen("QEnvironment:")); const uint32_t bytes_left = packet.GetBytesLeft(); if (bytes_left > 0) { - m_process_launch_info.GetEnvironmentEntries().AppendArgument( - llvm::StringRef::withNullAsEmpty(packet.Peek())); + m_process_launch_info.GetEnvironment().insert(packet.Peek()); return SendOKResponse(); } return SendErrorResponse(12); @@ -960,7 +959,7 @@ if (bytes_left > 0) { std::string str; packet.GetHexByteString(str); - m_process_launch_info.GetEnvironmentEntries().AppendArgument(str); + m_process_launch_info.GetEnvironment().insert(str); return SendOKResponse(); } return SendErrorResponse(12); Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -121,6 +121,7 @@ /// response was received. //------------------------------------------------------------------ int SendEnvironmentPacket(char const *name_equal_value); + int SendEnvironment(const Environment &env); int SendLaunchArchPacket(const char *arch); Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -822,6 +822,15 @@ return -1; } +int GDBRemoteCommunicationClient::SendEnvironment(const Environment &env) { + for (const auto &KV: env) { + int r = SendEnvironmentPacket(Environment::compose(KV).c_str()); + if (r != 0) + return r; + } + return 0; +} + int GDBRemoteCommunicationClient::SendEnvironmentPacket( char const *name_equal_value) { if (name_equal_value && name_equal_value[0]) { Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -1206,11 +1206,7 @@ } // Copy the current environment to the gdbserver/debugserver instance - StringList env; - if (Host::GetEnvironment(env)) { - for (size_t i = 0; i < env.GetSize(); ++i) - launch_info.GetEnvironmentEntries().AppendArgument(env[i]); - } + launch_info.GetEnvironment() = Host::GetEnvironment(); // Close STDIN, STDOUT and STDERR. launch_info.AppendCloseFileAction(STDIN_FILENO); Index: source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp =================================================================== --- source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp +++ source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp @@ -423,16 +423,7 @@ } // Send the environment and the program + arguments after we connect - const char **envp = - launch_info.GetEnvironmentEntries().GetConstArgumentVector(); - - if (envp) { - const char *env_entry; - for (int i = 0; (env_entry = envp[i]); ++i) { - if (m_gdb_client.SendEnvironmentPacket(env_entry) != 0) - break; - } - } + m_gdb_client.SendEnvironment(launch_info.GetEnvironment()); ArchSpec arch_spec = launch_info.GetArchitecture(); const char *arch_triple = arch_spec.GetTriple().str().c_str(); Index: source/Plugins/Platform/Windows/PlatformWindows.h =================================================================== --- source/Plugins/Platform/Windows/PlatformWindows.h +++ source/Plugins/Platform/Windows/PlatformWindows.h @@ -118,7 +118,7 @@ bool CanDebugProcess() override; - size_t GetEnvironment(StringList &env) override; + Environment GetEnvironment() override; // FIXME not sure what the _sigtramp equivalent would be on this platform void CalculateTrapHandlerSymbolNames() override {} Index: source/Plugins/Platform/Windows/PlatformWindows.cpp =================================================================== --- source/Plugins/Platform/Windows/PlatformWindows.cpp +++ source/Plugins/Platform/Windows/PlatformWindows.cpp @@ -587,14 +587,14 @@ bool PlatformWindows::CanDebugProcess() { return true; } -size_t PlatformWindows::GetEnvironment(StringList &env) { +Environment PlatformWindows::GetEnvironment() { if (IsRemote()) { if (m_remote_platform_sp) - return m_remote_platform_sp->GetEnvironment(env); - return 0; + return m_remote_platform_sp->GetEnvironment(); + return Environment(); } - return Host::GetEnvironment(env); + return Host::GetEnvironment(); } ConstString PlatformWindows::GetFullNameForDylib(ConstString basename) { Index: source/Plugins/Platform/POSIX/PlatformPOSIX.h =================================================================== --- source/Plugins/Platform/POSIX/PlatformPOSIX.h +++ source/Plugins/Platform/POSIX/PlatformPOSIX.h @@ -85,7 +85,7 @@ const lldb::UnixSignalsSP &GetRemoteUnixSignals() override; - size_t GetEnvironment(lldb_private::StringList &environment) override; + lldb_private::Environment GetEnvironment() override; bool IsConnected() const override; Index: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp =================================================================== --- source/Plugins/Platform/POSIX/PlatformPOSIX.cpp +++ source/Plugins/Platform/POSIX/PlatformPOSIX.cpp @@ -658,13 +658,13 @@ return false; } -size_t PlatformPOSIX::GetEnvironment(StringList &env) { +Environment PlatformPOSIX::GetEnvironment() { if (IsRemote()) { if (m_remote_platform_sp) - return m_remote_platform_sp->GetEnvironment(env); - return 0; + return m_remote_platform_sp->GetEnvironment(); + return Environment(); } - return Host::GetEnvironment(env); + return Host::GetEnvironment(); } bool PlatformPOSIX::GetRemoteOSKernelDescription(std::string &s) { Index: source/Plugins/Platform/MacOSX/PlatformiOSSimulatorCoreSimulatorSupport.mm =================================================================== --- source/Plugins/Platform/MacOSX/PlatformiOSSimulatorCoreSimulatorSupport.mm +++ source/Plugins/Platform/MacOSX/PlatformiOSSimulatorCoreSimulatorSupport.mm @@ -478,26 +478,17 @@ [options setObject:args_array forKey:kSimDeviceSpawnArguments]; } - if (launch_info.GetEnvironmentEntries().GetArgumentCount()) { - const Args &envs(launch_info.GetEnvironmentEntries()); - NSMutableDictionary *env_dict = [[NSMutableDictionary alloc] init]; - for (size_t idx = 0; idx < envs.GetArgumentCount(); idx++) { - llvm::StringRef arg_sr(envs.GetArgumentAtIndex(idx)); - auto first_eq = arg_sr.find('='); - if (first_eq == llvm::StringRef::npos) - continue; - llvm::StringRef key = arg_sr.substr(0, first_eq); - llvm::StringRef value = arg_sr.substr(first_eq + 1); - - NSString *key_ns = [NSString stringWithUTF8String:key.str().c_str()]; - NSString *value_ns = [NSString stringWithUTF8String:value.str().c_str()]; - - [env_dict setValue:value_ns forKey:key_ns]; - } + NSMutableDictionary *env_dict = [[NSMutableDictionary alloc] init]; + + for (const auto &KV: launch_info.GetEnvironment()) { + NSString *key_ns = [NSString stringWithUTF8String:KV.first().str().c_str()]; + NSString *value_ns = [NSString stringWithUTF8String:KV.second.c_str()]; - [options setObject:env_dict forKey:kSimDeviceSpawnEnvironment]; + [env_dict setValue:value_ns forKey:key_ns]; } + [options setObject:env_dict forKey:kSimDeviceSpawnEnvironment]; + Status error; File stdin_file; File stdout_file; Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp =================================================================== --- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -1282,14 +1282,8 @@ // /bin/sh re-exec's itself as /bin/bash requiring another resume. // But it only does this if the COMMAND_MODE environment variable // is set to "legacy". - const char **envp = - launch_info.GetEnvironmentEntries().GetConstArgumentVector(); - if (envp != NULL) { - for (int i = 0; envp[i] != NULL; i++) { - if (strcmp(envp[i], "COMMAND_MODE=legacy") == 0) - return 2; - } - } + if (launch_info.GetEnvironment().lookup("COMMAND_MODE") == "legacy") + return 2; return 1; } else if (strcmp(shell_name, "csh") == 0 || strcmp(shell_name, "tcsh") == 0 || @@ -1667,25 +1661,13 @@ if (process && strstr(GetPluginName().GetCString(), "-simulator")) { lldb_private::ProcessInstanceInfo proc_info; if (Host::GetProcessInfo(process->GetID(), proc_info)) { - Args &env = proc_info.GetEnvironmentEntries(); - const size_t n = env.GetArgumentCount(); - const llvm::StringRef k_runtime_version("SIMULATOR_RUNTIME_VERSION="); - const llvm::StringRef k_dyld_root_path("DYLD_ROOT_PATH="); - std::string dyld_root_path; - - for (size_t i = 0; i < n; ++i) { - const char *env_cstr = env.GetArgumentAtIndex(i); - if (env_cstr) { - llvm::StringRef env_str(env_cstr); - if (env_str.consume_front(k_runtime_version)) { - if (Args::StringToVersion(env_str, major, minor, update)) - return true; - } else if (env_str.consume_front(k_dyld_root_path)) { - dyld_root_path = env_str; - } - } - } + const Environment &env = proc_info.GetEnvironment(); + + if (Args::StringToVersion(env.lookup("SIMULATOR_RUNTIME_VERSION"), major, + minor, update)) + return true; + std::string dyld_root_path = env.lookup("DYLD_ROOT_PATH"); if (!dyld_root_path.empty()) { dyld_root_path += "/System/Library/CoreServices/SystemVersion.plist"; ApplePropertyList system_version_plist(dyld_root_path.c_str()); @@ -1756,14 +1738,12 @@ // LLDB *not* to muck with the OS_ACTIVITY_DT_MODE flag when they // specifically want it unset. const char *disable_env_var = "IDE_DISABLED_OS_ACTIVITY_DT_MODE"; - auto &env_vars = launch_info.GetEnvironmentEntries(); - if (!env_vars.ContainsEnvironmentVariable(llvm::StringRef(disable_env_var))) { + auto &env_vars = launch_info.GetEnvironment(); + if (!env_vars.count(disable_env_var)) { // We want to make sure that OS_ACTIVITY_DT_MODE is set so that // we get os_log and NSLog messages mirrored to the target process // stderr. - if (!env_vars.ContainsEnvironmentVariable( - llvm::StringRef("OS_ACTIVITY_DT_MODE"))) - env_vars.AppendArgument(llvm::StringRef("OS_ACTIVITY_DT_MODE=enable")); + env_vars.try_emplace("OS_ACTIVITY_DT_MODE", "enable"); } // Let our parent class do the real launching. Index: source/Interpreter/Args.cpp =================================================================== --- source/Interpreter/Args.cpp +++ source/Interpreter/Args.cpp @@ -900,50 +900,6 @@ return result; } -void Args::AddOrReplaceEnvironmentVariable(llvm::StringRef env_var_name, - llvm::StringRef new_value) { - if (env_var_name.empty()) - return; - - // Build the new entry. - std::string var_string(env_var_name); - if (!new_value.empty()) { - var_string += "="; - var_string += new_value; - } - - size_t index = 0; - if (ContainsEnvironmentVariable(env_var_name, &index)) { - ReplaceArgumentAtIndex(index, var_string); - return; - } - - // We didn't find it. Append it instead. - AppendArgument(var_string); -} - -bool Args::ContainsEnvironmentVariable(llvm::StringRef env_var_name, - size_t *argument_index) const { - // Validate args. - if (env_var_name.empty()) - return false; - - // Check each arg to see if it matches the env var name. - for (auto arg : llvm::enumerate(m_entries)) { - llvm::StringRef name, value; - std::tie(name, value) = arg.value().ref.split('='); - if (name != env_var_name) - continue; - - if (argument_index) - *argument_index = arg.index(); - return true; - } - - // We didn't find a match. - return false; -} - size_t Args::FindArgumentIndexForOption(Option *long_options, int long_options_index) const { char short_buffer[3]; Index: source/Host/windows/Host.cpp =================================================================== --- source/Host/windows/Host.cpp +++ source/Host/windows/Host.cpp @@ -258,24 +258,23 @@ return error; } -size_t Host::GetEnvironment(StringList &env) { +Environment Host::GetEnvironment() { + Environment env; // The environment block on Windows is a contiguous buffer of NULL terminated - // strings, - // where the end of the environment block is indicated by two consecutive - // NULLs. + // strings, where the end of the environment block is indicated by two + // consecutive NULLs. LPWCH environment_block = ::GetEnvironmentStringsW(); - env.Clear(); while (*environment_block != L'\0') { std::string current_var; auto current_var_size = wcslen(environment_block) + 1; if (!llvm::convertWideToUTF8(environment_block, current_var)) { environment_block += current_var_size; continue; } if (current_var[0] != '=') - env.AppendString(current_var); + env.insert(current_var); environment_block += current_var_size; } - return env.GetSize(); + return env; } Index: source/Host/posix/ProcessLauncherPosixFork.cpp =================================================================== --- source/Host/posix/ProcessLauncherPosixFork.cpp +++ source/Host/posix/ProcessLauncherPosixFork.cpp @@ -38,17 +38,12 @@ using namespace lldb; using namespace lldb_private; -static void FixupEnvironment(Args &env) { +static void FixupEnvironment(Environment &env) { #ifdef __ANDROID__ // If there is no PATH variable specified inside the environment then set the // path to /system/bin. It is required because the default path used by // execve() is wrong on android. - static const char *path = "PATH="; - for (auto &entry : env.entries()) { - if (entry.ref.startswith(path)) - return; - } - env.AppendArgument(llvm::StringRef("PATH=/system/bin")); + env.try_emplace("PATH", "/system/bin"); #endif } @@ -132,9 +127,9 @@ ExitWithError(error_fd, "chdir"); DisableASLRIfRequested(error_fd, info); - Args env = info.GetEnvironmentEntries(); + Environment env = info.GetEnvironment(); FixupEnvironment(env); - const char **envp = env.GetConstArgumentVector(); + Environment::Envp envp = env.getEnvp(); // Clear the signal mask to prevent the child from being affected by // any masking done by the parent. @@ -159,8 +154,7 @@ } // Execute. We should never return... - execve(argv[0], const_cast<char *const *>(argv), - const_cast<char *const *>(envp)); + execve(argv[0], const_cast<char *const *>(argv), envp); #if defined(__linux__) if (errno == ETXTBSY) { @@ -177,8 +171,7 @@ // this state should clear up quickly, wait a while and then give it one // more go. usleep(50000); - execve(argv[0], const_cast<char *const *>(argv), - const_cast<char *const *>(envp)); + execve(argv[0], const_cast<char *const *>(argv), envp); } #endif Index: source/Host/openbsd/Host.cpp =================================================================== --- source/Host/openbsd/Host.cpp +++ source/Host/openbsd/Host.cpp @@ -45,16 +45,17 @@ using namespace lldb; using namespace lldb_private; -size_t Host::GetEnvironment(StringList &env) { +Environment Host::GetEnvironment() { + Environment env; char *v; char **var = environ; for (; var != NULL && *var != NULL; ++var) { v = strchr(*var, (int)'-'); if (v == NULL) continue; - env.AppendString(v); + env.insert(v); } - return env.GetSize(); + return env; } static bool Index: source/Host/netbsd/Host.cpp =================================================================== --- source/Host/netbsd/Host.cpp +++ source/Host/netbsd/Host.cpp @@ -48,14 +48,7 @@ using namespace lldb; using namespace lldb_private; -size_t Host::GetEnvironment(StringList &env) { - char **host_env = environ; - char *env_entry; - size_t i; - for (i = 0; (env_entry = host_env[i]) != NULL; ++i) - env.AppendString(env_entry); - return i; -} +Environment Host::GetEnvironment() { return Environment(environ); } static bool GetNetBSDProcessArgs(const ProcessInstanceInfoMatch *match_info_ptr, ProcessInstanceInfo &process_info) { Index: source/Host/macosx/Host.mm =================================================================== --- source/Host/macosx/Host.mm +++ source/Host/macosx/Host.mm @@ -428,33 +428,16 @@ command.PutCString(" --disable-aslr"); // We are launching on this host in a terminal. So compare the environment on - // the host - // to what is supplied in the launch_info. Any items that aren't in the host - // environment - // need to be sent to darwin-debug. If we send all environment entries, we - // might blow the - // max command line length, so we only send user modified entries. - const char **envp = - launch_info.GetEnvironmentEntries().GetConstArgumentVector(); - - StringList host_env; - const size_t host_env_count = Host::GetEnvironment(host_env); - - if (envp && envp[0]) { - const char *env_entry; - for (size_t env_idx = 0; (env_entry = envp[env_idx]) != NULL; ++env_idx) { - bool add_entry = true; - for (size_t i = 0; i < host_env_count; ++i) { - const char *host_env_entry = host_env.GetStringAtIndex(i); - if (strcmp(env_entry, host_env_entry) == 0) { - add_entry = false; - break; - } - } - if (add_entry) { - command.Printf(" --env='%s'", env_entry); - } - } + // the host to what is supplied in the launch_info. Any items that aren't in + // the host environment need to be sent to darwin-debug. If we send all + // environment entries, we might blow the max command line length, so we only + // send user modified entries. + Environment host_env = Host::GetEnvironment(); + + for (const auto &KV: launch_info.GetEnvironment()) { + auto host_entry = host_env.find(KV.first()); + if (host_entry == host_env.end() || host_entry->second != KV.second) + command.Format(" --env='{0}'", Environment::compose(KV)); } command.PutCString(" -- "); @@ -638,13 +621,8 @@ #endif // #if !defined(__arm__) && !defined(__arm64__) && !defined(__aarch64__) } -size_t Host::GetEnvironment(StringList &env) { - char **host_env = *_NSGetEnviron(); - char *env_entry; - size_t i; - for (i = 0; (env_entry = host_env[i]) != NULL; ++i) - env.AppendString(env_entry); - return i; +Environment Host::GetEnvironment() { + return Environment(*_NSGetEnviron()); } static bool GetMacOSXProcessCPUType(ProcessInstanceInfo &process_info) { @@ -767,7 +745,7 @@ proc_args.AppendArgument(llvm::StringRef(cstr)); } - Args &proc_env = process_info.GetEnvironmentEntries(); + Environment &proc_env = process_info.GetEnvironment(); while ((cstr = data.GetCStr(&offset))) { if (cstr[0] == '\0') break; @@ -782,7 +760,7 @@ llvm::Triple::MacOSX); } - proc_env.AppendArgument(llvm::StringRef(cstr)); + proc_env.insert(cstr); } return true; } @@ -936,6 +914,14 @@ } } +static void PackageXPCEnvironment(xpc_object_t message, llvm::StringRef prefix, const Environment &env) { + xpc_dictionary_set_int64(message, (prefix+"Count").str().c_str(), env.size()); + size_t i = 0; + for (const auto &KV: env) { + xpc_dictionary_set_string(message, (prefix+llvm::Twine(i)).str().c_str(), Environment::compose(KV).c_str()); + } +} + /* A valid authorizationRef means that - there is the LaunchUsingXPCRightName rights in the /etc/authorization @@ -1138,8 +1124,8 @@ PackageXPCArguments(message, LauncherXPCServiceArgPrefxKey, launch_info.GetArguments()); - PackageXPCArguments(message, LauncherXPCServiceEnvPrefxKey, - launch_info.GetEnvironmentEntries()); + PackageXPCEnvironment(message, LauncherXPCServiceEnvPrefxKey, + launch_info.GetEnvironment()); // Posix spawn stuff. xpc_dictionary_set_int64(message, LauncherXPCServiceCPUTypeKey, @@ -1353,8 +1339,7 @@ const char *tmp_argv[2]; char *const *argv = const_cast<char *const *>( launch_info.GetArguments().GetConstArgumentVector()); - char *const *envp = const_cast<char *const *>( - launch_info.GetEnvironmentEntries().GetConstArgumentVector()); + Environment::Envp envp = launch_info.GetEnvironment().getEnvp(); if (argv == NULL) { // posix_spawn gets very unhappy if it doesn't have at least the program // name in argv[0]. One of the side affects I have noticed is the @@ -1422,7 +1407,7 @@ "error: {0}, ::posix_spawnp(pid => {1}, path = '{2}', " "file_actions = {3}, " "attr = {4}, argv = {5}, envp = {6} )", - error, result_pid, exe_path, &file_actions, &attr, argv, envp); + error, result_pid, exe_path, &file_actions, &attr, argv, envp.get()); if (log) { for (int ii = 0; argv[ii]; ++ii) LLDB_LOG(log, "argv[{0}] = '{1}'", ii, argv[ii]); @@ -1438,7 +1423,7 @@ LLDB_LOG(log, "error: {0}, ::posix_spawnp ( pid => {1}, path = '{2}', " "file_actions = NULL, attr = {3}, argv = {4}, envp = {5} )", - error, result_pid, exe_path, &attr, argv, envp); + error, result_pid, exe_path, &attr, argv, envp.get()); if (log) { for (int ii = 0; argv[ii]; ++ii) LLDB_LOG(log, "argv[{0}] = '{1}'", ii, argv[ii]); Index: source/Host/linux/Host.cpp =================================================================== --- source/Host/linux/Host.cpp +++ source/Host/linux/Host.cpp @@ -199,7 +199,7 @@ while (!Rest.empty()) { llvm::StringRef Var; std::tie(Var, Rest) = Rest.split('\0'); - process_info.GetEnvironmentEntries().AppendArgument(Var); + process_info.GetEnvironment().insert(Var); } llvm::StringRef Arg0; @@ -297,14 +297,7 @@ return GetProcessAndStatInfo(pid, process_info, State, tracerpid); } -size_t Host::GetEnvironment(StringList &env) { - char **host_env = environ; - char *env_entry; - size_t i; - for (i = 0; (env_entry = host_env[i]) != NULL; ++i) - env.AppendString(env_entry); - return i; -} +Environment Host::GetEnvironment() { return Environment(environ); } Status Host::ShellExpandArguments(ProcessLaunchInfo &launch_info) { return Status("unimplemented"); Index: source/Host/freebsd/Host.cpp =================================================================== --- source/Host/freebsd/Host.cpp +++ source/Host/freebsd/Host.cpp @@ -243,14 +243,7 @@ return false; } -size_t Host::GetEnvironment(StringList &env) { - char **host_env = environ; - char *env_entry; - size_t i; - for (i = 0; (env_entry = host_env[i]) != NULL; ++i) - env.AppendString(env_entry); - return i; -} +Environment Host::GetEnvironment() { return Environment(environ); } Status Host::ShellExpandArguments(ProcessLaunchInfo &launch_info) { return Status("unimplemented"); Index: source/Commands/CommandObjectProcess.cpp =================================================================== --- source/Commands/CommandObjectProcess.cpp +++ source/Commands/CommandObjectProcess.cpp @@ -205,11 +205,7 @@ if (target->GetDisableSTDIO()) m_options.launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO); - Args environment; - target->GetEnvironmentAsArgs(environment); - if (environment.GetArgumentCount() > 0) - m_options.launch_info.GetEnvironmentEntries().AppendArguments( - environment); + m_options.launch_info.GetEnvironment() = target->GetEnvironment(); if (!target_settings_argv0.empty()) { m_options.launch_info.GetArguments().AppendArgument( Index: source/API/SBTarget.cpp =================================================================== --- source/API/SBTarget.cpp +++ source/API/SBTarget.cpp @@ -290,7 +290,7 @@ if (argv) launch_info.GetArguments().AppendArguments(argv); if (envp) - launch_info.GetEnvironmentEntries().SetArguments(envp); + launch_info.GetEnvironment() = Environment(envp); if (listener.IsValid()) launch_info.SetListener(listener.GetSP()); @@ -340,7 +340,7 @@ } } - lldb_private::ProcessLaunchInfo &launch_info = sb_launch_info.ref(); + lldb_private::ProcessLaunchInfo launch_info = sb_launch_info.ref(); if (!launch_info.GetExecutableFile()) { Module *exe_module = target_sp->GetExecutableModulePointer(); @@ -353,6 +353,7 @@ launch_info.GetArchitecture() = arch_spec; error.SetError(target_sp->Launch(launch_info, NULL)); + sb_launch_info.set_ref(launch_info); sb_process.SetSP(target_sp->GetProcessSP()); } else { error.SetErrorString("SBTarget is invalid"); @@ -2195,7 +2196,7 @@ lldb::SBLaunchInfo launch_info(NULL); TargetSP target_sp(GetSP()); if (target_sp) - launch_info.ref() = m_opaque_sp->GetProcessLaunchInfo(); + launch_info.set_ref(m_opaque_sp->GetProcessLaunchInfo()); return launch_info; } Index: source/API/SBProcess.cpp =================================================================== --- source/API/SBProcess.cpp +++ source/API/SBProcess.cpp @@ -140,7 +140,7 @@ if (argv) launch_info.GetArguments().AppendArguments(argv); if (envp) - launch_info.GetEnvironmentEntries().SetArguments(envp); + launch_info.GetEnvironment() = Environment(envp); error.SetError(process_sp->Launch(launch_info)); } else { error.SetErrorString("must be in eStateConnected to call RemoteLaunch"); Index: source/API/SBPlatform.cpp =================================================================== --- source/API/SBPlatform.cpp +++ source/API/SBPlatform.cpp @@ -416,7 +416,10 @@ SBError SBPlatform::Launch(SBLaunchInfo &launch_info) { return ExecuteConnected([&](const lldb::PlatformSP &platform_sp) { - return platform_sp->LaunchProcess(launch_info.ref()); + ProcessLaunchInfo info = launch_info.ref(); + Status error = platform_sp->LaunchProcess(info); + launch_info.set_ref(info); + return error; }); } Index: source/API/SBLaunchInfo.cpp =================================================================== --- source/API/SBLaunchInfo.cpp +++ source/API/SBLaunchInfo.cpp @@ -16,21 +16,41 @@ using namespace lldb; using namespace lldb_private; +class lldb_private::SBLaunchInfoImpl: public ProcessLaunchInfo { +public: + SBLaunchInfoImpl() + : ProcessLaunchInfo(), m_envp(GetEnvironment().getEnvp()) {} + + const char *const *GetEnvp() const { return m_envp; } + void RegenerateEnvp() { m_envp = GetEnvironment().getEnvp(); } + + SBLaunchInfoImpl &operator=(const ProcessLaunchInfo &rhs) { + ProcessLaunchInfo::operator=(rhs); + RegenerateEnvp(); + return *this; + } + +private: + Environment::Envp m_envp; +}; + SBLaunchInfo::SBLaunchInfo(const char **argv) - : m_opaque_sp(new ProcessLaunchInfo()) { + : m_opaque_sp(new SBLaunchInfoImpl()) { m_opaque_sp->GetFlags().Reset(eLaunchFlagDebug | eLaunchFlagDisableASLR); if (argv && argv[0]) m_opaque_sp->GetArguments().SetArguments(argv); } SBLaunchInfo::~SBLaunchInfo() {} -lldb_private::ProcessLaunchInfo &SBLaunchInfo::ref() { return *m_opaque_sp; } - const lldb_private::ProcessLaunchInfo &SBLaunchInfo::ref() const { return *m_opaque_sp; } +void SBLaunchInfo::set_ref(const ProcessLaunchInfo &info) { + *m_opaque_sp = info; +} + lldb::pid_t SBLaunchInfo::GetProcessID() { return m_opaque_sp->GetProcessID(); } uint32_t SBLaunchInfo::GetUserID() { return m_opaque_sp->GetUserID(); } @@ -83,23 +103,22 @@ } uint32_t SBLaunchInfo::GetNumEnvironmentEntries() { - return m_opaque_sp->GetEnvironmentEntries().GetArgumentCount(); + return m_opaque_sp->GetEnvironment().size(); } const char *SBLaunchInfo::GetEnvironmentEntryAtIndex(uint32_t idx) { - return m_opaque_sp->GetEnvironmentEntries().GetArgumentAtIndex(idx); + if (idx > GetNumEnvironmentEntries()) + return nullptr; + return m_opaque_sp->GetEnvp()[idx]; } void SBLaunchInfo::SetEnvironmentEntries(const char **envp, bool append) { - if (append) { - if (envp) - m_opaque_sp->GetEnvironmentEntries().AppendArguments(envp); - } else { - if (envp) - m_opaque_sp->GetEnvironmentEntries().SetArguments(envp); - else - m_opaque_sp->GetEnvironmentEntries().Clear(); - } + Environment env(envp); + if (append) + m_opaque_sp->GetEnvironment().insert(env.begin(), env.end()); + else + m_opaque_sp->GetEnvironment() = env; + m_opaque_sp->RegenerateEnvp(); } void SBLaunchInfo::Clear() { m_opaque_sp->Clear(); } Index: packages/Python/lldbsuite/test/python_api/sblaunchinfo/TestSBLaunchInfo.py =================================================================== --- /dev/null +++ packages/Python/lldbsuite/test/python_api/sblaunchinfo/TestSBLaunchInfo.py @@ -0,0 +1,31 @@ +""" +Test SBLaunchInfo +""" + +from __future__ import print_function + + +from lldbsuite.test.lldbtest import * + + +def lookup(info, key): + for i in range(info.GetNumEnvironmentEntries()): + KeyEqValue = info.GetEnvironmentEntryAtIndex(i) + Key, Value = KeyEqValue.split("=") + if Key == key: + return Value + return "" + +class BreakpointAPITestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + NO_DEBUG_INFO_TESTCASE = True + + def test_environment_getset(self): + info = lldb.SBLaunchInfo(None) + info.SetEnvironmentEntries(["FOO=BAR"], False) + self.assertEquals(1, info.GetNumEnvironmentEntries()) + info.SetEnvironmentEntries(["BAR=BAZ"], True) + self.assertEquals(2, info.GetNumEnvironmentEntries()) + self.assertEquals("BAR", lookup(info, "FOO")) + self.assertEquals("BAZ", lookup(info, "BAR")) Index: include/lldb/Utility/Environment.h =================================================================== --- /dev/null +++ include/lldb/Utility/Environment.h @@ -0,0 +1,95 @@ +//===-- Environment.h -------------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_UTILITY_ENVIRONMENT_H +#define LLDB_UTILITY_ENVIRONMENT_H + +#include "llvm/ADT/StringMap.h" +#include "llvm/Support/Allocator.h" +#include "llvm/Support/FormatProviders.h" + +namespace lldb_private { + +class Environment: private llvm::StringMap<std::string> { + using Base = llvm::StringMap<std::string>; + +public: + class Envp { + public: + Envp(Envp &&RHS) = default; + Envp &operator=(Envp &&RHS) = default; + + char *const *get() const { return Data; } + operator char *const *() const { return get(); } + + private: + explicit Envp(const Environment &Env); + char *make_entry(llvm::StringRef Key, llvm::StringRef Value); + Envp(const Envp &) = delete; + Envp &operator=(const Envp &) = delete; + friend class Environment; + + llvm::BumpPtrAllocator Allocator; + char **Data; + }; + + using Base::const_iterator; + using Base::iterator; + using Base::value_type; + + using Base::begin; + using Base::clear; + using Base::count; + using Base::end; + using Base::erase; + using Base::find; + using Base::insert; + using Base::lookup; + using Base::size; + using Base::try_emplace; + using Base::operator[]; + + Environment() : Base() {} + Environment(const Environment &RHS) : Base(RHS) {} + Environment(Environment &&RHS) : Base(std::move(RHS)) {} + Environment(char *const *Env) + : Environment(const_cast<const char *const *>(Env)) {} + Environment(const char *const *Env); + + Environment &operator=(Environment RHS) { + Base::operator=(std::move(RHS)); + return *this; + } + + std::pair<iterator, bool> insert(llvm::StringRef KeyEqValue) { + return insert(KeyEqValue.split('=')); + } + + void insert(const_iterator first, const_iterator last); + + Envp getEnvp() const { return Envp(*this); } + + static std::string compose(const value_type &KeyValue) { + return (KeyValue.first() + "=" + KeyValue.second).str(); + } +}; + +} // namespace lldb_private + +namespace llvm { +template <> struct format_provider<lldb_private::Environment> { + static void format(const lldb_private::Environment &Env, raw_ostream &Stream, + StringRef Style) { + for (const auto &KV : Env) + Stream << "env[" << KV.first() << "] = " << KV.second << "\n"; + } +}; +} + +#endif // #ifndef LLDB_UTILITY_ENVIRONMENT_H Index: include/lldb/Target/Target.h =================================================================== --- include/lldb/Target/Target.h +++ include/lldb/Target/Target.h @@ -115,9 +115,8 @@ void SetRunArguments(const Args &args); - size_t GetEnvironmentAsArgs(Args &env) const; - - void SetEnvironmentFromArgs(const Args &env); + Environment GetEnvironment() const; + void SetEnvironment(Environment env); bool GetSkipPrologue() const; Index: include/lldb/Target/ProcessInfo.h =================================================================== --- include/lldb/Target/ProcessInfo.h +++ include/lldb/Target/ProcessInfo.h @@ -13,6 +13,7 @@ // LLDB headers #include "lldb/Interpreter/Args.h" #include "lldb/Utility/ArchSpec.h" +#include "lldb/Utility/Environment.h" #include "lldb/Utility/FileSpec.h" namespace lldb_private { @@ -81,18 +82,17 @@ void SetArguments(char const **argv, bool first_arg_is_executable); - Args &GetEnvironmentEntries() { return m_environment; } - - const Args &GetEnvironmentEntries() const { return m_environment; } + Environment &GetEnvironment() { return m_environment; } + const Environment &GetEnvironment() const { return m_environment; } protected: FileSpec m_executable; std::string m_arg0; // argv[0] if supported. If empty, then use m_executable. // Not all process plug-ins support specifying an argv[0] // that differs from the resolved platform executable // (which is in m_executable) Args m_arguments; // All program arguments except argv[0] - Args m_environment; + Environment m_environment; uint32_t m_uid; uint32_t m_gid; ArchSpec m_arch; Index: include/lldb/Target/Platform.h =================================================================== --- include/lldb/Target/Platform.h +++ include/lldb/Target/Platform.h @@ -633,7 +633,7 @@ //---------------------------------------------------------------------- virtual Status Install(const FileSpec &src, const FileSpec &dst); - virtual size_t GetEnvironment(StringList &environment); + virtual Environment GetEnvironment(); virtual bool GetFileExists(const lldb_private::FileSpec &file_spec); Index: include/lldb/Interpreter/Args.h =================================================================== --- include/lldb/Interpreter/Args.h +++ include/lldb/Interpreter/Args.h @@ -21,6 +21,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" // Project includes +#include "lldb/Utility/Environment.h" #include "lldb/Utility/Status.h" #include "lldb/lldb-private-types.h" #include "lldb/lldb-types.h" @@ -95,6 +96,12 @@ //------------------------------------------------------------------ ~Args(); + explicit Args(const Environment &env) : Args() { + SetArguments(const_cast<const char **>(env.getEnvp().get())); + } + + explicit operator Environment() const { return GetConstArgumentVector(); } + //------------------------------------------------------------------ /// Dump all entries to the stream \a s using label \a label_name. /// @@ -433,38 +440,6 @@ static std::string EscapeLLDBCommandArgument(const std::string &arg, char quote_char); - //------------------------------------------------------------------ - /// Add or replace an environment variable with the given value. - /// - /// This command adds the environment variable if it is not already - /// present using the given value. If the environment variable is - /// already in the list, it replaces the first such occurrence - /// with the new value. - //------------------------------------------------------------------ - void AddOrReplaceEnvironmentVariable(llvm::StringRef env_var_name, - llvm::StringRef new_value); - - /// Return whether a given environment variable exists. - /// - /// This command treats Args like a list of environment variables, - /// as used in ProcessLaunchInfo. It treats each argument as - /// an {env_var_name}={value} or an {env_var_name} entry. - /// - /// @param[in] env_var_name - /// Specifies the name of the environment variable to check. - /// - /// @param[out] argument_index - /// If non-null, then when the environment variable is found, - /// the index of the argument position will be returned in - /// the size_t pointed to by this argument. - /// - /// @return - /// true if the specified env var name exists in the list in - /// either of the above-mentioned formats; otherwise, false. - //------------------------------------------------------------------ - bool ContainsEnvironmentVariable(llvm::StringRef env_var_name, - size_t *argument_index = nullptr) const; - private: size_t FindArgumentIndexForOption(Option *long_options, int long_options_index) const; Index: include/lldb/Host/Host.h =================================================================== --- include/lldb/Host/Host.h +++ include/lldb/Host/Host.h @@ -12,8 +12,8 @@ #include "lldb/Host/File.h" #include "lldb/Host/HostThread.h" +#include "lldb/Utility/Environment.h" #include "lldb/Utility/FileSpec.h" -#include "lldb/Utility/StringList.h" #include "lldb/lldb-private-forward.h" #include "lldb/lldb-private.h" #include <cerrno> @@ -242,7 +242,7 @@ static bool OpenFileInExternalEditor(const FileSpec &file_spec, uint32_t line_no); - static size_t GetEnvironment(StringList &env); + static Environment GetEnvironment(); static std::unique_ptr<Connection> CreateDefaultConnection(llvm::StringRef url); Index: include/lldb/API/SBLaunchInfo.h =================================================================== --- include/lldb/API/SBLaunchInfo.h +++ include/lldb/API/SBLaunchInfo.h @@ -12,6 +12,10 @@ #include "lldb/API/SBDefines.h" +namespace lldb_private { + class SBLaunchInfoImpl; +} + namespace lldb { class SBPlatform; @@ -141,11 +145,10 @@ friend class SBPlatform; friend class SBTarget; - lldb_private::ProcessLaunchInfo &ref(); - const lldb_private::ProcessLaunchInfo &ref() const; + void set_ref(const lldb_private::ProcessLaunchInfo &info); - ProcessLaunchInfoSP m_opaque_sp; + std::shared_ptr<lldb_private::SBLaunchInfoImpl> m_opaque_sp; }; } // namespace lldb
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits