Merge "Revert^2 "Verify profile wrt dex file in dex2oat"""
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 21d3895..28e1d94 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -712,6 +712,10 @@
}
}
+ bool VerifyProfileData() {
+ return profile_compilation_info_->VerifyProfileData(dex_files_);
+ }
+
void ParseInstructionSetVariant(const StringPiece& option, ParserOptions* parser_options) {
DCHECK(option.starts_with("--instruction-set-variant="));
StringPiece str = option.substr(strlen("--instruction-set-variant=")).data();
@@ -3103,6 +3107,19 @@
return setup_code;
}
+ // TODO: Due to the cyclic dependencies, profile loading and verifying are
+ // being done separately. Refactor and place the two next to each other.
+ // If verification fails, we don't abort the compilation and instead log an
+ // error.
+ // TODO(b/62602192, b/65260586): We should consider aborting compilation when
+ // the profile verification fails.
+ // Note: If dex2oat fails, installd will remove the oat files causing the app
+ // to fallback to apk with possible in-memory extraction. We want to avoid
+ // that, and thus we're lenient towards profile corruptions.
+ if (dex2oat->UseProfile()) {
+ dex2oat->VerifyProfileData();
+ }
+
// Helps debugging on device. Can be used to determine which dalvikvm instance invoked a dex2oat
// instance. Used by tools/bisection_search/bisection_search.py.
VLOG(compiler) << "Running dex2oat (parent PID = " << getppid() << ")";
diff --git a/runtime/jit/profile_compilation_info.cc b/runtime/jit/profile_compilation_info.cc
index 12fa49e..1eba306 100644
--- a/runtime/jit/profile_compilation_info.cc
+++ b/runtime/jit/profile_compilation_info.cc
@@ -1053,6 +1053,89 @@
}
}
+bool ProfileCompilationInfo::VerifyProfileData(const std::vector<const DexFile*>& dex_files) {
+ std::unordered_map<std::string, const DexFile*> key_to_dex_file;
+ for (const DexFile* dex_file : dex_files) {
+ key_to_dex_file.emplace(GetProfileDexFileKey(dex_file->GetLocation()), dex_file);
+ }
+ for (const DexFileData* dex_data : info_) {
+ const auto it = key_to_dex_file.find(dex_data->profile_key);
+ if (it == key_to_dex_file.end()) {
+ // It is okay if profile contains data for additional dex files.
+ continue;
+ }
+ const DexFile* dex_file = it->second;
+ const std::string& dex_location = dex_file->GetLocation();
+ if (!ChecksumMatch(dex_data->checksum, dex_file->GetLocationChecksum())) {
+ LOG(ERROR) << "Dex checksum mismatch while verifying profile "
+ << "dex location " << dex_location << " (checksum="
+ << dex_file->GetLocationChecksum() << ", profile checksum="
+ << dex_data->checksum;
+ return false;
+ }
+ // Verify method_encoding.
+ for (const auto& method_it : dex_data->method_map) {
+ size_t method_id = (size_t)(method_it.first);
+ if (method_id >= dex_file->NumMethodIds()) {
+ LOG(ERROR) << "Invalid method id in profile file. dex location="
+ << dex_location << " method_id=" << method_id << " NumMethodIds="
+ << dex_file->NumMethodIds();
+ return false;
+ }
+
+ // Verify class indices of inline caches.
+ const InlineCacheMap &inline_cache_map = method_it.second;
+ for (const auto& inline_cache_it : inline_cache_map) {
+ const DexPcData dex_pc_data = inline_cache_it.second;
+ if (dex_pc_data.is_missing_types || dex_pc_data.is_megamorphic) {
+ // No class indices to verify.
+ continue;
+ }
+
+ const ClassSet &classes = dex_pc_data.classes;
+ SafeMap<uint8_t, std::vector<dex::TypeIndex>> dex_to_classes_map;
+ // Group the classes by dex. We expect that most of the classes will come from
+ // the same dex, so this will be more efficient than encoding the dex index
+ // for each class reference.
+ GroupClassesByDex(classes, &dex_to_classes_map);
+ for (const auto &dex_it : dex_to_classes_map) {
+ uint8_t dex_profile_index = dex_it.first;
+ const auto dex_file_inline_cache_it = key_to_dex_file.find(
+ info_[dex_profile_index]->profile_key);
+ if (dex_file_inline_cache_it == key_to_dex_file.end()) {
+ // It is okay if profile contains data for additional dex files.
+ continue;
+ }
+ const DexFile *dex_file_for_inline_cache_check = dex_file_inline_cache_it->second;
+ const std::vector<dex::TypeIndex> &dex_classes = dex_it.second;
+ for (size_t i = 0; i < dex_classes.size(); i++) {
+ if (dex_classes[i].index_ >= dex_file_for_inline_cache_check->NumTypeIds()) {
+ LOG(ERROR) << "Invalid inline cache in profile file. dex location="
+ << dex_location << " method_id=" << method_id
+ << " dex_profile_index="
+ << static_cast<uint16_t >(dex_profile_index) << " type_index="
+ << dex_classes[i].index_
+ << " NumTypeIds="
+ << dex_file_for_inline_cache_check->NumTypeIds();
+ return false;
+ }
+ }
+ }
+ }
+ }
+ // Verify class_ids.
+ for (const auto& class_id : dex_data->class_set) {
+ if (class_id.index_ >= dex_file->NumTypeIds()) {
+ LOG(ERROR) << "Invalid class id in profile file. dex_file location "
+ << dex_location << " class_id=" << class_id.index_ << " NumClassIds="
+ << dex_file->NumClassDefs();
+ return false;
+ }
+ }
+ }
+ return true;
+}
+
// TODO(calin): fail fast if the dex checksums don't match.
ProfileCompilationInfo::ProfileLoadSatus ProfileCompilationInfo::LoadInternal(
int fd, std::string* error, bool merge_classes) {
diff --git a/runtime/jit/profile_compilation_info.h b/runtime/jit/profile_compilation_info.h
index 009554c..09de29e 100644
--- a/runtime/jit/profile_compilation_info.h
+++ b/runtime/jit/profile_compilation_info.h
@@ -305,6 +305,15 @@
// If merge_classes is set to false, classes will not be merged/loaded.
bool Load(int fd, bool merge_classes = true);
+ // Verify integrity of the profile file with the provided dex files.
+ // If there exists a DexData object which maps to a dex_file, then it verifies that:
+ // - The checksums of the DexData and dex_file are equals.
+ // - No method id exceeds NumMethodIds corresponding to the dex_file.
+ // - No class id exceeds NumTypeIds corresponding to the dex_file.
+ // - For every inline_caches, class_ids does not exceed NumTypeIds corresponding to
+ // the dex_file they are in.
+ bool VerifyProfileData(const std::vector<const DexFile *> &dex_files);
+
// Load profile information from the given file
// If the current profile is non-empty the load will fail.
// If clear_if_invalid is true and the file is invalid the method clears the
diff --git a/test/707-checker-invalid-profile/check b/test/707-checker-invalid-profile/check
new file mode 100755
index 0000000..976afc4
--- /dev/null
+++ b/test/707-checker-invalid-profile/check
@@ -0,0 +1,21 @@
+#!/bin/bash
+#
+# Copyright (C) 2017 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# When profile verification fails, dex2oat logs an error. The following
+# command strips out the error message.
+grep -v -f $1 $2 > $1
+
+./default-check "$@"
diff --git a/test/707-checker-invalid-profile/expected.txt b/test/707-checker-invalid-profile/expected.txt
index e69de29..4d84c96 100644
--- a/test/707-checker-invalid-profile/expected.txt
+++ b/test/707-checker-invalid-profile/expected.txt
@@ -0,0 +1 @@
+Invalid inline cache in profile file.
