Add comment & refactor check for Obsolete class in FindFieldFromCode
Add a comment explaining why we need a check for a class not being an
obsolete class in FindFieldFromCode. It is needed since if we
structurally redefine a class that is in the middle of running its
<clinit> or <init> function the classes final fields will be set from
a function with the obsolete class as its declaring-class.
Also refactor the code a bit to put this logic in ArtField.
Test: None
Change-Id: I9ebb0c89568b9d8e5a60d44c982584b174370eef
diff --git a/runtime/art_field-inl.h b/runtime/art_field-inl.h
index 41808dd..5ab6d91 100644
--- a/runtime/art_field-inl.h
+++ b/runtime/art_field-inl.h
@@ -40,6 +40,17 @@
return GetDeclaringClass<kWithoutReadBarrier>()->IsProxyClass<kVerifyNone>();
}
+// We are only ever allowed to set our own final fields. We do need to be careful since if a
+// structural redefinition occurs during <clinit> we can end up trying to set the non-obsolete
+// class's fields from the obsolete class. This is something we want to allow. This is tested by
+// run-test 2002-virtual-structural-initializing.
+inline bool ArtField::CanBeChangedBy(ArtMethod* method) {
+ ObjPtr<mirror::Class> declaring_class(GetDeclaringClass());
+ ObjPtr<mirror::Class> referring_class(method->GetDeclaringClass());
+ return !IsFinal() || (declaring_class == referring_class) ||
+ UNLIKELY(referring_class->IsObsoleteVersionOf(declaring_class));
+}
+
template<ReadBarrierOption kReadBarrierOption>
inline ObjPtr<mirror::Class> ArtField::GetDeclaringClass() {
GcRootSource gc_root_source(this);
diff --git a/runtime/art_field.h b/runtime/art_field.h
index c149003..86f67ad 100644
--- a/runtime/art_field.h
+++ b/runtime/art_field.h
@@ -233,6 +233,9 @@
std::string PrettyField(bool with_type = true)
REQUIRES_SHARED(Locks::mutator_lock_);
+ // Returns true if a set-* instruction in the given method is allowable.
+ ALWAYS_INLINE inline bool CanBeChangedBy(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_);
+
private:
bool IsProxyField() REQUIRES_SHARED(Locks::mutator_lock_);
diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h
index c67b1b0..338928e 100644
--- a/runtime/entrypoints/entrypoint_utils-inl.h
+++ b/runtime/entrypoints/entrypoint_utils-inl.h
@@ -358,8 +358,7 @@
DCHECK(self->IsExceptionPending()); // Throw exception and unwind.
return nullptr; // Failure.
}
- if (UNLIKELY(is_set && resolved_field->IsFinal() && (fields_class != referring_class) &&
- !referring_class->IsObsoleteVersionOf(fields_class))) {
+ if (UNLIKELY(is_set && !resolved_field->CanBeChangedBy(referrer))) {
ThrowIllegalAccessErrorFinalField(referrer, resolved_field);
return nullptr; // Failure.
} else {
@@ -630,7 +629,7 @@
ObjPtr<mirror::Class> referring_class = referrer->GetDeclaringClass();
if (UNLIKELY(!referring_class->CanAccess(fields_class) ||
!referring_class->CanAccessMember(fields_class, resolved_field->GetAccessFlags()) ||
- (is_set && resolved_field->IsFinal() && (fields_class != referring_class)))) {
+ (is_set && !resolved_field->CanBeChangedBy(referrer)))) {
// Illegal access.
return nullptr;
}
