Handle gracefully profiles with invalid classes or methods
Bug: 38410980
Test: m test-art-host-run-test-707
(cherry picked from commit 08556886a16ff2bb9fc3f184ac699de21c0369cd)
Merged-In: Id28aa1eb333f0f4843722d091c6c7a877c0c78f6
Change-Id: I64f8e3baa7860571e3dc249ce218d4a0eb59d84d
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 2f09bc8..79f5091 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -664,6 +664,12 @@
ObjPtr<mirror::DexCache> dex_cache =
dex_profile_index_to_dex_cache[class_ref.dex_profile_index];
DCHECK(dex_cache != nullptr);
+
+ if (!dex_cache->GetDexFile()->IsTypeIndexValid(class_ref.type_index)) {
+ VLOG(compiler) << "Profile data corrupt: type index " << class_ref.type_index
+ << "is invalid in location" << dex_cache->GetDexFile()->GetLocation();
+ return kInlineCacheNoData;
+ }
ObjPtr<mirror::Class> clazz = ClassLinker::LookupResolvedType(
class_ref.type_index,
dex_cache,
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 48c9910..8b0b45d 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1433,12 +1433,8 @@
Runtime* runtime = Runtime::Current();
CHECK(runtime != nullptr);
// Filter out class path classes since we don't want to include these in the image.
- std::unordered_set<std::string> dex_files_locations;
- for (const DexFile* dex_file : dex_files_) {
- dex_files_locations.insert(dex_file->GetLocation());
- }
std::set<DexCacheResolvedClasses> resolved_classes(
- profile_compilation_info_->GetResolvedClasses(dex_files_locations));
+ profile_compilation_info_->GetResolvedClasses(dex_files_));
image_classes_.reset(new std::unordered_set<std::string>(
runtime->GetClassLinker()->GetClassDescriptorsForResolvedClasses(resolved_classes)));
VLOG(compiler) << "Loaded " << image_classes_->size()
diff --git a/profman/profile_assistant_test.cc b/profman/profile_assistant_test.cc
index 41b9f99..1c32898 100644
--- a/profman/profile_assistant_test.cc
+++ b/profman/profile_assistant_test.cc
@@ -759,4 +759,63 @@
CheckProfileInfo(profile1, info1);
}
+TEST_F(ProfileAssistantTest, TestProfileCreateWithInvalidData) {
+ // Create the profile content.
+ std::vector<std::string> profile_methods = {
+ "LTestInline;->inlineMonomorphic(LSuper;)I+invalid_class",
+ "LTestInline;->invalid_method",
+ "invalid_class"
+ };
+ std::string input_file_contents;
+ for (std::string& m : profile_methods) {
+ input_file_contents += m + std::string("\n");
+ }
+
+ // Create the profile and save it to disk.
+ ScratchFile profile_file;
+ std::string dex_filename = GetTestDexFileName("ProfileTestMultiDex");
+ ASSERT_TRUE(CreateProfile(input_file_contents,
+ profile_file.GetFilename(),
+ dex_filename));
+
+ // Load the profile from disk.
+ ProfileCompilationInfo info;
+ profile_file.GetFile()->ResetOffset();
+ ASSERT_TRUE(info.Load(GetFd(profile_file)));
+
+ // Load the dex files and verify that the profile contains the expected methods info.
+ ScopedObjectAccess soa(Thread::Current());
+ jobject class_loader = LoadDex("ProfileTestMultiDex");
+ ASSERT_NE(class_loader, nullptr);
+
+ ArtMethod* inline_monomorphic = GetVirtualMethod(class_loader,
+ "LTestInline;",
+ "inlineMonomorphic");
+ const DexFile* dex_file = inline_monomorphic->GetDexFile();
+
+ // Verify that the inline cache contains the invalid type.
+ std::unique_ptr<ProfileCompilationInfo::OfflineProfileMethodInfo> pmi =
+ info.GetMethod(dex_file->GetLocation(),
+ dex_file->GetLocationChecksum(),
+ inline_monomorphic->GetDexMethodIndex());
+ ASSERT_TRUE(pmi != nullptr);
+ ASSERT_EQ(pmi->inline_caches->size(), 1u);
+ const ProfileCompilationInfo::DexPcData& dex_pc_data = pmi->inline_caches->begin()->second;
+ dex::TypeIndex invalid_class_index(std::numeric_limits<uint16_t>::max() - 1);
+ ASSERT_EQ(1u, dex_pc_data.classes.size());
+ ASSERT_EQ(invalid_class_index, dex_pc_data.classes.begin()->type_index);
+
+ // Verify that the start-up classes contain the invalid class.
+ std::set<dex::TypeIndex> classes;
+ std::set<uint16_t> methods;
+ ASSERT_TRUE(info.GetClassesAndMethods(*dex_file, &classes, &methods));
+ ASSERT_EQ(1u, classes.size());
+ ASSERT_TRUE(classes.find(invalid_class_index) != classes.end());
+
+ // Verify that the invalid method is in the profile.
+ ASSERT_EQ(2u, methods.size());
+ uint16_t invalid_method_index = std::numeric_limits<uint16_t>::max() - 1;
+ ASSERT_TRUE(methods.find(invalid_method_index) != methods.end());
+}
+
} // namespace art
diff --git a/profman/profman.cc b/profman/profman.cc
index f60aa32..5bb019d 100644
--- a/profman/profman.cc
+++ b/profman/profman.cc
@@ -143,6 +143,8 @@
// Separators used when parsing human friendly representation of profiles.
static const std::string kMethodSep = "->";
static const std::string kMissingTypesMarker = "missing_types";
+static const std::string kInvalidClassDescriptor = "invalid_class";
+static const std::string kInvalidMethod = "invalid_method";
static const std::string kClassAllMethods = "*";
static constexpr char kProfileParsingInlineChacheSep = '+';
static constexpr char kProfileParsingTypeSep = ',';
@@ -561,8 +563,23 @@
bool FindClass(const std::vector<std::unique_ptr<const DexFile>>& dex_files,
const std::string& klass_descriptor,
/*out*/ProfileMethodInfo::ProfileClassReference* class_ref) {
+ constexpr uint16_t kInvalidTypeIndex = std::numeric_limits<uint16_t>::max() - 1;
for (const std::unique_ptr<const DexFile>& dex_file_ptr : dex_files) {
const DexFile* dex_file = dex_file_ptr.get();
+ if (klass_descriptor == kInvalidClassDescriptor) {
+ if (kInvalidTypeIndex >= dex_file->NumTypeIds()) {
+ // The dex file does not contain all possible type ids which leaves us room
+ // to add an "invalid" type id.
+ class_ref->dex_file = dex_file;
+ class_ref->type_index = dex::TypeIndex(kInvalidTypeIndex);
+ return true;
+ } else {
+ // The dex file contains all possible type ids. We don't have any free type id
+ // that we can use as invalid.
+ continue;
+ }
+ }
+
const DexFile::TypeId* type_id = dex_file->FindTypeId(klass_descriptor.c_str());
if (type_id == nullptr) {
continue;
@@ -582,14 +599,23 @@
// Find the method specified by method_spec in the class class_ref.
uint32_t FindMethodIndex(const ProfileMethodInfo::ProfileClassReference& class_ref,
const std::string& method_spec) {
+ const DexFile* dex_file = class_ref.dex_file;
+ if (method_spec == kInvalidMethod) {
+ constexpr uint16_t kInvalidMethodIndex = std::numeric_limits<uint16_t>::max() - 1;
+ return kInvalidMethodIndex >= dex_file->NumMethodIds()
+ ? kInvalidMethodIndex
+ : DexFile::kDexNoIndex;
+ }
+
std::vector<std::string> name_and_signature;
Split(method_spec, kProfileParsingFirstCharInSignature, &name_and_signature);
if (name_and_signature.size() != 2) {
LOG(ERROR) << "Invalid method name and signature " << method_spec;
+ return DexFile::kDexNoIndex;
}
+
const std::string& name = name_and_signature[0];
const std::string& signature = kProfileParsingFirstCharInSignature + name_and_signature[1];
- const DexFile* dex_file = class_ref.dex_file;
const DexFile::StringId* name_id = dex_file->FindStringId(name.c_str());
if (name_id == nullptr) {
@@ -655,9 +681,12 @@
// The possible line formats are:
// "LJustTheCass;".
// "LTestInline;->inlinePolymorphic(LSuper;)I+LSubA;,LSubB;,LSubC;".
+ // "LTestInline;->inlinePolymorphic(LSuper;)I+LSubA;,LSubB;,invalid_class".
// "LTestInline;->inlineMissingTypes(LSuper;)I+missing_types".
// "LTestInline;->inlineNoInlineCaches(LSuper;)I".
// "LTestInline;->*".
+ // "invalid_class".
+ // "LTestInline;->invalid_method".
// The method and classes are searched only in the given dex files.
bool ProcessLine(const std::vector<std::unique_ptr<const DexFile>>& dex_files,
const std::string& line,
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 694c113..146030b 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -8948,6 +8948,12 @@
<< info.GetClasses().size() << " classes";
DCHECK_EQ(dex_file->GetLocationChecksum(), info.GetLocationChecksum());
for (dex::TypeIndex type_idx : info.GetClasses()) {
+ if (!dex_file->IsTypeIndexValid(type_idx)) {
+ // Something went bad. The profile is probably corrupted. Abort and return an emtpy set.
+ LOG(WARNING) << "Corrupted profile: invalid type index "
+ << type_idx.index_ << " in dex " << location;
+ return std::unordered_set<std::string>();
+ }
const DexFile::TypeId& type_id = dex_file->GetTypeId(type_idx);
const char* descriptor = dex_file->GetTypeDescriptor(type_id);
ret.insert(descriptor);
diff --git a/runtime/dex_file.h b/runtime/dex_file.h
index 36c7341..591ba42 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -582,6 +582,10 @@
return header_->type_ids_size_;
}
+ bool IsTypeIndexValid(dex::TypeIndex idx) const {
+ return idx.IsValid() && idx.index_ < NumTypeIds();
+ }
+
// Returns the TypeId at the specified index.
const TypeId& GetTypeId(dex::TypeIndex idx) const {
DCHECK_LT(idx.index_, NumTypeIds()) << GetLocation();
diff --git a/runtime/jit/profile_compilation_info.cc b/runtime/jit/profile_compilation_info.cc
index f676369..1a950d1 100644
--- a/runtime/jit/profile_compilation_info.cc
+++ b/runtime/jit/profile_compilation_info.cc
@@ -1167,16 +1167,24 @@
}
std::set<DexCacheResolvedClasses> ProfileCompilationInfo::GetResolvedClasses(
- const std::unordered_set<std::string>& dex_files_locations) const {
- std::unordered_map<std::string, std::string> key_to_location_map;
- for (const std::string& location : dex_files_locations) {
- key_to_location_map.emplace(GetProfileDexFileKey(location), location);
+ const std::vector<const DexFile*>& dex_files) const {
+ 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);
}
std::set<DexCacheResolvedClasses> ret;
for (const DexFileData* dex_data : info_) {
- const auto& it = key_to_location_map.find(dex_data->profile_key);
- if (it != key_to_location_map.end()) {
- DexCacheResolvedClasses classes(it->second, it->second, dex_data->checksum);
+ const auto it = key_to_dex_file.find(dex_data->profile_key);
+ if (it != key_to_dex_file.end()) {
+ const DexFile* dex_file = it->second;
+ const std::string& dex_location = dex_file->GetLocation();
+ if (dex_data->checksum != it->second->GetLocationChecksum()) {
+ LOG(ERROR) << "Dex checksum mismatch when getting resolved classes from profile for "
+ << "location " << dex_location << " (checksum=" << dex_file->GetLocationChecksum()
+ << ", profile checksum=" << dex_data->checksum;
+ return std::set<DexCacheResolvedClasses>();
+ }
+ DexCacheResolvedClasses classes(dex_location, dex_location, dex_data->checksum);
classes.AddClasses(dex_data->class_set.begin(), dex_data->class_set.end());
ret.insert(classes);
}
diff --git a/runtime/jit/profile_compilation_info.h b/runtime/jit/profile_compilation_info.h
index 446bef8..ec7822d 100644
--- a/runtime/jit/profile_compilation_info.h
+++ b/runtime/jit/profile_compilation_info.h
@@ -259,7 +259,7 @@
// Return the class descriptors for all of the classes in the profiles' class sets.
std::set<DexCacheResolvedClasses> GetResolvedClasses(
- const std::unordered_set<std::string>& dex_files_locations) const;
+ const std::vector<const DexFile*>& dex_files_) const;
// Return the profile key associated with the given dex location.
static std::string GetProfileDexFileKey(const std::string& dex_location);
diff --git a/test/707-checker-invalid-profile/expected.txt b/test/707-checker-invalid-profile/expected.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/707-checker-invalid-profile/expected.txt
diff --git a/test/707-checker-invalid-profile/info.txt b/test/707-checker-invalid-profile/info.txt
new file mode 100644
index 0000000..4b59eff
--- /dev/null
+++ b/test/707-checker-invalid-profile/info.txt
@@ -0,0 +1,2 @@
+Verify the compiler can handle an invalid profile with methods
+and classes exceeding the dex file limits.
diff --git a/test/707-checker-invalid-profile/profile b/test/707-checker-invalid-profile/profile
new file mode 100644
index 0000000..5979dd2
--- /dev/null
+++ b/test/707-checker-invalid-profile/profile
@@ -0,0 +1,4 @@
+LMain;->attemptInlineMonomorphic(LMain;)I+invalid_class
+LMain;->attemptInlinePolymorphic(LMain;)I+LMain;,invalid_class
+LMain;->invalid_method
+invalid_class
\ No newline at end of file
diff --git a/test/707-checker-invalid-profile/run b/test/707-checker-invalid-profile/run
new file mode 100644
index 0000000..146e180
--- /dev/null
+++ b/test/707-checker-invalid-profile/run
@@ -0,0 +1,17 @@
+#!/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.
+
+exec ${RUN} $@ --profile -Xcompiler-option --compiler-filter=speed-profile
diff --git a/test/707-checker-invalid-profile/src/Main.java b/test/707-checker-invalid-profile/src/Main.java
new file mode 100644
index 0000000..003f0e8
--- /dev/null
+++ b/test/707-checker-invalid-profile/src/Main.java
@@ -0,0 +1,43 @@
+/*
+ * 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.
+ */
+
+class Unrelated {
+}
+
+public class Main {
+
+ /// CHECK-START: int Main.attemptInlineMonomorphic(Main) inliner (after)
+ /// CHECK: InvokeVirtual method_name:Main.getValue
+ public static int attemptInlineMonomorphic(Main a) {
+ return a.getValue();
+ }
+
+ /// CHECK-START: int Main.attemptInlinePolymorphic(Main) inliner (after)
+ /// CHECK: InvokeVirtual method_name:Main.getValue
+ public static int attemptInlinePolymorphic(Main a) {
+ return a.getValue();
+ }
+
+ public int getValue() {
+ return 42;
+ }
+
+ public static void main(String[] args) {
+ attemptInlineMonomorphic(new Main());
+ attemptInlinePolymorphic(new Main());
+ }
+
+}
