[Lldb-commits] [lldb] [lldb][Process/FreeBSDKernelCore] Fix thread ordering (PR #187976)

[David Spickett via lldb-commits]

================
@@ -247,26 +247,34 @@ bool ProcessFreeBSDKernelCore::DoUpdateThreadList(ThreadList &old_thread_list,
     // https://cgit.freebsd.org/src/tree/sys/sys/param.h
     constexpr size_t fbsd_maxcomlen = 19;
 
-    // Iterate through a linked list of all processes. New processes are added
-    // to the head of this list. Which means that earlier PIDs are actually at
-    // the end of the list, so we have to walk it backwards. First collect all
-    // the processes in the list order.
-    std::vector<lldb::addr_t> process_addrs;
+    // Iterate through a linked list of all processes then order incrementally
+    // by pid. Though new processes are added to the head of this list, process
+    // ids may be reused as well. So we cannot rely on it being in a particular
+    // order.
+    std::vector<std::pair<lldb::addr_t, int32_t>> process_addrs;
     if (lldb::addr_t allproc_addr = FindSymbol("allproc");
         allproc_addr != LLDB_INVALID_ADDRESS) {
       for (lldb::addr_t proc = ReadPointerFromMemory(allproc_addr, error);
            proc != 0 && proc != LLDB_INVALID_ADDRESS && error.Success();
-           proc = ReadPointerFromMemory(proc + offset_p_list, error))
-        process_addrs.push_back(proc);
+           proc = ReadPointerFromMemory(proc + offset_p_list, error)) {
+        int32_t pid =
+            ReadSignedIntegerFromMemory(proc + offset_p_pid, 4, -1, error);
+        if (error.Fail())
+          return false;
+        process_addrs.emplace_back(proc, pid);
+      }
     }
 
-    // Processes are in the linked list in descending PID order, so we must walk
-    // them in reverse to get ascending PID order.
-    for (auto proc_it = process_addrs.rbegin(); proc_it != process_addrs.rend();
-         ++proc_it) {
-      lldb::addr_t proc = *proc_it;
-      int32_t pid =
-          ReadSignedIntegerFromMemory(proc + offset_p_pid, 4, -1, error);
+    if (error.Fail())
+      return false;
----------------
DavidSpickett wrote:

Sounds good, I think fixing it here would get in the way of the real purpose of this PR.

https://github.com/llvm/llvm-project/pull/187976