[Lldb-commits] [lldb] r355323 - Refactor user/group name resolving code

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Mon Mar 4 10:48:00 PST 2019


Author: labath
Date: Mon Mar  4 10:48:00 2019
New Revision: 355323

URL: http://llvm.org/viewvc/llvm-project?rev=355323&view=rev
Log:
Refactor user/group name resolving code

Summary:
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.

Reviewers: zturner, clayborg, jingham

Subscribers: mgorny, lldb-commits

Differential Revision: https://reviews.llvm.org/D58167

Added:
    lldb/trunk/include/lldb/Utility/UserIDResolver.h
    lldb/trunk/source/Utility/UserIDResolver.cpp
    lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp
    lldb/trunk/unittests/Utility/UserIDResolverTest.cpp
Modified:
    lldb/trunk/include/lldb/Host/HostInfoBase.h
    lldb/trunk/include/lldb/Host/posix/HostInfoPosix.h
    lldb/trunk/include/lldb/Target/Platform.h
    lldb/trunk/include/lldb/Target/Process.h
    lldb/trunk/include/lldb/Target/RemoteAwarePlatform.h
    lldb/trunk/source/Commands/CommandObjectPlatform.cpp
    lldb/trunk/source/Host/posix/HostInfoPosix.cpp
    lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.h
    lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
    lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
    lldb/trunk/source/Target/Platform.cpp
    lldb/trunk/source/Target/Process.cpp
    lldb/trunk/source/Target/RemoteAwarePlatform.cpp
    lldb/trunk/source/Utility/CMakeLists.txt
    lldb/trunk/unittests/Target/CMakeLists.txt
    lldb/trunk/unittests/Utility/CMakeLists.txt

Modified: lldb/trunk/include/lldb/Host/HostInfoBase.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/HostInfoBase.h?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/HostInfoBase.h (original)
+++ lldb/trunk/include/lldb/Host/HostInfoBase.h Mon Mar  4 10:48:00 2019
@@ -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 @@ public:
   //---------------------------------------------------------------------------
   static ArchSpec GetAugmentedArchSpec(llvm::StringRef triple);
 
+  static UserIDResolver &GetUserIDResolver();
+
 protected:
   static bool ComputeSharedLibraryDirectory(FileSpec &file_spec);
   static bool ComputeSupportExeDirectory(FileSpec &file_spec);

Modified: lldb/trunk/include/lldb/Host/posix/HostInfoPosix.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/posix/HostInfoPosix.h?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/posix/HostInfoPosix.h (original)
+++ lldb/trunk/include/lldb/Host/posix/HostInfoPosix.h Mon Mar  4 10:48:00 2019
@@ -20,8 +20,7 @@ class HostInfoPosix : public HostInfoBas
 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();

Modified: lldb/trunk/include/lldb/Target/Platform.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Platform.h?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Platform.h (original)
+++ lldb/trunk/include/lldb/Target/Platform.h Mon Mar  4 10:48:00 2019
@@ -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 @@ public:
 
   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 @@ protected:
   // 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 @@ protected:
   //------------------------------------------------------------------
   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);

Modified: lldb/trunk/include/lldb/Target/Process.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Mon Mar  4 10:48:00 2019
@@ -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 @@ public:
     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:

Modified: lldb/trunk/include/lldb/Target/RemoteAwarePlatform.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/RemoteAwarePlatform.h?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/RemoteAwarePlatform.h (original)
+++ lldb/trunk/include/lldb/Target/RemoteAwarePlatform.h Mon Mar  4 10:48:00 2019
@@ -70,8 +70,7 @@ public:
                          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;

Added: lldb/trunk/include/lldb/Utility/UserIDResolver.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/UserIDResolver.h?rev=355323&view=auto
==============================================================================
--- lldb/trunk/include/lldb/Utility/UserIDResolver.h (added)
+++ lldb/trunk/include/lldb/Utility/UserIDResolver.h Mon Mar  4 10:48:00 2019
@@ -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

Modified: lldb/trunk/source/Commands/CommandObjectPlatform.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectPlatform.cpp?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectPlatform.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectPlatform.cpp Mon Mar  4 10:48:00 2019
@@ -1151,10 +1151,9 @@ protected:
           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 @@ protected:
                 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 @@ protected:
               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",

Modified: lldb/trunk/source/Host/posix/HostInfoPosix.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/HostInfoPosix.cpp?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/source/Host/posix/HostInfoPosix.cpp (original)
+++ lldb/trunk/source/Host/posix/HostInfoPosix.cpp Mon Mar  4 10:48:00 2019
@@ -48,40 +48,38 @@ bool HostInfoPosix::GetHostname(std::str
 #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 @@ const char *HostInfoPosix::LookupGroupNa
   // 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(); }

Modified: lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.h?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.h (original)
+++ lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.h Mon Mar  4 10:48:00 2019
@@ -62,6 +62,10 @@ public:
 
   void CalculateTrapHandlerSymbolNames() override;
 
+  UserIDResolver &GetUserIDResolver() override {
+    return UserIDResolver::GetNoopResolver();
+  }
+
 protected:
   lldb::PlatformSP m_remote_platform_sp;
 

Modified: lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp Mon Mar  4 10:48:00 2019
@@ -340,29 +340,20 @@ const char *PlatformRemoteGDBServer::Get
   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(

Modified: lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h (original)
+++ lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h Mon Mar  4 10:48:00 2019
@@ -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 @@ public:
   // 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 @@ private:
                                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);
 };
 

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp Mon Mar  4 10:48:00 2019
@@ -436,10 +436,10 @@ GDBRemoteCommunicationServerCommon::Hand
   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 @@ GDBRemoteCommunicationServerCommon::Hand
   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());
     }
   }

Modified: lldb/trunk/source/Target/Platform.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Platform.cpp?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/source/Target/Platform.cpp (original)
+++ lldb/trunk/source/Target/Platform.cpp Mon Mar  4 10:48:00 2019
@@ -384,10 +384,10 @@ Platform::Platform(bool is_host)
     : 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 @@ bool Platform::SetRemoteWorkingDirectory
   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

Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Mon Mar  4 10:48:00 2019
@@ -278,8 +278,7 @@ bool ProcessProperties::GetStopOnExec()
       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 @@ void ProcessInstanceInfo::Dump(Stream &s
     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::DumpTableHeade
   }
 }
 
-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 @@ Status Process::Attach(ProcessAttachInfo
                 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,

Modified: lldb/trunk/source/Target/RemoteAwarePlatform.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/RemoteAwarePlatform.cpp?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/source/Target/RemoteAwarePlatform.cpp (original)
+++ lldb/trunk/source/Target/RemoteAwarePlatform.cpp Mon Mar  4 10:48:00 2019
@@ -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 @@ const char *RemoteAwarePlatform::GetHost
   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() {

Modified: lldb/trunk/source/Utility/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/CMakeLists.txt?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/source/Utility/CMakeLists.txt (original)
+++ lldb/trunk/source/Utility/CMakeLists.txt Mon Mar  4 10:48:00 2019
@@ -85,6 +85,7 @@ add_lldb_library(lldbUtility
   TildeExpressionResolver.cpp
   Timer.cpp
   UserID.cpp
+  UserIDResolver.cpp
   UriParser.cpp
   UUID.cpp
   VASprintf.cpp

Added: lldb/trunk/source/Utility/UserIDResolver.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/UserIDResolver.cpp?rev=355323&view=auto
==============================================================================
--- lldb/trunk/source/Utility/UserIDResolver.cpp (added)
+++ lldb/trunk/source/Utility/UserIDResolver.cpp Mon Mar  4 10:48:00 2019
@@ -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; }

Modified: lldb/trunk/unittests/Target/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Target/CMakeLists.txt?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/unittests/Target/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Target/CMakeLists.txt Mon Mar  4 10:48:00 2019
@@ -2,6 +2,7 @@ add_lldb_unittest(TargetTests
   MemoryRegionInfoTest.cpp
   ModuleCacheTest.cpp
   PathMappingListTest.cpp
+  ProcessInstanceInfoTest.cpp
 
   LINK_LIBS
       lldbCore

Added: lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp?rev=355323&view=auto
==============================================================================
--- lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp (added)
+++ lldb/trunk/unittests/Target/ProcessInstanceInfoTest.cpp Mon Mar  4 10:48:00 2019
@@ -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());
+}

Modified: lldb/trunk/unittests/Utility/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/CMakeLists.txt?rev=355323&r1=355322&r2=355323&view=diff
==============================================================================
--- lldb/trunk/unittests/Utility/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Utility/CMakeLists.txt Mon Mar  4 10:48:00 2019
@@ -34,6 +34,7 @@ add_lldb_unittest(UtilityTests
   TimeoutTest.cpp
   TimerTest.cpp
   UriParserTest.cpp
+  UserIDResolverTest.cpp
   UUIDTest.cpp
   VASprintfTest.cpp
   VMRangeTest.cpp

Added: lldb/trunk/unittests/Utility/UserIDResolverTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/UserIDResolverTest.cpp?rev=355323&view=auto
==============================================================================
--- lldb/trunk/unittests/Utility/UserIDResolverTest.cpp (added)
+++ lldb/trunk/unittests/Utility/UserIDResolverTest.cpp Mon Mar  4 10:48:00 2019
@@ -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));
+}




More information about the lldb-commits mailing list