Don't assume the backing thread shares a Protocol ID

[JDevlieghere]

Summary

When we're dealing with virtual (memory) threads created by the OS
plugins, there's no guarantee that the real thread and the backing
thread share a protocol ID. Instead, we should iterate over the memory
threads to find the virtual thread that is backed by the current real
thread.

rdar://36485830

[JDevlieghere]

(I'll update the diff once I've figured out how to test this)

[clayborg]

source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

1831–1832

I would add a new function to ThreadList:

ThreadSP ThreadList::GetBackingThread(const ThreadSP &real_thread);

Since there is a mutex inside the thread list, we don't want another thread to be able to mutate the thread list while doing this iteration. So this code would become:

ThreadSP memory_thread_sp = m_thread_list.GetBackingThread(thread_sp);
if (memory_thread_sp)
  thread_sp = memory_thread_sp;

[JDevlieghere]

• Address Greg's comment and factor out the method.
• Add a test case.

include/lldb/Target/ThreadList.h

public:
 
   lldb::ThreadSP RemoveThreadByProtocolID(lldb::tid_t tid,
                                           bool can_update = true);
 
   lldb::ThreadSP FindThreadByIndexID(uint32_t index_id, bool can_update = true);
 
   lldb::ThreadSP GetThreadSPForThreadPtr(Thread *thread_ptr);
 
+  lldb::ThreadSP GetBackingThread(const lldb::ThreadSP &real_thread);
+
   bool ShouldStop(Event *event_ptr);
 
   Vote ShouldReportStop(Event *event_ptr);
 
   Vote ShouldReportRun(Event *event_ptr);
 
   void RefreshStateAfterStop();
 

packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestThreadSelectionBug.py

+from __future__ import print_function
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestThreadSelectionBug(GDBRemoteTestBase):
+    def test(self):
+        class MyResponder(MockGDBServerResponder):
+            def cont(self):
+                # Simulate process stopping due to a raise(SIGINT)
+                return "T01reason:signal"
+
+        self.server.responder = MyResponder()
+        target = self.createTarget("a.yaml")
+        process = self.connect(target)
+        python_os_plugin_path = os.path.join(self.getSourceDir(),
+                                             'operating_system.py')
+        command = "settings set target.process.python-os-plugin-path '{}'".format(
+            python_os_plugin_path)
+        self.dbg.HandleCommand(command)
+
+        self.assertTrue(process, PROCESS_IS_VALID)
+        self.assertEqual(process.GetNumThreads(), 3)
+
+        # Verify our OS plug-in threads showed up
+        thread = process.GetThreadByID(0x1)
+        self.assertTrue(
+            thread.IsValid(),
+            "Make sure there is a thread 0x1 after we load the python OS plug-in")
+        thread = process.GetThreadByID(0x2)
+        self.assertTrue(
+            thread.IsValid(),
+            "Make sure there is a thread 0x2 after we load the python OS plug-in")
+        thread = process.GetThreadByID(0x3)
+        self.assertTrue(
+            thread.IsValid(),
+            "Make sure there is a thread 0x3 after we load the python OS plug-in")
+
+        # Verify that a thread other than 3 is selected.
+        thread = process.GetSelectedThread()
+        self.assertNotEqual(thread.GetThreadID(), 0x3)
+
+        # Verify that we select the thread backed by physical thread 1, rather
+        # than virtual thread 1. The mapping comes from the OS plugin, where we
+        # specified that thread 3 is backed by real thread 1.
+        process.Continue()
+        thread = process.GetSelectedThread()
+        self.assertEqual(thread.GetThreadID(), 0x3)

packages/Python/lldbsuite/test/functionalities/gdb_remote_client/operating_system.py

+import lldb
+import struct
+
+
+class OperatingSystemPlugIn(object):
+    """Class that provides data for an instance of a LLDB 'OperatingSystemPython' plug-in class"""
+
+    def __init__(self, process):
+        '''Initialization needs a valid.SBProcess object.
+
+        This plug-in will get created after a live process is valid and has stopped for the first time.
+        '''
+        self.process = None
+        self.registers = None
+        self.threads = None
+        if isinstance(process, lldb.SBProcess) and process.IsValid():
+            self.process = process
+            self.threads = None  # Will be an dictionary containing info for each thread
+
+    def get_target(self):
+        return self.process.target
+
+    def get_thread_info(self):
+        if not self.threads:
+            self.threads = [{
+                'tid': 0x1,
+                'name': 'one',
+                'queue': 'queue1',
+                'state': 'stopped',
+                'stop_reason': 'none'
+            }, {
+                'tid': 0x2,
+                'name': 'two',
+                'queue': 'queue2',
+                'state': 'stopped',
+                'stop_reason': 'none'
+            }, {
+                'tid': 0x3,
+                'name': 'three',
+                'queue': 'queue3',
+                'state': 'stopped',
+                'stop_reason': 'sigstop',
+                'core': 0
+            }]
+        return self.threads

source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

if (thread_sp) {
 
       if (dispatch_queue_t != LLDB_INVALID_ADDRESS)
         gdb_thread->SetQueueLibdispatchQueueAddress(dispatch_queue_t);
 
       // Make sure we update our thread stop reason just once
       if (!thread_sp->StopInfoIsUpToDate()) {
         thread_sp->SetStopInfo(StopInfoSP());
         // If there's a memory thread backed by this thread, we need to use it
-        // to calcualte StopInfo.
-        ThreadSP memory_thread_sp =
-            m_thread_list.FindThreadByProtocolID(thread_sp->GetProtocolID());
-        if (memory_thread_sp)
+        // to calculate StopInfo.
+        if (ThreadSP memory_thread_sp =
+                m_thread_list.GetBackingThread(thread_sp))
           thread_sp = memory_thread_sp;
 
         if (exc_type != 0) {
           const size_t exc_data_size = exc_data.size();
 
           thread_sp->SetStopInfo(
               StopInfoMachException::CreateStopReasonWithMachException(
                   *thread_sp, exc_type, exc_data_size,
                   exc_data_size >= 1 ? exc_data[0] : 0,
                   exc_data_size >= 2 ? exc_data[1] : 0,
                   exc_data_size >= 3 ? exc_data[2] : 0));
         } else {
           bool handled = false;

source/Target/ThreadList.cpp

for (idx = 0; idx < num_threads; ++idx) {
         thread_sp = m_threads[idx];
         break;
       }
     }
   }
   return thread_sp;
 }
 
+ThreadSP ThreadList::GetBackingThread(const ThreadSP &real_thread) {
+  std::lock_guard<std::recursive_mutex> guard(GetMutex());
+
+  ThreadSP thread_sp;
+  const uint32_t num_threads = m_threads.size();
+  for (uint32_t idx = 0; idx < num_threads; ++idx) {
+    if (m_threads[idx]->GetBackingThread() == real_thread) {
+      thread_sp = m_threads[idx];
+      break;
+    }
+  }
+  return thread_sp;
+}
+
 ThreadSP ThreadList::FindThreadByIndexID(uint32_t index_id, bool can_update) {
   std::lock_guard<std::recursive_mutex> guard(GetMutex());
 
   if (can_update)
     m_process->UpdateThreadListIfNeeded();
 
   ThreadSP thread_sp;
   const uint32_t num_threads = m_threads.size();