diff options
| -rw-r--r-- | runtime/verifier/method_verifier.cc | 23 | ||||
| -rw-r--r-- | runtime/verifier/reg_type.cc | 76 | ||||
| -rw-r--r-- | runtime/verifier/reg_type.h | 7 | ||||
| -rw-r--r-- | runtime/verifier/reg_type_cache.cc | 40 | ||||
| -rw-r--r-- | test/800-smali/expected.txt | 5 | ||||
| -rw-r--r-- | test/800-smali/smali/b_27799205_1.smali | 37 | ||||
| -rw-r--r-- | test/800-smali/smali/b_27799205_2.smali | 37 | ||||
| -rw-r--r-- | test/800-smali/smali/b_27799205_3.smali | 39 | ||||
| -rw-r--r-- | test/800-smali/smali/b_27799205_4.smali | 39 | ||||
| -rw-r--r-- | test/800-smali/smali/b_27799205_5.smali | 39 | ||||
| -rw-r--r-- | test/800-smali/smali/b_27799205_helper.smali | 40 | ||||
| -rw-r--r-- | test/800-smali/src/Main.java | 9 |
12 files changed, 376 insertions, 15 deletions
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index 537d9c9311..9bcbdbd261 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -2425,6 +2425,10 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) { 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 @@ void MethodVerifier::VerifyNewArray(const Instruction* inst, bool is_filled, boo 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 @@ void MethodVerifier::VerifyAGet(const Instruction* inst, } } 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 @@ void MethodVerifier::VerifyAPut(const Instruction* inst, 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 0894f5d1ee..bd0308b0f1 100644 --- a/runtime/verifier/reg_type.cc +++ b/runtime/verifier/reg_type.cc @@ -517,9 +517,21 @@ const RegType& RegType::GetSuperClass(RegTypeCache* cache) const { } } +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::IsObjectArrayTypes() const SHARED_REQUIRES(Locks::mutator_lock_) { } } -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 @@ UnresolvedMergedType::UnresolvedMergedType(const RegType& resolved, } } 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 @@ bool RegType::CanAssignArray(const RegType& src, RegTypeCache& reg_types, 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 7c7981e5e7..4837490e60 100644 --- a/runtime/verifier/reg_type.h +++ b/runtime/verifier/reg_type.h @@ -172,8 +172,8 @@ class RegType { } 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 @@ class UnresolvedMergedType FINAL : public UnresolvedType { 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 b171b75e97..208ef51d28 100644 --- a/runtime/verifier/reg_type_cache.cc +++ b/runtime/verifier/reg_type_cache.cc @@ -347,29 +347,39 @@ const RegType& RegTypeCache::FromUnresolvedMerge(const RegType& left, const RegT 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 @@ const RegType& RegTypeCache::FromUnresolvedMerge(const RegType& left, const RegT 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 @@ const RegType& RegTypeCache::GetComponentType(const RegType& array, mirror::Clas 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 8808a50f75..5a3857d07c 100644 --- a/test/800-smali/expected.txt +++ b/test/800-smali/expected.txt @@ -60,4 +60,9 @@ b/26594149 (7) 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 0000000000..92bfc80494 --- /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 0000000000..e730b1e9de --- /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 0000000000..1cb025ed79 --- /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 0000000000..e42951ac91 --- /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 0000000000..6c7b18331a --- /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 0000000000..145c93db38 --- /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 4e6de46caa..20c306589f 100644 --- a/test/800-smali/src/Main.java +++ b/test/800-smali/src/Main.java @@ -164,6 +164,15 @@ public class Main { 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() { |