Fix compiling boot image extension with assume-verified.
The change https://android-review.googlesource.com/1246288
enabled AOT initialization of classes for the compiler
filter "assume-verified". However, this was untested and
broken because aborted transactions would leave created
array classes with partially rolled-back state on the heap.
Note that for other compiler filters, the verification step
creates those array classes outside of a transaction.
We fix this problem by making sure that the initialization
of an array class does not create any transaction records
for the class fields (we are keeping the defined class).
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Test: aosp_taimen-userdebug boots.
Test: adb root && \
adb shell stop && \
adb shell setprop dalvik.vm.boot-image \
'boot.art:/nonx/boot-framework.art!/system/etc/boot-image.prof' && \
adb shell setprop dalvik.vm.image-dex2oat-filter \
assume-verified && \
adb shell start # Starts correctly.
Test: Extract the boot image extension compilation command
from the verbose log, change compiler filter to
"assume-verified" and execute with no crashes.
Bug: 119800099
Change-Id: Id57a01af0a015c75a16eea12fd19599ae76ce49a
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index cde0432..e1dc544 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -139,6 +139,7 @@
#include "thread-inl.h"
#include "thread_list.h"
#include "trace.h"
+#include "transaction.h"
#include "utils/dex_cache_arrays_layout-inl.h"
#include "verifier/class_verifier.h"
#include "well_known_classes.h"
@@ -2179,7 +2180,7 @@
// class loader is only the initiating loader but not the defining loader.
// Avoid read barrier since we are comparing against null.
if (klass->GetClassLoader<kDefaultVerifyFlags, kWithoutReadBarrier>() != nullptr) {
- klass->SetClassLoader</*kCheckTransaction=*/ false>(loader);
+ klass->SetClassLoader(loader);
}
}
}
@@ -2636,7 +2637,7 @@
// Arrays are access-checks-clean and preverified.
access_flags |= kAccVerificationAttempted;
- array_class->SetAccessFlags(access_flags);
+ array_class->SetAccessFlagsDuringLinking(access_flags);
// Array classes are fully initialized either during single threaded startup,
// or from a pre-fence visitor, so visibly initialized.
@@ -3694,7 +3695,7 @@
klass->SetClass(GetClassRoot<mirror::Class>(this));
uint32_t access_flags = dex_class_def.GetJavaAccessFlags();
CHECK_EQ(access_flags & ~kAccJavaFlagsMask, 0U);
- klass->SetAccessFlags(access_flags);
+ klass->SetAccessFlagsDuringLinking(access_flags);
klass->SetClassLoader(class_loader);
DCHECK_EQ(klass->GetPrimitiveType(), Primitive::kPrimNot);
mirror::Class::SetStatus(klass, ClassStatus::kIdx, nullptr);
@@ -4245,7 +4246,7 @@
CHECK(primitive_class != nullptr) << "OOM for primitive class " << type;
// Do not hold lock on the primitive class object, the initialization of
// primitive classes is done while the process is still single threaded.
- primitive_class->SetAccessFlags(
+ primitive_class->SetAccessFlagsDuringLinking(
kAccPublic | kAccFinal | kAccAbstract | kAccVerificationAttempted);
primitive_class->SetPrimitiveType(type);
primitive_class->SetIfTable(GetClassRoot<mirror::Object>(this)->GetIfTable());
@@ -4362,6 +4363,7 @@
auto visitor = [this, array_class_size, component_type](ObjPtr<mirror::Object> obj,
size_t usable_size)
REQUIRES_SHARED(Locks::mutator_lock_) {
+ ScopedAssertNoNewTransactionRecords sanntr("CreateArrayClass");
mirror::Class::InitializeClassVisitor init_class(array_class_size);
init_class(obj, usable_size);
ObjPtr<mirror::Class> klass = ObjPtr<mirror::Class>::DownCast(obj);
@@ -4962,7 +4964,8 @@
temp_klass->SetObjectSize(sizeof(mirror::Proxy));
// Set the class access flags incl. VerificationAttempted, so we do not try to set the flag on
// the methods.
- temp_klass->SetAccessFlags(kAccClassIsProxy | kAccPublic | kAccFinal | kAccVerificationAttempted);
+ temp_klass->SetAccessFlagsDuringLinking(
+ kAccClassIsProxy | kAccPublic | kAccFinal | kAccVerificationAttempted);
temp_klass->SetClassLoader(soa.Decode<mirror::ClassLoader>(loader));
DCHECK_EQ(temp_klass->GetPrimitiveType(), Primitive::kPrimNot);
temp_klass->SetName(soa.Decode<mirror::String>(name));
diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h
index 6c45ebf..ef82d23 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -78,7 +78,8 @@
DCHECK(old_super_class == nullptr || old_super_class == new_super_class);
}
DCHECK(new_super_class != nullptr);
- SetFieldObject<false>(OFFSET_OF_OBJECT_MEMBER(Class, super_class_), new_super_class);
+ SetFieldObject</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ OFFSET_OF_OBJECT_MEMBER(Class, super_class_), new_super_class);
}
inline bool Class::HasSuperClass() {
@@ -299,7 +300,8 @@
}
inline void Class::SetVTable(ObjPtr<PointerArray> new_vtable) {
- SetFieldObject<false>(OFFSET_OF_OBJECT_MEMBER(Class, vtable_), new_vtable);
+ SetFieldObject</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ OFFSET_OF_OBJECT_MEMBER(Class, vtable_), new_vtable);
}
template<VerifyObjectFlags kVerifyFlags>
@@ -345,7 +347,8 @@
}
inline void Class::SetEmbeddedVTableLength(int32_t len) {
- SetField32<false>(MemberOffset(EmbeddedVTableLengthOffset()), len);
+ SetField32</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ MemberOffset(EmbeddedVTableLengthOffset()), len);
}
inline ImTable* Class::GetImt(PointerSize pointer_size) {
@@ -353,7 +356,8 @@
}
inline void Class::SetImt(ImTable* imt, PointerSize pointer_size) {
- return SetFieldPtrWithSize<false>(ImtPtrOffset(pointer_size), imt, pointer_size);
+ return SetFieldPtrWithSize</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ ImtPtrOffset(pointer_size), imt, pointer_size);
}
inline MemberOffset Class::EmbeddedVTableEntryOffset(uint32_t i, PointerSize pointer_size) {
@@ -367,7 +371,8 @@
inline void Class::SetEmbeddedVTableEntryUnchecked(
uint32_t i, ArtMethod* method, PointerSize pointer_size) {
- SetFieldPtrWithSize<false>(EmbeddedVTableEntryOffset(i, pointer_size), method, pointer_size);
+ SetFieldPtrWithSize</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ EmbeddedVTableEntryOffset(i, pointer_size), method, pointer_size);
}
inline void Class::SetEmbeddedVTableEntry(uint32_t i, ArtMethod* method, PointerSize pointer_size) {
@@ -671,7 +676,8 @@
inline void Class::SetIfTable(ObjPtr<IfTable> new_iftable) {
DCHECK(new_iftable != nullptr) << PrettyClass(this);
- SetFieldObject<false>(IfTableOffset(), new_iftable);
+ SetFieldObject</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ IfTableOffset(), new_iftable);
}
inline LengthPrefixedArray<ArtField>* Class::GetIFieldsPtr() {
@@ -919,8 +925,15 @@
klass->SetDexClassDefIndex(DexFile::kDexNoIndex16); // Default to no valid class def index.
klass->SetDexTypeIndex(dex::TypeIndex(DexFile::kDexNoIndex16)); // Default to no valid type
// index.
- // Default to force slow path until initialized.
- klass->SetObjectSizeAllocFastPath(std::numeric_limits<uint32_t>::max());
+ // Default to force slow path until visibly initialized.
+ // There is no need for release store (volatile) in pre-fence visitor.
+ klass->SetField32</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ ObjectSizeAllocFastPathOffset(), std::numeric_limits<uint32_t>::max());
+}
+
+inline void Class::SetAccessFlagsDuringLinking(uint32_t new_access_flags) {
+ SetField32</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ AccessFlagsOffset(), new_access_flags);
}
inline void Class::SetAccessFlags(uint32_t new_access_flags) {
@@ -936,11 +949,8 @@
}
inline void Class::SetClassFlags(uint32_t new_flags) {
- if (Runtime::Current()->IsActiveTransaction()) {
- SetField32<true>(OFFSET_OF_OBJECT_MEMBER(Class, class_flags_), new_flags);
- } else {
- SetField32<false>(OFFSET_OF_OBJECT_MEMBER(Class, class_flags_), new_flags);
- }
+ SetField32</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ OFFSET_OF_OBJECT_MEMBER(Class, class_flags_), new_flags);
}
inline uint32_t Class::NumDirectInterfaces() {
@@ -1020,7 +1030,8 @@
DCHECK(GetComponentType() == nullptr);
DCHECK(new_component_type != nullptr);
// Component type is invariant: use non-transactional mode without check.
- SetFieldObject<false, false>(ComponentTypeOffset(), new_component_type);
+ SetFieldObject</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ ComponentTypeOffset(), new_component_type);
}
inline size_t Class::GetComponentSize() {
@@ -1188,14 +1199,9 @@
return component->IsPrimitive() || component->CannotBeAssignedFromOtherTypes();
}
-template <bool kCheckTransaction>
inline void Class::SetClassLoader(ObjPtr<ClassLoader> new_class_loader) {
- if (kCheckTransaction && Runtime::Current()->IsActiveTransaction()) {
- SetFieldObject<true>(OFFSET_OF_OBJECT_MEMBER(Class, class_loader_), new_class_loader);
- } else {
- DCHECK(!Runtime::Current()->IsActiveTransaction());
- SetFieldObject<false>(OFFSET_OF_OBJECT_MEMBER(Class, class_loader_), new_class_loader);
- }
+ SetFieldObject</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ OFFSET_OF_OBJECT_MEMBER(Class, class_loader_), new_class_loader);
}
inline void Class::SetRecursivelyInitialized() {
@@ -1207,7 +1213,7 @@
inline void Class::SetHasDefaultMethods() {
DCHECK_EQ(GetLockOwnerThreadId(), Thread::Current()->GetThreadId());
uint32_t flags = GetField32(OFFSET_OF_OBJECT_MEMBER(Class, access_flags_));
- SetAccessFlags(flags | kAccHasDefaultMethod);
+ SetAccessFlagsDuringLinking(flags | kAccHasDefaultMethod);
}
} // namespace mirror
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index e57cc98..a2d76cd 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -362,7 +362,8 @@
LOG(FATAL_WITHOUT_ABORT) << new_class_size << " vs " << GetClassSize();
LOG(FATAL) << "class=" << PrettyTypeOf();
}
- SetField32Transaction(OFFSET_OF_OBJECT_MEMBER(Class, class_size_), new_class_size);
+ SetField32</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ OFFSET_OF_OBJECT_MEMBER(Class, class_size_), new_class_size);
}
ObjPtr<Class> Class::GetObsoleteClass() {
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index 3e24346..48e1c3c 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -216,6 +216,10 @@
}
void SetClassFlags(uint32_t new_flags) REQUIRES_SHARED(Locks::mutator_lock_);
+ // Set access flags during linking, these cannot be rolled back by a Transaction.
+ void SetAccessFlagsDuringLinking(uint32_t new_access_flags) REQUIRES_SHARED(Locks::mutator_lock_);
+
+ // Set access flags, recording the change if running inside a Transaction.
void SetAccessFlags(uint32_t new_access_flags) REQUIRES_SHARED(Locks::mutator_lock_);
// Returns true if the class is an enum.
@@ -258,7 +262,7 @@
ALWAYS_INLINE void SetFinalizable() REQUIRES_SHARED(Locks::mutator_lock_) {
uint32_t flags = GetField32(OFFSET_OF_OBJECT_MEMBER(Class, access_flags_));
- SetAccessFlags(flags | kAccClassIsFinalizable);
+ SetAccessFlagsDuringLinking(flags | kAccClassIsFinalizable);
}
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
@@ -403,7 +407,8 @@
DCHECK_EQ(v32 & kPrimitiveTypeMask, v32) << "upper 16 bits aren't zero";
// Store the component size shift in the upper 16 bits.
v32 |= Primitive::ComponentSizeShift(new_type) << kPrimitiveTypeSizeShiftShift;
- SetField32Transaction(OFFSET_OF_OBJECT_MEMBER(Class, primitive_type_), v32);
+ SetField32</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ OFFSET_OF_OBJECT_MEMBER(Class, primitive_type_), v32);
}
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
@@ -659,7 +664,6 @@
ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
ObjPtr<ClassLoader> GetClassLoader() ALWAYS_INLINE REQUIRES_SHARED(Locks::mutator_lock_);
- template <bool kCheckTransaction = true>
void SetClassLoader(ObjPtr<ClassLoader> new_cl) REQUIRES_SHARED(Locks::mutator_lock_);
static constexpr MemberOffset DexCacheOffset() {
@@ -1130,7 +1134,8 @@
}
void SetDexClassDefIndex(uint16_t class_def_idx) REQUIRES_SHARED(Locks::mutator_lock_) {
- SetField32Transaction(OFFSET_OF_OBJECT_MEMBER(Class, dex_class_def_idx_), class_def_idx);
+ SetField32</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ OFFSET_OF_OBJECT_MEMBER(Class, dex_class_def_idx_), class_def_idx);
}
dex::TypeIndex GetDexTypeIndex() REQUIRES_SHARED(Locks::mutator_lock_) {
@@ -1139,7 +1144,8 @@
}
void SetDexTypeIndex(dex::TypeIndex type_idx) REQUIRES_SHARED(Locks::mutator_lock_) {
- SetField32Transaction(OFFSET_OF_OBJECT_MEMBER(Class, dex_type_idx_), type_idx.index_);
+ SetField32</*kTransactionActive=*/ false, /*kCheckTransaction=*/ false>(
+ OFFSET_OF_OBJECT_MEMBER(Class, dex_type_idx_), type_idx.index_);
}
dex::TypeIndex FindTypeIndexInOtherDexFile(const DexFile& dex_file)
diff --git a/runtime/transaction.cc b/runtime/transaction.cc
index 6756c7b..5d9456b 100644
--- a/runtime/transaction.cc
+++ b/runtime/transaction.cc
@@ -45,7 +45,8 @@
rolling_back_(false),
heap_(Runtime::Current()->GetHeap()),
strict_(strict),
- root_(root) {
+ root_(root),
+ assert_no_new_records_reason_(nullptr) {
DCHECK(Runtime::Current()->IsAotCompiler());
}
@@ -170,6 +171,7 @@
bool is_volatile) {
DCHECK(obj != nullptr);
MutexLock mu(Thread::Current(), log_lock_);
+ DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
ObjectLog& object_log = object_logs_[obj];
object_log.LogBooleanValue(field_offset, value, is_volatile);
}
@@ -180,6 +182,7 @@
bool is_volatile) {
DCHECK(obj != nullptr);
MutexLock mu(Thread::Current(), log_lock_);
+ DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
ObjectLog& object_log = object_logs_[obj];
object_log.LogByteValue(field_offset, value, is_volatile);
}
@@ -190,6 +193,7 @@
bool is_volatile) {
DCHECK(obj != nullptr);
MutexLock mu(Thread::Current(), log_lock_);
+ DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
ObjectLog& object_log = object_logs_[obj];
object_log.LogCharValue(field_offset, value, is_volatile);
}
@@ -201,6 +205,7 @@
bool is_volatile) {
DCHECK(obj != nullptr);
MutexLock mu(Thread::Current(), log_lock_);
+ DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
ObjectLog& object_log = object_logs_[obj];
object_log.LogShortValue(field_offset, value, is_volatile);
}
@@ -212,6 +217,7 @@
bool is_volatile) {
DCHECK(obj != nullptr);
MutexLock mu(Thread::Current(), log_lock_);
+ DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
ObjectLog& object_log = object_logs_[obj];
object_log.Log32BitsValue(field_offset, value, is_volatile);
}
@@ -222,6 +228,7 @@
bool is_volatile) {
DCHECK(obj != nullptr);
MutexLock mu(Thread::Current(), log_lock_);
+ DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
ObjectLog& object_log = object_logs_[obj];
object_log.Log64BitsValue(field_offset, value, is_volatile);
}
@@ -232,6 +239,7 @@
bool is_volatile) {
DCHECK(obj != nullptr);
MutexLock mu(Thread::Current(), log_lock_);
+ DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
ObjectLog& object_log = object_logs_[obj];
object_log.LogReferenceValue(field_offset, value, is_volatile);
}
@@ -241,6 +249,7 @@
DCHECK(array->IsArrayInstance());
DCHECK(!array->IsObjectArray());
MutexLock mu(Thread::Current(), log_lock_);
+ DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
auto it = array_logs_.find(array);
if (it == array_logs_.end()) {
ArrayLog log;
@@ -254,6 +263,7 @@
DCHECK(dex_cache != nullptr);
DCHECK_LT(string_idx.index_, dex_cache->GetDexFile()->NumStringIds());
MutexLock mu(Thread::Current(), log_lock_);
+ DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
resolve_string_logs_.emplace_back(dex_cache, string_idx);
}
@@ -280,6 +290,7 @@
void Transaction::LogInternedString(InternStringLog&& log) {
Locks::intern_table_lock_->AssertExclusiveHeld(Thread::Current());
MutexLock mu(Thread::Current(), log_lock_);
+ DCHECK(assert_no_new_records_reason_ == nullptr) << assert_no_new_records_reason_;
intern_string_logs_.push_front(std::move(log));
}
@@ -698,4 +709,27 @@
}
}
+Transaction* ScopedAssertNoNewTransactionRecords::InstallAssertion(const char* reason) {
+ Transaction* transaction = nullptr;
+ if (kIsDebugBuild && Runtime::Current()->IsActiveTransaction()) {
+ transaction = Runtime::Current()->GetTransaction().get();
+ if (transaction != nullptr) {
+ MutexLock mu(Thread::Current(), transaction->log_lock_);
+ 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) {
+ CHECK(Runtime::Current()->GetTransaction().get() == transaction);
+ MutexLock mu(Thread::Current(), transaction->log_lock_);
+ 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 f05bfee..184280f 100644
--- a/runtime/transaction.h
+++ b/runtime/transaction.h
@@ -320,10 +320,31 @@
const bool strict_;
std::string abort_message_ GUARDED_BY(log_lock_);
mirror::Class* root_ GUARDED_BY(log_lock_);
+ const char* assert_no_new_records_reason_ GUARDED_BY(log_lock_);
+
+ 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_