Do not crash for broken stored VerifierDeps.
Propagate the error to the callers. Also avoid filling in
unused data structures when we want only verified classes.
Test: aosp_taimen-userdebug boots.
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Bug: 151220989
Change-Id: I3ad0d725251037006128c1f631e6bd6dcec3a592
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index ccb89a6..ee838da 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1962,6 +1962,20 @@
}
}
+ // Setup VerifierDeps for compilation and report if we fail to parse the data.
+ if (!DoEagerUnquickeningOfVdex() && input_vdex_file_ != nullptr) {
+ std::unique_ptr<verifier::VerifierDeps> verifier_deps(
+ new verifier::VerifierDeps(dex_files, /*output_only=*/ false));
+ if (!verifier_deps->ParseStoredData(dex_files, input_vdex_file_->GetVerifierDepsData())) {
+ return dex2oat::ReturnCode::kOther;
+ }
+ callbacks_->SetVerifierDeps(verifier_deps.release());
+ } else {
+ // Create the main VerifierDeps, here instead of in the compiler since we want to aggregate
+ // the results for all the dex files, not just the results for the current dex file.
+ callbacks_->SetVerifierDeps(new verifier::VerifierDeps(dex_files));
+ }
+
return dex2oat::ReturnCode::kNoFailure;
}
@@ -2099,9 +2113,6 @@
// Setup vdex for compilation.
const std::vector<const DexFile*>& dex_files = compiler_options_->dex_files_for_oat_file_;
if (!DoEagerUnquickeningOfVdex() && input_vdex_file_ != nullptr) {
- callbacks_->SetVerifierDeps(
- new verifier::VerifierDeps(dex_files, input_vdex_file_->GetVerifierDepsData()));
-
// TODO: we unquicken unconditionally, as we don't know
// if the boot image has changed. How exactly we'll know is under
// experimentation.
@@ -2111,10 +2122,6 @@
// optimization does not depend on the boot image (the optimization relies on not
// having final fields in a class, which does not change for an app).
input_vdex_file_->Unquicken(dex_files, /* decompile_return_instruction */ false);
- } else {
- // Create the main VerifierDeps, here instead of in the compiler since we want to aggregate
- // the results for all the dex files, not just the results for the current dex file.
- callbacks_->SetVerifierDeps(new verifier::VerifierDeps(dex_files));
}
// To allow initialization of classes that construct ThreadLocal objects in class initializer,
diff --git a/dex2oat/verifier_deps_test.cc b/dex2oat/verifier_deps_test.cc
index 4094425..79cd69a 100644
--- a/dex2oat/verifier_deps_test.cc
+++ b/dex2oat/verifier_deps_test.cc
@@ -478,7 +478,9 @@
jobject second_loader = LoadDex("VerifierDeps");
const auto& second_dex_files = GetDexFiles(second_loader);
- VerifierDeps decoded_deps(second_dex_files, ArrayRef<const uint8_t>(buffer));
+ VerifierDeps decoded_deps(second_dex_files, /*output_only=*/ false);
+ bool parsed = decoded_deps.ParseStoredData(second_dex_files, ArrayRef<const uint8_t>(buffer));
+ CHECK(parsed);
VerifierDeps::DexFileDeps* decoded_dex_deps =
decoded_deps.GetDexFileDeps(*second_dex_files.front());
@@ -1144,7 +1146,9 @@
verifier_deps_->Encode(dex_files_, &buffer);
ASSERT_FALSE(buffer.empty());
- VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
+ VerifierDeps decoded_deps(dex_files_, /*output_only=*/ false);
+ bool parsed = decoded_deps.ParseStoredData(dex_files_, ArrayRef<const uint8_t>(buffer));
+ ASSERT_TRUE(parsed);
ASSERT_TRUE(verifier_deps_->Equals(decoded_deps));
}
@@ -1170,7 +1174,9 @@
}
// Dump the new verifier deps to ensure it can properly read the data.
- VerifierDeps decoded_deps(dex_files, ArrayRef<const uint8_t>(buffer));
+ VerifierDeps decoded_deps(dex_files, /*output_only=*/ false);
+ bool parsed = decoded_deps.ParseStoredData(dex_files, ArrayRef<const uint8_t>(buffer));
+ ASSERT_TRUE(parsed);
std::ostringstream stream;
VariableIndentationOutputStream os(&stream);
decoded_deps.Dump(&os);
@@ -1430,7 +1436,9 @@
ScopedObjectAccess soa(Thread::Current());
LoadDexFile(soa, "VerifierDeps", multi);
}
- verifier::VerifierDeps decoded_deps(dex_files_, ArrayRef<const uint8_t>(buffer));
+ VerifierDeps decoded_deps(dex_files_, /*output_only=*/ false);
+ bool parsed = decoded_deps.ParseStoredData(dex_files_, ArrayRef<const uint8_t>(buffer));
+ ASSERT_TRUE(parsed);
if (verify_failure) {
// Just taint the decoded VerifierDeps with one invalid entry.
VerifierDeps::DexFileDeps* deps = decoded_deps.GetDexFileDeps(*primary_dex_file_);
diff --git a/oatdump/oatdump.cc b/oatdump/oatdump.cc
index 8d33970..3acea42 100644
--- a/oatdump/oatdump.cc
+++ b/oatdump/oatdump.cc
@@ -564,7 +564,11 @@
}
dex_files.push_back(dex_file);
}
- verifier::VerifierDeps deps(dex_files, oat_file_.GetVdexFile()->GetVerifierDepsData());
+ verifier::VerifierDeps deps(dex_files, /*output_only=*/ false);
+ if (!deps.ParseStoredData(dex_files, oat_file_.GetVdexFile()->GetVerifierDepsData())) {
+ os << "Error parsing verifier dependencies." << std::endl;
+ return false;
+ }
deps.Dump(&vios);
} else {
os << "UNRECOGNIZED vdex file, magic "
diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index 8d021aa..346c112 100644
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -165,6 +165,7 @@
virtual void PreSetup(const std::string& elf_filename) = 0;
+ // Setup functions return true on success, false on failure.
bool Setup(int zip_fd, ArrayRef<const std::string> dex_filenames, std::string* error_msg);
bool Setup(const std::vector<const DexFile*>& dex_files);
@@ -1450,12 +1451,13 @@
std::unique_ptr<VdexFile>&& vdex_file,
const std::string& location) {
std::unique_ptr<OatFileBackedByVdex> oat_file(new OatFileBackedByVdex(location));
- oat_file->Initialize(dex_files, std::move(vdex_file));
+ if (!oat_file->Setup(dex_files, std::move(vdex_file))) {
+ return nullptr;
+ }
return oat_file.release();
}
- void Initialize(const std::vector<const DexFile*>& dex_files,
- std::unique_ptr<VdexFile>&& vdex_file) {
+ bool Setup(const std::vector<const DexFile*>& dex_files, std::unique_ptr<VdexFile>&& vdex_file) {
DCHECK(!IsExecutable());
// SetVdex will take ownership of the VdexFile.
@@ -1478,10 +1480,18 @@
// Load VerifierDeps from VDEX and copy bit vectors of verified classes.
ArrayRef<const uint8_t> deps_data = GetVdexFile()->GetVerifierDepsData();
- verified_classes_per_dex_ = verifier::VerifierDeps::ParseVerifiedClasses(dex_files, deps_data);
+ if (!verifier::VerifierDeps::ParseVerifiedClasses(dex_files,
+ deps_data,
+ &verified_classes_per_dex_)) {
+ return false;
+ }
// Initialize OatDexFiles.
- Setup(dex_files);
+ if (!OatFileBase::Setup(dex_files)) {
+ return false;
+ }
+
+ return true;
}
bool IsClassVerifiedInVdex(const OatDexFile& oat_dex_file, uint16_t class_def_index) const {
diff --git a/runtime/oat_file_manager.cc b/runtime/oat_file_manager.cc
index a35f8f4..ff743d5 100644
--- a/runtime/oat_file_manager.cc
+++ b/runtime/oat_file_manager.cc
@@ -836,9 +836,10 @@
std::unique_ptr<OatFile> oat_file(OatFile::OpenFromVdex(MakeNonOwningPointerVector(dex_files),
std::move(vdex_file),
dex_location));
- DCHECK(oat_file != nullptr);
- VLOG(class_linker) << "Registering " << oat_file->GetLocation();
- *out_oat_file = RegisterOatFile(std::move(oat_file));
+ if (oat_file != nullptr) {
+ VLOG(class_linker) << "Registering " << oat_file->GetLocation();
+ *out_oat_file = RegisterOatFile(std::move(oat_file));
+ }
return dex_files;
}
diff --git a/runtime/verifier/verifier_deps.cc b/runtime/verifier/verifier_deps.cc
index 219627f..d06e3d4 100644
--- a/runtime/verifier/verifier_deps.cc
+++ b/runtime/verifier/verifier_deps.cc
@@ -45,9 +45,6 @@
}
}
-VerifierDeps::VerifierDeps(const std::vector<const DexFile*>& dex_files)
- : VerifierDeps(dex_files, /*output_only=*/ true) {}
-
// Perform logical OR on two bit vectors and assign back to LHS, i.e. `to_update |= other`.
// Size of the two vectors must be equal.
// Size of `other` must be equal to size of `to_update`.
@@ -581,11 +578,6 @@
namespace {
-static inline uint32_t DecodeUint32WithOverflowCheck(const uint8_t** in, const uint8_t* end) {
- CHECK_LT(*in, end);
- return DecodeUnsignedLeb128(in);
-}
-
template<typename T> inline uint32_t Encode(T in);
template<> inline uint32_t Encode<uint16_t>(uint16_t in) {
@@ -623,10 +615,14 @@
}
template<typename T1, typename T2>
-static inline void DecodeTuple(const uint8_t** in, const uint8_t* end, std::tuple<T1, T2>* t) {
- T1 v1 = Decode<T1>(DecodeUint32WithOverflowCheck(in, end));
- T2 v2 = Decode<T2>(DecodeUint32WithOverflowCheck(in, end));
- *t = std::make_tuple(v1, v2);
+static inline bool DecodeTuple(const uint8_t** in, const uint8_t* end, std::tuple<T1, T2>* t) {
+ uint32_t v1, v2;
+ if (UNLIKELY(!DecodeUnsignedLeb128Checked(in, end, &v1)) ||
+ UNLIKELY(!DecodeUnsignedLeb128Checked(in, end, &v2))) {
+ return false;
+ }
+ *t = std::make_tuple(Decode<T1>(v1), Decode<T2>(v2));
+ return true;
}
template<typename T1, typename T2, typename T3>
@@ -637,11 +633,15 @@
}
template<typename T1, typename T2, typename T3>
-static inline void DecodeTuple(const uint8_t** in, const uint8_t* end, std::tuple<T1, T2, T3>* t) {
- T1 v1 = Decode<T1>(DecodeUint32WithOverflowCheck(in, end));
- T2 v2 = Decode<T2>(DecodeUint32WithOverflowCheck(in, end));
- T3 v3 = Decode<T3>(DecodeUint32WithOverflowCheck(in, end));
- *t = std::make_tuple(v1, v2, v3);
+static inline bool DecodeTuple(const uint8_t** in, const uint8_t* end, std::tuple<T1, T2, T3>* t) {
+ uint32_t v1, v2, v3;
+ if (UNLIKELY(!DecodeUnsignedLeb128Checked(in, end, &v1)) ||
+ UNLIKELY(!DecodeUnsignedLeb128Checked(in, end, &v2)) ||
+ UNLIKELY(!DecodeUnsignedLeb128Checked(in, end, &v3))) {
+ return false;
+ }
+ *t = std::make_tuple(Decode<T1>(v1), Decode<T2>(v2), Decode<T3>(v3));
+ return true;
}
template<typename T>
@@ -652,15 +652,23 @@
}
}
-template<typename T>
-static inline void DecodeSet(const uint8_t** in, const uint8_t* end, std::set<T>* set) {
+template<bool kFillSet, typename T>
+static inline bool DecodeSet(const uint8_t** in, const uint8_t* end, std::set<T>* set) {
DCHECK(set->empty());
- size_t num_entries = DecodeUint32WithOverflowCheck(in, end);
- for (size_t i = 0; i < num_entries; ++i) {
- T tuple;
- DecodeTuple(in, end, &tuple);
- set->emplace(tuple);
+ uint32_t num_entries;
+ if (UNLIKELY(!DecodeUnsignedLeb128Checked(in, end, &num_entries))) {
+ return false;
}
+ for (uint32_t i = 0; i < num_entries; ++i) {
+ T tuple;
+ if (UNLIKELY(!DecodeTuple(in, end, &tuple))) {
+ return false;
+ }
+ if (kFillSet) {
+ set->emplace(tuple);
+ }
+ }
+ return true;
}
static inline void EncodeUint16SparseBitVector(std::vector<uint8_t>* out,
@@ -675,18 +683,31 @@
}
}
-static inline void DecodeUint16SparseBitVector(const uint8_t** in,
+template<bool kFillVector>
+static inline bool DecodeUint16SparseBitVector(const uint8_t** in,
const uint8_t* end,
- std::vector<bool>* vector,
- bool sparse_value) {
- DCHECK(IsUint<16>(vector->size()));
- std::fill(vector->begin(), vector->end(), !sparse_value);
- size_t num_entries = DecodeUint32WithOverflowCheck(in, end);
- for (size_t i = 0; i < num_entries; ++i) {
- uint16_t idx = Decode<uint16_t>(DecodeUint32WithOverflowCheck(in, end));
- DCHECK_LT(idx, vector->size());
- (*vector)[idx] = sparse_value;
+ size_t num_class_defs,
+ bool sparse_value,
+ /*out*/std::vector<bool>* vector) {
+ if (kFillVector) {
+ DCHECK_EQ(vector->size(), num_class_defs);
+ DCHECK(IsUint<16>(vector->size()));
+ std::fill(vector->begin(), vector->end(), !sparse_value);
}
+ uint32_t num_entries;
+ if (UNLIKELY(!DecodeUnsignedLeb128Checked(in, end, &num_entries))) {
+ return false;
+ }
+ for (uint32_t i = 0; i < num_entries; ++i) {
+ uint32_t idx;
+ if (UNLIKELY(!DecodeUnsignedLeb128Checked(in, end, &idx)) || idx >= vector->size()) {
+ return false;
+ }
+ if (kFillVector) {
+ (*vector)[idx] = sparse_value;
+ }
+ }
+ return true;
}
static inline void EncodeStringVector(std::vector<uint8_t>* out,
@@ -700,18 +721,31 @@
}
}
-static inline void DecodeStringVector(const uint8_t** in,
+template<bool kFillVector>
+static inline bool DecodeStringVector(const uint8_t** in,
const uint8_t* end,
std::vector<std::string>* strings) {
DCHECK(strings->empty());
- size_t num_strings = DecodeUint32WithOverflowCheck(in, end);
- strings->reserve(num_strings);
- for (size_t i = 0; i < num_strings; ++i) {
- CHECK_LT(*in, end);
- const char* string_start = reinterpret_cast<const char*>(*in);
- strings->emplace_back(std::string(string_start));
- *in += strings->back().length() + 1;
+ uint32_t num_strings;
+ if (UNLIKELY(!DecodeUnsignedLeb128Checked(in, end, &num_strings))) {
+ return false;
}
+ if (kFillVector) {
+ strings->reserve(num_strings);
+ }
+ for (uint32_t i = 0; i < num_strings; ++i) {
+ const char* string_start = reinterpret_cast<const char*>(*in);
+ const char* string_end = reinterpret_cast<const char*>(memchr(string_start, 0, end - *in));
+ if (UNLIKELY(string_end == nullptr)) {
+ return false;
+ }
+ size_t string_length = string_end - string_start;
+ if (kFillVector) {
+ strings->emplace_back(string_start, string_length);
+ }
+ *in += string_length + 1;
+ }
+ return true;
}
static inline std::string ToHex(uint32_t value) {
@@ -737,60 +771,79 @@
}
}
-void VerifierDeps::DecodeDexFileDeps(DexFileDeps& deps,
+template <bool kOnlyVerifiedClasses>
+bool VerifierDeps::DecodeDexFileDeps(DexFileDeps& deps,
const uint8_t** data_start,
- const uint8_t* data_end) {
- DecodeStringVector(data_start, data_end, &deps.strings_);
- DecodeSet(data_start, data_end, &deps.assignable_types_);
- DecodeSet(data_start, data_end, &deps.unassignable_types_);
- DecodeSet(data_start, data_end, &deps.classes_);
- DecodeSet(data_start, data_end, &deps.fields_);
- DecodeSet(data_start, data_end, &deps.methods_);
- DecodeUint16SparseBitVector(data_start,
- data_end,
- &deps.verified_classes_,
- /* sparse_value= */ false);
- DecodeUint16SparseBitVector(data_start,
- data_end,
- &deps.redefined_classes_,
- /* sparse_value= */ true);
+ const uint8_t* data_end,
+ size_t num_class_defs) {
+ return
+ DecodeStringVector</*kFillVector=*/ !kOnlyVerifiedClasses>(
+ data_start, data_end, &deps.strings_) &&
+ DecodeSet</*kFillSet=*/ !kOnlyVerifiedClasses>(
+ data_start, data_end, &deps.assignable_types_) &&
+ DecodeSet</*kFillSet=*/ !kOnlyVerifiedClasses>(
+ data_start, data_end, &deps.unassignable_types_) &&
+ DecodeSet</*kFillSet=*/ !kOnlyVerifiedClasses>(data_start, data_end, &deps.classes_) &&
+ DecodeSet</*kFillSet=*/ !kOnlyVerifiedClasses>(data_start, data_end, &deps.fields_) &&
+ DecodeSet</*kFillSet=*/ !kOnlyVerifiedClasses>(data_start, data_end, &deps.methods_) &&
+ DecodeUint16SparseBitVector</*kFillVector=*/ true>(
+ data_start, data_end, num_class_defs, /*sparse_value=*/ false, &deps.verified_classes_) &&
+ DecodeUint16SparseBitVector</*kFillVector=*/ !kOnlyVerifiedClasses>(
+ data_start, data_end, num_class_defs, /*sparse_value=*/ true, &deps.redefined_classes_);
}
-VerifierDeps::VerifierDeps(const std::vector<const DexFile*>& dex_files,
- ArrayRef<const uint8_t> data)
- : VerifierDeps(dex_files, /*output_only=*/ false) {
+bool VerifierDeps::ParseStoredData(const std::vector<const DexFile*>& dex_files,
+ ArrayRef<const uint8_t> data) {
if (data.empty()) {
// Return eagerly, as the first thing we expect from VerifierDeps data is
// the number of created strings, even if there is no dependency.
// Currently, only the boot image does not have any VerifierDeps data.
- return;
+ return true;
}
const uint8_t* data_start = data.data();
const uint8_t* data_end = data_start + data.size();
for (const DexFile* dex_file : dex_files) {
DexFileDeps* deps = GetDexFileDeps(*dex_file);
- DecodeDexFileDeps(*deps, &data_start, data_end);
+ size_t num_class_defs = dex_file->NumClassDefs();
+ if (UNLIKELY(!DecodeDexFileDeps</*kOnlyVerifiedClasses=*/ false>(*deps,
+ &data_start,
+ data_end,
+ num_class_defs))) {
+ LOG(ERROR) << "Failed to parse dex file dependencies for " << dex_file->GetLocation();
+ return false;
+ }
}
- CHECK_LE(data_start, data_end);
+ // TODO: We should check that `data_start == data_end`. Why are we passing excessive data?
+ return true;
}
-std::vector<std::vector<bool>> VerifierDeps::ParseVerifiedClasses(
+bool VerifierDeps::ParseVerifiedClasses(
const std::vector<const DexFile*>& dex_files,
- ArrayRef<const uint8_t> data) {
+ ArrayRef<const uint8_t> data,
+ /*out*/std::vector<std::vector<bool>>* verified_classes_per_dex) {
DCHECK(!data.empty());
DCHECK(!dex_files.empty());
+ DCHECK(verified_classes_per_dex->empty());
- std::vector<std::vector<bool>> verified_classes_per_dex;
- verified_classes_per_dex.reserve(dex_files.size());
+ verified_classes_per_dex->reserve(dex_files.size());
const uint8_t* data_start = data.data();
const uint8_t* data_end = data_start + data.size();
for (const DexFile* dex_file : dex_files) {
- DexFileDeps deps(dex_file->NumClassDefs());
- DecodeDexFileDeps(deps, &data_start, data_end);
- verified_classes_per_dex.push_back(std::move(deps.verified_classes_));
+ DexFileDeps deps(/*num_class_defs=*/ 0u); // Do not initialize sparse bool vectors.
+ size_t num_class_defs = dex_file->NumClassDefs();
+ deps.verified_classes_.resize(num_class_defs);
+ if (UNLIKELY(!DecodeDexFileDeps</*kOnlyVerifiedClasses=*/ true>(deps,
+ &data_start,
+ data_end,
+ num_class_defs))) {
+ LOG(ERROR) << "Failed to parse dex file dependencies for " << dex_file->GetLocation();
+ return false;
+ }
+ verified_classes_per_dex->push_back(std::move(deps.verified_classes_));
}
- return verified_classes_per_dex;
+ // TODO: We should check that `data_start == data_end`. Why are we passing excessive data?
+ return true;
}
bool VerifierDeps::Equals(const VerifierDeps& rhs) const {
diff --git a/runtime/verifier/verifier_deps.h b/runtime/verifier/verifier_deps.h
index 3980ca2..326ee53 100644
--- a/runtime/verifier/verifier_deps.h
+++ b/runtime/verifier/verifier_deps.h
@@ -58,9 +58,10 @@
// changes in the classpath.
class VerifierDeps {
public:
- explicit VerifierDeps(const std::vector<const DexFile*>& dex_files);
+ explicit VerifierDeps(const std::vector<const DexFile*>& dex_files, bool output_only = true);
- VerifierDeps(const std::vector<const DexFile*>& dex_files, ArrayRef<const uint8_t> data);
+ // Fill dependencies from stored data. Returns true on success, false on failure.
+ bool ParseStoredData(const std::vector<const DexFile*>& dex_files, ArrayRef<const uint8_t> data);
// Merge `other` into this `VerifierDeps`'. `other` and `this` must be for the
// same set of dex files.
@@ -147,9 +148,10 @@
// Parses raw VerifierDeps data to extract bitvectors of which class def indices
// were verified or not. The given `dex_files` must match the order and count of
// dex files used to create the VerifierDeps.
- static std::vector<std::vector<bool>> ParseVerifiedClasses(
+ static bool ParseVerifiedClasses(
const std::vector<const DexFile*>& dex_files,
- ArrayRef<const uint8_t> data);
+ ArrayRef<const uint8_t> data,
+ /*out*/std::vector<std::vector<bool>>* verified_classes_per_dex);
private:
static constexpr uint16_t kUnresolvedMarker = static_cast<uint16_t>(-1);
@@ -239,12 +241,13 @@
bool Equals(const DexFileDeps& rhs) const;
};
- VerifierDeps(const std::vector<const DexFile*>& dex_files, bool output_only);
-
// Helper function to share DexFileDeps decoding code.
- static void DecodeDexFileDeps(DexFileDeps& deps,
+ // Returns true on success, false on failure.
+ template <bool kOnlyVerifiedClasses>
+ static bool DecodeDexFileDeps(DexFileDeps& deps,
const uint8_t** data_start,
- const uint8_t* data_end);
+ const uint8_t* data_end,
+ size_t num_class_defs);
// Finds the DexFileDep instance associated with `dex_file`, or nullptr if
// `dex_file` is not reported as being compiled.