[linux] handle stop events while stepping

## Purpose
Properly handle a case that occasionally triggers
[this](https://github.com/compnerd/ds2/blob/main/Sources/Target/Linux/Process.cpp#L274)
`DS2BUG` fatal error in `Linux::Process::wait` when running some
multi-threaded lldb tests.

## Overview
* Handle the case where we get a stop event for a single-stepping thread
with `kEventNone` and `kReasonNone`
* Enable 15 lldb test cases that were previously flaky due to this bug
* Rework the `Linux::Process::wait` function be a little bit more
readable
* Handle all failure cases in `Linux::Process::wait` with the `CHK()`
macro

## Problem Details
The scenario where a `kEventNone` event with a reason other than
`kReasonThreadSpawn` is alluded to in
[this](https://github.com/compnerd/ds2/blob/main/Sources/Target/Linux/Process.cpp#L236-L238)
comment block of `Linux::Process::wait` but it suggests it will never
happen while the thread is single stepping. However, this scenario can
legitimately occur and is fairly easy to reproduce with the lldb
multithreaded tests. In this case, the reason will be `kReasonNone`.

The implementation already handled this `kEventNone` and `kReasonNone`
event by continuing the thread as long as it wasn't also being
single-stepped. This change updates it to step the thread in the same
situation if it is being single-stepped. However, I haven't 100%
convinced myself the implementation was correct to begin with (it could
probably use an overhaul). Intuitively, I would expect to ignore events
with `kEventNone` and `kReasonNone`, but that's not what it was doing
previously.

## Validation
The re-enabled tests now pass consistently locally on Android and Linux,
and have now passed on at least 10 automated runs.

Manually set some breakpoints and stepped in a simple multi-threaded
program. It behaved as expected.

Co-authored-by: Saleem Abdulrasool <compnerd@compnerd.org>

Author: andrurogerz

Sources/Target/Linux/Process.cpp

@@ -209,15 +209,15 @@ ErrorCode Process::wait() {
     }
 
     stepping = _currentThread->_state == Thread::kStepped;
-    _currentThread->updateStopInfo(status);
+    CHK(_currentThread->updateStopInfo(status));
 
     switch (_currentThread->_stopInfo.event) {
     case StopInfo::kEventNone:
       // If _stopInfo.event is kEventNone it means the thread is either
       // 1. stopped because another thread of the inferior stopped (e.g. because
       //    of a breakpoint or received a signal, etc) and ds2 sent a SIGSTOP
       //    to pause this thread while the other thread's stop is evaluated. In
-      //    this case we just resume the thread.
+      //    this case we just resume or step the thread.
       // 2. stopped because this thread called clone(2) (or similar e.g.
       //    pthread_create). In this case we have two subcases:
       //    a. The thread was running and hit the clone/spawn and thus was
@@ -232,48 +232,53 @@ ErrorCode Process::wait() {
       //       waitpid(3) call and thus have to handle the cloned thread
       //       and the currentThread at once. So we call step on the
       //       _currentThread and then beforeResume the cloned thread.
-      //
-      // We also have another theoretically possible case where we are not
-      // stopped by a clone but still stepping while encountering a kEventNone.
-      // This should not happen and we just assert that it does not.
-
-      if (_currentThread->_stopInfo.reason == StopInfo::kReasonThreadSpawn &&
-          stepping) { //(2b)
-        unsigned long spawnedThreadIdData;
-        CHK(ptrace().getEventMessage(_pid, spawnedThreadIdData));
-        auto const spawnedThreadId = static_cast<ThreadId>(spawnedThreadIdData);
-
-        if (_threads.find(spawnedThreadId) == _threads.end()) {
-          // If the newly cloned thread does not yet exist within the _threads
-          // vector then we have not yet seen it with waitpid(2). The most
-          // recent
-          // waitpid(2) returns a status guaranteeing a clone event did happen.
-          // Therefore, we wait and block until we see the cloned thread.
-          int spawnedThreadStatus;
-          ThreadId returnedThreadId =
-              blocking_waitpid(spawnedThreadId, &spawnedThreadStatus, __WALL);
-          if (spawnedThreadId != returnedThreadId)
-            return kErrorProcessNotFound;
-          DS2LOG(Debug, "child thread tid %d %s", returnedThreadId,
-                 Stringify::WaitStatus(spawnedThreadStatus));
-
-          _currentThread->step();
-          auto newThread = new Thread(this, returnedThreadId);
-          newThread->beforeResume();
-        } else {
-          // The new thread corresponding to this kReasonThreadSpawn was already
-          // caught by waitpid(2). Therefore, we only need to step the current
-          // thread.
-          _currentThread->step();
+
+      switch (_currentThread->_stopInfo.reason) {
+      case StopInfo::kReasonNone: // (1)
+        if (stepping)
+          CHK(_currentThread->step());
+        else
+          CHK(_currentThread->resume());
+        break;
+
+      case StopInfo::kReasonThreadSpawn:
+        if (!stepping) { // (2a)
+          CHK(_currentThread->resume());
+        } else { // (2b)
+          unsigned long spawnedThreadIdData;
+          CHK(ptrace().getEventMessage(_pid, spawnedThreadIdData));
+          auto const spawnedThreadId = static_cast<ThreadId>(spawnedThreadIdData);
+          if (_threads.find(spawnedThreadId) == _threads.end()) {
+            // If the newly cloned thread does not yet exist within the _threads
+            // vector then we have not yet seen it with waitpid(2). The most
+            // recent waitpid(2) returns a status guaranteeing a clone event did happen.
+            // Therefore, we wait and block until we see the cloned thread.
+            int spawnedThreadStatus;
+            ThreadId returnedThreadId =
+                blocking_waitpid(spawnedThreadId, &spawnedThreadStatus, __WALL);
+            if (spawnedThreadId != returnedThreadId)
+              return kErrorProcessNotFound;
+            DS2LOG(Debug, "child thread tid %d %s", returnedThreadId,
+                   Stringify::WaitStatus(spawnedThreadStatus));
+
+            CHK(_currentThread->step());
+            auto newThread = new Thread(this, returnedThreadId);
+            CHK(newThread->beforeResume());
+          } else {
+            // The new thread corresponding to this kReasonThreadSpawn was already
+            // caught by waitpid(2). Therefore, we only need to step the current
+            // thread.
+            CHK(_currentThread->step());
+          }
         }
-      } else if (stepping) {
+        break;
+
+      default:
         // We should never see a case where we're:
         //   1. stopped for event kEventNone
-        //   2. stepping
-        //   3. not stopped for reason kReasonThreadSpawn
+        //   2. not stopped for reason kReasonThreadSpawn or kReasonNone
         DS2BUG("inconsistent thread stop info");
-      } else {
-        _currentThread->resume(); // (1) and (2a)
+        break;
       }
       goto continue_waiting;
 

Support/Testing/Excluded/ds2/android-x86_64.excluded

@@ -6,28 +6,16 @@ TestBadAddressBreakpoints.BadAddressBreakpointTestCase.test_bad_address_breakpoi
 TestBreakpointInGlobalConstructor.TestBreakpointInGlobalConstructors.test
 TestBreakpointSetRestart.BreakpointSetRestart.test_breakpoint_set_restart_dwarf
 TestBreakpointSetRestart.BreakpointSetRestart.test_breakpoint_set_restart_dwo
-TestBreakOnLambdaCapture.TestBreakOnLambdaCapture.test_break_on_lambda_capture
 TestBtAliasRepeat.TestCase.test_dwarf
 TestBtAliasRepeat.TestCase.test_dwo
 TestChangeProcessGroup.ChangeProcessGroupTestCase.test_setpgid
-TestChangeValueAPI.ChangeValueAPITestCase.test_change_value_dwo
 TestCompletion.CommandLineCompletionTestCase.test_process_unload
-TestConcurrentBreakpointDelayBreakpointOneSignal.ConcurrentBreakpointDelayBreakpointOneSignal.test
-TestConcurrentBreakpointsDelayedBreakpointOneWatchpoint.ConcurrentBreakpointsDelayedBreakpointOneWatchpoint.test
-TestConcurrentDelayedCrashWithBreakpointSignal.ConcurrentDelayedCrashWithBreakpointSignal.test
-TestConcurrentDelayedCrashWithBreakpointWatchpoint.ConcurrentDelayedCrashWithBreakpointWatchpoint.test
 TestConcurrentNWatchNBreak.ConcurrentNWatchNBreak.test
 TestConcurrentSignalNWatchNBreak.ConcurrentSignalNWatchNBreak.test
-TestConcurrentSignalWatchBreak.ConcurrentSignalWatchBreak.test
-TestConcurrentTwoBreakpointsOneDelaySignal.ConcurrentTwoBreakpointsOneDelaySignal.test
-TestConcurrentTwoBreakpointsOneSignal.ConcurrentTwoBreakpointsOneSignal.test
-TestConcurrentTwoBreakpointsOneWatchpoint.ConcurrentTwoBreakpointsOneWatchpoint.test
-TestConcurrentTwoWatchpointsOneBreakpoint.ConcurrentTwoWatchpointsOneBreakpoint.test
 TestConcurrentVFork.TestConcurrentVFork.test_follow_child_fork_call_exec
 TestConcurrentVFork.TestConcurrentVFork.test_follow_child_fork_no_exec
 TestConcurrentVFork.TestConcurrentVFork.test_follow_child_vfork_call_exec
 TestConcurrentVFork.TestConcurrentVFork.test_follow_child_vfork_no_exec
-TestConcurrentWatchpointDelayWatchpointOneBreakpoint.ConcurrentWatchpointDelayWatchpointOneBreakpoint.test
 TestConstVariables.ConstVariableTestCase.test_and_run_command_dwarf
 TestConstVariables.ConstVariableTestCase.test_and_run_command_dwo
 TestContainerCommands.TestCmdContainer.test_container_add
@@ -149,9 +137,6 @@ TestTargetCreateDeps.TargetDependentsTestCase.test_dependents_implicit_default_l
 TestThreadAPI.ThreadAPITestCase.test_step_out_of_malloc_into_function_b_dwarf
 TestThreadAPI.ThreadAPITestCase.test_step_out_of_malloc_into_function_b_dwo
 TestThreadSelectionBug.TestThreadSelectionBug.test
-TestThreadStates.ThreadStateTestCase.test_process_interrupt
-TestThreadStepOut.ThreadStepOutTestCase.test_step_single_thread_dwo
-TestTwoHitsOneActual.TestTwoHitsOneActual.test_two_hits_one_actual
 TestUnalignedLargeWatchpoint.UnalignedLargeWatchpointTestCase.test_unaligned_large_watchpoint
 TestUnwindExpression.UnwindFromExpressionTest.test_unwind_expression_dwarf
 TestUnwindExpression.UnwindFromExpressionTest.test_unwind_expression_dwo