[lldb][tests] Test queue-specific breakpoints

[cassanova]

Summary

This commit adds tests to ensure that queue-specific breakpoints work as expected, as this feature wasn't being tested before.

[jasonmolenda]

This looks good to me.

[DavidSpickett]

Please include more context (https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface) in future patches.

Thanks for the tests regardless!

[mib]

lldb/test/API/macosx/queues/TestQueues.py

133

Could you replace that with the enum variable ?

388

Do you really need a main_thread variable since lldbutil.run_to_name_breakpoint would return a tuple with (target, process, thread, bkpt) ? Is the thread returned here different from the main_thread you created above ?

[cassanova]

lldb/test/API/macosx/queues/TestQueues.py

133

Yes the enum variable would be better here

388

The thread is the same actually, so it would probably be beneficial to just get the thread from run_to_name_breakpoint instead.

[cassanova]

Use the enum name for the stop reason when asserting that the queues hit their breakpoints instead of just the raw number.

Also, get the main thread from run_to_name_breakpoint instead of getting it from get_threads_stopped_at_breakpoint

[mib]

lldb/test/API/macosx/queues/TestQueues.py

204–206

Why not do it this way ? Takes less space and it's easier to read (for people who don't know what process_info[2] and process_info[3] refers to)

316

Do you still need this ?

387–389

ditto

[cassanova]

All variables needed from run_to_name_breakpoint are obtained in one line rather than getting them by index.

[cassanova]

lldb/test/API/macosx/queues/TestQueues.py

204–206

This is a much cleaner way to write it :)

[teemperor]

lldb/test/API/macosx/queues/TestQueues.py

131

missed assertEqual here?

[mib]

LGTM with @teemperor feedback !

lldb/test/API/macosx/queues/TestQueues.py

131

+1!

[cassanova]

lldb/test/API/macosx/queues/TestQueues.py

131

Yes, adding this in

[cassanova]

Changed an assertTrue to assertEqual.

[JDevlieghere]

lldb/test/API/macosx/queues/TestQueues.py

133

This should use self.assertStopReason for a better error message.

201–206

This seems like it should be its own test. Right now this is reassigning the target and process, which seems suspicious, and also changes the meaning of everything that comes after it.

[cassanova]

lldb/test/API/macosx/queues/TestQueues.py

201–206

Good point, I'm placing the queue specific tests into their own test.

[cassanova]

Added the queue-specific breakpoints to their own test.

[JDevlieghere]

lldb/test/API/macosx/queues/TestQueues.py

125

Any reason this should be a separate function? Can this be inlined in queue_specific_breakpoints?

[cassanova]

lldb/test/API/macosx/queues/TestQueues.py

125

It can be inlined, the reason I had it in a separate function is that it matched the rest of the test in having the assertions be in their own functions.

[JDevlieghere]

LGTM

lldb/test/API/macosx/queues/TestQueues.py

125

Fair enough, this test has a different structure than most. Let's keep it this way then for consistency.

lldb/test/API/macosx/queues/TestQueues.py

def check_queues_threads_match_queue(self, queue):
                  t.GetQueueName()))
             self.assertEqual(
                 t.GetQueue().GetQueueID(), queue.GetQueueID(),
                 "Thread #%d's Queue's QueueID of %d is not the same as the QueueID of its owning queue %d" %
                 (idx,
                  t.GetQueue().GetQueueID(),
                     queue.GetQueueID()))
 
+    def check_queue_breakpoints(self, queue1, queue2, queue_breakpoint):
+        queue1_thread = queue1.GetThreadAtIndex(0)
+        queue2_thread = queue2.GetThreadAtIndex(0)
+
+        self.assertEqual(queue_breakpoint.GetQueueName(), queue1.GetName(),
+                         "The breakpoint was set for queue %s, but the breakpoint's queue name is %s" % (queue_breakpoint.GetQueueName(), queue1.GetName()))
+        self.assertTrue(queue_breakpoint.GetHitCount() == 1,
+                        "The breakpoint for queue %s has not been hit" % (queue_breakpoint.GetQueueName()))
+        self.assertEqual(queue1_thread.GetStopReason(), lldb.eStopReasonBreakpoint,
+                         "Queue %s is not stopped at breakpoint %d" %
+                         (queue1.GetName(), queue_breakpoint.GetID()))
+        self.assertNotEqual(queue2_thread.GetStopReason(), lldb.eStopReasonBreakpoint,
+                            "Queue %s is stopped at breakpoint %d, but this breakpoint should only be hit for queue %s" %
+                            (queue2.GetName(), queue_breakpoint.GetID(), queue_breakpoint.GetQueueName()))
+
     def queues(self):
         """Test queues inspection SB APIs without libBacktraceRecording."""
         exe = self.getBuildArtifact("a.out")
 
         target = self.dbg.CreateTarget(exe)
         self.assertTrue(target, VALID_TARGET)
         self.main_source_spec = lldb.SBFileSpec(self.main_source)
         break1 = target.BreakpointCreateByName("stopper", 'a.out')
def queues(self):
         self.check_queue_kind(queue_performer_2, lldb.eQueueKindSerial)
         self.check_queue_kind(queue_performer_3, lldb.eQueueKindConcurrent)
 
         self.check_queues_threads_match_queue(queue_submittor_1)
         self.check_queues_threads_match_queue(queue_performer_1)
         self.check_queues_threads_match_queue(queue_performer_2)
         self.check_queues_threads_match_queue(queue_performer_3)
 
+        # Run the executable until the stopper function and get the breakpoint
+        # that's created from that. Then set the queue name of the breakpoint
+        # to be the name of the main thread
+        target, process, main_thread, queue_breakpoint = lldbutil.run_to_name_breakpoint(self, "stopper", only_one_thread=False)
+        queue_breakpoint.SetQueueName(main_thread.GetQueue().GetName())
+        self.check_queue_breakpoints(main_thread.GetQueue(), queue_submittor_1, queue_breakpoint)
+
         # We have threads running with all the different dispatch QoS service
         # levels - find those threads and check that we can get the correct
         # QoS name for each of them.
 
         user_initiated_thread = lldb.SBThread()
         user_interactive_thread = lldb.SBThread()
         utility_thread = lldb.SBThread()
         background_thread = lldb.SBThread()
def queues_with_libBacktraceRecording(self):
 
         self.assertTrue(process, PROCESS_IS_VALID)
 
         # The stop reason of the thread should be breakpoint.
         threads = lldbutil.get_threads_stopped_at_breakpoint(process, break1)
         if len(threads) != 1:
             self.fail("Failed to stop at breakpoint 1.")
 
+        main_thread = threads[0]
         self.inferior_process = process
 
         libbtr_module_filespec = lldb.SBFileSpec("libBacktraceRecording.dylib")
         libbtr_module = target.FindModule(libbtr_module_filespec)
         if not libbtr_module.IsValid():
             self.skipTest(
                 "Skipped because libBacktraceRecording.dylib was not loaded into the process.")
 
def queues_with_libBacktraceRecording(self):
         self.check_queue_kind(queue_performer_2, lldb.eQueueKindSerial)
         self.check_queue_kind(queue_performer_3, lldb.eQueueKindConcurrent)
 
         self.check_queues_threads_match_queue(queue_submittor_1)
         self.check_queues_threads_match_queue(queue_performer_1)
         self.check_queues_threads_match_queue(queue_performer_2)
         self.check_queues_threads_match_queue(queue_performer_3)
 
+        # Run the executable until the stopper function and get the breakpoint
+        # that's created from that. Then set the queue name of the breakpoint
+        # to be the name of the main thread
+        target, process, main_thread, queue_breakpoint = lldbutil.run_to_name_breakpoint(self, "stopper", only_one_thread=False)
+        queue_breakpoint.SetQueueName(main_thread.GetQueue().GetName())
+        self.check_queue_breakpoints(main_thread.GetQueue(), queue_submittor_1, queue_breakpoint)
+
         self.assertTrue(queue_performer_2.GetPendingItemAtIndex(
             0).IsValid(), "queue 2's pending item #0 is valid")
         self.assertTrue(queue_performer_2.GetPendingItemAtIndex(0).GetAddress().GetSymbol(
         ).GetName() == "doing_the_work_2", "queue 2's pending item #0 should be doing_the_work_2")
         self.assertEqual(
             queue_performer_2.GetNumPendingItems(), 9999,
             "verify that queue 2 still has 9999 pending items")
         self.assertTrue(queue_performer_2.GetPendingItemAtIndex(
             9998).IsValid(), "queue 2's pending item #9998 is valid")
         self.assertTrue(queue_performer_2.GetPendingItemAtIndex(9998).GetAddress().GetSymbol(
         ).GetName() == "doing_the_work_2", "queue 2's pending item #0 should be doing_the_work_2")
         self.assertTrue(queue_performer_2.GetPendingItemAtIndex(
             9999).IsValid() == False, "queue 2's pending item #9999 is invalid")