Always check result of Thread::ModifySuspendCount

Ensures that we never ignore the result of ModifySuspendCount so that
we can react if the suspend count is not updated as expected.

This CL does the following:
* Adds __attribute__((warn_unused_result)) on the method to raise an
  error at compilation time if the result is ignored.
* Wraps calls with DCHECK where the result used to be ignored.

Bug: 27385848
Test: make -j test-art-host
Test: art/tools/run-jdwp-tests.sh --mode=host --variant=X64
Change-Id: I2d0e1ab7158c70ec8076c8bae6e4b814aee75af6
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 008c388..87697c7 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -1451,7 +1451,8 @@
       MutexLock mu2(self, *Locks::thread_suspend_count_lock_);
 
       DCHECK_NE(GetState(), ThreadState::kRunnable);
-      CHECK(ModifySuspendCount(self, -1, nullptr, false));
+      bool updated = ModifySuspendCount(self, -1, nullptr, false);
+      DCHECK(updated);
     }
 
     return;  // We're done, break out of the loop.
diff --git a/runtime/thread.h b/runtime/thread.h
index de0b892..4d5d644 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -244,6 +244,7 @@
                           int delta,
                           AtomicInteger* suspend_barrier,
                           bool for_debugger)
+      WARN_UNUSED
       REQUIRES(Locks::thread_suspend_count_lock_);
 
   bool RequestCheckpoint(Closure* function)
@@ -1276,6 +1277,7 @@
                                   int delta,
                                   AtomicInteger* suspend_barrier,
                                   bool for_debugger)
+      WARN_UNUSED
       REQUIRES(Locks::thread_suspend_count_lock_);
 
   void RunCheckpointFunction();
diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc
index 8d72fe8..2e0d866 100644
--- a/runtime/thread_list.cc
+++ b/runtime/thread_list.cc
@@ -323,7 +323,8 @@
               // Spurious fail, try again.
               continue;
             }
-            thread->ModifySuspendCount(self, +1, nullptr, false);
+            bool updated = thread->ModifySuspendCount(self, +1, nullptr, false);
+            DCHECK(updated);
             suspended_count_modified_threads.push_back(thread);
             break;
           }
@@ -365,7 +366,8 @@
     checkpoint_function->Run(thread);
     {
       MutexLock mu2(self, *Locks::thread_suspend_count_lock_);
-      thread->ModifySuspendCount(self, -1, nullptr, false);
+      bool updated = thread->ModifySuspendCount(self, -1, nullptr, false);
+      DCHECK(updated);
     }
   }
 
@@ -565,7 +567,8 @@
       if ((state == kWaitingForGcThreadFlip || thread->IsTransitioningToRunnable()) &&
           thread->GetSuspendCount() == 1) {
         // The thread will resume right after the broadcast.
-        thread->ModifySuspendCount(self, -1, nullptr, false);
+        bool updated = thread->ModifySuspendCount(self, -1, nullptr, false);
+        DCHECK(updated);
         ++runnable_thread_count;
       } else {
         other_threads.push_back(thread);
@@ -598,7 +601,8 @@
     TimingLogger::ScopedTiming split4("ResumeOtherThreads", collector->GetTimings());
     MutexLock mu2(self, *Locks::thread_suspend_count_lock_);
     for (const auto& thread : other_threads) {
-      thread->ModifySuspendCount(self, -1, nullptr, false);
+      bool updated = thread->ModifySuspendCount(self, -1, nullptr, false);
+      DCHECK(updated);
     }
     Thread::resume_cond_->Broadcast(self);
   }
@@ -708,7 +712,8 @@
         continue;
       }
       VLOG(threads) << "requesting thread suspend: " << *thread;
-      thread->ModifySuspendCount(self, +1, &pending_threads, debug_suspend);
+      bool updated = thread->ModifySuspendCount(self, +1, &pending_threads, debug_suspend);
+      DCHECK(updated);
 
       // Must install the pending_threads counter first, then check thread->IsSuspend() and clear
       // the counter. Otherwise there's a race with Thread::TransitionFromRunnableToSuspended()
@@ -786,7 +791,8 @@
       if (thread == self) {
         continue;
       }
-      thread->ModifySuspendCount(self, -1, nullptr, false);
+      bool updated = thread->ModifySuspendCount(self, -1, nullptr, false);
+      DCHECK(updated);
     }
 
     // Broadcast a notification to all suspended threads, some or all of
@@ -828,7 +834,8 @@
           << ") thread not within thread list";
       return;
     }
-    thread->ModifySuspendCount(self, -1, nullptr, for_debugger);
+    bool updated = thread->ModifySuspendCount(self, -1, nullptr, for_debugger);
+    DCHECK(updated);
   }
 
   {
@@ -884,7 +891,11 @@
           // If we incremented the suspend count but the thread reset its peer, we need to
           // re-decrement it since it is shutting down and may deadlock the runtime in
           // ThreadList::WaitForOtherNonDaemonThreadsToExit.
-          suspended_thread->ModifySuspendCount(soa.Self(), -1, nullptr, debug_suspension);
+          bool updated = suspended_thread->ModifySuspendCount(soa.Self(),
+                                                              -1,
+                                                              nullptr,
+                                                              debug_suspension);
+          DCHECK(updated);
         }
         ThreadSuspendByPeerWarning(self,
                                    ::android::base::WARNING,
@@ -910,7 +921,8 @@
           }
           CHECK(suspended_thread == nullptr);
           suspended_thread = thread;
-          suspended_thread->ModifySuspendCount(self, +1, nullptr, debug_suspension);
+          bool updated = suspended_thread->ModifySuspendCount(self, +1, nullptr, debug_suspension);
+          DCHECK(updated);
           request_suspension = false;
         } else {
           // If the caller isn't requesting suspension, a suspension should have already occurred.
@@ -942,7 +954,11 @@
                                      peer);
           if (suspended_thread != nullptr) {
             CHECK_EQ(suspended_thread, thread);
-            suspended_thread->ModifySuspendCount(soa.Self(), -1, nullptr, debug_suspension);
+            bool updated = suspended_thread->ModifySuspendCount(soa.Self(),
+                                                                -1,
+                                                                nullptr,
+                                                                debug_suspension);
+            DCHECK(updated);
           }
           *timed_out = true;
           return nullptr;
@@ -1015,7 +1031,8 @@
             // which will allow this thread to be suspended.
             continue;
           }
-          thread->ModifySuspendCount(self, +1, nullptr, debug_suspension);
+          bool updated = thread->ModifySuspendCount(self, +1, nullptr, debug_suspension);
+          DCHECK(updated);
           suspended_thread = thread;
         } else {
           CHECK_EQ(suspended_thread, thread);
@@ -1046,7 +1063,8 @@
                                          "Thread suspension timed out",
                                          thread_id);
           if (suspended_thread != nullptr) {
-            thread->ModifySuspendCount(soa.Self(), -1, nullptr, debug_suspension);
+            bool updated = thread->ModifySuspendCount(soa.Self(), -1, nullptr, debug_suspension);
+            DCHECK(updated);
           }
           *timed_out = true;
           return nullptr;
@@ -1123,7 +1141,8 @@
     // to ensure that we're the only one fiddling with the suspend count
     // though.
     MutexLock mu(self, *Locks::thread_suspend_count_lock_);
-    self->ModifySuspendCount(self, +1, nullptr, true);
+    bool updated = self->ModifySuspendCount(self, +1, nullptr, true);
+    DCHECK(updated);
     CHECK_GT(self->GetSuspendCount(), 0);
 
     VLOG(threads) << *self << " self-suspending (debugger)";
@@ -1207,7 +1226,8 @@
           continue;
         }
         VLOG(threads) << "requesting thread resume: " << *thread;
-        thread->ModifySuspendCount(self, -1, nullptr, true);
+        bool updated = thread->ModifySuspendCount(self, -1, nullptr, true);
+        DCHECK(updated);
       }
     }
   }
@@ -1236,7 +1256,11 @@
       if (thread == self || thread->GetDebugSuspendCount() == 0) {
         continue;
       }
-      thread->ModifySuspendCount(self, -thread->GetDebugSuspendCount(), nullptr, true);
+      bool suspended = thread->ModifySuspendCount(self,
+                                                  -thread->GetDebugSuspendCount(),
+                                                  nullptr,
+                                                  true);
+      DCHECK(suspended);
     }
   }
 
@@ -1293,7 +1317,8 @@
       // daemons.
       CHECK(thread->IsDaemon()) << *thread;
       if (thread != self) {
-        thread->ModifySuspendCount(self, +1, nullptr, false);
+        bool updated = thread->ModifySuspendCount(self, +1, nullptr, false);
+        DCHECK(updated);
         ++daemons_left;
       }
       // We are shutting down the runtime, set the JNI functions of all the JNIEnvs to be
@@ -1352,10 +1377,12 @@
   // Modify suspend count in increments of 1 to maintain invariants in ModifySuspendCount. While
   // this isn't particularly efficient the suspend counts are most commonly 0 or 1.
   for (int delta = debug_suspend_all_count_; delta > 0; delta--) {
-    self->ModifySuspendCount(self, +1, nullptr, true);
+    bool updated = self->ModifySuspendCount(self, +1, nullptr, true);
+    DCHECK(updated);
   }
   for (int delta = suspend_all_count_ - debug_suspend_all_count_; delta > 0; delta--) {
-    self->ModifySuspendCount(self, +1, nullptr, false);
+    bool updated = self->ModifySuspendCount(self, +1, nullptr, false);
+    DCHECK(updated);
   }
   CHECK(!Contains(self));
   list_.push_back(self);
@@ -1450,11 +1477,13 @@
     MutexLock mu(self, *Locks::thread_list_lock_);
     MutexLock mu2(self, *Locks::thread_suspend_count_lock_);
     for (Thread* thread : list_) {
-      thread->ModifySuspendCount(self, +1, nullptr, false);
+      bool suspended = thread->ModifySuspendCount(self, +1, nullptr, false);
+      DCHECK(suspended);
       if (thread == self || thread->IsSuspended()) {
         threads_to_visit.push_back(thread);
       } else {
-        thread->ModifySuspendCount(self, -1, nullptr, false);
+        bool resumed = thread->ModifySuspendCount(self, -1, nullptr, false);
+        DCHECK(resumed);
       }
     }
   }
@@ -1469,7 +1498,8 @@
   {
     MutexLock mu2(self, *Locks::thread_suspend_count_lock_);
     for (Thread* thread : threads_to_visit) {
-      thread->ModifySuspendCount(self, -1, nullptr, false);
+      bool updated = thread->ModifySuspendCount(self, -1, nullptr, false);
+      DCHECK(updated);
     }
   }
 }