Merge "ART: Allow array-ness for unresolved merge types"
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() {
