Modernize TestThreadStepOut.py

[jingham]

Summary

This test was timing out on the bots occasionally, so I was poking around at it. I didn't see any obvious reason why it would stall, but it had a slightly odd way of rendezvousing the two test threads, and it also finished by running the inferior to completion, even though that wasn't relevant to the test at hand.

So I took the opportunity to modernize the test, and clean up these aspects of it. In order to do that I had to make run_to_source_breakpoint accept the case where the breakpoint it was setting had more than one thread hit the created breakpoint. So this diff also has the modifications to pass an "only_one_thread" parameter if you want to allow more than one thread stopping at the candidate breakpoint. The function still returns only one thread, since the vast majority of uses will require only one thread, and it's easy to find the threads stopped at a breakpoint if you need that.

[aprantl]

Thanks! Nitpicking inside.

lldb/packages/Python/lldbsuite/test/functionalities/thread/step_out/TestThreadStepOut.py

135

If self.bkpt_string and self.main_file are only used here, I would find this easier to read:

ldbutil.run_to_source_breakpoint(self, 'break here', lldb.SBFileSpec('main.cpp'), only_one_thread = False)

lldb/packages/Python/lldbsuite/test/lldbutil.py

781–786

this isn't used in the next condition?

783

assertEquals! :-)

785

assertGreater (see .../unittest2/case.py)

[jingham]

Addressed review comments.

[jingham]

lldb/packages/Python/lldbsuite/test/functionalities/thread/step_out/TestThreadStepOut.py

135

self.main_file is only used here, I moved it.

self.bkpt_string is used to calculate the line number self.breakpoint, and then the source breakpoint we set in run_to_source_breakpoint is supposed to match that line. So I think it is clearer to use the variable than to repeat the pattern twice.

lldb/packages/Python/lldbsuite/test/functionalities/thread/step_out/TestThreadStepOut.py

def test_python(self):
         """Test thread step out on one thread via Python API (dwarf)."""
         self.build(dictionary=self.getBuildFlags())
         self.step_out_test(self.step_out_with_python)
 
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number for our breakpoint.
-        self.breakpoint = line_number('main.cpp', '// Set breakpoint here')
+        self.bkpt_string = '// Set breakpoint here'
+        self.breakpoint = line_number('main.cpp', self.bkpt_string)       
+
         if "gcc" in self.getCompiler() or self.isIntelCompiler():
             self.step_out_destination = line_number(
                 'main.cpp', '// Expect to stop here after step-out (icc and gcc)')
         else:
             self.step_out_destination = line_number(
                 'main.cpp', '// Expect to stop here after step-out (clang)')
 
     def step_out_single_thread_with_cmd(self):
def step_out_with_python(self):
         desc = lldbutil.get_description(frame.GetLineEntry())
         expect = "main.cpp:%d" % self.step_out_destination
         self.assertTrue(
             expect in desc, "Expected %s but thread stopped at %s" %
             (expect, desc))
 
     def step_out_test(self, step_out_func):
         """Test single thread step out of a function."""
-        exe = self.getBuildArtifact("a.out")
-        self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
-
-        # This should create a breakpoint in the main thread.
-        lldbutil.run_break_set_by_file_and_line(
-            self, "main.cpp", self.breakpoint, num_expected_locations=1)
-
-        # The breakpoint list should show 1 location.
-        self.expect(
-            "breakpoint list -f",
-            "Breakpoint location shown correctly",
-            substrs=[
-                "1: file = 'main.cpp', line = %d, exact_match = 0, locations = 1" %
-                self.breakpoint])
-
-        # Run the program.
-        self.runCmd("run", RUN_SUCCEEDED)
+        (self.inferior_target, self.inferior_process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, self.bkpt_string, lldb.SBFileSpec('main.cpp'), only_one_thread = False)
 
-        # Get the target process
-        self.inferior_target = self.dbg.GetSelectedTarget()
-        self.inferior_process = self.inferior_target.GetProcess()
-
-        # Get the number of threads, ensure we see all three.
-        num_threads = self.inferior_process.GetNumThreads()
-        self.assertEqual(
-            num_threads,
-            3,
-            'Number of expected threads and actual threads do not match.')
+        # We hit the breakpoint on at least one thread.  If we hit it on both threads
+        # simultaneously, we can try the step out.  Otherwise, suspend the thread
+        # that hit the breakpoint, and continue till the second thread hits
+        # the breakpoint:
 
         (breakpoint_threads, other_threads) = ([], [])
         lldbutil.sort_stopped_threads(self.inferior_process,
                                       breakpoint_threads=breakpoint_threads,
                                       other_threads=other_threads)
-
-        while len(breakpoint_threads) < 2:
-            self.runCmd("thread continue %s" %
-                        " ".join([str(x.GetIndexID()) for x in other_threads]))
-            lldbutil.sort_stopped_threads(
-                self.inferior_process,
-                breakpoint_threads=breakpoint_threads,
-                other_threads=other_threads)
+        if len(breakpoint_threads) == 1:
+            success = thread.Suspend()
+            self.assertTrue(success, "Couldn't suspend a thread")
+            bkpt_threads = lldbutil.continue_to_breakpoint(bkpt)
+            self.assertEqual(len(bkpt_threads), 1, "Second thread stopped")
+            success = thread.Resume()
+            self.assertTrue(success, "Couldn't resume a thread")
 
         self.step_out_thread = breakpoint_threads[0]
 
         # Step out of thread stopped at breakpoint
         step_out_func()
-
-        # Run to completion
-        self.runCmd("continue")
-
-        # At this point, the inferior process should have exited.
-        self.assertTrue(self.inferior_process.GetState() ==
-                        lldb.eStateExited, PROCESS_EXITED)

lldb/packages/Python/lldbsuite/test/lldbutil.py

def run_to_breakpoint_make_target(test, exe_name = "a.out", in_cwd = True):
     if in_cwd:
         exe = test.getBuildArtifact(exe_name)
 
     # Create the target
     target = test.dbg.CreateTarget(exe)
     test.assertTrue(target, "Target: %s is not valid."%(exe_name))
     return target
 
-def run_to_breakpoint_do_run(test, target, bkpt, launch_info = None):
+def run_to_breakpoint_do_run(test, target, bkpt, launch_info = None, only_one_thread = True):
 
     # Launch the process, and do not stop at the entry point.
     if not launch_info:
         launch_info = lldb.SBLaunchInfo(None)
         launch_info.SetWorkingDirectory(test.get_process_working_directory())
 
     error = lldb.SBError()
     process = target.Launch(launch_info, error)
 
     test.assertTrue(process,
                     "Could not create a valid process for %s: %s"%(target.GetExecutable().GetFilename(),
                     error.GetCString()))
 
     # Frame #0 should be at our breakpoint.
     threads = get_threads_stopped_at_breakpoint(
                 process, bkpt)
 
-    test.assertTrue(len(threads) == 1, "Expected 1 thread to stop at breakpoint, %d did."%(len(threads)))
+    num_threads = len(threads)
+    if only_one_thread:
+        test.assertEqual(num_threads, 1, "Expected 1 thread to stop at breakpoint, %d did."%(num_threads))
+    else:
+        test.assertGreater(num_threads, 0, "No threads stopped at breakpoint")
+        
     thread = threads[0]
     return (target, process, thread, bkpt)
 
 def run_to_name_breakpoint (test, bkpt_name, launch_info = None,
                             exe_name = "a.out",
                             bkpt_module = None,
-                            in_cwd = True):
+                            in_cwd = True,
+                            only_one_thread = True):
     """Start up a target, using exe_name as the executable, and run it to
        a breakpoint set by name on bkpt_name restricted to bkpt_module.
 
        If you want to pass in launch arguments or environment
        variables, you can optionally pass in an SBLaunchInfo.  If you
        do that, remember to set the working directory as well.
 
        If your executable isn't called a.out, you can pass that in.
        And if your executable isn't in the CWD, pass in the absolute
        path to the executable in exe_name, and set in_cwd to False.
 
        If you need to restrict the breakpoint to a particular module,
        pass the module name (a string not a FileSpec) in bkpt_module.  If
        nothing is passed in setting will be unrestricted.
 
        If the target isn't valid, the breakpoint isn't found, or hit, the
        function will cause a testsuite failure.
 
        If successful it returns a tuple with the target process and
        thread that hit the breakpoint, and the breakpoint that we set
        for you.
+
+       If only_one_thread is true, we require that there be only one
+       thread stopped at the breakpoint.  Otherwise we only require one
+       or more threads stop there.  If there are more than one, we return
+       the first thread that stopped.
     """
 
     target = run_to_breakpoint_make_target(test, exe_name, in_cwd)
 
     breakpoint = target.BreakpointCreateByName(bkpt_name, bkpt_module)
 
 
     test.assertTrue(breakpoint.GetNumLocations() > 0,
                     "No locations found for name breakpoint: '%s'."%(bkpt_name))
-    return run_to_breakpoint_do_run(test, target, breakpoint, launch_info)
+    return run_to_breakpoint_do_run(test, target, breakpoint, launch_info, only_one_thread)
 
 def run_to_source_breakpoint(test, bkpt_pattern, source_spec,
                              launch_info = None, exe_name = "a.out",
                              bkpt_module = None,
-                             in_cwd = True):
+                             in_cwd = True,
+                             only_one_thread = True):
     """Start up a target, using exe_name as the executable, and run it to
        a breakpoint set by source regex bkpt_pattern.
 
        The rest of the behavior is the same as run_to_name_breakpoint.
     """
 
     target = run_to_breakpoint_make_target(test, exe_name, in_cwd)
     # Set the breakpoints
     breakpoint = target.BreakpointCreateBySourceRegex(
             bkpt_pattern, source_spec, bkpt_module)
     test.assertTrue(breakpoint.GetNumLocations() > 0,
         'No locations found for source breakpoint: "%s", file: "%s", dir: "%s"'
         %(bkpt_pattern, source_spec.GetFilename(), source_spec.GetDirectory()))
-    return run_to_breakpoint_do_run(test, target, breakpoint, launch_info)
+    return run_to_breakpoint_do_run(test, target, breakpoint, launch_info, only_one_thread)
 
 def run_to_line_breakpoint(test, source_spec, line_number, column = 0,
                            launch_info = None, exe_name = "a.out",
                            bkpt_module = None,
-                           in_cwd = True):
+                           in_cwd = True,
+                           only_one_thread = True):
     """Start up a target, using exe_name as the executable, and run it to
        a breakpoint set by (source_spec, line_number(, column)).
 
        The rest of the behavior is the same as run_to_name_breakpoint.
     """
 
     target = run_to_breakpoint_make_target(test, exe_name, in_cwd)
     # Set the breakpoints
     breakpoint = target.BreakpointCreateByLocation(
         source_spec, line_number, column, 0, lldb.SBFileSpecList())
     test.assertTrue(breakpoint.GetNumLocations() > 0,
         'No locations found for line breakpoint: "%s:%d(:%d)", dir: "%s"'
         %(source_spec.GetFilename(), line_number, column,
           source_spec.GetDirectory()))
-    return run_to_breakpoint_do_run(test, target, breakpoint, launch_info)
+    return run_to_breakpoint_do_run(test, target, breakpoint, launch_info, only_one_thread)
 
 
 def continue_to_breakpoint(process, bkpt):
     """ Continues the process, if it stops, returns the threads stopped at bkpt; otherwise, returns None"""
     process.Continue()
     if process.GetState() != lldb.eStateStopped:
         return None
     else: