ART: Allow array-ness for unresolved merge types
In case component types aren't resolvable, array types won't be
either. They then may be merged. The merge is still potentially
an array type.
Ensure that merging an unresolved array type with a primitive
array type or a non-array type will be resolved to java.lang.Object.
Added tests.
Bug: 27799205
Change-Id: I9beff75318814dddd842abd64ef9a5d2644d801e
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 537d9c9..9bcbdbd 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -2425,6 +2425,10 @@
if (!array_type.IsArrayTypes()) {
Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "invalid fill-array-data with array type "
<< array_type;
+ } else if (array_type.IsUnresolvedTypes()) {
+ // If it's an unresolved array type, it must be non-primitive.
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "invalid fill-array-data for array of type "
+ << array_type;
} else {
const RegType& component_type = reg_types_.GetComponentType(array_type, GetClassLoader());
DCHECK(!component_type.IsConflict());
@@ -4209,6 +4213,7 @@
const RegType& precise_type = reg_types_.FromUninitialized(res_type);
work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_22c(), precise_type);
} else {
+ DCHECK(!res_type.IsUnresolvedMergedReference());
// Verify each register. If "arg_count" is bad, VerifyRegisterType() will run off the end of
// the list and fail. It's legal, if silly, for arg_count to be zero.
const RegType& expected_type = reg_types_.GetComponentType(res_type, GetClassLoader());
@@ -4253,6 +4258,15 @@
}
} else if (!array_type.IsArrayTypes()) {
Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "not array type " << array_type << " with aget";
+ } else if (array_type.IsUnresolvedTypes()) {
+ // Unresolved array types must be reference array types.
+ if (is_primitive) {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "reference array type " << array_type
+ << " source for category 1 aget";
+ } else {
+ Fail(VERIFY_ERROR_NO_CLASS) << "cannot verify aget for " << array_type
+ << " because of missing class";
+ }
} else {
/* verify the class */
const RegType& component_type = reg_types_.GetComponentType(array_type, GetClassLoader());
@@ -4363,6 +4377,15 @@
work_line_->VerifyRegisterType(this, inst->VRegA_23x(), *modified_reg_type);
} else if (!array_type.IsArrayTypes()) {
Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "not array type " << array_type << " with aput";
+ } else if (array_type.IsUnresolvedTypes()) {
+ // Unresolved array types must be reference array types.
+ if (is_primitive) {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "put insn has type '" << insn_type
+ << "' but unresolved type '" << array_type << "'";
+ } else {
+ Fail(VERIFY_ERROR_NO_CLASS) << "cannot verify aput for " << array_type
+ << " because of missing class";
+ }
} else {
const RegType& component_type = reg_types_.GetComponentType(array_type, GetClassLoader());
const uint32_t vregA = inst->VRegA_23x();
diff --git a/runtime/verifier/reg_type.cc b/runtime/verifier/reg_type.cc
index 0894f5d..bd0308b 100644
--- a/runtime/verifier/reg_type.cc
+++ b/runtime/verifier/reg_type.cc
@@ -517,9 +517,21 @@
}
}
+bool RegType::IsJavaLangObject() const SHARED_REQUIRES(Locks::mutator_lock_) {
+ return IsReference() && GetClass()->IsObjectClass();
+}
+
bool RegType::IsObjectArrayTypes() const SHARED_REQUIRES(Locks::mutator_lock_) {
- if (IsUnresolvedTypes() && !IsUnresolvedMergedReference() && !IsUnresolvedSuperClass()) {
- // Primitive arrays will always resolve
+ if (IsUnresolvedTypes()) {
+ DCHECK(!IsUnresolvedMergedReference());
+
+ if (IsUnresolvedSuperClass()) {
+ // Cannot be an array, as the superclass of arrays is java.lang.Object (which cannot be
+ // unresolved).
+ return false;
+ }
+
+ // Primitive arrays will always resolve.
DCHECK(descriptor_[1] == 'L' || descriptor_[1] == '[');
return descriptor_[0] == '[';
} else if (HasClass()) {
@@ -530,12 +542,15 @@
}
}
-bool RegType::IsJavaLangObject() const SHARED_REQUIRES(Locks::mutator_lock_) {
- return IsReference() && GetClass()->IsObjectClass();
-}
-
bool RegType::IsArrayTypes() const SHARED_REQUIRES(Locks::mutator_lock_) {
- if (IsUnresolvedTypes() && !IsUnresolvedMergedReference() && !IsUnresolvedSuperClass()) {
+ if (IsUnresolvedTypes()) {
+ DCHECK(!IsUnresolvedMergedReference());
+
+ if (IsUnresolvedSuperClass()) {
+ // Cannot be an array, as the superclass of arrays is java.lang.Object (which cannot be
+ // unresolved).
+ return false;
+ }
return descriptor_[0] == '[';
} else if (HasClass()) {
return GetClass()->IsArrayClass();
@@ -793,11 +808,50 @@
}
}
void UnresolvedMergedType::CheckInvariants() const {
+ CHECK(reg_type_cache_ != nullptr);
+
// Unresolved merged types: merged types should be defined.
CHECK(descriptor_.empty()) << *this;
CHECK(klass_.IsNull()) << *this;
+
+ CHECK(!resolved_part_.IsConflict());
CHECK(resolved_part_.IsReferenceTypes());
CHECK(!resolved_part_.IsUnresolvedTypes());
+
+ CHECK(resolved_part_.IsZero() ||
+ !(resolved_part_.IsArrayTypes() && !resolved_part_.IsObjectArrayTypes()));
+
+ CHECK_GT(unresolved_types_.NumSetBits(), 0U);
+ bool unresolved_is_array =
+ reg_type_cache_->GetFromId(unresolved_types_.GetHighestBitSet()).IsArrayTypes();
+ for (uint32_t idx : unresolved_types_.Indexes()) {
+ const RegType& t = reg_type_cache_->GetFromId(idx);
+ CHECK_EQ(unresolved_is_array, t.IsArrayTypes());
+ }
+
+ if (!resolved_part_.IsZero()) {
+ CHECK_EQ(resolved_part_.IsArrayTypes(), unresolved_is_array);
+ }
+}
+
+bool UnresolvedMergedType::IsArrayTypes() const {
+ // For a merge to be an array, both the resolved and the unresolved part need to be object
+ // arrays.
+ // (Note: we encode a missing resolved part [which doesn't need to be an array] as zero.)
+
+ if (!resolved_part_.IsZero() && !resolved_part_.IsArrayTypes()) {
+ return false;
+ }
+
+ // It is enough to check just one of the merged types. Otherwise the merge should have been
+ // collapsed (checked in CheckInvariants on construction).
+ uint32_t idx = unresolved_types_.GetHighestBitSet();
+ const RegType& unresolved = reg_type_cache_->GetFromId(idx);
+ return unresolved.IsArrayTypes();
+}
+bool UnresolvedMergedType::IsObjectArrayTypes() const {
+ // Same as IsArrayTypes, as primitive arrays are always resolved.
+ return IsArrayTypes();
}
void UnresolvedReferenceType::CheckInvariants() const {
@@ -824,6 +878,14 @@
return false;
}
+ if (IsUnresolvedTypes() || src.IsUnresolvedTypes()) {
+ // An unresolved array type means that it's an array of some reference type. Reference arrays
+ // can never be assigned to primitive-type arrays, and vice versa. So it is a soft error if
+ // both arrays are reference arrays, otherwise a hard error.
+ *soft_error = IsObjectArrayTypes() && src.IsObjectArrayTypes();
+ return false;
+ }
+
const RegType& cmp1 = reg_types.GetComponentType(*this, class_loader.Get());
const RegType& cmp2 = reg_types.GetComponentType(src, class_loader.Get());
diff --git a/runtime/verifier/reg_type.h b/runtime/verifier/reg_type.h
index 7c7981e..4837490 100644
--- a/runtime/verifier/reg_type.h
+++ b/runtime/verifier/reg_type.h
@@ -172,8 +172,8 @@
}
virtual bool HasClassVirtual() const { return false; }
bool IsJavaLangObject() const SHARED_REQUIRES(Locks::mutator_lock_);
- bool IsArrayTypes() const SHARED_REQUIRES(Locks::mutator_lock_);
- bool IsObjectArrayTypes() const SHARED_REQUIRES(Locks::mutator_lock_);
+ virtual bool IsArrayTypes() const SHARED_REQUIRES(Locks::mutator_lock_);
+ virtual bool IsObjectArrayTypes() const SHARED_REQUIRES(Locks::mutator_lock_);
Primitive::Type GetPrimitiveType() const;
bool IsJavaLangObjectArray() const
SHARED_REQUIRES(Locks::mutator_lock_);
@@ -905,6 +905,9 @@
bool IsUnresolvedTypes() const OVERRIDE { return true; }
+ bool IsArrayTypes() const OVERRIDE SHARED_REQUIRES(Locks::mutator_lock_);
+ bool IsObjectArrayTypes() const OVERRIDE SHARED_REQUIRES(Locks::mutator_lock_);
+
std::string Dump() const OVERRIDE SHARED_REQUIRES(Locks::mutator_lock_);
private:
diff --git a/runtime/verifier/reg_type_cache.cc b/runtime/verifier/reg_type_cache.cc
index b171b75..208ef51 100644
--- a/runtime/verifier/reg_type_cache.cc
+++ b/runtime/verifier/reg_type_cache.cc
@@ -347,29 +347,39 @@
kDefaultArenaBitVectorBytes * kBitsPerByte, // Allocate at least 8 bytes.
true); // Is expandable.
const RegType* left_resolved;
+ bool left_unresolved_is_array;
if (left.IsUnresolvedMergedReference()) {
- const UnresolvedMergedType* left_merge = down_cast<const UnresolvedMergedType*>(&left);
- types.Copy(&left_merge->GetUnresolvedTypes());
- left_resolved = &left_merge->GetResolvedPart();
+ const UnresolvedMergedType& left_merge = *down_cast<const UnresolvedMergedType*>(&left);
+
+ types.Copy(&left_merge.GetUnresolvedTypes());
+ left_resolved = &left_merge.GetResolvedPart();
+ left_unresolved_is_array = left.IsArrayTypes();
} else if (left.IsUnresolvedTypes()) {
types.ClearAllBits();
types.SetBit(left.GetId());
left_resolved = &Zero();
+ left_unresolved_is_array = left.IsArrayTypes();
} else {
types.ClearAllBits();
left_resolved = &left;
+ left_unresolved_is_array = false;
}
const RegType* right_resolved;
+ bool right_unresolved_is_array;
if (right.IsUnresolvedMergedReference()) {
- const UnresolvedMergedType* right_merge = down_cast<const UnresolvedMergedType*>(&right);
- types.Union(&right_merge->GetUnresolvedTypes());
- right_resolved = &right_merge->GetResolvedPart();
+ const UnresolvedMergedType& right_merge = *down_cast<const UnresolvedMergedType*>(&right);
+
+ types.Union(&right_merge.GetUnresolvedTypes());
+ right_resolved = &right_merge.GetResolvedPart();
+ right_unresolved_is_array = right.IsArrayTypes();
} else if (right.IsUnresolvedTypes()) {
types.SetBit(right.GetId());
right_resolved = &Zero();
+ right_unresolved_is_array = right.IsArrayTypes();
} else {
right_resolved = &right;
+ right_unresolved_is_array = false;
}
// Merge the resolved parts. Left and right might be equal, so use SafeMerge.
@@ -379,6 +389,23 @@
return Conflict();
}
+ bool resolved_merged_is_array = resolved_parts_merged.IsArrayTypes();
+ if (left_unresolved_is_array || right_unresolved_is_array || resolved_merged_is_array) {
+ // Arrays involved, see if we need to merge to Object.
+
+ // Is the resolved part a primitive array?
+ if (resolved_merged_is_array && !resolved_parts_merged.IsObjectArrayTypes()) {
+ return JavaLangObject(false /* precise */);
+ }
+
+ // Is any part not an array (but exists)?
+ if ((!left_unresolved_is_array && left_resolved != &left) ||
+ (!right_unresolved_is_array && right_resolved != &right) ||
+ !resolved_merged_is_array) {
+ return JavaLangObject(false /* precise */);
+ }
+ }
+
// Check if entry already exists.
for (size_t i = primitive_count_; i < entries_.size(); i++) {
const RegType* cur_entry = entries_[i];
@@ -584,6 +611,7 @@
if (!array.IsArrayTypes()) {
return Conflict();
} else if (array.IsUnresolvedTypes()) {
+ DCHECK(!array.IsUnresolvedTypes()); // Caller must make sure not to ask for this.
const std::string descriptor(array.GetDescriptor().as_string());
return FromDescriptor(loader, descriptor.c_str() + 1, false);
} else {
diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt
index 8808a50..5a3857d 100644
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -60,4 +60,9 @@
b/26594149 (8)
b/27148248
b/26965384
+b/27799205 (1)
+b/27799205 (2)
+b/27799205 (3)
+b/27799205 (4)
+b/27799205 (5)
Done!
diff --git a/test/800-smali/smali/b_27799205_1.smali b/test/800-smali/smali/b_27799205_1.smali
new file mode 100644
index 0000000..92bfc80
--- /dev/null
+++ b/test/800-smali/smali/b_27799205_1.smali
@@ -0,0 +1,37 @@
+.class public LB27799205_1;
+.super Ljava/lang/Object;
+
+# A class with an unresolved array type should not fail hard (unless it's a primitive-type access).
+
+.method public static run()V
+.registers 1
+ return-void
+.end method
+
+.method public static test([Ljava/lang/Object;[Ldo/not/resolve/K;Z)V
+.registers 6
+ # Make v0, v1 and v2 null. We'll use v0 as a merge of the inputs, v1 as null, and v2 as 0.
+ const v0, 0
+ const v1, 0
+ const v2, 0
+
+ # Conditional jump so we have a merge point.
+ if-eqz v5, :LabelSelectUnresolved
+
+:LabelSelectResolved
+ move-object v0, v3
+ goto :LabelMerged
+
+:LabelSelectUnresolved
+ move-object v0, v4
+ goto :LabelMerged
+
+:LabelMerged
+ # At this point, v0 will be the unresolved merge.
+
+ # Test aput: v0[v2] = v1.
+ aput-object v1, v0, v2
+
+ return-void
+
+.end method
diff --git a/test/800-smali/smali/b_27799205_2.smali b/test/800-smali/smali/b_27799205_2.smali
new file mode 100644
index 0000000..e730b1e
--- /dev/null
+++ b/test/800-smali/smali/b_27799205_2.smali
@@ -0,0 +1,37 @@
+.class public LB27799205_2;
+.super Ljava/lang/Object;
+
+# A class with an unresolved array type should not fail hard (unless it's a primitive-type access).
+
+.method public static run()V
+.registers 1
+ return-void
+.end method
+
+.method public static test([Ljava/lang/Object;[Ldo/not/resolve/K;Z)V
+.registers 6
+ # Make v0, v1 and v2 null. We'll use v0 as a merge of the inputs, v1 as null, and v2 as 0.
+ const v0, 0
+ const v1, 0
+ const v2, 0
+
+ # Conditional jump so we have a merge point.
+ if-eqz v5, :LabelSelectUnresolved
+
+:LabelSelectResolved
+ move-object v0, v3
+ goto :LabelMerged
+
+:LabelSelectUnresolved
+ move-object v0, v4
+ goto :LabelMerged
+
+:LabelMerged
+ # At this point, v0 will be the unresolved merge.
+
+ # Test aput: v0[v2] = v1.
+ aput v1, v0, v2
+
+ return-void
+
+.end method
diff --git a/test/800-smali/smali/b_27799205_3.smali b/test/800-smali/smali/b_27799205_3.smali
new file mode 100644
index 0000000..1cb025e
--- /dev/null
+++ b/test/800-smali/smali/b_27799205_3.smali
@@ -0,0 +1,39 @@
+.class public LB27799205_3;
+.super Ljava/lang/Object;
+
+# A class with an unresolved array type should not fail hard (unless it's a primitive-type access).
+# Make sure that merging is pro-active.
+
+.method public static run()V
+.registers 1
+ return-void
+.end method
+
+# Use some non-object non-array input (non-Object because the merge should be Object).
+.method public static test(Ljava/lang/Integer;[Ldo/not/resolve/K;Z)V
+.registers 6
+ # Make v0, v1 and v2 null. We'll use v0 as a merge of the inputs, v1 as null, and v2 as 0.
+ const v0, 0
+ const v1, 0
+ const v2, 0
+
+ # Conditional jump so we have a merge point.
+ if-eqz v5, :LabelSelectUnresolved
+
+:LabelSelectResolved
+ move-object v0, v3
+ goto :LabelMerged
+
+:LabelSelectUnresolved
+ move-object v0, v4
+ goto :LabelMerged
+
+:LabelMerged
+ # At this point, v0 should be Object.
+
+ # Test aput-object: v0[v2] = v1. Should fail for v0 not being an array.
+ aput-object v1, v0, v2
+
+ return-void
+
+.end method
diff --git a/test/800-smali/smali/b_27799205_4.smali b/test/800-smali/smali/b_27799205_4.smali
new file mode 100644
index 0000000..e42951a
--- /dev/null
+++ b/test/800-smali/smali/b_27799205_4.smali
@@ -0,0 +1,39 @@
+.class public LB27799205_4;
+.super Ljava/lang/Object;
+
+# A class with an unresolved array type should not fail hard (unless it's a primitive-type access).
+# Make sure that merging is pro-active.
+
+.method public static run()V
+.registers 1
+ return-void
+.end method
+
+# Use some primitive-type array input.
+.method public static test([I[Ldo/not/resolve/K;Z)V
+.registers 6
+ # Make v0, v1 and v2 null. We'll use v0 as a merge of the inputs, v1 as null, and v2 as 0.
+ const v0, 0
+ const v1, 0
+ const v2, 0
+
+ # Conditional jump so we have a merge point.
+ if-eqz v5, :LabelSelectUnresolved
+
+:LabelSelectResolved
+ move-object v0, v3
+ goto :LabelMerged
+
+:LabelSelectUnresolved
+ move-object v0, v4
+ goto :LabelMerged
+
+:LabelMerged
+ # At this point, v0 should be Object.
+
+ # Test aput-object: v0[v2] = v1. Should fail for v0 not being an array.
+ aput-object v1, v0, v2
+
+ return-void
+
+.end method
diff --git a/test/800-smali/smali/b_27799205_5.smali b/test/800-smali/smali/b_27799205_5.smali
new file mode 100644
index 0000000..6c7b183
--- /dev/null
+++ b/test/800-smali/smali/b_27799205_5.smali
@@ -0,0 +1,39 @@
+.class public LB27799205_5;
+.super Ljava/lang/Object;
+
+# A class with an unresolved array type should not fail hard (unless it's a primitive-type access).
+# Make sure that merging is pro-active.
+
+.method public static run()V
+.registers 1
+ return-void
+.end method
+
+# Use some non-resolvable non-array type.
+.method public static test(Ldo/not/resolve/L;[Ldo/not/resolve/K;Z)V
+.registers 6
+ # Make v0, v1 and v2 null. We'll use v0 as a merge of the inputs, v1 as null, and v2 as 0.
+ const v0, 0
+ const v1, 0
+ const v2, 0
+
+ # Conditional jump so we have a merge point.
+ if-eqz v5, :LabelSelectUnresolved
+
+:LabelSelectResolved
+ move-object v0, v3
+ goto :LabelMerged
+
+:LabelSelectUnresolved
+ move-object v0, v4
+ goto :LabelMerged
+
+:LabelMerged
+ # At this point, v0 should be Object.
+
+ # Test aput-object: v0[v2] = v1. Should fail for v0 not being an array.
+ aput-object v1, v0, v2
+
+ return-void
+
+.end method
diff --git a/test/800-smali/smali/b_27799205_helper.smali b/test/800-smali/smali/b_27799205_helper.smali
new file mode 100644
index 0000000..145c93d
--- /dev/null
+++ b/test/800-smali/smali/b_27799205_helper.smali
@@ -0,0 +1,40 @@
+.class public LB27799205Helper;
+.super Ljava/lang/Object;
+
+# Helper for B27799205. Reflection tries to resolve all types. That's bad for intentionally
+# unresolved types. It makes it harder to distinguish what kind of error we got.
+
+.method public static run1()V
+.registers 1
+ invoke-static {}, LB27799205_1;->run()V
+
+ return-void
+.end method
+
+.method public static run2()V
+.registers 1
+ invoke-static {}, LB27799205_2;->run()V
+
+ return-void
+.end method
+
+.method public static run3()V
+.registers 1
+ invoke-static {}, LB27799205_3;->run()V
+
+ return-void
+.end method
+
+.method public static run4()V
+.registers 1
+ invoke-static {}, LB27799205_4;->run()V
+
+ return-void
+.end method
+
+.method public static run5()V
+.registers 1
+ invoke-static {}, LB27799205_5;->run()V
+
+ return-void
+.end method
diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java
index 4e6de46..20c3065 100644
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -164,6 +164,15 @@
null));
testCases.add(new TestCase("b/26965384", "B26965384", "run", null, new VerifyError(),
null));
+ testCases.add(new TestCase("b/27799205 (1)", "B27799205Helper", "run1", null, null, null));
+ testCases.add(new TestCase("b/27799205 (2)", "B27799205Helper", "run2", null,
+ new VerifyError(), null));
+ testCases.add(new TestCase("b/27799205 (3)", "B27799205Helper", "run3", null,
+ new VerifyError(), null));
+ testCases.add(new TestCase("b/27799205 (4)", "B27799205Helper", "run4", null,
+ new VerifyError(), null));
+ testCases.add(new TestCase("b/27799205 (5)", "B27799205Helper", "run5", null,
+ new VerifyError(), null));
}
public void runTests() {