Assert mutator doesn't get mark-stack assigned once destroyed

Write a non-null value to thread-local mark-stack pointer when
a mutator revokes the previously assigned thread-local mark-stack
so that we can catch the mutator if it ever invokes the read-barrier
or executes the flip function, both of which may assign mark-stack
to the mutator.

Test: art/test/testrunner/testrunner.py
Bug:140119552
Change-Id: I82f43c8a3aab6dacb0f6bd35471fc2cdd969b154

3 files changed, 15 insertions, 2 deletions

diff --git a/runtime/gc/collector/concurrent_copying-inl.h b/runtime/gc/collector/concurrent_copying-inl.h
index 2de79107f4..544258ebd9 100644
--- a/runtime/gc/collector/concurrent_copying-inl.h
+++ b/runtime/gc/collector/concurrent_copying-inl.h
@@ -199,6 +199,9 @@ inline mirror::Object* ConcurrentCopying::Mark(Thread* const self,
 inline mirror::Object* ConcurrentCopying::MarkFromReadBarrier(mirror::Object* from_ref) {
   mirror::Object* ret;
   Thread* const self = Thread::Current();
+  // TODO (lokeshgidra): Remove the check once b/140119552 is fixed.
+  CHECK(self->GetThreadLocalMarkStack()
+        != reinterpret_cast<accounting::AtomicStack<mirror::Object>*>(0x1));
   // We can get here before marking starts since we gray immune objects before the marking phase.
   if (from_ref == nullptr || !self->GetIsGcMarking()) {
     return from_ref;
diff --git a/runtime/gc/collector/concurrent_copying.cc b/runtime/gc/collector/concurrent_copying.cc
index afae3ef8fa..d34bbdbfaf 100644
--- a/runtime/gc/collector/concurrent_copying.cc
+++ b/runtime/gc/collector/concurrent_copying.cc
@@ -440,6 +440,9 @@ class ConcurrentCopying::ThreadFlipVisitor : public Closure, public RootVisitor
   void Run(Thread* thread) override REQUIRES_SHARED(Locks::mutator_lock_) {
     // Note: self is not necessarily equal to thread since thread may be suspended.
     Thread* self = Thread::Current();
+    // TODO (lokeshgidra): Remove once b/140119552 is fixed.
+    CHECK(self->GetThreadLocalMarkStack()
+          != reinterpret_cast<accounting::AtomicStack<mirror::Object>*>(0x1));
     CHECK(thread == self || thread->IsSuspended() || thread->GetState() == kWaitingPerformingGc)
         << thread->GetState() << " thread " << thread << " self " << self;
     thread->SetIsGcMarkingAndUpdateEntrypoints(true);
@@ -996,7 +999,9 @@ class ConcurrentCopying::RevokeThreadLocalMarkStackCheckpoint : public Closure {
         << thread->GetState() << " thread " << thread << " self " << self;
     // Revoke thread local mark stacks.
     accounting::AtomicStack<mirror::Object>* tl_mark_stack = thread->GetThreadLocalMarkStack();
-    if (tl_mark_stack != nullptr) {
+    // TODO (lokeshgidra): remove the 0x1 condition once b/140119552 is fixed.
+    if (tl_mark_stack != nullptr
+        && tl_mark_stack != reinterpret_cast<accounting::AtomicStack<mirror::Object>*>(0x1)) {
       MutexLock mu(self, concurrent_copying_->mark_stack_lock_);
       concurrent_copying_->revoked_mark_stacks_.push_back(tl_mark_stack);
       thread->SetThreadLocalMarkStack(nullptr);
@@ -2059,6 +2064,9 @@ void ConcurrentCopying::RevokeThreadLocalMarkStack(Thread* thread) {
     RemoveThreadMarkStackMapping(thread, tl_mark_stack);
     thread->SetThreadLocalMarkStack(nullptr);
   }
+  // TODO (lokeshgidra): storing a non-null value to diagnose b/140119552.
+  // The CL to be reverted once the issue is fixed.
+  thread->SetThreadLocalMarkStack(reinterpret_cast<accounting::AtomicStack<mirror::Object>*>(0x1));
 }
 
 void ConcurrentCopying::ProcessMarkStack() {
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 59a38e161d..6f0776b6b5 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -2466,7 +2466,9 @@ Thread::~Thread() {
     Runtime::Current()->GetHeap()->ConcurrentCopyingCollector()
         ->AssertNoThreadMarkStackMapping(this);
     gc::accounting::AtomicStack<mirror::Object>* tl_mark_stack = GetThreadLocalMarkStack();
-    CHECK(tl_mark_stack == nullptr) << "mark-stack: " << tl_mark_stack;
+    CHECK(tl_mark_stack == nullptr
+          || tl_mark_stack == reinterpret_cast<gc::accounting::AtomicStack<mirror::Object>*>(0x1))
+        << "mark-stack: " << tl_mark_stack;
   }
   // Make sure we processed all deoptimization requests.
   CHECK(tlsPtr_.deoptimization_context_stack == nullptr) << "Missed deoptimization";