diff options
author | Jeff Hao <jeffhao@google.com> | 2013-07-31 13:47:31 -0700 |
---|---|---|
committer | Jeff Hao <jeffhao@google.com> | 2013-08-01 13:47:26 -0700 |
commit | b24b4a7e0c4f9bbea49f9dd95b2600080c8293d9 (patch) | |
tree | a59863dc34ae5262a418d855fe3f55eeb452b9bd /runtime | |
parent | 8d4fb0eb94ea3dd5db9461230e2c11926e4ebdb4 (diff) | |
download | art-b24b4a7e0c4f9bbea49f9dd95b2600080c8293d9.zip art-b24b4a7e0c4f9bbea49f9dd95b2600080c8293d9.tar.gz art-b24b4a7e0c4f9bbea49f9dd95b2600080c8293d9.tar.bz2 |
Make verifier allow integral types to be put in integral type arrays.
This fixes a problem where the verifier was rejecting when an integer
is put into a byte array. This also more closely matches the RI.
Also fixes various issues with debugging checks caught by cts.
Bug 10097083
Change-Id: Ie816fcdd85d6dc898feffa1e3fea8cfc2c6946ff
Diffstat (limited to 'runtime')
-rw-r--r-- | runtime/dex_file.cc | 1 | ||||
-rw-r--r-- | runtime/stack.cc | 6 | ||||
-rw-r--r-- | runtime/verifier/method_verifier.cc | 75 | ||||
-rw-r--r-- | runtime/verifier/register_line.cc | 4 | ||||
-rw-r--r-- | runtime/verifier/register_line.h | 2 |
5 files changed, 64 insertions, 24 deletions
diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc index cbdc430..aaff0fc 100644 --- a/runtime/dex_file.cc +++ b/runtime/dex_file.cc @@ -313,7 +313,6 @@ void DexFile::InitMembers() { method_ids_ = reinterpret_cast<const MethodId*>(b + h->method_ids_off_); proto_ids_ = reinterpret_cast<const ProtoId*>(b + h->proto_ids_off_); class_defs_ = reinterpret_cast<const ClassDef*>(b + h->class_defs_off_); - DCHECK_EQ(size_, header_->file_size_) << GetLocation(); } bool DexFile::CheckMagicAndVersion() const { diff --git a/runtime/stack.cc b/runtime/stack.cc index aeb15f0..a74bcdb 100644 --- a/runtime/stack.cc +++ b/runtime/stack.cc @@ -266,7 +266,11 @@ void StackVisitor::SanityCheckFrame() const { // Frame sanity. size_t frame_size = method->GetFrameSizeInBytes(); CHECK_NE(frame_size, 0u); - CHECK_LT(frame_size, 1024u); + // A rough guess at an upper size we expect to see for a frame. The 256 is + // a dex register limit. The 16 incorporates callee save spills and + // outgoing argument set up. + const size_t kMaxExpectedFrameSize = 256 * sizeof(word) + 16; + CHECK_LE(frame_size, kMaxExpectedFrameSize); size_t return_pc_offset = method->GetReturnPcOffsetInBytes(); CHECK_LT(return_pc_offset, frame_size); } diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index 6cc74de..9fb3b07 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -2819,8 +2819,10 @@ const RegType& MethodVerifier::ResolveClassAndCheckAccess(uint32_t class_idx) { dex_cache_->SetResolvedType(class_idx, result.GetClass()); } // Check if access is allowed. Unresolved types use xxxWithAccessCheck to - // check at runtime if access is allowed and so pass here. - if (!result.IsUnresolvedTypes() && !referrer.IsUnresolvedTypes() && !referrer.CanAccess(result)) { + // check at runtime if access is allowed and so pass here. If result is + // primitive, skip the access check. + if (result.IsNonZeroReferenceTypes() && !result.IsUnresolvedTypes() && + !referrer.IsUnresolvedTypes() && !referrer.CanAccess(result)) { Fail(VERIFY_ERROR_ACCESS_CLASS) << "illegal class access: '" << referrer << "' -> '" << result << "'"; } @@ -3310,25 +3312,56 @@ void MethodVerifier::VerifyAPut(const Instruction* inst, } else if (!array_type.IsArrayTypes()) { Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "not array type " << array_type << " with aput"; } else { - /* verify the class */ const RegType& component_type = reg_types_.GetComponentType(array_type, class_loader_); - if (!component_type.IsReferenceTypes() && !is_primitive) { - Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "primitive array type " << array_type - << " source for aput-object"; - } else if (component_type.IsNonZeroReferenceTypes() && is_primitive) { - Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "reference array type " << array_type - << " source for category 1 aput"; - } else if (is_primitive && !insn_type.Equals(component_type) && - !((insn_type.IsInteger() && component_type.IsFloat()) || - (insn_type.IsLong() && component_type.IsDouble()))) { - Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "array type " << array_type - << " incompatible with aput of type " << insn_type; + if (is_primitive) { + // Primitive array assignability rules are weaker than regular assignability rules. + bool instruction_compatible; + bool value_compatible; + const RegType& value_type = work_line_->GetRegisterType(inst->VRegA_23x()); + if (component_type.IsNonZeroReferenceTypes()) { + Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "reference array type " << array_type + << " source for category 1 aput"; + return; + } else if (component_type.IsIntegralTypes()) { + instruction_compatible = insn_type.IsIntegralTypes(); + value_compatible = value_type.IsIntegralTypes(); + } else if (component_type.IsFloat()) { + instruction_compatible = insn_type.IsInteger(); // no aput-float, so expect aput-int + value_compatible = value_type.IsFloatTypes(); + } else if (component_type.IsLong()) { + instruction_compatible = insn_type.IsLong(); + value_compatible = value_type.IsLongTypes(); + } else if (component_type.IsDouble()) { + instruction_compatible = insn_type.IsLong(); // no aput-double, so expect aput-long + value_compatible = value_type.IsDoubleTypes(); + } else { + instruction_compatible = false; // reference array with primitive store + value_compatible = false; // unused + } + if (!instruction_compatible) { + // This is a global failure rather than a class change failure as the instructions and + // the descriptors for the type should have been consistent within the same file at + // compile time + Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "expected array to be of type '" << insn_type + << "' but found type '" << component_type + << "' in aput"; + return; + } + if (!value_compatible) { + Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "unexpected value in v" << inst->VRegA_23x() + << " of type " << value_type << " but expected " << component_type << " for aput"; + return; + } } else { - // The instruction agrees with the type of array, confirm the value to be stored does too - // Note: we use the instruction type (rather than the component type) for aput-object as - // incompatible classes will be caught at runtime as an array store exception - work_line_->VerifyRegisterType(inst->VRegA_23x(), - is_primitive ? component_type : insn_type); + if (!component_type.IsReferenceTypes()) { + Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "primitive array type " << array_type + << " source for aput-object"; + } else { + // The instruction agrees with the type of array, confirm the value to be stored does too + // Note: we use the instruction type (rather than the component type) for aput-object as + // incompatible classes will be caught at runtime as an array store exception + work_line_->VerifyRegisterType(inst->VRegA_23x(), insn_type); + } } } } @@ -3756,6 +3789,10 @@ bool MethodVerifier::UpdateRegisters(uint32_t next_insn, const RegisterLine* mer if (!insn_flags_[next_insn].IsReturn()) { target_line->CopyFromLine(merge_line); } else { + // Verify that the monitor stack is empty on return. + if (!merge_line->VerifyMonitorStackEmpty()) { + return false; + } // For returns we only care about the operand to the return, all other registers are dead. // Initialize them as conflicts so they don't add to GC and deoptimization information. const Instruction* ret_inst = Instruction::At(code_item_->insns_ + next_insn); diff --git a/runtime/verifier/register_line.cc b/runtime/verifier/register_line.cc index 7965c06..24a626b 100644 --- a/runtime/verifier/register_line.cc +++ b/runtime/verifier/register_line.cc @@ -38,7 +38,7 @@ bool RegisterLine::CheckConstructorReturn() const { bool RegisterLine::SetRegisterType(uint32_t vdst, const RegType& new_type) { DCHECK_LT(vdst, num_regs_); if (new_type.IsLowHalf() || new_type.IsHighHalf()) { - verifier_->Fail(VERIFY_ERROR_BAD_CLASS_SOFT) << "Expected category1 register type not '" + verifier_->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "Expected category1 register type not '" << new_type << "'"; return false; } else if (new_type.IsConflict()) { // should only be set during a merge @@ -448,7 +448,7 @@ void RegisterLine::PopMonitor(uint32_t reg_idx) { } } -bool RegisterLine::VerifyMonitorStackEmpty() { +bool RegisterLine::VerifyMonitorStackEmpty() const { if (MonitorStackDepth() != 0) { verifier_->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "expected empty monitor stack"; return false; diff --git a/runtime/verifier/register_line.h b/runtime/verifier/register_line.h index f380877..f19dcca 100644 --- a/runtime/verifier/register_line.h +++ b/runtime/verifier/register_line.h @@ -268,7 +268,7 @@ class RegisterLine { // We expect no monitors to be held at certain points, such a method returns. Verify the stack // is empty, failing and returning false if not. - bool VerifyMonitorStackEmpty(); + bool VerifyMonitorStackEmpty() const; bool MergeRegisters(const RegisterLine* incoming_line) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); |