Replace `ScopedAssertNoNewTransactionRecords`...

... with `ScopedAssertNoTransactionChecks`. The new check
is stronger than the old one but does not work during early
setup when we do not have a `Thread` object yet.

Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Change-Id: Iba5a5cda0d97993ff324b4d11de02cb07f770699

10 files changed, 100 insertions, 78 deletions

diff --git a/compiler/utils/assembler_thumb_test_expected.cc.inc b/compiler/utils/assembler_thumb_test_expected.cc.inc
index 6e0048eb20..5184b2c897 100644
--- a/compiler/utils/assembler_thumb_test_expected.cc.inc
+++ b/compiler/utils/assembler_thumb_test_expected.cc.inc
@@ -154,7 +154,7 @@ const char* const VixlJniHelpersResults = {
   "     210: f8d9 8020     ldr.w r8, [r9, #32]\n"
   "     214: 4770          bx lr\n"
   "     216: f8d9 0094     ldr.w r0, [r9, #148]\n"
-  "     21a: f8d9 e2c4     ldr.w lr, [r9, #708]\n"
+  "     21a: f8d9 e2c0     ldr.w lr, [r9, #704]\n"
   "     21e: 47f0          blx lr\n"
 };
 
diff --git a/dex2oat/linker/oat_writer.cc b/dex2oat/linker/oat_writer.cc
index 78b3f972f1..a0056c85c4 100644
--- a/dex2oat/linker/oat_writer.cc
+++ b/dex2oat/linker/oat_writer.cc
@@ -2358,7 +2358,7 @@ size_t OatWriter::InitOatCodeDexFiles(size_t offset) {
 
   if (HasImage()) {
     ScopedObjectAccess soa(Thread::Current());
-    ScopedAssertNoThreadSuspension sants("Init image method visitor", Thread::Current());
+    ScopedAssertNoThreadSuspension sants("Init image method visitor");
     InitImageMethodVisitor image_visitor(this, offset, dex_files_);
     success = VisitDexMethods(&image_visitor);
     image_visitor.Postprocess();
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 4d3eaea37f..72e4d14789 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -148,6 +148,7 @@
 #include "profile/profile_compilation_info.h"
 #include "runtime.h"
 #include "runtime_callbacks.h"
+#include "scoped_assert_no_transaction_checks.h"
 #include "scoped_thread_state_change-inl.h"
 #include "startup_completed_task.h"
 #include "thread-inl.h"
@@ -4659,7 +4660,7 @@ ObjPtr<mirror::Class> ClassLinker::CreateArrayClass(Thread* self,
   auto visitor = [this, array_class_size, component_type](ObjPtr<mirror::Object> obj,
                                                           size_t usable_size)
       REQUIRES_SHARED(Locks::mutator_lock_) {
-    ScopedAssertNoNewTransactionRecords sanntr("CreateArrayClass");
+    ScopedAssertNoTransactionChecks santc("CreateArrayClass");
     mirror::Class::InitializeClassVisitor init_class(array_class_size);
     init_class(obj, usable_size);
     ObjPtr<mirror::Class> klass = ObjPtr<mirror::Class>::DownCast(obj);
diff --git a/runtime/entrypoints_order_test.cc b/runtime/entrypoints_order_test.cc
index 20f2863bb5..b25e01067b 100644
--- a/runtime/entrypoints_order_test.cc
+++ b/runtime/entrypoints_order_test.cc
@@ -105,16 +105,14 @@ class EntrypointsOrderTest : public CommonArtTest {
                         frame_id_to_shadow_frame, sizeof(void*));
     EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, frame_id_to_shadow_frame, name, sizeof(void*));
     EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, name, pthread_self, sizeof(void*));
-    EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, pthread_self, last_no_thread_suspension_cause,
-                        sizeof(void*));
-    EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, last_no_thread_suspension_cause,
-                        active_suspendall_barrier, sizeof(void*));
+    EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, pthread_self, active_suspendall_barrier, sizeof(void*));
     EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, active_suspendall_barrier,
                         active_suspend1_barriers, sizeof(void*));
-    EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, active_suspend1_barriers, thread_local_pos, sizeof(void*));
+    EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, active_suspend1_barriers, thread_local_start,
+                        sizeof(void*));
+    EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, thread_local_start, thread_local_pos, sizeof(void*));
     EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, thread_local_pos, thread_local_end, sizeof(void*));
-    EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, thread_local_end, thread_local_start, sizeof(void*));
-    EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, thread_local_start, thread_local_limit, sizeof(void*));
+    EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, thread_local_end, thread_local_limit, sizeof(void*));
     EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, thread_local_limit, thread_local_objects, sizeof(void*));
     EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, thread_local_objects, checkpoint_function, sizeof(size_t));
     EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, checkpoint_function, jni_entrypoints,
@@ -139,11 +137,17 @@ class EntrypointsOrderTest : public CommonArtTest {
         Thread, tlsPtr_, method_trace_buffer, method_trace_buffer_index, sizeof(void*));
     EXPECT_OFFSET_DIFFP(
         Thread, tlsPtr_, method_trace_buffer_index, thread_exit_flags, sizeof(void*));
+    EXPECT_OFFSET_DIFFP(
+        Thread, tlsPtr_, thread_exit_flags, last_no_thread_suspension_cause, sizeof(void*));
+    EXPECT_OFFSET_DIFFP(Thread, tlsPtr_, last_no_thread_suspension_cause,
+                        last_no_transaction_checks_cause, sizeof(void*));
     // The first field after tlsPtr_ is forced to a 16 byte alignment so it might have some space.
     auto offset_tlsptr_end = OFFSETOF_MEMBER(Thread, tlsPtr_) +
         sizeof(decltype(reinterpret_cast<Thread*>(16)->tlsPtr_));
-    CHECKED(offset_tlsptr_end - OFFSETOF_MEMBER(Thread, tlsPtr_.thread_exit_flags) == sizeof(void*),
-            "async_exception last field");
+    CHECKED(
+        offset_tlsptr_end - OFFSETOF_MEMBER(Thread, tlsPtr_.last_no_transaction_checks_cause) ==
+            sizeof(void*),
+        "last_no_transaction_checks_cause last field");
   }
 
   void CheckJniEntryPoints() {
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 152180985e..701d68176c 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -3492,4 +3492,13 @@ void Runtime::AddExtraBootDexFiles(const std::string& filename,
   GetClassLinker()->AddExtraBootDexFiles(Thread::Current(), std::move(dex_files));
 }
 
+void Runtime::DCheckNoTransactionCheckAllowed() {
+  if (kIsDebugBuild) {
+    Thread* self = Thread::Current();
+    if (self != nullptr) {
+      self->AssertNoTransactionCheckAllowed();
+    }
+  }
+}
+
 }  // namespace art
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 89020ef5ae..afb8ebd3c0 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -631,6 +631,9 @@ class Runtime {
   }
 
   bool IsActiveTransaction() {
+    if (kIsDebugBuild) {
+      DCheckNoTransactionCheckAllowed();
+    }
     return active_transaction_;
   }
 
@@ -1179,6 +1182,8 @@ class Runtime {
 
   void AppendToBootClassPath(const std::string& filename, const std::string& location);
 
+  void DCheckNoTransactionCheckAllowed();
+
   // Don't use EXPORT ("default" visibility), because quick_entrypoints_x86.o
   // refers to this symbol and it can't link with R_386_PC32 relocation.
   // A pointer to the active runtime or null.
diff --git a/runtime/scoped_assert_no_transaction_checks.h b/runtime/scoped_assert_no_transaction_checks.h
new file mode 100644
index 0000000000..cbdff5955b
--- /dev/null
+++ b/runtime/scoped_assert_no_transaction_checks.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef ART_RUNTIME_SCOPED_ASSERT_NO_TRANSACTION_CHECKS_H_
+#define ART_RUNTIME_SCOPED_ASSERT_NO_TRANSACTION_CHECKS_H_
+
+#include "thread-current-inl.h"
+
+namespace art {
+
+class ScopedAssertNoTransactionChecks {
+ public:
+  explicit ScopedAssertNoTransactionChecks(const char* cause)
+      : self_(kIsDebugBuild ? Thread::Current() : nullptr),
+        old_cause_(self_ != nullptr ? self_->tlsPtr_.last_no_transaction_checks_cause : nullptr) {
+    DCHECK(cause != nullptr);
+    if (kIsDebugBuild && self_ != nullptr) {
+      self_->tlsPtr_.last_no_transaction_checks_cause = cause;
+    }
+  }
+
+  ~ScopedAssertNoTransactionChecks() {
+    if (kIsDebugBuild && self_ != nullptr) {
+      self_->tlsPtr_.last_no_transaction_checks_cause = old_cause_;
+    }
+  }
+
+ private:
+  Thread* const self_;
+  const char* const old_cause_;
+};
+
+}  // namespace art
+
+#endif  // ART_RUNTIME_SCOPED_ASSERT_NO_TRANSACTION_CHECKS_H_
diff --git a/runtime/thread.h b/runtime/thread.h
index 912015a07f..464b343b1e 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -548,6 +548,11 @@ class EXPORT Thread {
 
   void AssertThreadSuspensionIsAllowable(bool check_locks = true) const;
 
+  void AssertNoTransactionCheckAllowed() const {
+    CHECK(tlsPtr_.last_no_transaction_checks_cause == nullptr)
+        << tlsPtr_.last_no_transaction_checks_cause;
+  }
+
   // Return true if thread suspension is allowable.
   bool IsThreadSuspensionAllowable() const;
 
@@ -2131,12 +2136,11 @@ class EXPORT Thread {
                                frame_id_to_shadow_frame(nullptr),
                                name(nullptr),
                                pthread_self(0),
-                               last_no_thread_suspension_cause(nullptr),
                                active_suspendall_barrier(nullptr),
                                active_suspend1_barriers(nullptr),
+                               thread_local_start(nullptr),
                                thread_local_pos(nullptr),
                                thread_local_end(nullptr),
-                               thread_local_start(nullptr),
                                thread_local_limit(nullptr),
                                thread_local_objects(0),
                                checkpoint_function(nullptr),
@@ -2149,7 +2153,9 @@ class EXPORT Thread {
                                top_reflective_handle_scope(nullptr),
                                method_trace_buffer(nullptr),
                                method_trace_buffer_index(0),
-                               thread_exit_flags(nullptr) {
+                               thread_exit_flags(nullptr),
+                               last_no_thread_suspension_cause(nullptr),
+                               last_no_transaction_checks_cause(nullptr) {
       std::fill(held_mutexes, held_mutexes + kLockLevelCount, nullptr);
     }
 
@@ -2248,9 +2254,6 @@ class EXPORT Thread {
     // A cached pthread_t for the pthread underlying this Thread*.
     pthread_t pthread_self;
 
-    // If no_thread_suspension_ is > 0, what is causing that assertion.
-    const char* last_no_thread_suspension_cause;
-
     // After a thread observes a suspend request and enters a suspended state,
     // it notifies the requestor by arriving at a "suspend barrier". This consists of decrementing
     // the atomic integer representing the barrier. (This implementation was introduced in 2015 to
@@ -2267,14 +2270,14 @@ class EXPORT Thread {
     // The struct as a whole is still stored on the requesting thread's stack.
     WrappedSuspend1Barrier* active_suspend1_barriers GUARDED_BY(Locks::thread_suspend_count_lock_);
 
+    // Thread-local allocation pointer. Can be moved below the following two to correct alignment.
+    uint8_t* thread_local_start;
+
     // thread_local_pos and thread_local_end must be consecutive for ldrd and are 8 byte aligned for
     // potentially better performance.
     uint8_t* thread_local_pos;
     uint8_t* thread_local_end;
 
-    // Thread-local allocation pointer. Can be moved above the preceding two to correct alignment.
-    uint8_t* thread_local_start;
-
     // Thread local limit is how much we can expand the thread local buffer to, it is greater or
     // equal to thread_local_end.
     uint8_t* thread_local_limit;
@@ -2329,6 +2332,13 @@ class EXPORT Thread {
 
     // Pointer to the first node of an intrusively doubly-linked list of ThreadExitFlags.
     ThreadExitFlag* thread_exit_flags GUARDED_BY(Locks::thread_list_lock_);
+
+    // If no_thread_suspension_ is > 0, what is causing that assertion.
+    const char* last_no_thread_suspension_cause;
+
+    // If the thread is asserting that there should be no transaction checks,
+    // what is causing that assertion (debug builds only).
+    const char* last_no_transaction_checks_cause;
   } tlsPtr_;
 
   // Small thread-local cache to be used from the interpreter.
@@ -2387,6 +2397,7 @@ class EXPORT Thread {
   friend class gc::collector::SemiSpace;  // For getting stack traces.
   friend class Runtime;  // For CreatePeer.
   friend class QuickExceptionHandler;  // For dumping the stack.
+  friend class ScopedAssertNoTransactionChecks;
   friend class ScopedThreadStateChange;
   friend class StubTest;  // For accessing entrypoints.
   friend class ThreadList;  // For ~Thread, Destroy and EnsureFlipFunctionStarted.
diff --git a/runtime/transaction.cc b/runtime/transaction.cc
index 9b3985503a..ea0bcb4776 100644
--- a/runtime/transaction.cc
+++ b/runtime/transaction.cc
@@ -57,8 +57,7 @@ Transaction::Transaction(bool strict,
       heap_(Runtime::Current()->GetHeap()),
       strict_(strict),
       root_(root),
-      last_allocated_object_(nullptr),
-      assert_no_new_records_reason_(nullptr) {
+      last_allocated_object_(nullptr) {
   DCHECK(Runtime::Current()->IsAotCompiler());
   DCHECK_NE(arena_stack != nullptr, arena_pool != nullptr);
 }
@@ -210,7 +209,6 @@ void Transaction::RecordWriteFieldBoolean(mirror::Object* obj,
                                           uint8_t value,
                                           bool is_volatile) {
   DCHECK(obj != nullptr);
-  DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
   if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.LogBooleanValue(field_offset, value, is_volatile);
@@ -222,7 +220,6 @@ void Transaction::RecordWriteFieldByte(mirror::Object* obj,
                                        int8_t value,
                                        bool is_volatile) {
   DCHECK(obj != nullptr);
-  DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
   if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.LogByteValue(field_offset, value, is_volatile);
@@ -234,7 +231,6 @@ void Transaction::RecordWriteFieldChar(mirror::Object* obj,
                                        uint16_t value,
                                        bool is_volatile) {
   DCHECK(obj != nullptr);
-  DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
   if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.LogCharValue(field_offset, value, is_volatile);
@@ -247,7 +243,6 @@ void Transaction::RecordWriteFieldShort(mirror::Object* obj,
                                         int16_t value,
                                         bool is_volatile) {
   DCHECK(obj != nullptr);
-  DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
   if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.LogShortValue(field_offset, value, is_volatile);
@@ -260,7 +255,6 @@ void Transaction::RecordWriteField32(mirror::Object* obj,
                                      uint32_t value,
                                      bool is_volatile) {
   DCHECK(obj != nullptr);
-  DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
   if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.Log32BitsValue(field_offset, value, is_volatile);
@@ -272,7 +266,6 @@ void Transaction::RecordWriteField64(mirror::Object* obj,
                                      uint64_t value,
                                      bool is_volatile) {
   DCHECK(obj != nullptr);
-  DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
   if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.Log64BitsValue(field_offset, value, is_volatile);
@@ -284,7 +277,6 @@ void Transaction::RecordWriteFieldReference(mirror::Object* obj,
                                             mirror::Object* value,
                                             bool is_volatile) {
   DCHECK(obj != nullptr);
-  DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
   if (obj != last_allocated_object_) {
     ObjectLog& object_log = GetOrCreateObjectLog(obj);
     object_log.LogReferenceValue(field_offset, value, is_volatile);
@@ -295,7 +287,6 @@ void Transaction::RecordWriteArray(mirror::Array* array, size_t index, uint64_t
   DCHECK(array != nullptr);
   DCHECK(array->IsArrayInstance());
   DCHECK(!array->IsObjectArray());
-  DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
   if (array != last_allocated_object_) {
     ArrayLog& array_log = array_logs_.GetOrCreate(array, [&]() { return ArrayLog(&allocator_); });
     array_log.LogValue(index, value);
@@ -306,7 +297,6 @@ void Transaction::RecordResolveString(ObjPtr<mirror::DexCache> dex_cache,
                                       dex::StringIndex string_idx) {
   DCHECK(dex_cache != nullptr);
   DCHECK_LT(string_idx.index_, dex_cache->GetDexFile()->NumStringIds());
-  DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
   resolve_string_logs_.emplace_front(dex_cache, string_idx);
 }
 
@@ -314,7 +304,6 @@ void Transaction::RecordResolveMethodType(ObjPtr<mirror::DexCache> dex_cache,
                                           dex::ProtoIndex proto_idx) {
   DCHECK(dex_cache != nullptr);
   DCHECK_LT(proto_idx.index_, dex_cache->GetDexFile()->NumProtoIds());
-  DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
   resolve_method_type_logs_.emplace_front(dex_cache, proto_idx);
 }
 
@@ -340,7 +329,6 @@ void Transaction::RecordWeakStringRemoval(ObjPtr<mirror::String> s) {
 
 void Transaction::LogInternedString(InternStringLog&& log) {
   Locks::intern_table_lock_->AssertExclusiveHeld(Thread::Current());
-  DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
   intern_string_logs_.push_front(std::move(log));
 }
 
@@ -801,27 +789,4 @@ void Transaction::ArrayLog::UndoArrayWrite(mirror::Array* array,
   }
 }
 
-Transaction* ScopedAssertNoNewTransactionRecords::InstallAssertion(const char* reason) {
-  Transaction* transaction = nullptr;
-  if (kIsDebugBuild && Runtime::Current()->IsActiveTransaction()) {
-    AotClassLinker* class_linker = down_cast<AotClassLinker*>(Runtime::Current()->GetClassLinker());
-    transaction = class_linker->GetTransaction();
-    if (transaction != nullptr) {
-      CHECK(transaction->assert_no_new_records_reason_ == nullptr)
-          << "old: " << transaction->assert_no_new_records_reason_ << " new: " << reason;
-      transaction->assert_no_new_records_reason_ = reason;
-    }
-  }
-  return transaction;
-}
-
-void ScopedAssertNoNewTransactionRecords::RemoveAssertion(Transaction* transaction) {
-  if (kIsDebugBuild) {
-    AotClassLinker* class_linker = down_cast<AotClassLinker*>(Runtime::Current()->GetClassLinker());
-    CHECK(class_linker->GetTransaction() == transaction);
-    CHECK(transaction->assert_no_new_records_reason_ != nullptr);
-    transaction->assert_no_new_records_reason_ = nullptr;
-  }
-}
-
 }  // namespace art
diff --git a/runtime/transaction.h b/runtime/transaction.h
index 60d1b3e3fd..324188614f 100644
--- a/runtime/transaction.h
+++ b/runtime/transaction.h
@@ -380,31 +380,10 @@ class Transaction final {
   std::string abort_message_;
   mirror::Class* root_;
   mirror::Object* last_allocated_object_;
-  const char* assert_no_new_records_reason_;
-
-  friend class ScopedAssertNoNewTransactionRecords;
 
   DISALLOW_COPY_AND_ASSIGN(Transaction);
 };
 
-class ScopedAssertNoNewTransactionRecords {
- public:
-  explicit ScopedAssertNoNewTransactionRecords(const char* reason)
-    : transaction_(kIsDebugBuild ? InstallAssertion(reason) : nullptr) {}
-
-  ~ScopedAssertNoNewTransactionRecords() {
-    if (kIsDebugBuild && transaction_ != nullptr) {
-      RemoveAssertion(transaction_);
-    }
-  }
-
- private:
-  static Transaction* InstallAssertion(const char* reason);
-  static void RemoveAssertion(Transaction* transaction);
-
-  Transaction* transaction_;
-};
-
 }  // namespace art
 
 #endif  // ART_RUNTIME_TRANSACTION_H_