summaryrefslogtreecommitdiffstats
path: root/runtime/verifier
diff options
context:
space:
mode:
authorStephen Kyle <stephen.kyle@arm.com>2014-12-17 17:10:02 +0000
committerAndreas Gampe <agampe@google.com>2014-12-22 13:26:32 -0800
commit7e541c91997b7747fa79014a8ea540395e54efc8 (patch)
treeccd47096963dae48cad1a268f545fdd9b86d788e /runtime/verifier
parent5c79aec9d53d1320041d5a52e5115d78d16035b7 (diff)
downloadart-7e541c91997b7747fa79014a8ea540395e54efc8.zip
art-7e541c91997b7747fa79014a8ea540395e54efc8.tar.gz
art-7e541c91997b7747fa79014a8ea540395e54efc8.tar.bz2
ART: Fix verification of constructors.
Summary: A constructor must call its superclass constructor. However, if one replaces the invoke-direct superclass.<init>() instruction with a variety of instructions, the verifier would NOT complain that the superclass constructor hadn't been called. Detailed explanation: This was because if we are verifying the return-void insn of a constructor, then we check that the register line doesn't contain a register with an UninitializedThis type. With a method like follows: Class.<init>()V: return-void Then we hit the return-void, see the UninitializedThis, and fail the method. However, with a method like follows: Class.<init>()V: nop return-void Any insn that continues or branches onto a return-void instruction will mark all of the registers as Conflict. This meant that the check in return-void for an UninitializedThis residing the register line would _always_ pass if there were any insns before it - the entire line had been set to Conflict. The fix is to bring the check for an UninitializedThis forward to the point just before we set all registers to Conflict, if we're about to hit a return-void insn in a constructor. It still needs to be done again in the verification of return-void itself, to avoid the solo return-void case. This patch also deals with the case where the only remaining UninitializedThis reference is overwritten, to avoid a method like the following from getting through verification: Class.<init>()V: const/4 v0, 0 return-void Bug: 18800943 Change-Id: I2e317261844d3b6c78e35228669f3da173316570 Fuzzed-With: https://android-review.googlesource.com/#/c/119463/
Diffstat (limited to 'runtime/verifier')
-rw-r--r--runtime/verifier/method_verifier.cc36
-rw-r--r--runtime/verifier/method_verifier.h20
-rw-r--r--runtime/verifier/register_line.cc43
-rw-r--r--runtime/verifier/register_line.h12
4 files changed, 99 insertions, 12 deletions
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 81172cb..66846b5 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -103,6 +103,14 @@ ALWAYS_INLINE static inline bool FailOrAbort(MethodVerifier* verifier, bool cond
return false;
}
+static void SafelyMarkAllRegistersAsConflicts(MethodVerifier* verifier, RegisterLine* reg_line) {
+ if (verifier->IsConstructor()) {
+ // Before we mark all regs as conflicts, check that we don't have an uninitialized this.
+ reg_line->CheckConstructorReturn(verifier);
+ }
+ reg_line->MarkAllRegistersAsConflicts(verifier);
+}
+
MethodVerifier::FailureKind MethodVerifier::VerifyClass(Thread* self,
mirror::Class* klass,
bool allow_soft_failures,
@@ -1559,6 +1567,16 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
std::unique_ptr<RegisterLine> branch_line;
std::unique_ptr<RegisterLine> fallthrough_line;
+ /*
+ * If we are in a constructor, and we currently have an UninitializedThis type
+ * in a register somewhere, we need to make sure it isn't overwritten.
+ */
+ bool track_uninitialized_this = false;
+ size_t uninitialized_this_loc = 0;
+ if (IsConstructor()) {
+ track_uninitialized_this = work_line_->GetUninitializedThisLoc(this, &uninitialized_this_loc);
+ }
+
switch (inst->Opcode()) {
case Instruction::NOP:
/*
@@ -2769,6 +2787,20 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
*/
} // end - switch (dec_insn.opcode)
+ /*
+ * If we are in a constructor, and we had an UninitializedThis type
+ * in a register somewhere, we need to make sure it wasn't overwritten.
+ */
+ if (track_uninitialized_this) {
+ bool was_invoke_direct = (inst->Opcode() == Instruction::INVOKE_DIRECT ||
+ inst->Opcode() == Instruction::INVOKE_DIRECT_RANGE);
+ if (work_line_->WasUninitializedThisOverwritten(this, uninitialized_this_loc,
+ was_invoke_direct)) {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD)
+ << "Constructor failed to initialize this object";
+ }
+ }
+
if (have_pending_hard_failure_) {
if (Runtime::Current()->IsCompiler()) {
/* When compiling, check that the last failure is a hard failure */
@@ -2950,7 +2982,7 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
const Instruction* ret_inst = Instruction::At(code_item_->insns_ + next_insn_idx);
Instruction::Code opcode = ret_inst->Opcode();
if ((opcode == Instruction::RETURN_VOID) || (opcode == Instruction::RETURN_VOID_BARRIER)) {
- work_line_->MarkAllRegistersAsConflicts(this);
+ SafelyMarkAllRegistersAsConflicts(this, work_line_.get());
} else {
if (opcode == Instruction::RETURN_WIDE) {
work_line_->MarkAllRegistersAsConflictsExceptWide(this, ret_inst->VRegA_11x());
@@ -4105,7 +4137,7 @@ bool MethodVerifier::UpdateRegisters(uint32_t next_insn, RegisterLine* merge_lin
const Instruction* ret_inst = Instruction::At(code_item_->insns_ + next_insn);
Instruction::Code opcode = ret_inst->Opcode();
if ((opcode == Instruction::RETURN_VOID) || (opcode == Instruction::RETURN_VOID_BARRIER)) {
- target_line->MarkAllRegistersAsConflicts(this);
+ SafelyMarkAllRegistersAsConflicts(this, target_line);
} else {
target_line->CopyFromLine(merge_line);
if (opcode == Instruction::RETURN_WIDE) {
diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h
index c3bd4af..15a09c5 100644
--- a/runtime/verifier/method_verifier.h
+++ b/runtime/verifier/method_verifier.h
@@ -243,6 +243,16 @@ class MethodVerifier {
bool is_range)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+ // Is the method being verified a constructor?
+ bool IsConstructor() const {
+ return (method_access_flags_ & kAccConstructor) != 0;
+ }
+
+ // Is the method verified static?
+ bool IsStatic() const {
+ return (method_access_flags_ & kAccStatic) != 0;
+ }
+
private:
// Private constructor for dumping.
MethodVerifier(Thread* self, const DexFile* dex_file, Handle<mirror::DexCache> dex_cache,
@@ -625,16 +635,6 @@ class MethodVerifier {
bool UpdateRegisters(uint32_t next_insn, RegisterLine* merge_line, bool update_merge_line)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
- // Is the method being verified a constructor?
- bool IsConstructor() const {
- return (method_access_flags_ & kAccConstructor) != 0;
- }
-
- // Is the method verified static?
- bool IsStatic() const {
- return (method_access_flags_ & kAccStatic) != 0;
- }
-
// Return the register type for the method.
const RegType& GetMethodReturnType() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
diff --git a/runtime/verifier/register_line.cc b/runtime/verifier/register_line.cc
index 72d7938..3b09871 100644
--- a/runtime/verifier/register_line.cc
+++ b/runtime/verifier/register_line.cc
@@ -25,6 +25,49 @@
namespace art {
namespace verifier {
+bool RegisterLine::WasUninitializedThisOverwritten(MethodVerifier* verifier,
+ size_t this_loc,
+ bool was_invoke_direct) const {
+ DCHECK(verifier->IsConstructor());
+
+ // Is the UnintializedThis type still there?
+ if (GetRegisterType(verifier, this_loc).IsUninitializedThisReference() ||
+ GetRegisterType(verifier, this_loc).IsUnresolvedAndUninitializedThisReference()) {
+ return false;
+ }
+
+ // If there is an initialized reference here now, did we just perform an invoke-direct? Note that
+ // this is the correct approach for dex bytecode: results of invoke-direct are stored in the
+ // result register. Overwriting "this_loc" can only be done by a constructor call.
+ if (GetRegisterType(verifier, this_loc).IsReferenceTypes() && was_invoke_direct) {
+ return false;
+ // Otherwise we could have just copied a different initialized reference to this location.
+ }
+
+ // The UnintializedThis in the register is gone, so check to see if it's somewhere else now.
+ for (size_t i = 0; i < num_regs_; i++) {
+ if (GetRegisterType(verifier, i).IsUninitializedThisReference() ||
+ GetRegisterType(verifier, i).IsUnresolvedAndUninitializedThisReference()) {
+ // We found it somewhere else...
+ return false;
+ }
+ }
+
+ // The UninitializedThis is gone from the original register, and now we can't find it.
+ return true;
+}
+
+bool RegisterLine::GetUninitializedThisLoc(MethodVerifier* verifier, size_t* vreg) const {
+ for (size_t i = 0; i < num_regs_; i++) {
+ if (GetRegisterType(verifier, i).IsUninitializedThisReference() ||
+ GetRegisterType(verifier, i).IsUnresolvedAndUninitializedThisReference()) {
+ *vreg = i;
+ return true;
+ }
+ }
+ return false;
+}
+
bool RegisterLine::CheckConstructorReturn(MethodVerifier* verifier) const {
for (size_t i = 0; i < num_regs_; i++) {
if (GetRegisterType(verifier, i).IsUninitializedThisReference() ||
diff --git a/runtime/verifier/register_line.h b/runtime/verifier/register_line.h
index 52b5c13..ca61a0b 100644
--- a/runtime/verifier/register_line.h
+++ b/runtime/verifier/register_line.h
@@ -157,6 +157,18 @@ class RegisterLine {
*/
bool CheckConstructorReturn(MethodVerifier* verifier) const;
+ /*
+ * Check if an UninitializedThis at the specified location has been overwritten before
+ * being correctly initialized.
+ */
+ bool WasUninitializedThisOverwritten(MethodVerifier* verifier, size_t this_loc,
+ bool was_invoke_direct) const;
+
+ /*
+ * Get the first location of an UninitializedThis type, or return kInvalidVreg if there are none.
+ */
+ bool GetUninitializedThisLoc(MethodVerifier* verifier, size_t* vreg) const;
+
// Compare two register lines. Returns 0 if they match.
// Using this for a sort is unwise, since the value can change based on machine endianness.
int CompareLine(const RegisterLine* line2) const {