Fix RegType invariant checks.
Move the checks from the base constructor to constructors of
the most derived classes. While in the base constructor, the
vtable points to the base class implementations of virtual
function, so we were not checking what we intended. And when
the check failed, we tried to call a pure virtual function
RegType::Dump() and crashed while constructing an abort
message.
Also, avoid an unnecessary std::string construction in
RegType::CheckInvariants() as StringPiece can be compared
with const char* directly.
Test: testrunner.py --host
Bug: 32548008
Change-Id: Ice7ca298132f87f8b1671b6578aeec221f0e7930
diff --git a/runtime/verifier/reg_type.cc b/runtime/verifier/reg_type.cc
index 52f7e34..740b7dd 100644
--- a/runtime/verifier/reg_type.cc
+++ b/runtime/verifier/reg_type.cc
@@ -309,6 +309,7 @@
// Note: no check for IsInstantiable() here. We may produce this in case an InstantiationError
// would be thrown at runtime, but we need to continue verification and *not* create a
// hard failure or abort.
+ CheckConstructorInvariants(this);
}
std::string UnresolvedMergedType::Dump() const {
@@ -789,7 +790,7 @@
if (!klass_.IsNull()) {
CHECK(!descriptor_.empty()) << *this;
std::string temp;
- CHECK_EQ(descriptor_.ToString(), klass_.Read()->GetDescriptor(&temp)) << *this;
+ CHECK_EQ(descriptor_, klass_.Read()->GetDescriptor(&temp)) << *this;
}
}
@@ -820,9 +821,7 @@
reg_type_cache_(reg_type_cache),
resolved_part_(resolved),
unresolved_types_(unresolved, false, unresolved.GetAllocator()) {
- if (kIsDebugBuild) {
- CheckInvariants();
- }
+ CheckConstructorInvariants(this);
}
void UnresolvedMergedType::CheckInvariants() const {
CHECK(reg_type_cache_ != nullptr);
diff --git a/runtime/verifier/reg_type.h b/runtime/verifier/reg_type.h
index 472381d..dedf77f 100644
--- a/runtime/verifier/reg_type.h
+++ b/runtime/verifier/reg_type.h
@@ -274,14 +274,17 @@
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
: descriptor_(descriptor),
klass_(klass),
- cache_id_(cache_id) {
+ cache_id_(cache_id) {}
+
+ template <typename Class>
+ void CheckConstructorInvariants(Class* this_ ATTRIBUTE_UNUSED) const
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ static_assert(std::is_final<Class>::value, "Class must be final.");
if (kIsDebugBuild) {
CheckInvariants();
}
}
- void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_);
-
const StringPiece descriptor_;
mutable GcRoot<mirror::Class> klass_; // Non-const only due to moving classes.
const uint16_t cache_id_;
@@ -289,6 +292,8 @@
friend class RegTypeCache;
private:
+ virtual void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_);
+
/*
* A basic Join operation on classes. For a pair of types S and T the Join, written S v T = J, is
* S <: J, T <: J and for-all U such that S <: U, T <: U then J <: U. That is J is the parent of
@@ -339,7 +344,9 @@
private:
ConflictType(mirror::Class* klass, const StringPiece& descriptor,
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
- : RegType(klass, descriptor, cache_id) {}
+ : RegType(klass, descriptor, cache_id) {
+ CheckConstructorInvariants(this);
+ }
static const ConflictType* instance_;
};
@@ -368,7 +375,9 @@
private:
UndefinedType(mirror::Class* klass, const StringPiece& descriptor,
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
- : RegType(klass, descriptor, cache_id) {}
+ : RegType(klass, descriptor, cache_id) {
+ CheckConstructorInvariants(this);
+ }
static const UndefinedType* instance_;
};
@@ -387,7 +396,7 @@
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_);
};
-class IntegerType : public Cat1Type {
+class IntegerType FINAL : public Cat1Type {
public:
bool IsInteger() const OVERRIDE { return true; }
std::string Dump() const OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
@@ -401,7 +410,9 @@
private:
IntegerType(mirror::Class* klass, const StringPiece& descriptor,
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
- : Cat1Type(klass, descriptor, cache_id) {}
+ : Cat1Type(klass, descriptor, cache_id) {
+ CheckConstructorInvariants(this);
+ }
static const IntegerType* instance_;
};
@@ -419,7 +430,9 @@
private:
BooleanType(mirror::Class* klass, const StringPiece& descriptor,
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
- : Cat1Type(klass, descriptor, cache_id) {}
+ : Cat1Type(klass, descriptor, cache_id) {
+ CheckConstructorInvariants(this);
+ }
static const BooleanType* instance_;
};
@@ -438,7 +451,9 @@
private:
ByteType(mirror::Class* klass, const StringPiece& descriptor,
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
- : Cat1Type(klass, descriptor, cache_id) {}
+ : Cat1Type(klass, descriptor, cache_id) {
+ CheckConstructorInvariants(this);
+ }
static const ByteType* instance_;
};
@@ -456,7 +471,9 @@
private:
ShortType(mirror::Class* klass, const StringPiece& descriptor,
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
- : Cat1Type(klass, descriptor, cache_id) {}
+ : Cat1Type(klass, descriptor, cache_id) {
+ CheckConstructorInvariants(this);
+ }
static const ShortType* instance_;
};
@@ -474,7 +491,9 @@
private:
CharType(mirror::Class* klass, const StringPiece& descriptor,
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
- : Cat1Type(klass, descriptor, cache_id) {}
+ : Cat1Type(klass, descriptor, cache_id) {
+ CheckConstructorInvariants(this);
+ }
static const CharType* instance_;
};
@@ -492,7 +511,9 @@
private:
FloatType(mirror::Class* klass, const StringPiece& descriptor,
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
- : Cat1Type(klass, descriptor, cache_id) {}
+ : Cat1Type(klass, descriptor, cache_id) {
+ CheckConstructorInvariants(this);
+ }
static const FloatType* instance_;
};
@@ -517,7 +538,9 @@
private:
LongLoType(mirror::Class* klass, const StringPiece& descriptor,
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
- : Cat2Type(klass, descriptor, cache_id) {}
+ : Cat2Type(klass, descriptor, cache_id) {
+ CheckConstructorInvariants(this);
+ }
static const LongLoType* instance_;
};
@@ -535,7 +558,9 @@
private:
LongHiType(mirror::Class* klass, const StringPiece& descriptor,
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
- : Cat2Type(klass, descriptor, cache_id) {}
+ : Cat2Type(klass, descriptor, cache_id) {
+ CheckConstructorInvariants(this);
+ }
static const LongHiType* instance_;
};
@@ -554,7 +579,9 @@
private:
DoubleLoType(mirror::Class* klass, const StringPiece& descriptor,
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
- : Cat2Type(klass, descriptor, cache_id) {}
+ : Cat2Type(klass, descriptor, cache_id) {
+ CheckConstructorInvariants(this);
+ }
static const DoubleLoType* instance_;
};
@@ -572,7 +599,9 @@
private:
DoubleHiType(mirror::Class* klass, const StringPiece& descriptor,
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
- : Cat2Type(klass, descriptor, cache_id) {}
+ : Cat2Type(klass, descriptor, cache_id) {
+ CheckConstructorInvariants(this);
+ }
static const DoubleHiType* instance_;
};
@@ -637,7 +666,9 @@
public:
PreciseConstType(uint32_t constant, uint16_t cache_id)
REQUIRES_SHARED(Locks::mutator_lock_)
- : ConstantType(constant, cache_id) {}
+ : ConstantType(constant, cache_id) {
+ CheckConstructorInvariants(this);
+ }
bool IsPreciseConstant() const OVERRIDE { return true; }
@@ -648,7 +679,9 @@
public:
PreciseConstLoType(uint32_t constant, uint16_t cache_id)
REQUIRES_SHARED(Locks::mutator_lock_)
- : ConstantType(constant, cache_id) {}
+ : ConstantType(constant, cache_id) {
+ CheckConstructorInvariants(this);
+ }
bool IsPreciseConstantLo() const OVERRIDE { return true; }
std::string Dump() const OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
};
@@ -657,7 +690,9 @@
public:
PreciseConstHiType(uint32_t constant, uint16_t cache_id)
REQUIRES_SHARED(Locks::mutator_lock_)
- : ConstantType(constant, cache_id) {}
+ : ConstantType(constant, cache_id) {
+ CheckConstructorInvariants(this);
+ }
bool IsPreciseConstantHi() const OVERRIDE { return true; }
std::string Dump() const OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
};
@@ -667,6 +702,7 @@
ImpreciseConstType(uint32_t constat, uint16_t cache_id)
REQUIRES_SHARED(Locks::mutator_lock_)
: ConstantType(constat, cache_id) {
+ CheckConstructorInvariants(this);
}
bool IsImpreciseConstant() const OVERRIDE { return true; }
std::string Dump() const OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
@@ -676,7 +712,9 @@
public:
ImpreciseConstLoType(uint32_t constant, uint16_t cache_id)
REQUIRES_SHARED(Locks::mutator_lock_)
- : ConstantType(constant, cache_id) {}
+ : ConstantType(constant, cache_id) {
+ CheckConstructorInvariants(this);
+ }
bool IsImpreciseConstantLo() const OVERRIDE { return true; }
std::string Dump() const OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
};
@@ -685,7 +723,9 @@
public:
ImpreciseConstHiType(uint32_t constant, uint16_t cache_id)
REQUIRES_SHARED(Locks::mutator_lock_)
- : ConstantType(constant, cache_id) {}
+ : ConstantType(constant, cache_id) {
+ CheckConstructorInvariants(this);
+ }
bool IsImpreciseConstantHi() const OVERRIDE { return true; }
std::string Dump() const OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
};
@@ -718,7 +758,9 @@
const StringPiece& descriptor,
uint32_t allocation_pc, uint16_t cache_id)
REQUIRES_SHARED(Locks::mutator_lock_)
- : UninitializedType(klass, descriptor, allocation_pc, cache_id) {}
+ : UninitializedType(klass, descriptor, allocation_pc, cache_id) {
+ CheckConstructorInvariants(this);
+ }
bool IsUninitializedReference() const OVERRIDE { return true; }
@@ -735,9 +777,7 @@
uint32_t allocation_pc, uint16_t cache_id)
REQUIRES_SHARED(Locks::mutator_lock_)
: UninitializedType(nullptr, descriptor, allocation_pc, cache_id) {
- if (kIsDebugBuild) {
- CheckInvariants();
- }
+ CheckConstructorInvariants(this);
}
bool IsUnresolvedAndUninitializedReference() const OVERRIDE { return true; }
@@ -747,7 +787,7 @@
std::string Dump() const OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
private:
- void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_);
+ void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_) OVERRIDE;
};
// Similar to UninitializedReferenceType but special case for the this argument
@@ -759,9 +799,7 @@
uint16_t cache_id)
REQUIRES_SHARED(Locks::mutator_lock_)
: UninitializedType(klass, descriptor, 0, cache_id) {
- if (kIsDebugBuild) {
- CheckInvariants();
- }
+ CheckConstructorInvariants(this);
}
virtual bool IsUninitializedThisReference() const OVERRIDE { return true; }
@@ -771,7 +809,7 @@
std::string Dump() const OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
private:
- void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_);
+ void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_) OVERRIDE;
};
class UnresolvedUninitializedThisRefType FINAL : public UninitializedType {
@@ -780,9 +818,7 @@
uint16_t cache_id)
REQUIRES_SHARED(Locks::mutator_lock_)
: UninitializedType(nullptr, descriptor, 0, cache_id) {
- if (kIsDebugBuild) {
- CheckInvariants();
- }
+ CheckConstructorInvariants(this);
}
bool IsUnresolvedAndUninitializedThisReference() const OVERRIDE { return true; }
@@ -792,7 +828,7 @@
std::string Dump() const OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
private:
- void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_);
+ void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_) OVERRIDE;
};
// A type of register holding a reference to an Object of type GetClass or a
@@ -801,7 +837,9 @@
public:
ReferenceType(mirror::Class* klass, const StringPiece& descriptor,
uint16_t cache_id) REQUIRES_SHARED(Locks::mutator_lock_)
- : RegType(klass, descriptor, cache_id) {}
+ : RegType(klass, descriptor, cache_id) {
+ CheckConstructorInvariants(this);
+ }
bool IsReference() const OVERRIDE { return true; }
@@ -848,9 +886,7 @@
UnresolvedReferenceType(const StringPiece& descriptor, uint16_t cache_id)
REQUIRES_SHARED(Locks::mutator_lock_)
: UnresolvedType(descriptor, cache_id) {
- if (kIsDebugBuild) {
- CheckInvariants();
- }
+ CheckConstructorInvariants(this);
}
bool IsUnresolvedReference() const OVERRIDE { return true; }
@@ -860,7 +896,7 @@
std::string Dump() const OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
private:
- void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_);
+ void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_) OVERRIDE;
};
// Type representing the super-class of an unresolved type.
@@ -872,9 +908,7 @@
: UnresolvedType("", cache_id),
unresolved_child_id_(child_id),
reg_type_cache_(reg_type_cache) {
- if (kIsDebugBuild) {
- CheckInvariants();
- }
+ CheckConstructorInvariants(this);
}
bool IsUnresolvedSuperClass() const OVERRIDE { return true; }
@@ -889,7 +923,7 @@
std::string Dump() const OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
private:
- void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_);
+ void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_) OVERRIDE;
const uint16_t unresolved_child_id_;
const RegTypeCache* const reg_type_cache_;
@@ -925,7 +959,7 @@
std::string Dump() const OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
private:
- void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_);
+ void CheckInvariants() const REQUIRES_SHARED(Locks::mutator_lock_) OVERRIDE;
const RegTypeCache* const reg_type_cache_;