Fix handling of unresolved references in verifier.
The verifier should not treat use of unresolved references as a reason to reject
the entire class. Instead, the verifier treats the instruction as a throw. If
that class is run, the interpreter with extra checks will throw an exception.
Bug: 10457426
Change-Id: I3799da843a7ffb3519bbf6dc13a6276519d9cb95
diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index 86a6aea..36b250c 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -22,6 +22,7 @@
template<InvokeType type, bool is_range, bool do_access_check>
bool DoInvoke(Thread* self, ShadowFrame& shadow_frame,
const Instruction* inst, JValue* result) {
+ bool do_assignability_check = do_access_check;
uint32_t method_idx = (is_range) ? inst->VRegB_3rc() : inst->VRegB_35c();
uint32_t vregC = (is_range) ? inst->VRegC_3rc() : inst->VRegC_35c();
Object* receiver = (type == kStatic) ? NULL : shadow_frame.GetVRegReference(vregC);
@@ -61,6 +62,10 @@
++cur_reg;
}
+ const DexFile::TypeList* params;
+ if (do_assignability_check) {
+ params = mh.GetParameterTypeList();
+ }
size_t arg_offset = (receiver == NULL) ? 0 : 1;
const char* shorty = mh.GetShorty();
uint32_t arg[5];
@@ -73,6 +78,23 @@
switch (shorty[shorty_pos + 1]) {
case 'L': {
Object* o = shadow_frame.GetVRegReference(arg_pos);
+ if (do_assignability_check && o != NULL) {
+ Class* arg_type = mh.GetClassFromTypeIdx(params->GetTypeItem(shorty_pos).type_idx_);
+ if (arg_type == NULL) {
+ CHECK(self->IsExceptionPending());
+ return false;
+ }
+ if (!o->VerifierInstanceOf(arg_type)) {
+ // This should never happen.
+ self->ThrowNewExceptionF(self->GetCurrentLocationForThrow(),
+ "Ljava/lang/VirtualMachineError;",
+ "Invoking %s with bad arg %d, type '%s' not instance of '%s'",
+ mh.GetName(), shorty_pos,
+ ClassHelper(o->GetClass()).GetDescriptor(),
+ ClassHelper(arg_type).GetDescriptor());
+ return false;
+ }
+ }
new_shadow_frame->SetVRegReference(cur_reg, o);
break;
}
diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index a6c5e62..3ad1935 100644
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -215,6 +215,7 @@
template<FindFieldType find_type, Primitive::Type field_type, bool do_access_check>
static inline bool DoFieldPut(Thread* self, const ShadowFrame& shadow_frame,
const Instruction* inst, uint16_t inst_data) {
+ bool do_assignability_check = do_access_check;
bool is_static = (find_type == StaticObjectWrite) || (find_type == StaticPrimitiveWrite);
uint32_t field_idx = is_static ? inst->VRegB_21c() : inst->VRegC_22c();
ArtField* f = FindFieldFromCode(field_idx, shadow_frame.GetMethod(), self,
@@ -255,9 +256,24 @@
case Primitive::kPrimLong:
f->SetLong(obj, shadow_frame.GetVRegLong(vregA));
break;
- case Primitive::kPrimNot:
- f->SetObj(obj, shadow_frame.GetVRegReference(vregA));
+ case Primitive::kPrimNot: {
+ Object* reg = shadow_frame.GetVRegReference(vregA);
+ if (do_assignability_check && reg != NULL) {
+ Class* field_class = FieldHelper(f).GetType();
+ if (!reg->VerifierInstanceOf(field_class)) {
+ // This should never happen.
+ self->ThrowNewExceptionF(self->GetCurrentLocationForThrow(),
+ "Ljava/lang/VirtualMachineError;",
+ "Put '%s' that is not instance of field '%s' in '%s'",
+ ClassHelper(reg->GetClass()).GetDescriptor(),
+ ClassHelper(field_class).GetDescriptor(),
+ ClassHelper(f->GetDeclaringClass()).GetDescriptor());
+ return false;
+ }
+ }
+ f->SetObj(obj, reg);
break;
+ }
default:
LOG(FATAL) << "Unreachable: " << field_type;
}
@@ -479,7 +495,8 @@
}
static inline void TraceExecution(const ShadowFrame& shadow_frame, const Instruction* inst,
- const uint32_t dex_pc, MethodHelper& mh) {
+ const uint32_t dex_pc, MethodHelper& mh)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
const bool kTracing = false;
if (kTracing) {
#define TRACE_LOG std::cerr
diff --git a/runtime/interpreter/interpreter_goto_table_impl.cc b/runtime/interpreter/interpreter_goto_table_impl.cc
index d70b80e..018add3 100644
--- a/runtime/interpreter/interpreter_goto_table_impl.cc
+++ b/runtime/interpreter/interpreter_goto_table_impl.cc
@@ -74,6 +74,7 @@
template<bool do_access_check>
JValue ExecuteGotoImpl(Thread* self, MethodHelper& mh, const DexFile::CodeItem* code_item,
ShadowFrame& shadow_frame, JValue result_register) {
+ bool do_assignability_check = do_access_check;
if (UNLIKELY(!shadow_frame.HasReferenceArray())) {
LOG(FATAL) << "Invalid shadow frame for interpreter use";
return JValue();
@@ -264,11 +265,28 @@
HANDLE_INSTRUCTION_START(RETURN_OBJECT) {
JValue result;
+ Object* obj_result = shadow_frame.GetVRegReference(inst->VRegA_11x(inst_data));
result.SetJ(0);
- result.SetL(shadow_frame.GetVRegReference(inst->VRegA_11x(inst_data)));
+ result.SetL(obj_result);
if (UNLIKELY(self->TestAllFlags())) {
CheckSuspend(self);
}
+ if (do_assignability_check && obj_result != NULL) {
+ Class* return_type = MethodHelper(shadow_frame.GetMethod()).GetReturnType();
+ if (return_type == NULL) {
+ // Return the pending exception.
+ HANDLE_PENDING_EXCEPTION();
+ }
+ if (!obj_result->VerifierInstanceOf(return_type)) {
+ // This should never happen.
+ self->ThrowNewExceptionF(self->GetCurrentLocationForThrow(),
+ "Ljava/lang/VirtualMachineError;",
+ "Returning '%s' that is not instance of return type '%s'",
+ ClassHelper(obj_result->GetClass()).GetDescriptor(),
+ ClassHelper(return_type).GetDescriptor());
+ HANDLE_PENDING_EXCEPTION();
+ }
+ }
if (UNLIKELY(instrumentation->HasMethodExitListeners())) {
instrumentation->MethodExitEvent(self, shadow_frame.GetThisObject(code_item->ins_size_),
shadow_frame.GetMethod(), dex_pc,
@@ -512,6 +530,12 @@
Object* exception = shadow_frame.GetVRegReference(inst->VRegA_11x(inst_data));
if (UNLIKELY(exception == NULL)) {
ThrowNullPointerException(NULL, "throw with null exception");
+ } else if (do_assignability_check && !exception->GetClass()->IsThrowableClass()) {
+ // This should never happen.
+ self->ThrowNewExceptionF(self->GetCurrentLocationForThrow(),
+ "Ljava/lang/VirtualMachineError;",
+ "Throwing '%s' that is not instance of Throwable",
+ ClassHelper(exception->GetClass()).GetDescriptor());
} else {
self->SetException(shadow_frame.GetCurrentLocationForThrow(), exception->AsThrowable());
}
diff --git a/runtime/interpreter/interpreter_switch_impl.cc b/runtime/interpreter/interpreter_switch_impl.cc
index d49807c..2d658b3 100644
--- a/runtime/interpreter/interpreter_switch_impl.cc
+++ b/runtime/interpreter/interpreter_switch_impl.cc
@@ -53,6 +53,7 @@
template<bool do_access_check>
static JValue ExecuteSwitchImpl(Thread* self, MethodHelper& mh, const DexFile::CodeItem* code_item,
ShadowFrame& shadow_frame, JValue result_register) {
+ bool do_assignability_check = do_access_check;
if (UNLIKELY(!shadow_frame.HasReferenceArray())) {
LOG(FATAL) << "Invalid shadow frame for interpreter use";
return JValue();
@@ -226,11 +227,28 @@
case Instruction::RETURN_OBJECT: {
PREAMBLE();
JValue result;
+ Object* obj_result = shadow_frame.GetVRegReference(inst->VRegA_11x(inst_data));
result.SetJ(0);
- result.SetL(shadow_frame.GetVRegReference(inst->VRegA_11x(inst_data)));
+ result.SetL(obj_result);
if (UNLIKELY(self->TestAllFlags())) {
CheckSuspend(self);
}
+ if (do_assignability_check && obj_result != NULL) {
+ Class* return_type = MethodHelper(shadow_frame.GetMethod()).GetReturnType();
+ if (return_type == NULL) {
+ // Return the pending exception.
+ HANDLE_PENDING_EXCEPTION();
+ }
+ if (!obj_result->VerifierInstanceOf(return_type)) {
+ // This should never happen.
+ self->ThrowNewExceptionF(self->GetCurrentLocationForThrow(),
+ "Ljava/lang/VirtualMachineError;",
+ "Returning '%s' that is not instance of return type '%s'",
+ ClassHelper(obj_result->GetClass()).GetDescriptor(),
+ ClassHelper(return_type).GetDescriptor());
+ HANDLE_PENDING_EXCEPTION();
+ }
+ }
if (UNLIKELY(instrumentation->HasMethodExitListeners())) {
instrumentation->MethodExitEvent(self, shadow_frame.GetThisObject(code_item->ins_size_),
shadow_frame.GetMethod(), inst->GetDexPc(insns),
@@ -472,6 +490,12 @@
Object* exception = shadow_frame.GetVRegReference(inst->VRegA_11x(inst_data));
if (UNLIKELY(exception == NULL)) {
ThrowNullPointerException(NULL, "throw with null exception");
+ } else if (do_assignability_check && !exception->GetClass()->IsThrowableClass()) {
+ // This should never happen.
+ self->ThrowNewExceptionF(self->GetCurrentLocationForThrow(),
+ "Ljava/lang/VirtualMachineError;",
+ "Throwing '%s' that is not instance of Throwable",
+ ClassHelper(exception->GetClass()).GetDescriptor());
} else {
self->SetException(shadow_frame.GetCurrentLocationForThrow(), exception->AsThrowable());
}
diff --git a/runtime/mirror/object-inl.h b/runtime/mirror/object-inl.h
index 63396d2..5ed3db3 100644
--- a/runtime/mirror/object-inl.h
+++ b/runtime/mirror/object-inl.h
@@ -71,6 +71,12 @@
Monitor::Wait(self, this, ms, ns, true, kTimedWaiting);
}
+inline bool Object::VerifierInstanceOf(const Class* klass) const {
+ DCHECK(klass != NULL);
+ DCHECK(GetClass() != NULL);
+ return klass->IsInterface() || InstanceOf(klass);
+}
+
inline bool Object::InstanceOf(const Class* klass) const {
DCHECK(klass != NULL);
DCHECK(GetClass() != NULL);
diff --git a/runtime/mirror/object.h b/runtime/mirror/object.h
index efaa183..e105525 100644
--- a/runtime/mirror/object.h
+++ b/runtime/mirror/object.h
@@ -71,6 +71,11 @@
void SetClass(Class* new_klass);
+ // The verifier treats all interfaces as java.lang.Object and relies on runtime checks in
+ // invoke-interface to detect incompatible interface types.
+ bool VerifierInstanceOf(const Class* klass) const
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+
bool InstanceOf(const Class* klass) const
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 9811926..924a1bb 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -439,6 +439,8 @@
// to an interpreter.
error = VERIFY_ERROR_BAD_CLASS_SOFT;
} else {
+ // If we fail again at runtime, mark that this instruction would throw and force this
+ // method to be executed using the interpreter with checks.
have_pending_runtime_throw_failure_ = true;
}
break;
@@ -1580,11 +1582,13 @@
Fail(VERIFY_ERROR_BAD_CLASS_SOFT) << "returning uninitialized object '"
<< reg_type << "'";
} else if (!return_type.IsAssignableFrom(reg_type)) {
- Fail(reg_type.IsUnresolvedTypes() ?
- VERIFY_ERROR_BAD_CLASS_SOFT :
- VERIFY_ERROR_BAD_CLASS_HARD)
- << "returning '" << reg_type << "', but expected from declaration '"
- << return_type << "'";
+ if (reg_type.IsUnresolvedTypes() || return_type.IsUnresolvedTypes()) {
+ Fail(VERIFY_ERROR_NO_CLASS) << " can't resolve returned type '" << return_type
+ << "' or '" << reg_type << "'";
+ } else {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "returning '" << reg_type
+ << "', but expected from declaration '" << return_type << "'";
+ }
}
}
}
@@ -1804,8 +1808,8 @@
case Instruction::THROW: {
const RegType& res_type = work_line_->GetRegisterType(inst->VRegA_11x());
if (!reg_types_.JavaLangThrowable(false).IsAssignableFrom(res_type)) {
- Fail(VERIFY_ERROR_BAD_CLASS_SOFT) << "thrown class " << res_type
- << " not instanceof Throwable";
+ Fail(res_type.IsUnresolvedTypes() ? VERIFY_ERROR_NO_CLASS : VERIFY_ERROR_BAD_CLASS_SOFT)
+ << "thrown class " << res_type << " not instanceof Throwable";
}
break;
}
@@ -1929,8 +1933,8 @@
const RegType& orig_type = work_line_->GetRegisterType(instance_of_inst->VRegB_22c());
const RegType& cast_type = ResolveClassAndCheckAccess(instance_of_inst->VRegC_22c());
- if (!cast_type.IsUnresolvedTypes() && !cast_type.GetClass()->IsInterface() &&
- !cast_type.IsAssignableFrom(orig_type)) {
+ if (!cast_type.IsUnresolvedTypes() && !orig_type.IsUnresolvedTypes() &&
+ !cast_type.GetClass()->IsInterface() && !cast_type.IsAssignableFrom(orig_type)) {
RegisterLine* update_line = new RegisterLine(code_item_->registers_size_, this);
if (inst->Opcode() == Instruction::IF_EQZ) {
fallthrough_line.reset(update_line);
@@ -2610,7 +2614,7 @@
info_messages_ << "Rejecting opcode " << inst->DumpString(dex_file_);
return false;
} else if (have_pending_runtime_throw_failure_) {
- /* slow path will throw, mark following code as unreachable */
+ /* checking interpreter will throw, mark following code as unreachable */
opcode_flags = Instruction::kThrow;
}
/*
@@ -2861,7 +2865,8 @@
} else if (!reg_types_.JavaLangThrowable(false).IsAssignableFrom(exception)) {
// We don't know enough about the type and the common path merge will result in
// Conflict. Fail here knowing the correct thing can be done at runtime.
- Fail(VERIFY_ERROR_BAD_CLASS_SOFT) << "unexpected non-exception class " << exception;
+ Fail(exception.IsUnresolvedTypes() ? VERIFY_ERROR_NO_CLASS :
+ VERIFY_ERROR_BAD_CLASS_SOFT) << "unexpected non-exception class " << exception;
return reg_types_.Conflict();
} else if (common_super->Equals(exception)) {
// odd case, but nothing to do
@@ -3042,7 +3047,8 @@
reg_types_.FromClass(ClassHelper(klass).GetDescriptor(), klass,
klass->CannotBeAssignedFromOtherTypes());
if (!res_method_class.IsAssignableFrom(actual_arg_type)) {
- Fail(VERIFY_ERROR_BAD_CLASS_SOFT) << "'this' argument '" << actual_arg_type
+ Fail(actual_arg_type.IsUnresolvedTypes() ? VERIFY_ERROR_NO_CLASS:
+ VERIFY_ERROR_BAD_CLASS_SOFT) << "'this' argument '" << actual_arg_type
<< "' not instance of '" << res_method_class << "'";
return NULL;
}
@@ -3166,7 +3172,8 @@
reg_types_.FromClass(ClassHelper(klass).GetDescriptor(), klass,
klass->CannotBeAssignedFromOtherTypes());
if (!res_method_class.IsAssignableFrom(actual_arg_type)) {
- Fail(VERIFY_ERROR_BAD_CLASS_SOFT) << "'this' argument '" << actual_arg_type
+ Fail(actual_arg_type.IsUnresolvedTypes() ? VERIFY_ERROR_NO_CLASS :
+ VERIFY_ERROR_BAD_CLASS_SOFT) << "'this' argument '" << actual_arg_type
<< "' not instance of '" << res_method_class << "'";
return NULL;
}
diff --git a/runtime/verifier/register_line.cc b/runtime/verifier/register_line.cc
index 5affe47..a615cc1 100644
--- a/runtime/verifier/register_line.cc
+++ b/runtime/verifier/register_line.cc
@@ -44,12 +44,6 @@
} else if (new_type.IsConflict()) { // should only be set during a merge
verifier_->Fail(VERIFY_ERROR_BAD_CLASS_SOFT) << "Set register to unknown type " << new_type;
return false;
- } else if (verifier_->CanLoadClasses() && !Runtime::Current()->IsCompiler() &&
- new_type.IsUnresolvedTypes()) {
- // Unresolvable classes at runtime are bad and marked as a rewrite error.
- verifier_->Fail(VERIFY_ERROR_NO_CLASS) << "Set register to unresolved class '"
- << new_type << "' at runtime";
- return false;
} else {
line_[vdst] = new_type.GetId();
}
@@ -116,11 +110,15 @@
// Verify the src register type against the check type refining the type of the register
const RegType& src_type = GetRegisterType(vsrc);
if (!(check_type.IsAssignableFrom(src_type))) {
- // Hard fail if one of the types is primitive, since they are concretely known.
- enum VerifyError fail_type = (!check_type.IsNonZeroReferenceTypes() ||
- !src_type.IsNonZeroReferenceTypes())
- ? VERIFY_ERROR_BAD_CLASS_HARD
- : VERIFY_ERROR_BAD_CLASS_SOFT;
+ enum VerifyError fail_type;
+ if (!check_type.IsNonZeroReferenceTypes() || !src_type.IsNonZeroReferenceTypes()) {
+ // Hard fail if one of the types is primitive, since they are concretely known.
+ fail_type = VERIFY_ERROR_BAD_CLASS_HARD;
+ } else if (check_type.IsUnresolvedTypes() || src_type.IsUnresolvedTypes()) {
+ fail_type = VERIFY_ERROR_NO_CLASS;
+ } else {
+ fail_type = VERIFY_ERROR_BAD_CLASS_SOFT;
+ }
verifier_->Fail(fail_type) << "register v" << vsrc << " has type "
<< src_type << " but expected " << check_type;
return false;
