diff options
-rw-r--r-- | runtime/verifier/method_verifier.h | 18 | ||||
-rw-r--r-- | runtime/verifier/reg_type.h | 16 | ||||
-rw-r--r-- | runtime/verifier/register_line-inl.h | 11 | ||||
-rw-r--r-- | runtime/verifier/register_line.cc | 8 | ||||
-rw-r--r-- | runtime/verifier/register_line.h | 31 | ||||
-rw-r--r-- | test/800-smali/expected.txt | 1 | ||||
-rw-r--r-- | test/800-smali/smali/b_17978759.smali | 28 | ||||
-rw-r--r-- | test/800-smali/src/Main.java | 80 |
8 files changed, 123 insertions, 70 deletions
diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h index 357acf0..0c4bf3c 100644 --- a/runtime/verifier/method_verifier.h +++ b/runtime/verifier/method_verifier.h @@ -18,32 +18,26 @@ #define ART_RUNTIME_VERIFIER_METHOD_VERIFIER_H_ #include <memory> -#include <set> #include <vector> -#include "base/casts.h" #include "base/macros.h" -#include "base/stl_util.h" -#include "class_reference.h" #include "dex_file.h" -#include "dex_instruction.h" #include "handle.h" #include "instruction_flags.h" #include "method_reference.h" -#include "reg_type.h" #include "reg_type_cache.h" -#include "register_line.h" -#include "safe_map.h" namespace art { +class Instruction; struct ReferenceMap2Visitor; -template<class T> class Handle; namespace verifier { -class MethodVerifier; class DexPcToReferenceMap; +class MethodVerifier; +class RegisterLine; +class RegType; /* * "Direct" and "virtual" methods are stored independently. The type of call used to invoke the @@ -128,6 +122,8 @@ class PcToRegisterLineTable { private: std::unique_ptr<RegisterLine*[]> register_lines_; size_t size_; + + DISALLOW_COPY_AND_ASSIGN(PcToRegisterLineTable); }; // The verifier @@ -733,6 +729,8 @@ class MethodVerifier { // even though we might detect to be a compiler. Should only be set when running // VerifyMethodAndDump. const bool verify_to_dump_; + + DISALLOW_COPY_AND_ASSIGN(MethodVerifier); }; std::ostream& operator<<(std::ostream& os, const MethodVerifier::FailureKind& rhs); diff --git a/runtime/verifier/reg_type.h b/runtime/verifier/reg_type.h index 34d6caa..05958b5 100644 --- a/runtime/verifier/reg_type.h +++ b/runtime/verifier/reg_type.h @@ -17,17 +17,14 @@ #ifndef ART_RUNTIME_VERIFIER_REG_TYPE_H_ #define ART_RUNTIME_VERIFIER_REG_TYPE_H_ -#include <limits> #include <stdint.h> +#include <limits> #include <set> #include <string> -#include "jni.h" - #include "base/macros.h" #include "base/mutex.h" #include "gc_root.h" -#include "globals.h" #include "object_callbacks.h" #include "primitive.h" @@ -35,6 +32,7 @@ namespace art { namespace mirror { class Class; } // namespace mirror + namespace verifier { class RegTypeCache; @@ -578,17 +576,17 @@ class ConstantType : public RegType { bool IsConstantChar() const OVERRIDE { return IsConstant() && ConstantValue() >= 0 && - ConstantValue() <= std::numeric_limits<jchar>::max(); + ConstantValue() <= std::numeric_limits<uint16_t>::max(); } bool IsConstantByte() const OVERRIDE { return IsConstant() && - ConstantValue() >= std::numeric_limits<jbyte>::min() && - ConstantValue() <= std::numeric_limits<jbyte>::max(); + ConstantValue() >= std::numeric_limits<int8_t>::min() && + ConstantValue() <= std::numeric_limits<int8_t>::max(); } bool IsConstantShort() const OVERRIDE { return IsConstant() && - ConstantValue() >= std::numeric_limits<jshort>::min() && - ConstantValue() <= std::numeric_limits<jshort>::max(); + ConstantValue() >= std::numeric_limits<int16_t>::min() && + ConstantValue() <= std::numeric_limits<int16_t>::max(); } virtual bool IsConstantTypes() const OVERRIDE { return true; } diff --git a/runtime/verifier/register_line-inl.h b/runtime/verifier/register_line-inl.h index 219e687..244deed 100644 --- a/runtime/verifier/register_line-inl.h +++ b/runtime/verifier/register_line-inl.h @@ -114,6 +114,17 @@ inline void RegisterLine::CopyRegister2(MethodVerifier* verifier, uint32_t vdst, } } +inline size_t RegisterLine::GetMaxNonZeroReferenceReg(MethodVerifier* verifier, + size_t max_ref_reg) const { + size_t i = static_cast<int>(max_ref_reg) < 0 ? 0 : max_ref_reg; + for (; i < num_regs_; i++) { + if (GetRegisterType(verifier, i).IsNonZeroReferenceTypes()) { + max_ref_reg = i; + } + } + return max_ref_reg; +} + inline bool RegisterLine::VerifyRegisterType(MethodVerifier* verifier, uint32_t vsrc, const RegType& check_type) { // Verify the src register type against the check type refining the type of the register diff --git a/runtime/verifier/register_line.cc b/runtime/verifier/register_line.cc index 3139204..72d7938 100644 --- a/runtime/verifier/register_line.cc +++ b/runtime/verifier/register_line.cc @@ -310,8 +310,12 @@ void RegisterLine::PushMonitor(MethodVerifier* verifier, uint32_t reg_idx, int32 verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "monitor-enter stack overflow: " << monitors_.size(); } else { - SetRegToLockDepth(reg_idx, monitors_.size()); - monitors_.push_back(insn_idx); + if (SetRegToLockDepth(reg_idx, monitors_.size())) { + monitors_.push_back(insn_idx); + } else { + verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "unexpected monitor-enter on register v" << + reg_idx; + } } } diff --git a/runtime/verifier/register_line.h b/runtime/verifier/register_line.h index 8f7823a..52b5c13 100644 --- a/runtime/verifier/register_line.h +++ b/runtime/verifier/register_line.h @@ -20,14 +20,16 @@ #include <memory> #include <vector> -#include "dex_instruction.h" -#include "reg_type.h" #include "safe_map.h" namespace art { + +class Instruction; + namespace verifier { class MethodVerifier; +class RegType; /* * Register type categories, for type checking. @@ -275,15 +277,7 @@ class RegisterLine { bool MergeRegisters(MethodVerifier* verifier, const RegisterLine* incoming_line) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); - size_t GetMaxNonZeroReferenceReg(MethodVerifier* verifier, size_t max_ref_reg) { - size_t i = static_cast<int>(max_ref_reg) < 0 ? 0 : max_ref_reg; - for (; i < num_regs_; i++) { - if (GetRegisterType(verifier, i).IsNonZeroReferenceTypes()) { - max_ref_reg = i; - } - } - return max_ref_reg; - } + size_t GetMaxNonZeroReferenceReg(MethodVerifier* verifier, size_t max_ref_reg) const; // Write a bit at each register location that holds a reference. void WriteReferenceBitMap(MethodVerifier* verifier, std::vector<uint8_t>* data, size_t max_bytes); @@ -313,15 +307,18 @@ class RegisterLine { } } - void SetRegToLockDepth(size_t reg, size_t depth) { + bool SetRegToLockDepth(size_t reg, size_t depth) { CHECK_LT(depth, 32u); - DCHECK(!IsSetLockDepth(reg, depth)); + if (IsSetLockDepth(reg, depth)) { + return false; // Register already holds lock so locking twice is erroneous. + } auto it = reg_to_lock_depths_.find(reg); if (it == reg_to_lock_depths_.end()) { reg_to_lock_depths_.Put(reg, 1 << depth); } else { it->second |= (1 << depth); } + return true; } void ClearRegToLockDepth(size_t reg, size_t depth) { @@ -347,21 +344,23 @@ class RegisterLine { SetResultTypeToUnknown(verifier); } - // Storage for the result register's type, valid after an invocation + // Storage for the result register's type, valid after an invocation. uint16_t result_[2]; // Length of reg_types_ const uint32_t num_regs_; - // A stack of monitor enter locations + // A stack of monitor enter locations. std::vector<uint32_t, TrackingAllocator<uint32_t, kAllocatorTagVerifier>> monitors_; // A map from register to a bit vector of indices into the monitors_ stack. As we pop the monitor // stack we verify that monitor-enter/exit are correctly nested. That is, if there was a - // monitor-enter on v5 and then on v6, we expect the monitor-exit to be on v6 then on v5 + // monitor-enter on v5 and then on v6, we expect the monitor-exit to be on v6 then on v5. AllocationTrackingSafeMap<uint32_t, uint32_t, kAllocatorTagVerifier> reg_to_lock_depths_; // An array of RegType Ids associated with each dex register. uint16_t line_[0]; + + DISALLOW_COPY_AND_ASSIGN(RegisterLine); }; } // namespace verifier diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt index 3e3955b..f766b0a 100644 --- a/test/800-smali/expected.txt +++ b/test/800-smali/expected.txt @@ -1,4 +1,5 @@ b/17790197 +b/17978759 FloatBadArgReg negLong Done! diff --git a/test/800-smali/smali/b_17978759.smali b/test/800-smali/smali/b_17978759.smali new file mode 100644 index 0000000..07bcae5 --- /dev/null +++ b/test/800-smali/smali/b_17978759.smali @@ -0,0 +1,28 @@ +.class public LB17978759; +.super Ljava/lang/Object; + + .method public constructor <init>()V + .registers 1 + invoke-direct {p0}, Ljava/lang/Object;-><init>()V + return-void + .end method + + .method public test()V + .registers 2 + + move-object v0, p0 + # v0 and p0 alias + monitor-enter p0 + # monitor-enter on p0 + monitor-exit v0 + # monitor-exit on v0, however, verifier doesn't track this and so this is + # a warning. Verifier will still think p0 is locked. + + move-object v0, p0 + # v0 will now appear locked. + monitor-enter v0 + # Attempt to lock v0 twice is a verifier failure. + monitor-exit v0 + + return-void + .end method diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java index 87549d9..014edc0 100644 --- a/test/800-smali/src/Main.java +++ b/test/800-smali/src/Main.java @@ -15,6 +15,7 @@ */ import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.LinkedList; import java.util.List; @@ -49,6 +50,7 @@ public class Main { testCases = new LinkedList<TestCase>(); testCases.add(new TestCase("b/17790197", "B17790197", "getInt", null, null, 100)); + testCases.add(new TestCase("b/17978759", "B17978759", "test", null, new VerifyError(), null)); testCases.add(new TestCase("FloatBadArgReg", "FloatBadArgReg", "getInt", new Object[]{100}, null, 100)); testCases.add(new TestCase("negLong", "negLong", "negLong", null, null, 122142L)); @@ -66,47 +68,59 @@ public class Main { } private void runTest(TestCase tc) throws Exception { - Class<?> c = Class.forName(tc.testClass); - - Method[] methods = c.getDeclaredMethods(); - - // For simplicity we assume that test methods are not overloaded. So searching by name - // will give us the method we need to run. - Method method = null; - for (Method m : methods) { - if (m.getName().equals(tc.testMethodName)) { - method = m; - break; - } - } - - if (method == null) { - throw new IllegalArgumentException("Could not find test method " + tc.testMethodName + - " in class " + tc.testClass + " for test " + tc.testName); - } - Exception errorReturn = null; try { - Object retValue = method.invoke(null, tc.values); - if (tc.expectedException != null) { - errorReturn = new IllegalStateException("Expected an exception in test " + - tc.testName); + Class<?> c = Class.forName(tc.testClass); + + Method[] methods = c.getDeclaredMethods(); + + // For simplicity we assume that test methods are not overloaded. So searching by name + // will give us the method we need to run. + Method method = null; + for (Method m : methods) { + if (m.getName().equals(tc.testMethodName)) { + method = m; + break; + } } - if (tc.expectedReturn == null && retValue != null) { - errorReturn = new IllegalStateException("Expected a null result in test " + - tc.testName); - } else if (tc.expectedReturn != null && - (retValue == null || !tc.expectedReturn.equals(retValue))) { - errorReturn = new IllegalStateException("Expected return " + tc.expectedReturn + - ", but got " + retValue); + + if (method == null) { + errorReturn = new IllegalArgumentException("Could not find test method " + + tc.testMethodName + " in class " + + tc.testClass + " for test " + + tc.testName); + } else { + Object retValue; + if (Modifier.isStatic(method.getModifiers())) { + retValue = method.invoke(null, tc.values); + } else { + retValue = method.invoke(method.getDeclaringClass().newInstance(), tc.values); + } + if (tc.expectedException != null) { + errorReturn = new IllegalStateException("Expected an exception in test " + + tc.testName); + } + if (tc.expectedReturn == null && retValue != null) { + errorReturn = new IllegalStateException("Expected a null result in test " + + tc.testName); + } else if (tc.expectedReturn != null && + (retValue == null || !tc.expectedReturn.equals(retValue))) { + errorReturn = new IllegalStateException("Expected return " + + tc.expectedReturn + + ", but got " + retValue); + } else { + // Expected result, do nothing. + } } - } catch (Exception exc) { + } catch (Throwable exc) { if (tc.expectedException == null) { errorReturn = new IllegalStateException("Did not expect exception", exc); } else if (!tc.expectedException.getClass().equals(exc.getClass())) { errorReturn = new IllegalStateException("Expected " + - tc.expectedException.getClass().getName() + - ", but got " + exc.getClass(), exc); + tc.expectedException.getClass().getName() + + ", but got " + exc.getClass(), exc); + } else { + // Expected exception, do nothing. } } finally { if (errorReturn != null) { |