Merge "ART: Add option to retain lock levels in verifier"
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index a506071..efefa8b 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -1387,13 +1387,15 @@
if (declaring_class.IsJavaLangObject()) {
// "this" is implicitly initialized.
reg_line->SetThisInitialized();
- reg_line->SetRegisterType(this, arg_start + cur_arg, declaring_class);
+ reg_line->SetRegisterType<LockOp::kClear>(this, arg_start + cur_arg, declaring_class);
} else {
- reg_line->SetRegisterType(this, arg_start + cur_arg,
- reg_types_.UninitializedThisArgument(declaring_class));
+ reg_line->SetRegisterType<LockOp::kClear>(
+ this,
+ arg_start + cur_arg,
+ reg_types_.UninitializedThisArgument(declaring_class));
}
} else {
- reg_line->SetRegisterType(this, arg_start + cur_arg, declaring_class);
+ reg_line->SetRegisterType<LockOp::kClear>(this, arg_start + cur_arg, declaring_class);
}
cur_arg++;
}
@@ -1425,26 +1427,26 @@
DCHECK(HasFailures());
return false;
}
- reg_line->SetRegisterType(this, arg_start + cur_arg, reg_type);
+ reg_line->SetRegisterType<LockOp::kClear>(this, arg_start + cur_arg, reg_type);
}
break;
case 'Z':
- reg_line->SetRegisterType(this, arg_start + cur_arg, reg_types_.Boolean());
+ reg_line->SetRegisterType<LockOp::kClear>(this, arg_start + cur_arg, reg_types_.Boolean());
break;
case 'C':
- reg_line->SetRegisterType(this, arg_start + cur_arg, reg_types_.Char());
+ reg_line->SetRegisterType<LockOp::kClear>(this, arg_start + cur_arg, reg_types_.Char());
break;
case 'B':
- reg_line->SetRegisterType(this, arg_start + cur_arg, reg_types_.Byte());
+ reg_line->SetRegisterType<LockOp::kClear>(this, arg_start + cur_arg, reg_types_.Byte());
break;
case 'I':
- reg_line->SetRegisterType(this, arg_start + cur_arg, reg_types_.Integer());
+ reg_line->SetRegisterType<LockOp::kClear>(this, arg_start + cur_arg, reg_types_.Integer());
break;
case 'S':
- reg_line->SetRegisterType(this, arg_start + cur_arg, reg_types_.Short());
+ reg_line->SetRegisterType<LockOp::kClear>(this, arg_start + cur_arg, reg_types_.Short());
break;
case 'F':
- reg_line->SetRegisterType(this, arg_start + cur_arg, reg_types_.Float());
+ reg_line->SetRegisterType<LockOp::kClear>(this, arg_start + cur_arg, reg_types_.Float());
break;
case 'J':
case 'D': {
@@ -1787,7 +1789,7 @@
* that as part of extracting the exception type from the catch block list.
*/
const RegType& res_type = GetCaughtExceptionType();
- work_line_->SetRegisterType(this, inst->VRegA_11x(), res_type);
+ work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_11x(), res_type);
break;
}
case Instruction::RETURN_VOID:
@@ -1887,26 +1889,26 @@
/* could be boolean, int, float, or a null reference */
case Instruction::CONST_4: {
int32_t val = static_cast<int32_t>(inst->VRegB_11n() << 28) >> 28;
- work_line_->SetRegisterType(this, inst->VRegA_11n(),
- DetermineCat1Constant(val, need_precise_constants_));
+ work_line_->SetRegisterType<LockOp::kClear>(
+ this, inst->VRegA_11n(), DetermineCat1Constant(val, need_precise_constants_));
break;
}
case Instruction::CONST_16: {
int16_t val = static_cast<int16_t>(inst->VRegB_21s());
- work_line_->SetRegisterType(this, inst->VRegA_21s(),
- DetermineCat1Constant(val, need_precise_constants_));
+ work_line_->SetRegisterType<LockOp::kClear>(
+ this, inst->VRegA_21s(), DetermineCat1Constant(val, need_precise_constants_));
break;
}
case Instruction::CONST: {
int32_t val = inst->VRegB_31i();
- work_line_->SetRegisterType(this, inst->VRegA_31i(),
- DetermineCat1Constant(val, need_precise_constants_));
+ work_line_->SetRegisterType<LockOp::kClear>(
+ this, inst->VRegA_31i(), DetermineCat1Constant(val, need_precise_constants_));
break;
}
case Instruction::CONST_HIGH16: {
int32_t val = static_cast<int32_t>(inst->VRegB_21h() << 16);
- work_line_->SetRegisterType(this, inst->VRegA_21h(),
- DetermineCat1Constant(val, need_precise_constants_));
+ work_line_->SetRegisterType<LockOp::kClear>(
+ this, inst->VRegA_21h(), DetermineCat1Constant(val, need_precise_constants_));
break;
}
/* could be long or double; resolved upon use */
@@ -1939,19 +1941,21 @@
break;
}
case Instruction::CONST_STRING:
- work_line_->SetRegisterType(this, inst->VRegA_21c(), reg_types_.JavaLangString());
+ work_line_->SetRegisterType<LockOp::kClear>(
+ this, inst->VRegA_21c(), reg_types_.JavaLangString());
break;
case Instruction::CONST_STRING_JUMBO:
- work_line_->SetRegisterType(this, inst->VRegA_31c(), reg_types_.JavaLangString());
+ work_line_->SetRegisterType<LockOp::kClear>(
+ this, inst->VRegA_31c(), reg_types_.JavaLangString());
break;
case Instruction::CONST_CLASS: {
// Get type from instruction if unresolved then we need an access check
// TODO: check Compiler::CanAccessTypeWithoutChecks returns false when res_type is unresolved
const RegType& res_type = ResolveClassAndCheckAccess(inst->VRegB_21c());
// Register holds class, ie its type is class, on error it will hold Conflict.
- work_line_->SetRegisterType(this, inst->VRegA_21c(),
- res_type.IsConflict() ? res_type
- : reg_types_.JavaLangClass());
+ work_line_->SetRegisterType<LockOp::kClear>(
+ this, inst->VRegA_21c(), res_type.IsConflict() ? res_type
+ : reg_types_.JavaLangClass());
break;
}
case Instruction::MONITOR_ENTER:
@@ -2032,7 +2036,9 @@
DCHECK_NE(failures_.size(), 0U);
if (!is_checkcast) {
- work_line_->SetRegisterType(this, inst->VRegA_22c(), reg_types_.Boolean());
+ work_line_->SetRegisterType<LockOp::kClear>(this,
+ inst->VRegA_22c(),
+ reg_types_.Boolean());
}
break; // bad class
}
@@ -2053,9 +2059,11 @@
}
} else {
if (is_checkcast) {
- work_line_->SetRegisterType(this, inst->VRegA_21c(), res_type);
+ work_line_->SetRegisterType<LockOp::kKeep>(this, inst->VRegA_21c(), res_type);
} else {
- work_line_->SetRegisterType(this, inst->VRegA_22c(), reg_types_.Boolean());
+ work_line_->SetRegisterType<LockOp::kClear>(this,
+ inst->VRegA_22c(),
+ reg_types_.Boolean());
}
}
break;
@@ -2066,7 +2074,9 @@
if (!res_type.IsArrayTypes() && !res_type.IsZero()) { // ie not an array or null
Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "array-length on non-array " << res_type;
} else {
- work_line_->SetRegisterType(this, inst->VRegA_12x(), reg_types_.Integer());
+ work_line_->SetRegisterType<LockOp::kClear>(this,
+ inst->VRegA_12x(),
+ reg_types_.Integer());
}
} else {
Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "array-length on non-array " << res_type;
@@ -2091,7 +2101,7 @@
// initialized must be marked invalid.
work_line_->MarkUninitRefsAsInvalid(this, uninit_type);
// add the new uninitialized reference to the register state
- work_line_->SetRegisterType(this, inst->VRegA_21c(), uninit_type);
+ work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_21c(), uninit_type);
break;
}
case Instruction::NEW_ARRAY:
@@ -2113,7 +2123,7 @@
if (!work_line_->VerifyRegisterType(this, inst->VRegC_23x(), reg_types_.Float())) {
break;
}
- work_line_->SetRegisterType(this, inst->VRegA_23x(), reg_types_.Integer());
+ work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_23x(), reg_types_.Integer());
break;
case Instruction::CMPL_DOUBLE:
case Instruction::CMPG_DOUBLE:
@@ -2125,7 +2135,7 @@
reg_types_.DoubleHi())) {
break;
}
- work_line_->SetRegisterType(this, inst->VRegA_23x(), reg_types_.Integer());
+ work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_23x(), reg_types_.Integer());
break;
case Instruction::CMP_LONG:
if (!work_line_->VerifyRegisterTypeWide(this, inst->VRegB_23x(), reg_types_.LongLo(),
@@ -2136,7 +2146,7 @@
reg_types_.LongHi())) {
break;
}
- work_line_->SetRegisterType(this, inst->VRegA_23x(), reg_types_.Integer());
+ work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_23x(), reg_types_.Integer());
break;
case Instruction::THROW: {
const RegType& res_type = work_line_->GetRegisterType(this, inst->VRegA_11x());
@@ -2291,7 +2301,9 @@
branch_line.reset(update_line);
}
update_line->CopyFromLine(work_line_.get());
- update_line->SetRegisterType(this, instance_of_inst->VRegB_22c(), cast_type);
+ update_line->SetRegisterType<LockOp::kKeep>(this,
+ instance_of_inst->VRegB_22c(),
+ cast_type);
if (!insn_flags_[instance_of_idx].IsBranchTarget() && 0 != instance_of_idx) {
// See if instance-of was preceded by a move-object operation, common due to the small
// register encoding space of instance-of, and propagate type information to the source
@@ -2309,17 +2321,23 @@
switch (move_inst->Opcode()) {
case Instruction::MOVE_OBJECT:
if (move_inst->VRegA_12x() == instance_of_inst->VRegB_22c()) {
- update_line->SetRegisterType(this, move_inst->VRegB_12x(), cast_type);
+ update_line->SetRegisterType<LockOp::kKeep>(this,
+ move_inst->VRegB_12x(),
+ cast_type);
}
break;
case Instruction::MOVE_OBJECT_FROM16:
if (move_inst->VRegA_22x() == instance_of_inst->VRegB_22c()) {
- update_line->SetRegisterType(this, move_inst->VRegB_22x(), cast_type);
+ update_line->SetRegisterType<LockOp::kKeep>(this,
+ move_inst->VRegB_22x(),
+ cast_type);
}
break;
case Instruction::MOVE_OBJECT_16:
if (move_inst->VRegA_32x() == instance_of_inst->VRegB_22c()) {
- update_line->SetRegisterType(this, move_inst->VRegB_32x(), cast_type);
+ update_line->SetRegisterType<LockOp::kKeep>(this,
+ move_inst->VRegB_32x(),
+ cast_type);
}
break;
default:
@@ -3010,7 +3028,7 @@
// is good enough for some other verification to occur without hard-failing.
const uint32_t vreg_target_object = inst->VRegA_22x(); // box-lambda vA, vB
const RegType& reg_type = reg_types_.JavaLangObject(need_precise_constants_);
- work_line_->SetRegisterType(this, vreg_target_object, reg_type);
+ work_line_->SetRegisterType<LockOp::kClear>(this, vreg_target_object, reg_type);
break;
}
@@ -3839,7 +3857,7 @@
work_line_->VerifyRegisterType(this, inst->VRegB_22c(), reg_types_.Integer());
/* set register type to array class */
const RegType& precise_type = reg_types_.FromUninitialized(res_type);
- work_line_->SetRegisterType(this, inst->VRegA_22c(), precise_type);
+ work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_22c(), precise_type);
} else {
// 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.
@@ -3876,7 +3894,7 @@
// instruction type. TODO: have a proper notion of bottom here.
if (!is_primitive || insn_type.IsCategory1Types()) {
// Reference or category 1
- work_line_->SetRegisterType(this, inst->VRegA_23x(), reg_types_.Zero());
+ work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_23x(), reg_types_.Zero());
} else {
// Category 2
work_line_->SetRegisterTypeWide(this, inst->VRegA_23x(),
@@ -3904,7 +3922,7 @@
// instruction, which can't differentiate object types and ints from floats, longs from
// doubles.
if (!component_type.IsLowHalf()) {
- work_line_->SetRegisterType(this, inst->VRegA_23x(), component_type);
+ work_line_->SetRegisterType<LockOp::kClear>(this, inst->VRegA_23x(), component_type);
} else {
work_line_->SetRegisterTypeWide(this, inst->VRegA_23x(), component_type,
component_type.HighHalf(®_types_));
@@ -4208,13 +4226,13 @@
<< "' but found type '" << *field_type
<< "' in get-object";
if (error != VERIFY_ERROR_BAD_CLASS_HARD) {
- work_line_->SetRegisterType(this, vregA, reg_types_.Conflict());
+ work_line_->SetRegisterType<LockOp::kClear>(this, vregA, reg_types_.Conflict());
}
return;
}
}
if (!field_type->IsLowHalf()) {
- work_line_->SetRegisterType(this, vregA, *field_type);
+ work_line_->SetRegisterType<LockOp::kClear>(this, vregA, *field_type);
} else {
work_line_->SetRegisterTypeWide(this, vregA, *field_type, field_type->HighHalf(®_types_));
}
@@ -4357,12 +4375,12 @@
<< " to be compatible with type '" << insn_type
<< "' but found type '" << *field_type
<< "' in get-object";
- work_line_->SetRegisterType(this, vregA, reg_types_.Conflict());
+ work_line_->SetRegisterType<LockOp::kClear>(this, vregA, reg_types_.Conflict());
return;
}
}
if (!field_type->IsLowHalf()) {
- work_line_->SetRegisterType(this, vregA, *field_type);
+ work_line_->SetRegisterType<LockOp::kClear>(this, vregA, *field_type);
} else {
work_line_->SetRegisterTypeWide(this, vregA, *field_type, field_type->HighHalf(®_types_));
}
diff --git a/runtime/verifier/register_line-inl.h b/runtime/verifier/register_line-inl.h
index 9cd2bdf..bee5834 100644
--- a/runtime/verifier/register_line-inl.h
+++ b/runtime/verifier/register_line-inl.h
@@ -31,6 +31,7 @@
return verifier->GetRegTypeCache()->GetFromId(line_[vsrc]);
}
+template <LockOp kLockOp>
inline bool RegisterLine::SetRegisterType(MethodVerifier* verifier, uint32_t vdst,
const RegType& new_type) {
DCHECK_LT(vdst, num_regs_);
@@ -43,8 +44,16 @@
// as they are not accessed, and our backends can handle this nowadays.
line_[vdst] = new_type.GetId();
}
- // Clear the monitor entry bits for this register.
- ClearAllRegToLockDepths(vdst);
+ switch (kLockOp) {
+ case LockOp::kClear:
+ // Clear the monitor entry bits for this register.
+ ClearAllRegToLockDepths(vdst);
+ break;
+ case LockOp::kKeep:
+ // Should only be doing this with reference types.
+ DCHECK(new_type.IsReferenceTypes());
+ break;
+ }
return true;
}
@@ -89,7 +98,7 @@
TypeCategory cat) {
DCHECK(cat == kTypeCategory1nr || cat == kTypeCategoryRef);
const RegType& type = GetRegisterType(verifier, vsrc);
- if (!SetRegisterType(verifier, vdst, type)) {
+ if (!SetRegisterType<LockOp::kClear>(verifier, vdst, type)) {
return;
}
if (!type.IsConflict() && // Allow conflicts to be copied around.
diff --git a/runtime/verifier/register_line.cc b/runtime/verifier/register_line.cc
index f286a45..bb6df76 100644
--- a/runtime/verifier/register_line.cc
+++ b/runtime/verifier/register_line.cc
@@ -155,6 +155,9 @@
for (const auto& monitor : monitors_) {
result += StringPrintf("{%d},", monitor);
}
+ for (auto& pairs : reg_to_lock_depths_) {
+ result += StringPrintf("<%d -> %x>", pairs.first, pairs.second);
+ }
return result;
}
@@ -175,7 +178,7 @@
<< "copyRes1 v" << vdst << "<- result0" << " type=" << type;
} else {
DCHECK(verifier->GetRegTypeCache()->GetFromId(result_[1]).IsUndefined());
- SetRegisterType(verifier, vdst, type);
+ SetRegisterType<LockOp::kClear>(verifier, vdst, type);
result_[0] = verifier->GetRegTypeCache()->Undefined().GetId();
}
}
@@ -201,7 +204,7 @@
void RegisterLine::CheckUnaryOp(MethodVerifier* verifier, const Instruction* inst,
const RegType& dst_type, const RegType& src_type) {
if (VerifyRegisterType(verifier, inst->VRegB_12x(), src_type)) {
- SetRegisterType(verifier, inst->VRegA_12x(), dst_type);
+ SetRegisterType<LockOp::kClear>(verifier, inst->VRegA_12x(), dst_type);
}
}
@@ -225,7 +228,7 @@
const RegType& dst_type,
const RegType& src_type1, const RegType& src_type2) {
if (VerifyRegisterTypeWide(verifier, inst->VRegB_12x(), src_type1, src_type2)) {
- SetRegisterType(verifier, inst->VRegA_12x(), dst_type);
+ SetRegisterType<LockOp::kClear>(verifier, inst->VRegA_12x(), dst_type);
}
}
@@ -241,11 +244,13 @@
DCHECK(dst_type.IsInteger());
if (GetRegisterType(verifier, vregB).IsBooleanTypes() &&
GetRegisterType(verifier, vregC).IsBooleanTypes()) {
- SetRegisterType(verifier, inst->VRegA_23x(), verifier->GetRegTypeCache()->Boolean());
+ SetRegisterType<LockOp::kClear>(verifier,
+ inst->VRegA_23x(),
+ verifier->GetRegTypeCache()->Boolean());
return;
}
}
- SetRegisterType(verifier, inst->VRegA_23x(), dst_type);
+ SetRegisterType<LockOp::kClear>(verifier, inst->VRegA_23x(), dst_type);
}
}
@@ -279,11 +284,13 @@
DCHECK(dst_type.IsInteger());
if (GetRegisterType(verifier, vregA).IsBooleanTypes() &&
GetRegisterType(verifier, vregB).IsBooleanTypes()) {
- SetRegisterType(verifier, vregA, verifier->GetRegTypeCache()->Boolean());
+ SetRegisterType<LockOp::kClear>(verifier,
+ vregA,
+ verifier->GetRegTypeCache()->Boolean());
return;
}
}
- SetRegisterType(verifier, vregA, dst_type);
+ SetRegisterType<LockOp::kClear>(verifier, vregA, dst_type);
}
}
@@ -321,11 +328,13 @@
/* check vB with the call, then check the constant manually */
const uint32_t val = is_lit16 ? inst->VRegC_22s() : inst->VRegC_22b();
if (GetRegisterType(verifier, vregB).IsBooleanTypes() && (val == 0 || val == 1)) {
- SetRegisterType(verifier, vregA, verifier->GetRegTypeCache()->Boolean());
+ SetRegisterType<LockOp::kClear>(verifier,
+ vregA,
+ verifier->GetRegTypeCache()->Boolean());
return;
}
}
- SetRegisterType(verifier, vregA, dst_type);
+ SetRegisterType<LockOp::kClear>(verifier, vregA, dst_type);
}
}
diff --git a/runtime/verifier/register_line.h b/runtime/verifier/register_line.h
index a9c4c95..41f1e28 100644
--- a/runtime/verifier/register_line.h
+++ b/runtime/verifier/register_line.h
@@ -47,6 +47,12 @@
kTypeCategoryRef = 3, // object reference
};
+// What to do with the lock levels when setting the register type.
+enum class LockOp {
+ kClear, // Clear the lock levels recorded.
+ kKeep // Leave the lock levels alone.
+};
+
// During verification, we associate one of these with every "interesting" instruction. We track
// the status of all registers, and (if the method has any monitor-enter instructions) maintain a
// stack of entered monitors (identified by code unit offset).
@@ -83,6 +89,15 @@
// Set the type of register N, verifying that the register is valid. If "newType" is the "Lo"
// part of a 64-bit value, register N+1 will be set to "newType+1".
// The register index was validated during the static pass, so we don't need to check it here.
+ //
+ // LockOp::kClear should be used by default; it will clear the lock levels associated with the
+ // register. An example is setting the register type because an instruction writes to the
+ // register.
+ // LockOp::kKeep keeps the lock levels of the register and only changes the register type. This
+ // is typical when the underlying value did not change, but we have "different" type information
+ // available now. An example is sharpening types after a check-cast. Note that when given kKeep,
+ // the new_type is dchecked to be a reference type.
+ template <LockOp kLockOp>
ALWAYS_INLINE bool SetRegisterType(MethodVerifier* verifier, uint32_t vdst,
const RegType& new_type)
SHARED_REQUIRES(Locks::mutator_lock_);
diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt
index 6a452eb..6568eac 100644
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -42,4 +42,6 @@
b/23201502 (double)
b/23300986
b/23300986 (2)
+b/23502994 (if-eqz)
+b/23502994 (check-cast)
Done!
diff --git a/test/800-smali/smali/b_23502994.smali b/test/800-smali/smali/b_23502994.smali
new file mode 100644
index 0000000..d1d0554
--- /dev/null
+++ b/test/800-smali/smali/b_23502994.smali
@@ -0,0 +1,45 @@
+.class public LB23502994;
+
+.super Ljava/lang/Object;
+
+.method public static runIF_EQZ(Ljava/lang/Object;)V
+ .registers 3
+ monitor-enter v2 # Lock on parameter
+
+ # Sharpen, and try to unlock (in both branches). We should not lose the lock info when we make
+ # the register type more precise.
+
+ instance-of v0, v2, Ljava/lang/String;
+ if-eqz v0, :LnotString
+
+ # At this point v2 is of type Ljava/lang/String;
+ monitor-exit v2
+
+ goto :Lend
+
+:LnotString
+ monitor-exit v2 # Unlock the else branch
+
+ # Fall-through.
+
+:Lend
+ return-void
+
+.end method
+
+
+.method public static runCHECKCAST(Ljava/lang/Object;)V
+ .registers 3
+ monitor-enter v2 # Lock on parameter
+
+ # Sharpen, and try to unlock. We should not lose the lock info when we make the register type
+ # more precise.
+
+ check-cast v2, Ljava/lang/String;
+
+ # At this point v2 is of type Ljava/lang/String;
+ monitor-exit v2
+
+ return-void
+
+.end method
diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java
index 183958a..ba4990a 100644
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -129,6 +129,10 @@
new Object[] { new Object() }, null, null));
testCases.add(new TestCase("b/23300986 (2)", "B23300986", "runAliasBeforeEnter",
new Object[] { new Object() }, null, null));
+ testCases.add(new TestCase("b/23502994 (if-eqz)", "B23502994", "runIF_EQZ",
+ new Object[] { new Object() }, null, null));
+ testCases.add(new TestCase("b/23502994 (check-cast)", "B23502994", "runCHECKCAST",
+ new Object[] { "abc" }, null, null));
}
public void runTests() {