[lldb/Plugin] Sort the ScriptedProcess' thread list before creating threads

lldb/examples/python/crashlog.py

@@ -76,10 +76,12 @@ class Thread:
 
         def __init__(self, index, app_specific_backtrace):
             self.index = index
+            self.id = index
             self.frames = list()
             self.idents = list()
             self.registers = dict()
             self.reason = None
+            self.name = None
             self.queue = None
             self.crashed = False
             self.app_specific_backtrace = app_specific_backtrace
@@ -521,14 +523,18 @@ def parse_threads(self, json_threads):
         for json_thread in json_threads:
             thread = self.crashlog.Thread(idx, False)
             if 'name' in json_thread:
+                thread.name = json_thread['name']
                 thread.reason = json_thread['name']
+            if 'id' in json_thread:
+                thread.id = int(json_thread['id'])
             if json_thread.get('triggered', False):
                 self.crashlog.crashed_thread_idx = idx
                 thread.crashed = True
                 if 'threadState' in json_thread:
                     thread.registers = self.parse_thread_registers(
                         json_thread['threadState'])
-            thread.queue = json_thread.get('queue')
+            if 'queue' in json_thread:
+                thread.queue = json_thread.get('queue')
             self.parse_frames(thread, json_thread.get('frames', []))
             self.crashlog.threads.append(thread)
             idx += 1

lldb/examples/python/scripted_process/crashlog_scripted_process.py

@@ -135,15 +135,12 @@ def __init__(self, process, args, crashlog_thread):
 
         self.backing_thread = crashlog_thread
         self.idx = self.backing_thread.index
+        self.tid = self.backing_thread.id
+        self.name = self.backing_thread.name
+        self.queue = self.backing_thread.queue
         self.has_crashed = (self.scripted_process.crashed_thread_idx == self.idx)
         self.create_stackframes()
 
-    def get_thread_id(self) -> int:
-        return self.idx
-
-    def get_name(self) -> str:
-        return CrashLogScriptedThread.__name__ + ".thread-" + str(self.idx)
-
     def get_state(self):
         if not self.has_crashed:
             return lldb.eStateStopped

lldb/examples/python/scripted_process/scripted_process.py

@@ -219,8 +219,8 @@ def __init__(self, scripted_process, args):
         self.scripted_process = None
         self.process = None
         self.args = None
-
-        self.id = None
+        self.idx = 0
+        self.tid = 0
         self.idx = None
         self.name = None
         self.queue = None
@@ -236,24 +236,29 @@ def __init__(self, scripted_process, args):
             self.process = self.target.GetProcess()
             self.get_register_info()
 
+    def get_thread_idx(self):
+        """ Get the scripted thread index.
+
+        Returns:
+            int: The index of the scripted thread in the scripted process.
+        """
+        return self.idx
 
-    @abstractmethod
     def get_thread_id(self):
         """ Get the scripted thread identifier.
 
         Returns:
             int: The identifier of the scripted thread.
         """
-        pass
+        return self.tid
 
-    @abstractmethod
     def get_name(self):
         """ Get the scripted thread name.
 
         Returns:
             str: The name of the scripted thread.
         """
-        pass
+        return self.name
 
     def get_state(self):
         """ Get the scripted thread state type.
@@ -277,7 +282,7 @@ def get_queue(self):
         Returns:
             str: The queue name associated with the scripted thread.
         """
-        pass
+        return self.queue
 
     @abstractmethod
     def get_stop_reason(self):

lldb/include/lldb/Utility/StructuredData.h

@@ -376,15 +376,15 @@ class StructuredData {
       }
     }
 
-    ObjectSP GetKeys() const {
-      auto object_sp = std::make_shared<Array>();
+    ArraySP GetKeys() const {
+      auto array_sp = std::make_shared<Array>();
       collection::const_iterator iter;
       for (iter = m_dict.begin(); iter != m_dict.end(); ++iter) {
         auto key_object_sp = std::make_shared<String>();
         key_object_sp->SetValue(iter->first.AsCString());
-        object_sp->Push(key_object_sp);
+        array_sp->Push(key_object_sp);
       }
-      return object_sp;
+      return array_sp;
     }
 
     ObjectSP GetValueForKey(llvm::StringRef key) const {

lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp

@@ -309,35 +309,55 @@ bool ScriptedProcess::DoUpdateThreadList(ThreadList &old_thread_list,
 
   StructuredData::DictionarySP thread_info_sp = GetInterface().GetThreadsInfo();
 
-  // FIXME: Need to sort the dictionary otherwise the thread ids won't match the
-  // thread indices.
-
   if (!thread_info_sp)
     return ScriptedInterface::ErrorWithMessage<bool>(
         LLVM_PRETTY_FUNCTION,
         "Couldn't fetch thread list from Scripted Process.", error);
 
+  // Because `StructuredData::Dictionary` uses a `std::map<ConstString,
+  // ObjectSP>` for storage, each item is sorted based on the key alphabetical
+  // order. Since `GetThreadsInfo` provides thread indices as the key element,
+  // thread info comes ordered alphabetically, instead of numerically, so we
+  // need to sort the thread indices before creating thread.
+
+  StructuredData::ArraySP keys = thread_info_sp->GetKeys();
+
+  std::map<size_t, StructuredData::ObjectSP> sorted_threads;
+  auto sort_keys = [&sorted_threads,
+                    &thread_info_sp](StructuredData::Object *item) -> bool {
+    if (!item)
+      return false;
+
+    llvm::StringRef key = item->GetStringValue();
+    size_t idx = 0;
+
+    // Make sure the provided index is actually an integer
+    if (!llvm::to_integer(key, idx))
+      return false;
+
+    sorted_threads[idx] = thread_info_sp->GetValueForKey(key);
+    return true;
+  };
+
+  size_t thread_count = thread_info_sp->GetSize();
+
+  if (!keys->ForEach(sort_keys) || sorted_threads.size() != thread_count)
+    // Might be worth showing the unsorted thread list instead of return early.
+    return ScriptedInterface::ErrorWithMessage<bool>(
+        LLVM_PRETTY_FUNCTION, "Couldn't sort thread list.", error);
+
   auto create_scripted_thread =
-      [this, &old_thread_list, &error,
-       &new_thread_list](ConstString key, StructuredData::Object *val) -> bool {
-    if (!val)
-      return ScriptedInterface::ErrorWithMessage<bool>(
-          LLVM_PRETTY_FUNCTION, "Invalid thread info object", error);
+      [this, &error, &new_thread_list](
+          const std::pair<size_t, StructuredData::ObjectSP> pair) -> bool {
+    size_t idx = pair.first;
+    StructuredData::ObjectSP object_sp = pair.second;
 
-    lldb::tid_t tid = LLDB_INVALID_THREAD_ID;
-    if (!llvm::to_integer(key.AsCString(), tid))
+    if (!object_sp)
       return ScriptedInterface::ErrorWithMessage<bool>(
-          LLVM_PRETTY_FUNCTION, "Invalid thread id", error);
-
-    if (ThreadSP thread_sp =
-            old_thread_list.FindThreadByID(tid, false /*=can_update*/)) {
-      // If the thread was already in the old_thread_list,
-      // just add it back to the new_thread_list.
-      new_thread_list.AddThread(thread_sp);
-      return true;
-    }
+          LLVM_PRETTY_FUNCTION, "Invalid thread info object", error);
 
-    auto thread_or_error = ScriptedThread::Create(*this, val->GetAsGeneric());
+    auto thread_or_error =
+        ScriptedThread::Create(*this, object_sp->GetAsGeneric());
 
     if (!thread_or_error)
       return ScriptedInterface::ErrorWithMessage<bool>(
@@ -350,8 +370,7 @@ bool ScriptedProcess::DoUpdateThreadList(ThreadList &old_thread_list,
     if (!reg_ctx_sp)
       return ScriptedInterface::ErrorWithMessage<bool>(
           LLVM_PRETTY_FUNCTION,
-          llvm::Twine("Invalid Register Context for thread " +
-                      llvm::Twine(key.AsCString()))
+          llvm::Twine("Invalid Register Context for thread " + llvm::Twine(idx))
               .str(),
           error);
 
@@ -360,7 +379,7 @@ bool ScriptedProcess::DoUpdateThreadList(ThreadList &old_thread_list,
     return true;
   };
 
-  thread_info_sp->ForEach(create_scripted_thread);
+  llvm::for_each(sorted_threads, create_scripted_thread);
 
   return new_thread_list.GetSize(false) > 0;
 }

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test

@@ -4,17 +4,17 @@
 
 # RUN: cp %S/Inputs/scripted_crashlog.ips %t.crash
 # RUN: %python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash --offsets '{"main":160, "bar":20, "foo":24}' --json
-# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog -a -i %t.crash' 2>&1 -o "bt all" | FileCheck %s
+# RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 'crashlog -a -i %t.crash' 2>&1 -o "thread list" -o "bt all" | FileCheck %s
 
 # CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
 
 # CHECK: (lldb) process status
 # CHECK-NEXT: Process 24991 stopped
-# CHECK-NEXT: * thread #3, name = 'CrashLogScriptedThread.thread-2', stop reason = EXC_BAD_ACCESS
+# CHECK-NEXT: * thread #3, stop reason = EXC_BAD_ACCESS
 # CHECK-NEXT:     frame #0: 0x00000001047f5970 scripted_crashlog_json.test.tmp.out`bar
 
 # CHECK: (lldb) thread backtrace
-# CHECK-NEXT: * thread #3, name = 'CrashLogScriptedThread.thread-2', stop reason = EXC_BAD_ACCESS
+# CHECK-NEXT: * thread #3, stop reason = EXC_BAD_ACCESS
 # CHECK-NEXT:   * frame #0: 0x00000001047f5970 scripted_crashlog_json.test.tmp.out`bar
 # CHECK-NEXT:     frame #1: 0x00000001047f5998 scripted_crashlog_json.test.tmp.out`foo
 # CHECK-NEXT:     frame #2: 0x00000001047f5b04 scripted_crashlog_json.test.tmp.out`compute_pow
@@ -24,22 +24,29 @@
 # CHECK-NEXT:     frame #6: 0x000000018bf5326c libsystem_pthread.dylib`_pthread_start
 # CHECK-NEXT:     frame #7: 0x000000018bf4e08c libsystem_pthread.dylib`thread_start
 
+# CHECK: (lldb) thread list
+# CHECK-NEXT: Process 24991 stopped
+# CHECK-NEXT:  thread #1: tid = 0x4ea840, 0x000000018bf17854 libsystem_kernel.dylib`__ulock_wait{{.*}}, queue = 'com.apple.main-thread'
+# CHECK-NEXT:  thread #2: tid = 0x4ea850, 0x00000001047f59e8 scripted_crashlog_json.test.tmp.out`call_and_wait
+# CHECK-NEXT: * thread #3: tid = 0x4ea851, 0x00000001047f5970 scripted_crashlog_json.test.tmp.out`bar{{.*}}, stop reason = EXC_BAD_ACCESS
+
+
 # CHECK: (lldb) bt all
-# CHECK-NEXT:   thread #1, name = 'CrashLogScriptedThread.thread-0'
+# CHECK-NEXT:   thread #1
 # CHECK-NEXT:     frame #0: 0x000000018bf17854 libsystem_kernel.dylib`__ulock_wait
 # CHECK-NEXT:     frame #1: 0x000000018bf555a0 libsystem_pthread.dylib`_pthread_join
 # CHECK-NEXT:     frame #2: 0x000000018beae9c0 libc++.1.dylib`std::__1::thread::join
 # CHECK-NEXT:     frame #3: 0x00000001047f5bb8 scripted_crashlog_json.test.tmp.out`main
 # CHECK-NEXT:     frame #4: 0x0000000104ae5088 dyld`start
-# CHECK-NEXT:   thread #2, name = 'CrashLogScriptedThread.thread-1'
+# CHECK-NEXT:   thread #2
 # CHECK-NEXT:     frame #0: 0x00000001047f59e8 scripted_crashlog_json.test.tmp.out`call_and_wait
 # CHECK-NEXT:     frame #1: 0x00000001047f59d4 scripted_crashlog_json.test.tmp.out`call_and_wait
 # CHECK-NEXT:     frame #2: 0x00000001047f7690 scripted_crashlog_json.test.tmp.out`decltype
 # CHECK-NEXT:     frame #3: 0x00000001047f7614 scripted_crashlog_json.test.tmp.out`void std::__1::__thread_execute
 # CHECK-NEXT:     frame #4: 0x00000001047f6d58 scripted_crashlog_json.test.tmp.out`void* std::__1::__thread_proxy
 # CHECK-NEXT:     frame #5: 0x000000018bf5326c libsystem_pthread.dylib`_pthread_start
 # CHECK-NEXT:     frame #6: 0x000000018bf4e08c libsystem_pthread.dylib`thread_start
-# CHECK-NEXT: * thread #3, name = 'CrashLogScriptedThread.thread-2', stop reason = EXC_BAD_ACCESS
+# CHECK-NEXT: * thread #3, stop reason = EXC_BAD_ACCESS
 # CHECK-NEXT:   * frame #0: 0x00000001047f5970 scripted_crashlog_json.test.tmp.out`bar
 # CHECK-NEXT:     frame #1: 0x00000001047f5998 scripted_crashlog_json.test.tmp.out`foo
 # CHECK-NEXT:     frame #2: 0x00000001047f5b04 scripted_crashlog_json.test.tmp.out`compute_pow