Enabled verification in the compiler and some other verifier fixes.
The verifier still needs to be able to continue execution after it fails
to resolve a class or method. The structure for that doesn't exist yet.
Change-Id: Ie32f6f9971fed02b645ba715bc5164f86f70f1c6
diff --git a/src/class_linker.cc b/src/class_linker.cc
index dde29cf..037887d 100644
--- a/src/class_linker.cc
+++ b/src/class_linker.cc
@@ -1168,8 +1168,29 @@
return NULL;
}
+void ClassLinker::VerifyClass(Class* klass) {
+ if (klass->IsVerified()) {
+ return;
+ }
+
+ CHECK_EQ(klass->GetStatus(), Class::kStatusResolved);
+
+ klass->SetStatus(Class::kStatusVerifying);
+ if (!DexVerifier::VerifyClass(klass)) {
+ LOG(ERROR) << "Verification failed on class "
+ << klass->GetDescriptor()->ToModifiedUtf8();
+ Object* exception = Thread::Current()->GetException();
+ klass->SetVerifyErrorClass(exception->GetClass());
+ klass->SetStatus(Class::kStatusError);
+ return;
+ }
+
+ klass->SetStatus(Class::kStatusVerified);
+}
+
bool ClassLinker::InitializeClass(Class* klass) {
CHECK(klass->GetStatus() == Class::kStatusResolved ||
+ klass->GetStatus() == Class::kStatusVerified ||
klass->GetStatus() == Class::kStatusInitializing ||
klass->GetStatus() == Class::kStatusError)
<< PrettyDescriptor(klass->GetDescriptor()) << " is " << klass->GetStatus();
@@ -1185,18 +1206,10 @@
return false;
}
- CHECK(klass->GetStatus() == Class::kStatusResolved);
-
- klass->SetStatus(Class::kStatusVerifying);
- if (!DexVerifier::VerifyClass(klass)) {
- LG << "Verification failed"; // TODO: ThrowVerifyError
- Object* exception = self->GetException();
- klass->SetVerifyErrorClass(exception->GetClass());
- klass->SetStatus(Class::kStatusError);
+ VerifyClass(klass);
+ if (klass->GetStatus() != Class::kStatusVerified) {
return false;
}
-
- klass->SetStatus(Class::kStatusVerified);
}
if (klass->GetStatus() == Class::kStatusInitialized) {
diff --git a/src/class_linker.h b/src/class_linker.h
index e143fd8..d0d3827 100644
--- a/src/class_linker.h
+++ b/src/class_linker.h
@@ -155,6 +155,8 @@
ObjectArray<StackTraceElement>* AllocStackTraceElementArray(size_t length);
+ void VerifyClass(Class* klass);
+
private:
ClassLinker(InternTable*);
diff --git a/src/compiler.cc b/src/compiler.cc
index 8a382f5..528a0b2 100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -42,7 +42,7 @@
void Compiler::CompileAll(const ClassLoader* class_loader) {
Resolve(class_loader);
- // TODO: add verification step
+ Verify(class_loader);
// TODO: mark all verified classes initialized if they have no <clinit>
ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
@@ -58,7 +58,7 @@
void Compiler::CompileOne(Method* method) {
const ClassLoader* class_loader = method->GetDeclaringClass()->GetClassLoader();
Resolve(class_loader);
- // TODO: add verification step
+ Verify(class_loader);
CompileMethod(method);
SetCodeAndDirectMethods(class_loader);
}
@@ -101,6 +101,26 @@
}
}
+void Compiler::Verify(const ClassLoader* class_loader) {
+ const std::vector<const DexFile*>& class_path = ClassLoader::GetClassPath(class_loader);
+ for (size_t i = 0; i != class_path.size(); ++i) {
+ const DexFile* dex_file = class_path[i];
+ CHECK(dex_file != NULL);
+ VerifyDexFile(class_loader, *dex_file);
+ }
+}
+
+void Compiler::VerifyDexFile(const ClassLoader* class_loader, const DexFile& dex_file) {
+ ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+ for (size_t i = 0; i < dex_file.NumClassDefs(); i++) {
+ const DexFile::ClassDef& class_def = dex_file.GetClassDef(i);
+ const char* descriptor = dex_file.GetClassDescriptor(class_def);
+ Class* klass = class_linker->FindClass(descriptor, class_loader);
+ CHECK(klass != NULL);
+ class_linker->VerifyClass(klass);
+ }
+}
+
void Compiler::Compile(const ClassLoader* class_loader) {
const std::vector<const DexFile*>& class_path = ClassLoader::GetClassPath(class_loader);
for (size_t i = 0; i != class_path.size(); ++i) {
diff --git a/src/compiler.h b/src/compiler.h
index 958d9ab..5735c6b 100644
--- a/src/compiler.h
+++ b/src/compiler.h
@@ -35,6 +35,9 @@
void Resolve(const ClassLoader* class_loader);
void ResolveDexFile(const ClassLoader* class_loader, const DexFile& dex_file);
+ void Verify(const ClassLoader* class_loader);
+ void VerifyDexFile(const ClassLoader* class_loader, const DexFile& dex_file);
+
void Compile(const ClassLoader* class_loader);
void CompileDexFile(const ClassLoader* class_loader, const DexFile& dex_file);
void CompileClass(Class* klass);
diff --git a/src/dex_file.h b/src/dex_file.h
index 4448981..8e89e0a 100644
--- a/src/dex_file.h
+++ b/src/dex_file.h
@@ -438,6 +438,11 @@
return GetTypeDescriptor(type_id);
}
+ // Returns the prototype of a method id.
+ const char* GetMethodPrototype(const MethodId& method_id) const {
+ return dexStringById(method_id.proto_idx_);
+ }
+
// Returns the name of a method id.
const char* GetMethodName(const MethodId& method_id) const {
return dexStringById(method_id.name_idx_);
diff --git a/src/dex_verifier.cc b/src/dex_verifier.cc
index 3380981..5d622f9 100644
--- a/src/dex_verifier.cc
+++ b/src/dex_verifier.cc
@@ -104,16 +104,16 @@
for (size_t i = 0; i < klass->NumDirectMethods(); ++i) {
Method* method = klass->GetDirectMethod(i);
if (!VerifyMethod(method)) {
- LOG(ERROR) << "Verifier rejected class "
- << klass->GetDescriptor()->ToModifiedUtf8();
+ LOG(ERROR) << "Verifier rejected class "
+ << klass->GetDescriptor()->ToModifiedUtf8();
return false;
}
}
for (size_t i = 0; i < klass->NumVirtualMethods(); ++i) {
Method* method = klass->GetVirtualMethod(i);
if (!VerifyMethod(method)) {
- LOG(ERROR) << "Verifier rejected class "
- << klass->GetDescriptor()->ToModifiedUtf8();
+ LOG(ERROR) << "Verifier rejected class "
+ << klass->GetDescriptor()->ToModifiedUtf8();
return false;
}
}
@@ -1646,7 +1646,7 @@
break;
case Instruction::CONST_CLASS:
/* make sure we can resolve the class; access check is important */
- res_class = class_linker->ResolveType(*dex_file, dec_insn.vB_, klass);
+ res_class = ResolveClassAndCheckAccess(dex_file, dec_insn.vB_, klass, &failure);
if (res_class == NULL) {
const char* bad_class_desc = dex_file->dexStringByTypeIdx(dec_insn.vB_);
LOG(ERROR) << "VFY: unable to resolve const-class " << dec_insn.vB_
@@ -1697,7 +1697,7 @@
* If it fails, an exception is thrown, which we deal with later
* by ignoring the update to dec_insn.vA_ when branching to a handler.
*/
- res_class = class_linker->ResolveType(*dex_file, dec_insn.vB_, klass);
+ res_class = ResolveClassAndCheckAccess(dex_file, dec_insn.vB_, klass, &failure);
if (res_class == NULL) {
const char* bad_class_desc = dex_file->dexStringByTypeIdx(dec_insn.vB_);
LOG(ERROR) << "VFY: unable to resolve check-cast " << dec_insn.vB_
@@ -1726,7 +1726,7 @@
}
/* make sure we can resolve the class; access check is important */
- res_class = class_linker->ResolveType(*dex_file, dec_insn.vC_, klass);
+ res_class = ResolveClassAndCheckAccess(dex_file, dec_insn.vC_, klass, &failure);
if (res_class == NULL) {
const char* bad_class_desc = dex_file->dexStringByTypeIdx(dec_insn.vC_);
LOG(ERROR) << "VFY: unable to resolve instanceof " << dec_insn.vC_
@@ -1752,7 +1752,7 @@
break;
case Instruction::NEW_INSTANCE:
- res_class = class_linker->ResolveType(*dex_file, dec_insn.vB_, klass);
+ res_class = ResolveClassAndCheckAccess(dex_file, dec_insn.vB_, klass, &failure);
if (res_class == NULL) {
const char* bad_class_desc = dex_file->dexStringByTypeIdx(dec_insn.vB_);
LOG(ERROR) << "VFY: unable to resolve new-instance " << dec_insn.vB_
@@ -1787,7 +1787,7 @@
}
break;
case Instruction::NEW_ARRAY:
- res_class = class_linker->ResolveType(*dex_file, dec_insn.vC_, klass);
+ res_class = ResolveClassAndCheckAccess(dex_file, dec_insn.vC_, klass, &failure);
if (res_class == NULL) {
const char* bad_class_desc = dex_file->dexStringByTypeIdx(dec_insn.vC_);
LOG(ERROR) << "VFY: unable to resolve new-array " << dec_insn.vC_
@@ -1806,7 +1806,7 @@
break;
case Instruction::FILLED_NEW_ARRAY:
case Instruction::FILLED_NEW_ARRAY_RANGE:
- res_class = class_linker->ResolveType(*dex_file, dec_insn.vB_, klass);
+ res_class = ResolveClassAndCheckAccess(dex_file, dec_insn.vB_, klass, &failure);
if (res_class == NULL) {
const char* bad_class_desc = dex_file->dexStringByTypeIdx(dec_insn.vB_);
LOG(ERROR) << "VFY: unable to resolve filled-array " << dec_insn.vB_
@@ -3396,13 +3396,18 @@
}
if (failure != VERIFY_ERROR_NONE) {
- //if (failure == VERIFY_ERROR_GENERIC || gDvm.optimizing)
if (failure == VERIFY_ERROR_GENERIC) {
/* immediate failure, reject class */
LOG(ERROR) << "VFY: rejecting opcode 0x" << std::hex
<< (int) dec_insn.opcode_ << " at 0x" << insn_idx << std::dec;
return false;
} else {
+ // TODO: CHECK IF THIS WILL WORK!
+ /* ignore the failure and move on */
+ LOG(ERROR) << "VFY: failing opcode 0x" << std::hex
+ << (int) dec_insn.opcode_ << " at 0x" << insn_idx << std::dec;
+ failure = VERIFY_ERROR_NONE;
+#if 0
/* replace opcode and continue on */
LOG(ERROR) << "VFY: replacing opcode 0x" << std::hex
<< (int) dec_insn.opcode_ << " at 0x" << insn_idx << std::dec;
@@ -3419,6 +3424,7 @@
/* continue on as if we just handled a throw-verification-error */
failure = VERIFY_ERROR_NONE;
opcode_flag = Instruction::kThrow;
+#endif
}
}
@@ -3717,6 +3723,7 @@
// TODO: REPLACE FAILING OPCODES
//assert(width == 2 || width == 3);
//uint16_t new_val = Instruction::THROW_VERIFICATION_ERROR |
+ //uint16_t new_val = Instruction::UNUSED_ED |
//(failure << 8) | (ref_type << (8 + kVerifyErrorRefTypeShift));
//UpdateCodeUnit(method, insns, new_val);
@@ -3921,8 +3928,8 @@
if (handler.type_idx_ == DexFile::kDexNoIndex) {
klass = class_linker->FindSystemClass("Ljava/lang/Throwable;");
} else {
- klass = class_linker->ResolveType(*dex_file, handler.type_idx_,
- method->GetDeclaringClass());
+ klass = ResolveClassAndCheckAccess(dex_file, handler.type_idx_,
+ method->GetDeclaringClass(), failure);
}
if (klass == NULL) {
@@ -4418,13 +4425,33 @@
return DigForSuperclass(c1, c2);
}
+Class* DexVerifier::ResolveClassAndCheckAccess(const DexFile* dex_file,
+ uint32_t class_idx, const Class* referrer, VerifyError* failure) {
+ ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+ Class* res_class = class_linker->ResolveType(*dex_file, class_idx, referrer);
+
+ if (res_class == NULL) {
+ *failure = VERIFY_ERROR_NO_CLASS;
+ return NULL;
+ }
+
+ /* Check if access is allowed. */
+ if (!referrer->CanAccess(res_class)) {
+ LOG(ERROR) << "VFY: illegal class access: "
+ << referrer->GetDescriptor()->ToModifiedUtf8() << " -> "
+ << res_class->GetDescriptor()->ToModifiedUtf8();
+ *failure = VERIFY_ERROR_ACCESS_CLASS;
+ return NULL;
+ }
+
+ return res_class;
+}
+
DexVerifier::RegType DexVerifier::MergeTypes(RegType type1, RegType type2,
bool* changed) {
RegType result;
- /*
- * Check for trivial case so we don't have to hit memory.
- */
+ /* Check for trivial case so we don't have to hit memory. */
if (type1 == type2)
return type1;
@@ -4973,17 +5000,17 @@
res_method = class_linker->ResolveMethod(*dex_file, dec_insn->vB_, dex_cache,
class_loader, (method_type == METHOD_DIRECT || method_type == METHOD_STATIC));
- /* Scan all implemented interfaces for the method */
- if (method_type == METHOD_INTERFACE && res_method == NULL) {
-
- }
-
if (res_method == NULL) {
- LOG(ERROR) << "VFY: unable to resolve called method";
+ const DexFile::MethodId& method_id = dex_file->GetMethodId(dec_insn->vB_);
+ const char* method_name = dex_file->GetMethodName(method_id);
+ const char* method_proto = dex_file->GetMethodPrototype(method_id);
+ const char* class_descriptor = dex_file->GetMethodClassDescriptor(method_id);
+
+ LOG(ERROR) << "VFY: unable to resolve method " << dec_insn->vB_ << ": "
+ << class_descriptor << "." << method_name << " " << method_proto;
*failure = VERIFY_ERROR_NO_METHOD;
return NULL;
}
- Class* res_class = res_method->GetDeclaringClass();
/*
* Only time you can explicitly call a method starting with '<' is when
@@ -4993,7 +5020,7 @@
if (res_method->GetName()->Equals("<init>")) {
if (method_type != METHOD_DIRECT || !IsInitMethod(res_method)) {
LOG(ERROR) << "VFY: invalid call to "
- << res_class->GetDescriptor()->ToModifiedUtf8()
+ << res_method->GetDeclaringClass()->GetDescriptor()->ToModifiedUtf8()
<< "." << res_method->GetName();
goto bad_sig;
}
@@ -5005,7 +5032,7 @@
*/
if (!IsCorrectInvokeKind(method_type, res_method)) {
LOG(ERROR) << "VFY: invoke type does not match method type of "
- << res_class->GetDescriptor()->ToModifiedUtf8()
+ << res_method->GetDeclaringClass()->GetDescriptor()->ToModifiedUtf8()
<< "." << res_method->GetName()->ToModifiedUtf8();
*failure = VERIFY_ERROR_GENERIC;
diff --git a/src/dex_verifier.h b/src/dex_verifier.h
index a8351df..41223b2 100644
--- a/src/dex_verifier.h
+++ b/src/dex_verifier.h
@@ -34,6 +34,10 @@
class DexVerifier {
public:
+ /* Verify a class. Returns "true" on success. */
+ static bool VerifyClass(Class* klass);
+
+ private:
/*
* RegType holds information about the type of data held in a register.
* For most types it's a simple enum. For reference types it holds a
@@ -537,10 +541,6 @@
return (uint32_t) (kRegTypeUninit | (uidx << kRegTypeUninitShift));
}
- /* Verify a class. Returns "true" on success. */
- static bool VerifyClass(Class* klass);
-
- private:
/*
* Perform verification on a single method.
*
@@ -1240,6 +1240,17 @@
static Class* FindCommonSuperclass(Class* c1, Class* c2);
/*
+ * Resolves a class based on an index and performs access checks to ensure
+ * the referrer can access the resolved class.
+ *
+ * Exceptions caused by failures are cleared before returning.
+ *
+ * Sets "*failure" on failure.
+ */
+ static Class* ResolveClassAndCheckAccess(const DexFile* dex_file,
+ uint32_t class_idx, const Class* referrer, VerifyError* failure);
+
+ /*
* Merge two RegType values.
*
* Sets "*changed" to "true" if the result doesn't match "type1".
@@ -1252,7 +1263,7 @@
*
* The merge is a simple bitwise AND.
*
- * Sets "*pChanged" to "true" if the result doesn't match "ents1".
+ * Sets "*changed" to "true" if the result doesn't match "ents1".
*/
static MonitorEntries MergeMonitorEntries(MonitorEntries ents1,
MonitorEntries ents2, bool* changed);