ART: Ignore repeated field indexes when loading a class.
This provides a deterministic behavior for classes with
duplicate field indexes in class_data_item and avoids
failures when verifying the field ordering in debug build.
Regression test not feasible without hand-edited dex file.
(Smali deduplicates field definitions.)
Bug: 21868015
Change-Id: I4f97ba005e063686f8f44248ed7f1286d987888a
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 7936dd3..23c5942 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -2314,22 +2314,47 @@
// Class::VisitFieldRoots may miss some fields or methods.
ScopedAssertNoThreadSuspension nts(self, __FUNCTION__);
// Load static fields.
+ // We allow duplicate definitions of the same field in a class_data_item
+ // but ignore the repeated indexes here, b/21868015.
ClassDataItemIterator it(dex_file, class_data);
- const size_t num_sfields = it.NumStaticFields();
- ArtField* sfields = num_sfields != 0 ? AllocArtFieldArray(self, num_sfields) : nullptr;
- for (size_t i = 0; it.HasNextStaticField(); i++, it.Next()) {
- CHECK_LT(i, num_sfields);
- LoadField(it, klass, &sfields[i]);
+ ArtField* sfields =
+ it.NumStaticFields() != 0 ? AllocArtFieldArray(self, it.NumStaticFields()) : nullptr;
+ size_t num_sfields = 0;
+ uint32_t last_field_idx = 0u;
+ for (; it.HasNextStaticField(); it.Next()) {
+ uint32_t field_idx = it.GetMemberIndex();
+ DCHECK_GE(field_idx, last_field_idx); // Ordering enforced by DexFileVerifier.
+ if (num_sfields == 0 || LIKELY(field_idx > last_field_idx)) {
+ DCHECK_LT(num_sfields, it.NumStaticFields());
+ LoadField(it, klass, &sfields[num_sfields]);
+ ++num_sfields;
+ last_field_idx = field_idx;
+ }
}
klass->SetSFields(sfields);
klass->SetNumStaticFields(num_sfields);
DCHECK_EQ(klass->NumStaticFields(), num_sfields);
// Load instance fields.
- const size_t num_ifields = it.NumInstanceFields();
- ArtField* ifields = num_ifields != 0 ? AllocArtFieldArray(self, num_ifields) : nullptr;
- for (size_t i = 0; it.HasNextInstanceField(); i++, it.Next()) {
- CHECK_LT(i, num_ifields);
- LoadField(it, klass, &ifields[i]);
+ ArtField* ifields =
+ it.NumInstanceFields() != 0 ? AllocArtFieldArray(self, it.NumInstanceFields()) : nullptr;
+ size_t num_ifields = 0u;
+ last_field_idx = 0u;
+ for (; it.HasNextInstanceField(); it.Next()) {
+ uint32_t field_idx = it.GetMemberIndex();
+ DCHECK_GE(field_idx, last_field_idx); // Ordering enforced by DexFileVerifier.
+ if (num_ifields == 0 || LIKELY(field_idx > last_field_idx)) {
+ DCHECK_LT(num_ifields, it.NumInstanceFields());
+ LoadField(it, klass, &ifields[num_ifields]);
+ ++num_ifields;
+ last_field_idx = field_idx;
+ }
+ }
+ if (UNLIKELY(num_sfields != it.NumStaticFields()) ||
+ UNLIKELY(num_ifields != it.NumInstanceFields())) {
+ LOG(WARNING) << "Duplicate fields in class " << PrettyDescriptor(klass.Get())
+ << " (unique static fields: " << num_sfields << "/" << it.NumStaticFields()
+ << ", unique instance fields: " << num_ifields << "/" << it.NumInstanceFields() << ")";
+ // NOTE: Not shrinking the over-allocated sfields/ifields.
}
klass->SetIFields(ifields);
klass->SetNumInstanceFields(num_ifields);
diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index c1c7123..d30fac4 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -1113,9 +1113,8 @@
void ClassDataItemIterator::ReadClassDataField() {
field_.field_idx_delta_ = DecodeUnsignedLeb128(&ptr_pos_);
field_.access_flags_ = DecodeUnsignedLeb128(&ptr_pos_);
- if (last_idx_ != 0 && field_.field_idx_delta_ == 0) {
- LOG(WARNING) << "Duplicate field in " << dex_file_.GetLocation();
- }
+ // The user of the iterator is responsible for checking if there
+ // are unordered or duplicate indexes.
}
void ClassDataItemIterator::ReadClassDataMethod() {