ProcessInstanceInfoMatch: Don't match processes with no name if a name match was requested, take 2

[labath]

Summary

The previous attempt at making nameless process not match when searching for a
given name failed because the macos implementation was depending on this detail
in its partial matching strategy. Doing partial matching to avoid expensive
lookups is a perfectly valid thing to do, the way it was implemented seems
somewhat unexpected.

This patch implements it differently by providing special
methods in the ProcessInstanceInfoMatch which match only a subset of fields,
and changes mac host code to use those instead.

Then, it re-applies r373925 to get make the ProcessInstanceInfoMatch with a
name *not* match a nameless process.

[JDevlieghere]

Looks reasonable to me, but I'll defer to the people that actually know this code.

lldb/include/lldb/Utility/ProcessInfo.h

public:
   void SetMatchAllUsers(bool b) { m_match_all_users = b; }
 
   NameMatch GetNameMatchType() const { return m_name_match_type; }
 
   void SetNameMatchType(NameMatch name_match_type) {
     m_name_match_type = name_match_type;
   }
 
+  /// Return true iff the architecture in this object matches arch_spec.
+  bool ArchitectureMatches(const ArchSpec &arch_spec) const;
+
+  /// Return true iff the process name in this object matches process_name.
   bool NameMatches(const char *process_name) const;
 
+  /// Return true iff the process ID and parent process IDs in this object match
+  /// the ones in proc_info.
+  bool ProcessIDsMatch(const ProcessInstanceInfo &proc_info) const;
+
+  /// Return true iff the (both effective and real) user and group IDs in this
+  /// object match the ones in proc_info.
+  bool UserIDsMatch(const ProcessInstanceInfo &proc_info) const;
+
   bool Matches(const ProcessInstanceInfo &proc_info) const;
 
   bool MatchAllProcesses() const;
   void Clear();
 
 protected:
   ProcessInstanceInfo m_match_info;
   NameMatch m_name_match_type;
   bool m_match_all_users;
 };
 
 } // namespace lldb_private
 
 #endif // #ifndef LLDB_UTILITY_PROCESSINFO_H

lldb/source/Host/macosx/objcxx/Host.mm

for (size_t i = 0; i < actual_pid_count; i++) {
     process_info.SetGroupID(kinfo.kp_eproc.e_pcred.p_rgid);
     process_info.SetEffectiveUserID(kinfo.kp_eproc.e_ucred.cr_uid);
     if (kinfo.kp_eproc.e_ucred.cr_ngroups > 0)
       process_info.SetEffectiveGroupID(kinfo.kp_eproc.e_ucred.cr_groups[0]);
     else
       process_info.SetEffectiveGroupID(UINT32_MAX);
 
     // Make sure our info matches before we go fetch the name and cpu type
-    if (match_info.Matches(process_info)) {
-      // Get CPU type first so we can know to look for iOS simulator is we have
-      // x86 or x86_64
-      if (GetMacOSXProcessCPUType(process_info)) {
-        if (GetMacOSXProcessArgs(&match_info, process_info)) {
-          if (match_info.Matches(process_info))
-            process_infos.Append(process_info);
-        }
-      }
-    }
-  }
+    if (!match_info.UserIDsMatch(process_info) ||
+        !match_info.ProcessIDsMatch(process_info))
+      continue;
+
   return process_infos.GetSize();
 }
 
 bool Host::GetProcessInfo(lldb::pid_t pid, ProcessInstanceInfo &process_info) {
   process_info.SetProcessID(pid);
   bool success = false;
 
   // Get CPU type first so we can know to look for iOS simulator is we have x86

lldb/source/Utility/ProcessInfo.cpp

if (m_pid != LLDB_INVALID_PROCESS_ID) {
     } else {
       s.PutCString(GetName());
     }
 
     s.EOL();
   }
 }
 
+bool ProcessInstanceInfoMatch::ArchitectureMatches(
+    const ArchSpec &arch_spec) const {
+  return !m_match_info.GetArchitecture().IsValid() ||
+         m_match_info.GetArchitecture().IsCompatibleMatch(arch_spec);
+}
+
 bool ProcessInstanceInfoMatch::NameMatches(const char *process_name) const {
-  if (m_name_match_type == NameMatch::Ignore || process_name == nullptr)
+  if (m_name_match_type == NameMatch::Ignore)
     return true;
   const char *match_name = m_match_info.GetName();
   if (!match_name)
     return true;
 
   return lldb_private::NameMatches(process_name, m_name_match_type, match_name);
 }
 
-bool ProcessInstanceInfoMatch::Matches(
+bool ProcessInstanceInfoMatch::ProcessIDsMatch(
     const ProcessInstanceInfo &proc_info) const {
-  if (!NameMatches(proc_info.GetName()))
-    return false;
-
   if (m_match_info.ProcessIDIsValid() &&
       m_match_info.GetProcessID() != proc_info.GetProcessID())
     return false;
 
   if (m_match_info.ParentProcessIDIsValid() &&
       m_match_info.GetParentProcessID() != proc_info.GetParentProcessID())
     return false;
+  return true;
+}
 
+bool ProcessInstanceInfoMatch::UserIDsMatch(
+    const ProcessInstanceInfo &proc_info) const {
   if (m_match_info.UserIDIsValid() &&
       m_match_info.GetUserID() != proc_info.GetUserID())
     return false;
 
   if (m_match_info.GroupIDIsValid() &&
       m_match_info.GetGroupID() != proc_info.GetGroupID())
     return false;
 
   if (m_match_info.EffectiveUserIDIsValid() &&
       m_match_info.GetEffectiveUserID() != proc_info.GetEffectiveUserID())
     return false;
 
   if (m_match_info.EffectiveGroupIDIsValid() &&
       m_match_info.GetEffectiveGroupID() != proc_info.GetEffectiveGroupID())
     return false;
-
-  if (m_match_info.GetArchitecture().IsValid() &&
-      !m_match_info.GetArchitecture().IsCompatibleMatch(
-          proc_info.GetArchitecture()))
-    return false;
   return true;
 }
+bool ProcessInstanceInfoMatch::Matches(
+    const ProcessInstanceInfo &proc_info) const {
+  return ArchitectureMatches(proc_info.GetArchitecture()) &&
+         ProcessIDsMatch(proc_info) && UserIDsMatch(proc_info) &&
+         NameMatches(proc_info.GetName());
+}
 
 bool ProcessInstanceInfoMatch::MatchAllProcesses() const {
   if (m_name_match_type != NameMatch::Ignore)
     return false;
 
   if (m_match_info.ProcessIDIsValid())
     return false;
 

lldb/unittests/Utility/ProcessInstanceInfoTest.cpp

TEST(ProcessInstanceInfo, DumpTable_invalidUID) {
   info.DumpAsTableRow(s, resolver, show_args, verbose);
   EXPECT_STREQ(
       R"(PID    PARENT USER       TRIPLE                         NAME
 ====== ====== ========== ============================== ============================
 47     0                 aarch64-unknown-linux-android  a.out
 )",
       s.GetData());
 }
+
+TEST(ProcessInstanceInfoMatch, Name) {
+  ProcessInstanceInfo info_bar, info_empty;
+  info_bar.GetExecutableFile().SetFile("/foo/bar", FileSpec::Style::posix);
+
+  ProcessInstanceInfoMatch match;
+  match.SetNameMatchType(NameMatch::Equals);
+  match.GetProcessInfo().GetExecutableFile().SetFile("bar",
+                                                     FileSpec::Style::posix);
+
+  EXPECT_TRUE(match.Matches(info_bar));
+  EXPECT_FALSE(match.Matches(info_empty));
+
+  match.GetProcessInfo().GetExecutableFile() = FileSpec();
+  EXPECT_TRUE(match.Matches(info_bar));
+  EXPECT_TRUE(match.Matches(info_empty));
+}