Improve error messaging when debugserver fails to complete attaching to another process on Darwin systems

[jasonmolenda]

Summary

On Darwin, we first get a process' mach task port with task_for_pid(), and then we do a ptrace(PT_ATTACHEXC). We need to complete both before we are successfully attached. There are unusual failures where we get the task port and the ptrace may not have completed entirely, so lldb is going to exit out. An error message isn't created at the layer where this failure was detected, and then at a higher layer where we try to find a reason why the attach failed, we notice that the process is being "debugged" -- by ourselves -- and emit an error message saying it's being debugged.

This patch adds an error string at all the points where we can fail to complete this two-step attach process, and also adds a refinement to the "last ditch error reporting" scheme to confirm that the process' parent process isn't already this debugserver. (we half-attached to it)

[JDevlieghere]

LGTM modulo code duplication.

lldb/tools/debugserver/source/RNBRemote.cpp

3639–3644

As this is the exact same code as in MachProcess.mm, is this worth factoring out? Something like

std::optional<int> GetAttachedProcPID() {
  struct proc_bsdshortinfo proc;
  if (proc_pidinfo(pid, PROC_PIDT_SHORTBSDINFO, 0, &proc,
                   PROC_PIDT_SHORTBSDINFO_SIZE) == sizeof(proc)) {
    return proc.pbsi_ppid;
  }
  return std::null opt;
}

[jasonmolenda]

Update to incorporate Jonas' suggestion - adding MachProcess::GetParentProcessID and MachProcess::ProcessIsBeingDebugged, as well as the DNB passthrough functions. I didn't pass back optionals for the cases were we can fail, using "invalid pid number" and "is not being debugged" for when the calls fail, which isn't the correctest but these are only ending up in error messages so i'm not too pressed about it.

[jasonmolenda]

Update the patch; that last one included a separate set of changes I was working on with Alex yesterday.

lldb/tools/debugserver/source/DNB.h

 /// \return the iOSSupportVersion of the host OS.
 std::string DNBGetMacCatalystVersionString();
 
 /// \return true if debugserver is running in translation
 /// (is an x86_64 process on arm64)
 bool DNBDebugserverIsTranslated();
 
 bool DNBGetAddressingBits(uint32_t &addressing_bits);
+
+nub_process_t DNBGetParentProcessID(nub_process_t child_pid);
+
+bool DNBProcessIsBeingDebugged(nub_process_t pid);
+
 #endif

lldb/tools/debugserver/source/DNB.cpp

if (::sysctlbyname("machdep.virtual_address_size", &g_addressing_bits, &len,
       g_addressing_bits = 0;
     }
   });
 
   addressing_bits = g_addressing_bits;
 
   return addressing_bits > 0;
 }
+
+nub_process_t DNBGetParentProcessID(nub_process_t child_pid) {
+  return MachProcess::GetParentProcessID(child_pid);
+}
+
+bool DNBProcessIsBeingDebugged(nub_process_t pid) {
+  return MachProcess::ProcessIsBeingDebugged(pid);
+}

lldb/tools/debugserver/source/MacOSX/MachProcess.h

#if defined(WITH_BKS) || defined(WITH_FBS)
   pid_t BoardServiceForkChildForPTraceDebugging(
       const char *path, char const *argv[], char const *envp[], bool no_stdio,
       bool disable_aslr, const char *event_data, DNBError &launch_err);
   bool BoardServiceSendEvent(const char *event, DNBError &error);
 #endif
   static bool GetOSVersionNumbers(uint64_t *major, uint64_t *minor,
                                   uint64_t *patch);
   static std::string GetMacCatalystVersionString();
+
+  static nub_process_t GetParentProcessID(nub_process_t child_pid);
+
+  static bool ProcessIsBeingDebugged(nub_process_t pid);
+
 #ifdef WITH_BKS
   static void BKSCleanupAfterAttach(const void *attach_token,
                                     DNBError &err_str);
 #endif // WITH_BKS
 #ifdef WITH_FBS
   static void FBSCleanupAfterAttach(const void *attach_token,
                                     DNBError &err_str);
 #endif // WITH_FBS

lldb/tools/debugserver/source/MacOSX/MachProcess.mm

if (err.Success()) {
       return m_pid;
     } else {
       ::snprintf(err_str, err_len, "%s", err.AsString());
       DNBLogError(
           "[LaunchAttach] (%d) MachProcess::AttachForDebug error: failed to "
           "attach to pid %d",
           getpid(), pid);
 
-      struct kinfo_proc kinfo;
-      int mib[] = {CTL_KERN, KERN_PROC, KERN_PROC_PID, pid};
-      size_t len = sizeof(struct kinfo_proc);
-      if (sysctl(mib, sizeof(mib) / sizeof(mib[0]), &kinfo, &len, NULL, 0) == 0 && len > 0) {
-        if (kinfo.kp_proc.p_flag & P_TRACED) {
-          ::snprintf(err_str, err_len, "%s - process %d is already being debugged", err.AsString(), pid);
+      if (ProcessIsBeingDebugged(pid)) {
+        nub_process_t ppid = GetParentProcessID(pid);
+        if (ppid == getpid()) {
+          snprintf(err_str, err_len,
+                   "%s - Failed to attach to pid %d, AttachForDebug() "
+                   "unable to ptrace(PT_ATTACHEXC)",
+                   err.AsString(), m_pid);
+        } else {
+          snprintf(err_str, err_len,
+                   "%s - process %d is already being debugged by pid %d",
+                   err.AsString(), pid, ppid);
           DNBLogError(
               "[LaunchAttach] (%d) MachProcess::AttachForDebug pid %d is "
-              "already being debugged",
-              getpid(), pid);
+              "already being debugged by pid %d",
+              getpid(), pid, ppid);
         }
       }
     }
   }
   return INVALID_NUB_PROCESS;
 }
 
 Genealogy::ThreadActivitySP
NSDictionary *version_info =
        @"/System/Library/CoreServices/SystemVersion.plist"];
     NSString *version_value = [version_info objectForKey: @"iOSSupportVersion"];
     if (const char *version_str = [version_value UTF8String])
       return version_str;
   }
   return {};
 }
 
+nub_process_t MachProcess::GetParentProcessID(nub_process_t child_pid) {
+  struct proc_bsdshortinfo proc;
+  if (proc_pidinfo(child_pid, PROC_PIDT_SHORTBSDINFO, 0, &proc,
+                   PROC_PIDT_SHORTBSDINFO_SIZE) == sizeof(proc)) {
+    return proc.pbsi_ppid;
+  }
+  return INVALID_NUB_PROCESS;
+}
+
+bool MachProcess::ProcessIsBeingDebugged(nub_process_t pid) {
+  struct kinfo_proc kinfo;
+  int mib[] = {CTL_KERN, KERN_PROC, KERN_PROC_PID, pid};
+  size_t len = sizeof(struct kinfo_proc);
+  if (sysctl(mib, sizeof(mib) / sizeof(mib[0]), &kinfo, &len, NULL, 0) == 0 &&
+      (kinfo.kp_proc.p_flag & P_TRACED))
+    return true;
+  else
+    return false;
+}
+
 #if defined(WITH_SPRINGBOARD) || defined(WITH_BKS) || defined(WITH_FBS)
 /// Get the app bundle from the given path. Returns the empty string if the
 /// path doesn't appear to be an app bundle.
 static std::string GetAppBundle(std::string path) {
   auto pos = path.rfind(".app");
   // Path doesn't contain `.app`.
   if (pos == std::string::npos)
     return {};
if (launch_flavor == eLaunchFlavorPosixSpawn) {
         launch_err.Clear();
       } else {
         SetState(eStateExited);
         DNBError ptrace_err(ptrace_errno, DNBError::POSIX);
         DNBLogThreadedIf(LOG_PROCESS, "error: failed to attach to spawned pid "
                                       "%d (err = %i, errno = %i (%s))",
                          m_pid, err, ptrace_err.Status(),
                          ptrace_err.AsString());
-        launch_err.SetError(NUB_GENERIC_ERROR, DNBError::Generic);
+        char err_msg[PATH_MAX];
+
+        snprintf(err_msg, sizeof(err_msg),
+                 "Failed to attach to pid %d, LaunchForDebug() unable to "
+                 "ptrace(PT_ATTACHEXC)",
+                 m_pid);
+        launch_err.SetErrorString(err_msg);
       }
     } else {
       launch_err.Clear();
     }
   }
   return m_pid;
 }
 
DNBLog("[LaunchAttach] (%d) About to ptrace(PT_ATTACHEXC, %d)...", getpid(),
            m_pid);
     int err = ::ptrace(PT_ATTACHEXC, m_pid, 0, 0);
     DNBLog("[LaunchAttach] (%d) Completed ptrace(PT_ATTACHEXC, %d) == %d",
            getpid(), m_pid, err);
     if (err == 0) {
       m_flags |= eMachProcessFlagsAttached;
       DNBLogThreadedIf(LOG_PROCESS, "successfully attached to pid %d", m_pid);
     } else {
+      launch_err.SetErrorString(
+          "Failed to attach to pid %d, SBLaunchForDebug() unable to "
+          "ptrace(PT_ATTACHEXC)",
+          m_pid);
       SetState(eStateExited);
       DNBLogThreadedIf(LOG_PROCESS, "error: failed to attach to pid %d", m_pid);
     }
   }
   return m_pid;
 }
 
 #include <servers/bootstrap.h>
DNBLog("[LaunchAttach] (%d) About to ptrace(PT_ATTACHEXC, %d)...", getpid(),
            m_pid);
     int err = ::ptrace(PT_ATTACHEXC, m_pid, 0, 0);
     DNBLog("[LaunchAttach] (%d) Completed ptrace(PT_ATTACHEXC, %d) == %d",
            getpid(), m_pid, err);
     if (err == 0) {
       m_flags |= eMachProcessFlagsAttached;
       DNBLog("[LaunchAttach] successfully attached to pid %d", m_pid);
     } else {
+      launch_err.SetErrorString(
+          "Failed to attach to pid %d, BoardServiceLaunchForDebug() unable to "
+          "ptrace(PT_ATTACHEXC)",
+          m_pid);
       SetState(eStateExited);
       DNBLog("[LaunchAttach] END (%d) error: failed to attach to pid %d",
              getpid(), m_pid);
     }
   }
   return m_pid;
 }
 

lldb/tools/debugserver/source/RNBRemote.cpp

 }
 
 // NOTE: this should only be called after process_does_not_exist().
 // This sysctl will return uninitialized data if we ask for a pid
 // that doesn't exist.  The alternative would be to fetch all
 // processes and step through to find the one we're looking for
 // (as process_does_not_exist() does).
 static bool process_is_already_being_debugged (nub_process_t pid) {
-  struct kinfo_proc kinfo;
-  int mib[] = {CTL_KERN, KERN_PROC, KERN_PROC_PID, pid};
-  size_t len = sizeof(struct kinfo_proc);
-  if (sysctl(mib, sizeof(mib) / sizeof(mib[0]), &kinfo, &len, NULL, 0) != 0) {
-    return false; // pid doesn't exist? well, it's not being debugged...
-  }
-  if (kinfo.kp_proc.p_flag & P_TRACED)
-    return true; // is being debugged already
+  if (DNBProcessIsBeingDebugged(pid) && DNBGetParentProcessID(pid) != getpid())
+    return true;
   else
     return false;
 }
 
 // Test if this current login session has a connection to the
 // window server (if it does not have that access, it cannot ask
 // for debug permission by popping up a dialog box and attach
 // may fail outright).
 static bool login_session_has_gui_access () {
   // I believe this API only works on macOS.
 #if TARGET_OS_OSX == 0
   return true;
 #else
   auditinfo_addr_t info;
   getaudit_addr(&info, sizeof(info));
   if (info.ai_flags & AU_SESSION_FLAG_HAS_GRAPHIC_ACCESS)
     return true;
   else