labath created this revision. labath added reviewers: zturner, clayborg, jingham. Herald added a subscriber: mgorny.
This creates an abstract base class called "UserIDResolver", which can be implemented to provide user/group ID resolution capabilities for various objects. Posix host implement a PosixUserIDResolver, which does that using posix apis (getpwuid and friends). PlatformGDBRemote forwards queries over the gdb-remote link, etc. ProcessInstanceInfo class is refactored to make use of this interface instead of taking a platform pointer as an argument. The base resolver class already implements caching and thread-safety, so implementations don't have to worry about that. The main motivating factor for this was to remove external dependencies from the ProcessInstanceInfo class (so it can be put next to ProcessLaunchInfo and friends), but it has other benefits too: - ability to test the user name caching code - ability to test ProcessInstanceInfo dumping code - consistent interface for user/group resolution between Platform and Host classes. https://reviews.llvm.org/D58167 Files: include/lldb/Host/HostInfoBase.h include/lldb/Host/UserIDResolver.h include/lldb/Host/posix/HostInfoPosix.h include/lldb/Target/Platform.h include/lldb/Target/Process.h include/lldb/Target/RemoteAwarePlatform.h source/Commands/CommandObjectPlatform.cpp source/Host/CMakeLists.txt source/Host/common/UserIDResolver.cpp source/Host/posix/HostInfoPosix.cpp source/Plugins/Platform/Kalimba/PlatformKalimba.h source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp source/Target/Platform.cpp source/Target/Process.cpp source/Target/RemoteAwarePlatform.cpp unittests/Host/CMakeLists.txt unittests/Host/UserIDResolverTest.cpp unittests/Target/CMakeLists.txt unittests/Target/ProcessInstanceInfoTest.cpp
Index: unittests/Target/ProcessInstanceInfoTest.cpp =================================================================== --- /dev/null +++ unittests/Target/ProcessInstanceInfoTest.cpp @@ -0,0 +1,75 @@ +//===-- ProcessInstanceInfoTest.cpp -----------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Target/Process.h" +#include "gtest/gtest.h" + +using namespace lldb_private; + +namespace { +/// A very simple resolver which fails for even ids and returns a simple string +/// for odd ones. +class DummyUserIDResolver : public UserIDResolver { +protected: + llvm::Optional<std::string> DoGetUserName(id_t Uid) { + if (Uid % 2) + return ("user" + llvm::Twine(Uid)).str(); + return llvm::None; + } + + llvm::Optional<std::string> DoGetGroupName(id_t Uid) { + if (Uid % 2) + return ("group" + llvm::Twine(Uid)).str(); + return llvm::None; + } +}; +} // namespace + +TEST(ProcessInstanceInfo, Dump) { + ProcessInstanceInfo Info("a.out", ArchSpec("x86_64-pc-linux"), 47); + Info.SetUserID(1); + Info.SetEffectiveUserID(2); + Info.SetGroupID(3); + Info.SetEffectiveGroupID(4); + + DummyUserIDResolver Resolver; + StreamString S; + Info.Dump(S, Resolver); + EXPECT_STREQ(R"( pid = 47 + name = a.out + file = a.out + arch = x86_64-pc-linux + uid = 1 (user1) + gid = 3 (group3) + euid = 2 () + egid = 4 () +)", + S.GetData()); +} + +TEST(ProcessInstanceInfo, DumpTable) { + ProcessInstanceInfo Info("a.out", ArchSpec("x86_64-pc-linux"), 47); + Info.SetUserID(1); + Info.SetEffectiveUserID(2); + Info.SetGroupID(3); + Info.SetEffectiveGroupID(4); + + DummyUserIDResolver Resolver; + StreamString S; + + const bool show_args = false; + const bool verbose = true; + ProcessInstanceInfo::DumpTableHeader(S, show_args, verbose); + Info.DumpAsTableRow(S, Resolver, show_args, verbose); + EXPECT_STREQ( + R"(PID PARENT USER GROUP EFF USER EFF GROUP TRIPLE ARGUMENTS +====== ====== ========== ========== ========== ========== ======================== ============================ +47 0 user1 group3 2 4 x86_64-pc-linux +)", + S.GetData()); +} Index: unittests/Target/CMakeLists.txt =================================================================== --- unittests/Target/CMakeLists.txt +++ unittests/Target/CMakeLists.txt @@ -2,6 +2,7 @@ MemoryRegionInfoTest.cpp ModuleCacheTest.cpp PathMappingListTest.cpp + ProcessInstanceInfoTest.cpp LINK_LIBS lldbCore Index: unittests/Host/UserIDResolverTest.cpp =================================================================== --- /dev/null +++ unittests/Host/UserIDResolverTest.cpp @@ -0,0 +1,47 @@ +//===-- UserIDResolverTest.cpp ----------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Host/UserIDResolver.h" +#include "gmock/gmock.h" + +using namespace lldb_private; +using namespace testing; + +namespace { +class TestUserIDResolver : public UserIDResolver { +public: + MOCK_METHOD1(DoGetUserName, llvm::Optional<std::string>(id_t Uid)); + MOCK_METHOD1(DoGetGroupName, llvm::Optional<std::string>(id_t Uid)); +}; +} // namespace + +TEST(UserIDResolver, GetUserName) { + StrictMock<TestUserIDResolver> R; + llvm::StringRef User47("foo"); + EXPECT_CALL(R, DoGetUserName(47)).Times(1).WillOnce(Return(User47.str())); + EXPECT_CALL(R, DoGetUserName(42)).Times(1).WillOnce(Return(llvm::None)); + + // Call functions twice to make sure the caching works. + EXPECT_EQ(User47, R.GetUserName(47)); + EXPECT_EQ(User47, R.GetUserName(47)); + EXPECT_EQ(llvm::None, R.GetUserName(42)); + EXPECT_EQ(llvm::None, R.GetUserName(42)); +} + +TEST(UserIDResolver, GetGroupName) { + StrictMock<TestUserIDResolver> R; + llvm::StringRef Group47("foo"); + EXPECT_CALL(R, DoGetGroupName(47)).Times(1).WillOnce(Return(Group47.str())); + EXPECT_CALL(R, DoGetGroupName(42)).Times(1).WillOnce(Return(llvm::None)); + + // Call functions twice to make sure the caching works. + EXPECT_EQ(Group47, R.GetGroupName(47)); + EXPECT_EQ(Group47, R.GetGroupName(47)); + EXPECT_EQ(llvm::None, R.GetGroupName(42)); + EXPECT_EQ(llvm::None, R.GetGroupName(42)); +} Index: unittests/Host/CMakeLists.txt =================================================================== --- unittests/Host/CMakeLists.txt +++ unittests/Host/CMakeLists.txt @@ -11,6 +11,7 @@ SocketTest.cpp SymbolsTest.cpp TaskPoolTest.cpp + UserIDResolverTest.cpp ) if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android") Index: source/Target/RemoteAwarePlatform.cpp =================================================================== --- source/Target/RemoteAwarePlatform.cpp +++ source/Target/RemoteAwarePlatform.cpp @@ -8,6 +8,7 @@ #include "lldb/Target/RemoteAwarePlatform.h" #include "lldb/Host/Host.h" +#include "lldb/Host/HostInfo.h" using namespace lldb_private; @@ -70,25 +71,12 @@ return nullptr; } -const char *RemoteAwarePlatform::GetUserName(uint32_t uid) { - // Check the cache in Platform in case we have already looked this uid up - const char *user_name = Platform::GetUserName(uid); - if (user_name) - return user_name; - - if (IsRemote() && m_remote_platform_sp) - return m_remote_platform_sp->GetUserName(uid); - return nullptr; -} - -const char *RemoteAwarePlatform::GetGroupName(uint32_t gid) { - const char *group_name = Platform::GetGroupName(gid); - if (group_name) - return group_name; - - if (IsRemote() && m_remote_platform_sp) - return m_remote_platform_sp->GetGroupName(gid); - return nullptr; +UserIDResolver &RemoteAwarePlatform::GetUserIDResolver() { + if (IsHost()) + return HostInfo::GetUserIDResolver(); + if (m_remote_platform_sp) + return m_remote_platform_sp->GetUserIDResolver(); + return UserIDResolver::GetNoopResolver(); } Environment RemoteAwarePlatform::GetEnvironment() { Index: source/Target/Process.cpp =================================================================== --- source/Target/Process.cpp +++ source/Target/Process.cpp @@ -278,8 +278,7 @@ nullptr, idx, g_properties[idx].default_uint_value != 0); } -void ProcessInstanceInfo::Dump(Stream &s, Platform *platform) const { - const char *cstr; +void ProcessInstanceInfo::Dump(Stream &s, UserIDResolver &resolver) const { if (m_pid != LLDB_INVALID_PROCESS_ID) s.Printf(" pid = %" PRIu64 "\n", m_pid); @@ -311,26 +310,26 @@ s.EOL(); } - if (m_uid != UINT32_MAX) { - cstr = platform->GetUserName(m_uid); - s.Printf(" uid = %-5u (%s)\n", m_uid, cstr ? cstr : ""); + if (UserIDIsValid()) { + s.Format(" uid = {0,-5} ({1})\n", GetUserID(), + resolver.GetUserName(GetUserID()).getValueOr("")); } - if (m_gid != UINT32_MAX) { - cstr = platform->GetGroupName(m_gid); - s.Printf(" gid = %-5u (%s)\n", m_gid, cstr ? cstr : ""); + if (GroupIDIsValid()) { + s.Format(" gid = {0,-5} ({1})\n", GetGroupID(), + resolver.GetGroupName(GetGroupID()).getValueOr("")); } - if (m_euid != UINT32_MAX) { - cstr = platform->GetUserName(m_euid); - s.Printf(" euid = %-5u (%s)\n", m_euid, cstr ? cstr : ""); + if (EffectiveUserIDIsValid()) { + s.Format(" euid = {0,-5} ({1})\n", GetEffectiveUserID(), + resolver.GetUserName(GetEffectiveUserID()).getValueOr("")); } - if (m_egid != UINT32_MAX) { - cstr = platform->GetGroupName(m_egid); - s.Printf(" egid = %-5u (%s)\n", m_egid, cstr ? cstr : ""); + if (EffectiveGroupIDIsValid()) { + s.Format(" egid = {0,-5} ({1})\n", GetEffectiveGroupID(), + resolver.GetGroupName(GetEffectiveGroupID()).getValueOr("")); } } -void ProcessInstanceInfo::DumpTableHeader(Stream &s, Platform *platform, - bool show_args, bool verbose) { +void ProcessInstanceInfo::DumpTableHeader(Stream &s, bool show_args, + bool verbose) { const char *label; if (show_args || verbose) label = "ARGUMENTS"; @@ -350,49 +349,33 @@ } } -void ProcessInstanceInfo::DumpAsTableRow(Stream &s, Platform *platform, +void ProcessInstanceInfo::DumpAsTableRow(Stream &s, UserIDResolver &resolver, bool show_args, bool verbose) const { if (m_pid != LLDB_INVALID_PROCESS_ID) { - const char *cstr; s.Printf("%-6" PRIu64 " %-6" PRIu64 " ", m_pid, m_parent_pid); StreamString arch_strm; if (m_arch.IsValid()) m_arch.DumpTriple(arch_strm); - if (verbose) { - cstr = platform->GetUserName(m_uid); - if (cstr && - cstr[0]) // Watch for empty string that indicates lookup failed - s.Printf("%-10s ", cstr); - else - s.Printf("%-10u ", m_uid); - - cstr = platform->GetGroupName(m_gid); - if (cstr && - cstr[0]) // Watch for empty string that indicates lookup failed - s.Printf("%-10s ", cstr); - else - s.Printf("%-10u ", m_gid); - - cstr = platform->GetUserName(m_euid); - if (cstr && - cstr[0]) // Watch for empty string that indicates lookup failed - s.Printf("%-10s ", cstr); - else - s.Printf("%-10u ", m_euid); - - cstr = platform->GetGroupName(m_egid); - if (cstr && - cstr[0]) // Watch for empty string that indicates lookup failed - s.Printf("%-10s ", cstr); + auto print = [&](UserIDResolver::id_t id, + llvm::Optional<llvm::StringRef> (UserIDResolver::*get)( + UserIDResolver::id_t id)) { + if (auto name = (resolver.*get)(id)) + s.Format("{0,-10} ", *name); else - s.Printf("%-10u ", m_egid); + s.Format("{0,-10} ", id); + }; + if (verbose) { + print(m_uid, &UserIDResolver::GetUserName); + print(m_gid, &UserIDResolver::GetGroupName); + print(m_euid, &UserIDResolver::GetUserName); + print(m_egid, &UserIDResolver::GetGroupName); s.Printf("%-24s ", arch_strm.GetData()); } else { - s.Printf("%-10s %-24s ", platform->GetUserName(m_euid), - arch_strm.GetData()); + print(m_euid, &UserIDResolver::GetUserName); + s.Printf(" %-24s ", arch_strm.GetData()); } if (verbose || show_args) { @@ -3045,11 +3028,10 @@ process_name, sizeof(process_name)); if (num_matches > 1) { StreamString s; - ProcessInstanceInfo::DumpTableHeader(s, platform_sp.get(), true, - false); + ProcessInstanceInfo::DumpTableHeader(s, true, false); for (size_t i = 0; i < num_matches; i++) { process_infos.GetProcessInfoAtIndex(i).DumpAsTableRow( - s, platform_sp.get(), true, false); + s, platform_sp->GetUserIDResolver(), true, false); } error.SetErrorStringWithFormat( "more than one process named %s:\n%s", process_name, Index: source/Target/Platform.cpp =================================================================== --- source/Target/Platform.cpp +++ source/Target/Platform.cpp @@ -384,10 +384,10 @@ : m_is_host(is_host), m_os_version_set_while_connected(false), m_system_arch_set_while_connected(false), m_sdk_sysroot(), m_sdk_build(), m_working_dir(), m_remote_url(), m_name(), m_system_arch(), m_mutex(), - m_uid_map(), m_gid_map(), m_max_uid_name_len(0), m_max_gid_name_len(0), - m_supports_rsync(false), m_rsync_opts(), m_rsync_prefix(), - m_supports_ssh(false), m_ssh_opts(), m_ignores_remote_hostname(false), - m_trap_handlers(), m_calculated_trap_handlers(false), + m_max_uid_name_len(0), m_max_gid_name_len(0), m_supports_rsync(false), + m_rsync_opts(), m_rsync_prefix(), m_supports_ssh(false), m_ssh_opts(), + m_ignores_remote_hostname(false), m_trap_handlers(), + m_calculated_trap_handlers(false), m_module_cache(llvm::make_unique<ModuleCache>()) { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT)); if (log) @@ -834,34 +834,6 @@ return true; } -const char *Platform::GetUserName(uint32_t uid) { -#if !defined(LLDB_DISABLE_POSIX) - const char *user_name = GetCachedUserName(uid); - if (user_name) - return user_name; - if (IsHost()) { - std::string name; - if (HostInfo::LookupUserName(uid, name)) - return SetCachedUserName(uid, name.c_str(), name.size()); - } -#endif - return nullptr; -} - -const char *Platform::GetGroupName(uint32_t gid) { -#if !defined(LLDB_DISABLE_POSIX) - const char *group_name = GetCachedGroupName(gid); - if (group_name) - return group_name; - if (IsHost()) { - std::string name; - if (HostInfo::LookupGroupName(gid, name)) - return SetCachedGroupName(gid, name.c_str(), name.size()); - } -#endif - return nullptr; -} - bool Platform::SetOSVersion(llvm::VersionTuple version) { if (IsHost()) { // We don't need anyone setting the OS version for the host platform, we Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -436,10 +436,9 @@ packet.SetFilePos(::strlen("qUserName:")); uint32_t uid = packet.GetU32(UINT32_MAX); if (uid != UINT32_MAX) { - std::string name; - if (HostInfo::LookupUserName(uid, name)) { + if (auto name = HostInfo::GetUserIDResolver().GetUserName(uid)) { StreamString response; - response.PutStringAsRawHex8(name); + response.PutStringAsRawHex8(*name); return SendPacketNoLock(response.GetString()); } } @@ -457,10 +456,9 @@ packet.SetFilePos(::strlen("qGroupName:")); uint32_t gid = packet.GetU32(UINT32_MAX); if (gid != UINT32_MAX) { - std::string name; - if (HostInfo::LookupGroupName(gid, name)) { + if (auto name = HostInfo::GetUserIDResolver().GetGroupName(gid)) { StreamString response; - response.PutStringAsRawHex8(name); + response.PutStringAsRawHex8(*name); return SendPacketNoLock(response.GetString()); } } Index: source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h =================================================================== --- source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h +++ source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h @@ -19,7 +19,7 @@ namespace lldb_private { namespace platform_gdb_server { -class PlatformRemoteGDBServer : public Platform { +class PlatformRemoteGDBServer : public Platform, private UserIDResolver { public: static void Initialize(); @@ -100,9 +100,7 @@ // name if connected. const char *GetHostname() override; - const char *GetUserName(uint32_t uid) override; - - const char *GetGroupName(uint32_t gid) override; + UserIDResolver &GetUserIDResolver() override { return *this; } bool IsConnected() const override; @@ -195,6 +193,9 @@ const std::string &platform_hostname, uint16_t port, const char *socket_name); + llvm::Optional<std::string> DoGetUserName(UserIDResolver::id_t uid) override; + llvm::Optional<std::string> DoGetGroupName(UserIDResolver::id_t uid) override; + DISALLOW_COPY_AND_ASSIGN(PlatformRemoteGDBServer); }; Index: source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp =================================================================== --- source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp +++ source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp @@ -340,29 +340,20 @@ return m_name.c_str(); } -const char *PlatformRemoteGDBServer::GetUserName(uint32_t uid) { - // Try and get a cache user name first - const char *cached_user_name = Platform::GetUserName(uid); - if (cached_user_name) - return cached_user_name; +llvm::Optional<std::string> +PlatformRemoteGDBServer::DoGetUserName(UserIDResolver::id_t uid) { std::string name; if (m_gdb_client.GetUserName(uid, name)) - return SetCachedUserName(uid, name.c_str(), name.size()); - - SetUserNameNotFound(uid); // Negative cache so we don't keep sending packets - return NULL; + return std::move(name); + return llvm::None; } -const char *PlatformRemoteGDBServer::GetGroupName(uint32_t gid) { - const char *cached_group_name = Platform::GetGroupName(gid); - if (cached_group_name) - return cached_group_name; +llvm::Optional<std::string> +PlatformRemoteGDBServer::DoGetGroupName(UserIDResolver::id_t gid) { std::string name; if (m_gdb_client.GetGroupName(gid, name)) - return SetCachedGroupName(gid, name.c_str(), name.size()); - - SetGroupNameNotFound(gid); // Negative cache so we don't keep sending packets - return NULL; + return std::move(name); + return llvm::None; } uint32_t PlatformRemoteGDBServer::FindProcesses( Index: source/Plugins/Platform/Kalimba/PlatformKalimba.h =================================================================== --- source/Plugins/Platform/Kalimba/PlatformKalimba.h +++ source/Plugins/Platform/Kalimba/PlatformKalimba.h @@ -62,6 +62,10 @@ void CalculateTrapHandlerSymbolNames() override; + UserIDResolver &GetUserIDResolver() override { + return UserIDResolver::GetNoopResolver(); + } + protected: lldb::PlatformSP m_remote_platform_sp; Index: source/Host/posix/HostInfoPosix.cpp =================================================================== --- source/Host/posix/HostInfoPosix.cpp +++ source/Host/posix/HostInfoPosix.cpp @@ -48,66 +48,65 @@ #define USE_GETPWUID #endif -#ifdef USE_GETPWUID -static std::mutex s_getpwuid_lock; -#endif - -const char *HostInfoPosix::LookupUserName(uint32_t uid, - std::string &user_name) { +namespace { +class PosixUserIDResolver : public UserIDResolver { +protected: + llvm::Optional<std::string> DoGetUserName(id_t Uid) override; + llvm::Optional<std::string> DoGetGroupName(id_t Gid) override; +}; +} // namespace + +llvm::Optional<std::string> PosixUserIDResolver::DoGetUserName(id_t Uid) { #ifdef USE_GETPWUID // getpwuid_r is missing from android-9 - // make getpwuid thread safe with a mutex - std::lock_guard<std::mutex> lock(s_getpwuid_lock); + // UserIDResolver provides some thread safety by making sure noone calls this + // function concurrently, but using getpwuid is ultimately not thread-safe as + // we don't know who else might be calling it. struct passwd *user_info_ptr = ::getpwuid(uid); - if (user_info_ptr) { - user_name.assign(user_info_ptr->pw_name); - return user_name.c_str(); - } + if (user_info_ptr) + return std::string(user_info_ptr->pw_name); #else struct passwd user_info; struct passwd *user_info_ptr = &user_info; char user_buffer[PATH_MAX]; size_t user_buffer_size = sizeof(user_buffer); - if (::getpwuid_r(uid, &user_info, user_buffer, user_buffer_size, - &user_info_ptr) == 0) { - if (user_info_ptr) { - user_name.assign(user_info_ptr->pw_name); - return user_name.c_str(); - } + if (::getpwuid_r(Uid, &user_info, user_buffer, user_buffer_size, + &user_info_ptr) == 0 && + user_info_ptr) { + return std::string(user_info_ptr->pw_name); } #endif - user_name.clear(); - return nullptr; + return llvm::None; } -const char *HostInfoPosix::LookupGroupName(uint32_t gid, - std::string &group_name) { +llvm::Optional<std::string> PosixUserIDResolver::DoGetGroupName(id_t Gid) { #ifndef __ANDROID__ char group_buffer[PATH_MAX]; size_t group_buffer_size = sizeof(group_buffer); struct group group_info; struct group *group_info_ptr = &group_info; // Try the threadsafe version first - if (::getgrgid_r(gid, &group_info, group_buffer, group_buffer_size, + if (::getgrgid_r(Gid, &group_info, group_buffer, group_buffer_size, &group_info_ptr) == 0) { - if (group_info_ptr) { - group_name.assign(group_info_ptr->gr_name); - return group_name.c_str(); - } + if (group_info_ptr) + return std::string(group_info_ptr->gr_name); } else { // The threadsafe version isn't currently working for me on darwin, but the // non-threadsafe version is, so I am calling it below. - group_info_ptr = ::getgrgid(gid); - if (group_info_ptr) { - group_name.assign(group_info_ptr->gr_name); - return group_name.c_str(); - } + group_info_ptr = ::getgrgid(Gid); + if (group_info_ptr) + return std::string(group_info_ptr->gr_name); } - group_name.clear(); #else assert(false && "getgrgid_r() not supported on Android"); #endif - return NULL; + return llvm::None; +} + +static llvm::ManagedStatic<PosixUserIDResolver> g_user_id_resolver; + +UserIDResolver &HostInfoPosix::GetUserIDResolver() { + return *g_user_id_resolver; } uint32_t HostInfoPosix::GetUserID() { return getuid(); } Index: source/Host/common/UserIDResolver.cpp =================================================================== --- /dev/null +++ source/Host/common/UserIDResolver.cpp @@ -0,0 +1,44 @@ +//===-- UserIdResolver.cpp --------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Host/UserIDResolver.h" +#include "llvm/Support/ManagedStatic.h" + +using namespace lldb_private; + +UserIDResolver::~UserIDResolver() = default; + +llvm::Optional<llvm::StringRef> UserIDResolver::Get( + id_t Id, Map &Cache, + llvm::Optional<std::string> (UserIDResolver::*DoGet)(id_t)) { + + std::lock_guard<std::mutex> Guard(Mutex); + auto ItBool = Cache.try_emplace(Id, llvm::None); + if (ItBool.second) + ItBool.first->second = (this->*DoGet)(Id); + if (ItBool.first->second) + return llvm::StringRef(*ItBool.first->second); + return llvm::None; +} + +namespace { +class NoopResolver : public UserIDResolver { +protected: + llvm::Optional<std::string> DoGetUserName(id_t Uid) override { + return llvm::None; + } + + llvm::Optional<std::string> DoGetGroupName(id_t Gid) override { + return llvm::None; + } +}; +} // namespace + +static llvm::ManagedStatic<NoopResolver> g_noop_resolver; + +UserIDResolver &UserIDResolver::GetNoopResolver() { return *g_noop_resolver; } Index: source/Host/CMakeLists.txt =================================================================== --- source/Host/CMakeLists.txt +++ source/Host/CMakeLists.txt @@ -18,9 +18,9 @@ endmacro() add_host_subdirectory(common - common/File.cpp common/FileAction.cpp common/FileCache.cpp + common/File.cpp common/FileSystem.cpp common/GetOptInc.cpp common/Host.cpp @@ -31,26 +31,27 @@ common/LockFileBase.cpp common/MainLoop.cpp common/MonitoringProcessLauncher.cpp - common/NativeWatchpointList.cpp common/NativeProcessProtocol.cpp common/NativeRegisterContext.cpp common/NativeThreadProtocol.cpp + common/NativeWatchpointList.cpp common/OptionParser.cpp common/PipeBase.cpp common/ProcessInfo.cpp common/ProcessLaunchInfo.cpp common/ProcessRunLock.cpp common/PseudoTerminal.cpp - common/Socket.cpp common/SocketAddress.cpp + common/Socket.cpp common/StringConvert.cpp common/Symbols.cpp common/TaskPool.cpp common/TCPSocket.cpp common/Terminal.cpp common/ThreadLauncher.cpp - common/XML.cpp common/UDPSocket.cpp + common/UserIDResolver.cpp + common/XML.cpp ) if (NOT LLDB_DISABLE_LIBEDIT) Index: source/Commands/CommandObjectPlatform.cpp =================================================================== --- source/Commands/CommandObjectPlatform.cpp +++ source/Commands/CommandObjectPlatform.cpp @@ -1151,10 +1151,9 @@ if (pid != LLDB_INVALID_PROCESS_ID) { ProcessInstanceInfo proc_info; if (platform_sp->GetProcessInfo(pid, proc_info)) { - ProcessInstanceInfo::DumpTableHeader(ostrm, platform_sp.get(), - m_options.show_args, + ProcessInstanceInfo::DumpTableHeader(ostrm, m_options.show_args, m_options.verbose); - proc_info.DumpAsTableRow(ostrm, platform_sp.get(), + proc_info.DumpAsTableRow(ostrm, platform_sp->GetUserIDResolver(), m_options.show_args, m_options.verbose); result.SetStatus(eReturnStatusSuccessFinishResult); } else { @@ -1212,13 +1211,12 @@ result.AppendMessageWithFormat(" whose name %s \"%s\"", match_desc, match_name); result.AppendMessageWithFormat("\n"); - ProcessInstanceInfo::DumpTableHeader(ostrm, platform_sp.get(), - m_options.show_args, + ProcessInstanceInfo::DumpTableHeader(ostrm, m_options.show_args, m_options.verbose); for (uint32_t i = 0; i < matches; ++i) { proc_infos.GetProcessInfoAtIndex(i).DumpAsTableRow( - ostrm, platform_sp.get(), m_options.show_args, - m_options.verbose); + ostrm, platform_sp->GetUserIDResolver(), + m_options.show_args, m_options.verbose); } } } @@ -1453,7 +1451,7 @@ if (platform_sp->GetProcessInfo(pid, proc_info)) { ostrm.Printf("Process information for process %" PRIu64 ":\n", pid); - proc_info.Dump(ostrm, platform_sp.get()); + proc_info.Dump(ostrm, platform_sp->GetUserIDResolver()); } else { ostrm.Printf("error: no process information is available for " "process %" PRIu64 "\n", Index: include/lldb/Target/RemoteAwarePlatform.h =================================================================== --- include/lldb/Target/RemoteAwarePlatform.h +++ include/lldb/Target/RemoteAwarePlatform.h @@ -30,8 +30,7 @@ ArchSpec GetRemoteSystemArchitecture() override; const char *GetHostname() override; - const char *GetUserName(uint32_t uid) override; - const char *GetGroupName(uint32_t gid) override; + UserIDResolver &GetUserIDResolver() override; lldb_private::Environment GetEnvironment() override; bool IsConnected() const override; Index: include/lldb/Target/Process.h =================================================================== --- include/lldb/Target/Process.h +++ include/lldb/Target/Process.h @@ -31,6 +31,7 @@ #include "lldb/Host/ProcessInfo.h" #include "lldb/Host/ProcessLaunchInfo.h" #include "lldb/Host/ProcessRunLock.h" +#include "lldb/Host/UserIDResolver.h" #include "lldb/Interpreter/Options.h" #include "lldb/Symbol/ObjectFile.h" #include "lldb/Target/ExecutionContextScope.h" @@ -150,12 +151,11 @@ return m_parent_pid != LLDB_INVALID_PROCESS_ID; } - void Dump(Stream &s, Platform *platform) const; + void Dump(Stream &s, UserIDResolver &resolver) const; - static void DumpTableHeader(Stream &s, Platform *platform, bool show_args, - bool verbose); + static void DumpTableHeader(Stream &s, bool show_args, bool verbose); - void DumpAsTableRow(Stream &s, Platform *platform, bool show_args, + void DumpAsTableRow(Stream &s, UserIDResolver &resolver, bool show_args, bool verbose) const; protected: Index: include/lldb/Target/Platform.h =================================================================== --- include/lldb/Target/Platform.h +++ include/lldb/Target/Platform.h @@ -18,6 +18,7 @@ #include "lldb/Core/PluginInterface.h" #include "lldb/Core/UserSettingsController.h" +#include "lldb/Host/UserIDResolver.h" #include "lldb/Interpreter/Options.h" #include "lldb/Utility/ArchSpec.h" #include "lldb/Utility/ConstString.h" @@ -276,9 +277,7 @@ virtual bool SetRemoteWorkingDirectory(const FileSpec &working_dir); - virtual const char *GetUserName(uint32_t uid); - - virtual const char *GetGroupName(uint32_t gid); + virtual UserIDResolver &GetUserIDResolver() = 0; //------------------------------------------------------------------ /// Locate a file for a platform. @@ -916,8 +915,6 @@ // Mutex for modifying Platform data structures that should only be used for // non-reentrant code std::mutex m_mutex; - IDToNameMap m_uid_map; - IDToNameMap m_gid_map; size_t m_max_uid_name_len; size_t m_max_gid_name_len; bool m_supports_rsync; @@ -946,68 +943,6 @@ //------------------------------------------------------------------ virtual void CalculateTrapHandlerSymbolNames() = 0; - const char *GetCachedUserName(uint32_t uid) { - std::lock_guard<std::mutex> guard(m_mutex); - // return the empty string if our string is NULL so we can tell when things - // were in the negative cached (didn't find a valid user name, don't keep - // trying) - const auto pos = m_uid_map.find(uid); - return ((pos != m_uid_map.end()) ? pos->second.AsCString("") : nullptr); - } - - const char *SetCachedUserName(uint32_t uid, const char *name, - size_t name_len) { - std::lock_guard<std::mutex> guard(m_mutex); - ConstString const_name(name); - m_uid_map[uid] = const_name; - if (m_max_uid_name_len < name_len) - m_max_uid_name_len = name_len; - // Const strings lives forever in our const string pool, so we can return - // the const char * - return const_name.GetCString(); - } - - void SetUserNameNotFound(uint32_t uid) { - std::lock_guard<std::mutex> guard(m_mutex); - m_uid_map[uid] = ConstString(); - } - - void ClearCachedUserNames() { - std::lock_guard<std::mutex> guard(m_mutex); - m_uid_map.clear(); - } - - const char *GetCachedGroupName(uint32_t gid) { - std::lock_guard<std::mutex> guard(m_mutex); - // return the empty string if our string is NULL so we can tell when things - // were in the negative cached (didn't find a valid group name, don't keep - // trying) - const auto pos = m_gid_map.find(gid); - return ((pos != m_gid_map.end()) ? pos->second.AsCString("") : nullptr); - } - - const char *SetCachedGroupName(uint32_t gid, const char *name, - size_t name_len) { - std::lock_guard<std::mutex> guard(m_mutex); - ConstString const_name(name); - m_gid_map[gid] = const_name; - if (m_max_gid_name_len < name_len) - m_max_gid_name_len = name_len; - // Const strings lives forever in our const string pool, so we can return - // the const char * - return const_name.GetCString(); - } - - void SetGroupNameNotFound(uint32_t gid) { - std::lock_guard<std::mutex> guard(m_mutex); - m_gid_map[gid] = ConstString(); - } - - void ClearCachedGroupNames() { - std::lock_guard<std::mutex> guard(m_mutex); - m_gid_map.clear(); - } - Status GetCachedExecutable(ModuleSpec &module_spec, lldb::ModuleSP &module_sp, const FileSpecList *module_search_paths_ptr, Platform &remote_platform); Index: include/lldb/Host/posix/HostInfoPosix.h =================================================================== --- include/lldb/Host/posix/HostInfoPosix.h +++ include/lldb/Host/posix/HostInfoPosix.h @@ -20,8 +20,7 @@ public: static size_t GetPageSize(); static bool GetHostname(std::string &s); - static const char *LookupUserName(uint32_t uid, std::string &user_name); - static const char *LookupGroupName(uint32_t gid, std::string &group_name); + static UserIDResolver &GetUserIDResolver(); static uint32_t GetUserID(); static uint32_t GetGroupID(); Index: include/lldb/Host/UserIDResolver.h =================================================================== --- /dev/null +++ include/lldb/Host/UserIDResolver.h @@ -0,0 +1,56 @@ +//===-- UserIDResolver.h ----------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_HOST_USERIDRESOLVER_H +#define LLDB_HOST_USERIDRESOLVER_H + +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/StringRef.h" +#include <mutex> + +namespace lldb_private { + +/// An abstract interface for things that know how to map numeric user/group IDs +/// into names. It caches the resolved names to avoid repeating expensive +/// queries. The cache is internally protected by a mutex, so concurrent queries +/// are safe. +class UserIDResolver { +public: + typedef uint32_t id_t; + virtual ~UserIDResolver(); // anchor + + llvm::Optional<llvm::StringRef> GetUserName(id_t Uid) { + return Get(Uid, UidCache, &UserIDResolver::DoGetUserName); + } + llvm::Optional<llvm::StringRef> GetGroupName(id_t Gid) { + return Get(Gid, GidCache, &UserIDResolver::DoGetGroupName); + } + + /// Returns a resolver which returns a failure value for each query. Useful as + /// a fallback value for the case when we know all lookups will fail. + static UserIDResolver &GetNoopResolver(); + +protected: + virtual llvm::Optional<std::string> DoGetUserName(id_t Uid) = 0; + virtual llvm::Optional<std::string> DoGetGroupName(id_t Gid) = 0; + +private: + using Map = llvm::DenseMap<id_t, llvm::Optional<std::string>>; + + llvm::Optional<llvm::StringRef> + Get(id_t Id, Map &Cache, + llvm::Optional<std::string> (UserIDResolver::*DoGet)(id_t)); + + std::mutex Mutex; + Map UidCache; + Map GidCache; +}; + +} // namespace lldb_private + +#endif // #ifndef LLDB_HOST_USERIDRESOLVER_H Index: include/lldb/Host/HostInfoBase.h =================================================================== --- include/lldb/Host/HostInfoBase.h +++ include/lldb/Host/HostInfoBase.h @@ -9,10 +9,10 @@ #ifndef lldb_Host_HostInfoBase_h_ #define lldb_Host_HostInfoBase_h_ +#include "lldb/Host/UserIDResolver.h" #include "lldb/Utility/ArchSpec.h" #include "lldb/Utility/FileSpec.h" #include "lldb/lldb-enumerations.h" - #include "llvm/ADT/StringRef.h" #include <stdint.h> @@ -99,6 +99,8 @@ //--------------------------------------------------------------------------- static ArchSpec GetAugmentedArchSpec(llvm::StringRef triple); + static UserIDResolver &GetUserIDResolver(); + protected: static bool ComputeSharedLibraryDirectory(FileSpec &file_spec); static bool ComputeSupportExeDirectory(FileSpec &file_spec);
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits