This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB355323: Refactor user/group name resolving code (authored by labath, committed by ). Herald added a project: LLDB.
Changed prior to commit: https://reviews.llvm.org/D58167?vs=188448&id=189169#toc Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58167/new/ https://reviews.llvm.org/D58167 Files: include/lldb/Host/HostInfoBase.h include/lldb/Host/posix/HostInfoPosix.h include/lldb/Target/Platform.h include/lldb/Target/Process.h include/lldb/Target/RemoteAwarePlatform.h include/lldb/Utility/UserIDResolver.h source/Commands/CommandObjectPlatform.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 source/Utility/CMakeLists.txt source/Utility/UserIDResolver.cpp unittests/Target/CMakeLists.txt unittests/Target/ProcessInstanceInfoTest.cpp unittests/Utility/CMakeLists.txt unittests/Utility/UserIDResolverTest.cpp
Index: unittests/Utility/CMakeLists.txt =================================================================== --- unittests/Utility/CMakeLists.txt +++ unittests/Utility/CMakeLists.txt @@ -34,6 +34,7 @@ TimeoutTest.cpp TimerTest.cpp UriParserTest.cpp + UserIDResolverTest.cpp UUIDTest.cpp VASprintfTest.cpp VMRangeTest.cpp Index: unittests/Utility/UserIDResolverTest.cpp =================================================================== --- unittests/Utility/UserIDResolverTest.cpp +++ unittests/Utility/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/Utility/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 gid)); +}; +} // 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/Target/ProcessInstanceInfoTest.cpp =================================================================== --- unittests/Target/ProcessInstanceInfoTest.cpp +++ 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 gid) { + if (gid % 2) + return ("group" + llvm::Twine(gid)).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: 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/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -436,10 +436,10 @@ packet.SetFilePos(::strlen("qUserName:")); uint32_t uid = packet.GetU32(UINT32_MAX); if (uid != UINT32_MAX) { - std::string name; - if (HostInfo::LookupUserName(uid, name)) { + if (llvm::Optional<llvm::StringRef> name = + HostInfo::GetUserIDResolver().GetUserName(uid)) { StreamString response; - response.PutStringAsRawHex8(name); + response.PutStringAsRawHex8(*name); return SendPacketNoLock(response.GetString()); } } @@ -457,10 +457,10 @@ packet.SetFilePos(::strlen("qGroupName:")); uint32_t gid = packet.GetU32(UINT32_MAX); if (gid != UINT32_MAX) { - std::string name; - if (HostInfo::LookupGroupName(gid, name)) { + if (llvm::Optional<llvm::StringRef> name = + HostInfo::GetUserIDResolver().GetGroupName(gid)) { StreamString response; - response.PutStringAsRawHex8(name); + response.PutStringAsRawHex8(*name); return SendPacketNoLock(response.GetString()); } } 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: source/Host/posix/HostInfoPosix.cpp =================================================================== --- source/Host/posix/HostInfoPosix.cpp +++ source/Host/posix/HostInfoPosix.cpp @@ -48,40 +48,38 @@ #define USE_GETPWUID #endif -#ifdef USE_GETPWUID -static std::mutex s_getpwuid_lock; -#endif +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 -const char *HostInfoPosix::LookupUserName(uint32_t uid, - std::string &user_name) { +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(); - } + &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); @@ -90,24 +88,25 @@ // Try the threadsafe version first 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(); - } + 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/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/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) { @@ -3057,11 +3040,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/RemoteAwarePlatform.cpp =================================================================== --- source/Target/RemoteAwarePlatform.cpp +++ source/Target/RemoteAwarePlatform.cpp @@ -10,6 +10,7 @@ #include "lldb/Host/FileCache.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/Host.h" +#include "lldb/Host/HostInfo.h" using namespace lldb_private; @@ -204,25 +205,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/Utility/CMakeLists.txt =================================================================== --- source/Utility/CMakeLists.txt +++ source/Utility/CMakeLists.txt @@ -85,6 +85,7 @@ TildeExpressionResolver.cpp Timer.cpp UserID.cpp + UserIDResolver.cpp UriParser.cpp UUID.cpp VASprintf.cpp Index: source/Utility/UserIDResolver.cpp =================================================================== --- source/Utility/UserIDResolver.cpp +++ source/Utility/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/Utility/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::*do_get)(id_t)) { + + std::lock_guard<std::mutex> guard(m_mutex); + auto iter_bool = cache.try_emplace(id, llvm::None); + if (iter_bool.second) + iter_bool.first->second = (this->*do_get)(id); + if (iter_bool.first->second) + return llvm::StringRef(*iter_bool.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: include/lldb/Target/Platform.h =================================================================== --- include/lldb/Target/Platform.h +++ include/lldb/Target/Platform.h @@ -23,6 +23,7 @@ #include "lldb/Utility/ConstString.h" #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/Timeout.h" +#include "lldb/Utility/UserIDResolver.h" #include "lldb/lldb-private-forward.h" #include "lldb/lldb-public.h" #include "llvm/Support/VersionTuple.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/Target/RemoteAwarePlatform.h =================================================================== --- include/lldb/Target/RemoteAwarePlatform.h +++ include/lldb/Target/RemoteAwarePlatform.h @@ -70,8 +70,7 @@ const Timeout<std::micro> &timeout) 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 @@ -46,6 +46,7 @@ #include "lldb/Utility/Status.h" #include "lldb/Utility/StructuredData.h" #include "lldb/Utility/TraceOptions.h" +#include "lldb/Utility/UserIDResolver.h" #include "lldb/lldb-private.h" #include "llvm/ADT/ArrayRef.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/Host/HostInfoBase.h =================================================================== --- include/lldb/Host/HostInfoBase.h +++ include/lldb/Host/HostInfoBase.h @@ -11,8 +11,8 @@ #include "lldb/Utility/ArchSpec.h" #include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/UserIDResolver.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); 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/Utility/UserIDResolver.h =================================================================== --- include/lldb/Utility/UserIDResolver.h +++ include/lldb/Utility/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_UTILITY_USERIDRESOLVER_H +#define LLDB_UTILITY_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, m_uid_cache, &UserIDResolver::DoGetUserName); + } + llvm::Optional<llvm::StringRef> GetGroupName(id_t gid) { + return Get(gid, m_gid_cache, &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::*do_get)(id_t)); + + std::mutex m_mutex; + Map m_uid_cache; + Map m_gid_cache; +}; + +} // namespace lldb_private + +#endif // #ifndef LLDB_HOST_USERIDRESOLVER_H
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
