diff options
| -rw-r--r-- | compiler/dex/dex_to_dex_compiler.cc | 4 | ||||
| -rw-r--r-- | compiler/dex/dex_to_dex_decompiler_test.cc | 7 | ||||
| -rw-r--r-- | compiler/driver/compiler_driver.cc | 20 | ||||
| -rw-r--r-- | compiler/verifier_deps_test.cc | 5 | ||||
| -rw-r--r-- | libdexfile/Android.bp | 1 | ||||
| -rw-r--r-- | libdexfile/dex/class_accessor-inl.h | 76 | ||||
| -rw-r--r-- | libdexfile/dex/class_accessor.h | 138 | ||||
| -rw-r--r-- | libdexfile/dex/class_accessor_test.cc | 79 | ||||
| -rw-r--r-- | profman/profman.cc | 4 | ||||
| -rw-r--r-- | tools/dexanalyze/dexanalyze_experiments.cc | 4 |
10 files changed, 270 insertions, 68 deletions
diff --git a/compiler/dex/dex_to_dex_compiler.cc b/compiler/dex/dex_to_dex_compiler.cc index 68155d844a..fb6a72b1c5 100644 --- a/compiler/dex/dex_to_dex_compiler.cc +++ b/compiler/dex/dex_to_dex_compiler.cc @@ -635,13 +635,13 @@ void DexToDexCompiler::SetDexFiles(const std::vector<const DexFile*>& dex_files) std::unordered_set<const DexFile::CodeItem*> seen_code_items; for (const DexFile* dex_file : dex_files) { for (ClassAccessor accessor : dex_file->GetClasses()) { - accessor.VisitMethods([&](const ClassAccessor::Method& method) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { const DexFile::CodeItem* code_item = method.GetCodeItem(); // Detect the shared code items. if (!seen_code_items.insert(code_item).second) { shared_code_items_.insert(code_item); } - }); + } } } VLOG(compiler) << "Shared code items " << shared_code_items_.size(); diff --git a/compiler/dex/dex_to_dex_decompiler_test.cc b/compiler/dex/dex_to_dex_decompiler_test.cc index 082e6091d2..75de238211 100644 --- a/compiler/dex/dex_to_dex_decompiler_test.cc +++ b/compiler/dex/dex_to_dex_decompiler_test.cc @@ -85,10 +85,9 @@ class DexToDexDecompilerTest : public CommonCompilerTest { for (uint32_t i = 0; i < updated_dex_file->NumClassDefs(); ++i) { // Unquicken each method. ClassAccessor accessor(*updated_dex_file, updated_dex_file->GetClassDef(i)); - accessor.VisitMethods([&](const ClassAccessor::Method& method) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { CompiledMethod* compiled_method = compiler_driver_->GetCompiledMethod( - MethodReference(updated_dex_file, - method.GetIndex())); + method.GetReference()); ArrayRef<const uint8_t> table; if (compiled_method != nullptr) { table = compiled_method->GetVmapTable(); @@ -97,7 +96,7 @@ class DexToDexDecompilerTest : public CommonCompilerTest { *accessor.GetCodeItem(method), table, /* decompile_return_instruction */ true); - }); + } } // Make sure after unquickening we go back to the same contents as the original dex file. diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index decb330f3b..16f2d0f2cc 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -790,8 +790,7 @@ static void ResolveConstStrings(CompilerDriver* driver, // FIXME: Make sure that inlining honors this. b/26687569 continue; } - accessor.VisitMethods([&](const ClassAccessor::Method& method) - REQUIRES_SHARED(Locks::mutator_lock_) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { // Resolve const-strings in the code. Done to have deterministic allocation behavior. Right // now this is single-threaded for simplicity. // TODO: Collect the relevant string indices in parallel, then allocate them sequentially @@ -812,7 +811,7 @@ static void ResolveConstStrings(CompilerDriver* driver, break; } } - }); + } } } } @@ -880,10 +879,9 @@ static void InitializeTypeCheckBitstrings(CompilerDriver* driver, } // Direct and virtual methods. - accessor.VisitMethods([&](const ClassAccessor::Method& method) - REQUIRES_SHARED(Locks::mutator_lock_) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { InitializeTypeCheckBitstrings(driver, class_linker, dex_cache, *dex_file, method); - }); + } } } } @@ -1949,9 +1947,9 @@ bool CompilerDriver::FastVerify(jobject jclass_loader, // - We're only going to compile methods that did verify. // - Quickening will not do checkcast ellision. // TODO(ngeoffray): Reconsider this once we refactor compiler filters. - accessor.VisitMethods([&](const ClassAccessor::Method& method) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { verification_results_->CreateVerifiedMethodFor(method.GetReference()); - }); + } } } else if (!compiler_only_verifies) { // Make sure later compilation stages know they should not try to verify @@ -2747,12 +2745,12 @@ static void CompileDexFile(CompilerDriver* driver, // Compile direct and virtual methods. int64_t previous_method_idx = -1; - accessor.VisitMethods([&](const ClassAccessor::Method& method) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { const uint32_t method_idx = method.GetIndex(); if (method_idx == previous_method_idx) { // smali can create dex files with two encoded_methods sharing the same method_idx // http://code.google.com/p/smali/issues/detail?id=119 - return; + continue; } previous_method_idx = method_idx; compile_fn(soa.Self(), @@ -2767,7 +2765,7 @@ static void CompileDexFile(CompilerDriver* driver, dex_to_dex_compilation_level, compilation_enabled, dex_cache); - }); + } }; context.ForAllLambda(0, dex_file.NumClassDefs(), compile, thread_count); } diff --git a/compiler/verifier_deps_test.cc b/compiler/verifier_deps_test.cc index 103862beff..c0892ff466 100644 --- a/compiler/verifier_deps_test.cc +++ b/compiler/verifier_deps_test.cc @@ -155,8 +155,7 @@ class VerifierDepsTest : public CommonCompilerTest { bool has_failures = true; bool found_method = false; - accessor.VisitMethods([&](const ClassAccessor::Method& method) - REQUIRES_SHARED(Locks::mutator_lock_) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { ArtMethod* resolved_method = class_linker_->ResolveMethod<ClassLinker::ResolveMode::kNoChecks>( method.GetIndex(), @@ -186,7 +185,7 @@ class VerifierDepsTest : public CommonCompilerTest { has_failures = verifier.HasFailures(); found_method = true; } - }); + } CHECK(found_method) << "Expected to find method " << method_name; return !has_failures; } diff --git a/libdexfile/Android.bp b/libdexfile/Android.bp index 3818624d7a..06fd19e2fe 100644 --- a/libdexfile/Android.bp +++ b/libdexfile/Android.bp @@ -112,6 +112,7 @@ art_cc_test { ], srcs: [ "dex/art_dex_file_loader_test.cc", + "dex/class_accessor_test.cc", "dex/code_item_accessors_test.cc", "dex/compact_dex_file_test.cc", "dex/compact_offset_table_test.cc", diff --git a/libdexfile/dex/class_accessor-inl.h b/libdexfile/dex/class_accessor-inl.h index a082142366..49ca98d47f 100644 --- a/libdexfile/dex/class_accessor-inl.h +++ b/libdexfile/dex/class_accessor-inl.h @@ -50,6 +50,19 @@ inline const uint8_t* ClassAccessor::Field::Read(const uint8_t* ptr) { return ptr; } +template <typename DataType, typename Visitor> +inline const uint8_t* ClassAccessor::VisitMembers(size_t count, + const Visitor& visitor, + const uint8_t* ptr, + DataType* data) const { + DCHECK(data != nullptr); + for ( ; count != 0; --count) { + ptr = data->Read(ptr); + visitor(*data); + } + return ptr; +} + template <typename StaticFieldVisitor, typename InstanceFieldVisitor, typename DirectMethodVisitor, @@ -59,35 +72,15 @@ inline void ClassAccessor::VisitFieldsAndMethods( const InstanceFieldVisitor& instance_field_visitor, const DirectMethodVisitor& direct_method_visitor, const VirtualMethodVisitor& virtual_method_visitor) const { - const uint8_t* ptr = ptr_pos_; - { - Field data; - for (size_t i = 0; i < num_static_fields_; ++i) { - ptr = data.Read(ptr); - static_field_visitor(data); - } - } - { - Field data; - for (size_t i = 0; i < num_instance_fields_; ++i) { - ptr = data.Read(ptr); - instance_field_visitor(data); - } - } - { - Method data(dex_file_, /*is_static_or_direct*/ true); - for (size_t i = 0; i < num_direct_methods_; ++i) { - ptr = data.Read(ptr); - direct_method_visitor(data); - } - } - { - Method data(dex_file_, /*is_static_or_direct*/ false); - for (size_t i = 0; i < num_virtual_methods_; ++i) { - ptr = data.Read(ptr); - virtual_method_visitor(data); - } - } + Field field(dex_file_); + const uint8_t* ptr = VisitMembers(num_static_fields_, static_field_visitor, ptr_pos_, &field); + field.NextSection(); + ptr = VisitMembers(num_instance_fields_, instance_field_visitor, ptr, &field); + + Method method(dex_file_, /*is_static_or_direct*/ true); + ptr = VisitMembers(num_direct_methods_, direct_method_visitor, ptr, &method); + method.NextSection(); + ptr = VisitMembers(num_virtual_methods_, virtual_method_visitor, ptr, &method); } template <typename DirectMethodVisitor, @@ -110,12 +103,6 @@ inline void ClassAccessor::VisitFields(const StaticFieldVisitor& static_field_vi VoidFunctor()); } -// Visit direct and virtual methods. -template <typename MethodVisitor> -inline void ClassAccessor::VisitMethods(const MethodVisitor& method_visitor) const { - VisitMethods(method_visitor, method_visitor); -} - inline const DexFile::CodeItem* ClassAccessor::GetCodeItem(const Method& method) const { return dex_file_.GetCodeItem(method.GetCodeItemOffset()); } @@ -132,6 +119,25 @@ inline const DexFile::CodeItem* ClassAccessor::Method::GetCodeItem() const { return dex_file_.GetCodeItem(code_off_); } +inline IterationRange<ClassAccessor::DataIterator<ClassAccessor::Field>> ClassAccessor::GetFields() + const { + const uint32_t limit = num_static_fields_ + num_instance_fields_; + return { DataIterator<Field>(dex_file_, 0u, num_static_fields_, limit, ptr_pos_), + DataIterator<Field>(dex_file_, limit, num_static_fields_, limit, ptr_pos_) }; +} + +inline IterationRange<ClassAccessor::DataIterator<ClassAccessor::Method>> + ClassAccessor::GetMethods() const { + // Skip over the fields. + Field field(dex_file_); + const size_t skip_count = num_static_fields_ + num_instance_fields_; + const uint8_t* ptr_pos = VisitMembers(skip_count, VoidFunctor(), ptr_pos_, &field); + // Return the iterator pair for all the methods. + const uint32_t limit = num_direct_methods_ + num_virtual_methods_; + return { DataIterator<Method>(dex_file_, 0u, num_direct_methods_, limit, ptr_pos), + DataIterator<Method>(dex_file_, limit, num_direct_methods_, limit, ptr_pos) }; +} + } // namespace art #endif // ART_LIBDEXFILE_DEX_CLASS_ACCESSOR_INL_H_ diff --git a/libdexfile/dex/class_accessor.h b/libdexfile/dex/class_accessor.h index 72bc50b98c..7455704a7f 100644 --- a/libdexfile/dex/class_accessor.h +++ b/libdexfile/dex/class_accessor.h @@ -45,7 +45,7 @@ class ClassAccessor { return (GetAccessFlags() & kAccFinal) != 0; } - public: + protected: uint32_t index_ = 0u; uint32_t access_flags_ = 0u; }; @@ -72,8 +72,13 @@ class ClassAccessor { const DexFile::CodeItem* GetCodeItem() const; + bool IsStaticOrDirect() const { + return is_static_or_direct_; + } + private: - explicit Method(const DexFile& dex_file, bool is_static_or_direct) + explicit Method(const DexFile& dex_file, + bool is_static_or_direct = true) : dex_file_(dex_file), is_static_or_direct_(is_static_or_direct) {} @@ -94,8 +99,14 @@ class ClassAccessor { } } + void NextSection() { + DCHECK(is_static_or_direct_) << "Already in the virtual methods section"; + is_static_or_direct_ = false; + index_ = 0u; + } + const DexFile& dex_file_; - const bool is_static_or_direct_; + bool is_static_or_direct_ = true; uint32_t code_off_ = 0u; friend class ClassAccessor; @@ -103,12 +114,113 @@ class ClassAccessor { // A decoded version of the field of a class_data_item. class Field : public BaseItem { + public: + explicit Field(const DexFile& dex_file) : dex_file_(dex_file) {} + + const DexFile& GetDexFile() const { + return dex_file_; + } + private: const uint8_t* Read(const uint8_t* ptr); + void NextSection() { + index_ = 0u; + } + + const DexFile& dex_file_; friend class ClassAccessor; }; + template <typename DataType> + class DataIterator : public std::iterator<std::forward_iterator_tag, DataType> { + public: + using value_type = typename std::iterator<std::forward_iterator_tag, DataType>::value_type; + using difference_type = + typename std::iterator<std::forward_iterator_tag, value_type>::difference_type; + + DataIterator(const DexFile& dex_file, + uint32_t position, + uint32_t partition_pos, + uint32_t iterator_end, + const uint8_t* ptr_pos) + : data_(dex_file), + position_(position), + partition_pos_(partition_pos), + iterator_end_(iterator_end), + ptr_pos_(ptr_pos) { + ReadData(); + } + + bool IsValid() const { + return position_ < iterator_end_; + } + + // Value after modification. + DataIterator& operator++() { + ++position_; + ReadData(); + return *this; + } + + const value_type& operator*() const { + return data_; + } + + const value_type* operator->() const { + return &data_; + } + + bool operator==(const DataIterator& rhs) const { + DCHECK_EQ(&data_.dex_file_, &rhs.data_.dex_file_) << "Comparing different dex files."; + return position_ == rhs.position_; + } + + bool operator!=(const DataIterator& rhs) const { + return !(*this == rhs); + } + + bool operator<(const DataIterator& rhs) const { + DCHECK_EQ(&data_.dex_file_, &rhs.data_.dex_file_) << "Comparing different dex files."; + return position_ < rhs.position_; + } + + bool operator>(const DataIterator& rhs) const { + return rhs < *this; + } + + bool operator<=(const DataIterator& rhs) const { + return !(rhs < *this); + } + + bool operator>=(const DataIterator& rhs) const { + return !(*this < rhs); + } + + private: + // Read data at current position. + void ReadData() { + if (IsValid()) { + // At the end of the first section, go to the next section. + if (position_ == partition_pos_) { + data_.NextSection(); + } + DCHECK(ptr_pos_ != nullptr); + ptr_pos_ = data_.Read(ptr_pos_); + } + } + + DataType data_; + // Iterator position. + uint32_t position_; + // At partition_pos_, we go to the next section. + const uint32_t partition_pos_; + // At iterator_end_, the iterator is no longer valid. + const uint32_t iterator_end_; + // Internal data pointer. + const uint8_t* ptr_pos_; + }; + // Not explicit specifically for range-based loops. ALWAYS_INLINE ClassAccessor(const ClassIteratorData& data); @@ -118,7 +230,6 @@ class ClassAccessor { const DexFile::CodeItem* GetCodeItem(const Method& method) const; // Iterator data is not very iterator friendly, use visitors to get around this. - // No thread safety analysis since the visitor may require capabilities. template <typename StaticFieldVisitor, typename InstanceFieldVisitor, typename DirectMethodVisitor, @@ -126,8 +237,7 @@ class ClassAccessor { void VisitFieldsAndMethods(const StaticFieldVisitor& static_field_visitor, const InstanceFieldVisitor& instance_field_visitor, const DirectMethodVisitor& direct_method_visitor, - const VirtualMethodVisitor& virtual_method_visitor) const - NO_THREAD_SAFETY_ANALYSIS; + const VirtualMethodVisitor& virtual_method_visitor) const; template <typename DirectMethodVisitor, typename VirtualMethodVisitor> @@ -139,9 +249,11 @@ class ClassAccessor { void VisitFields(const StaticFieldVisitor& static_field_visitor, const InstanceFieldVisitor& instance_field_visitor) const; - // Visit direct and virtual methods. - template <typename MethodVisitor> - void VisitMethods(const MethodVisitor& method_visitor) const; + // Return the iteration range for all the fields. + IterationRange<DataIterator<Field>> GetFields() const; + + // Return the iteration range for all the methods. + IterationRange<DataIterator<Method>> GetMethods() const; uint32_t NumStaticFields() const { return num_static_fields_; @@ -170,6 +282,14 @@ class ClassAccessor { } protected: + // Template visitor to reduce copy paste for visiting elements. + // No thread safety analysis since the visitor may require capabilities. + template <typename DataType, typename Visitor> + const uint8_t* VisitMembers(size_t count, + const Visitor& visitor, + const uint8_t* ptr, + DataType* data) const NO_THREAD_SAFETY_ANALYSIS; + const DexFile& dex_file_; const dex::TypeIndex descriptor_index_ = {}; const uint8_t* ptr_pos_ = nullptr; // Pointer into stream of class_data_item. diff --git a/libdexfile/dex/class_accessor_test.cc b/libdexfile/dex/class_accessor_test.cc new file mode 100644 index 0000000000..95380d8140 --- /dev/null +++ b/libdexfile/dex/class_accessor_test.cc @@ -0,0 +1,79 @@ +/* + * Copyright (C) 2018 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. + */ + +#include "dex/class_accessor-inl.h" + +#include "base/common_art_test.h" + +namespace art { + +class ClassAccessorTest : public CommonArtTest {}; + +TEST_F(ClassAccessorTest, TestVisiting) { + std::vector<std::unique_ptr<const DexFile>> dex_files( + OpenDexFiles(GetLibCoreDexFileNames()[0].c_str())); + ASSERT_GT(dex_files.size(), 0u); + for (const std::unique_ptr<const DexFile>& dex_file : dex_files) { + uint32_t class_def_idx = 0u; + ASSERT_GT(dex_file->NumClassDefs(), 0u); + for (ClassAccessor accessor : dex_file->GetClasses()) { + const DexFile::ClassDef& class_def = dex_file->GetClassDef(class_def_idx); + EXPECT_EQ(accessor.GetDescriptor(), dex_file->StringByTypeIdx(class_def.class_idx_)); + ++class_def_idx; + // Check iterators against visitors. + auto methods = accessor.GetMethods(); + auto fields = accessor.GetFields(); + auto method_it = methods.begin(); + auto field_it = fields.begin(); + accessor.VisitFieldsAndMethods( + // Static fields. + [&](const ClassAccessor::Field& field) { + EXPECT_EQ(field.GetIndex(), field_it->GetIndex()); + EXPECT_EQ(field.GetAccessFlags(), field_it->GetAccessFlags()); + ++field_it; + }, + // Instance fields. + [&](const ClassAccessor::Field& field) { + EXPECT_EQ(field.GetIndex(), field_it->GetIndex()); + EXPECT_EQ(field.GetAccessFlags(), field_it->GetAccessFlags()); + ++field_it; + }, + // Direct methods. + [&](const ClassAccessor::Method& method) { + EXPECT_TRUE(method.IsStaticOrDirect()); + EXPECT_EQ(method.IsStaticOrDirect(), method_it->IsStaticOrDirect()); + EXPECT_EQ(method.GetIndex(), method_it->GetIndex()); + EXPECT_EQ(method.GetAccessFlags(), method_it->GetAccessFlags()); + EXPECT_EQ(method.GetCodeItem(), method_it->GetCodeItem()); + ++method_it; + }, + // Virtual methods. + [&](const ClassAccessor::Method& method) { + EXPECT_FALSE(method.IsStaticOrDirect()); + EXPECT_EQ(method.IsStaticOrDirect(), method_it->IsStaticOrDirect()); + EXPECT_EQ(method.GetIndex(), method_it->GetIndex()); + EXPECT_EQ(method.GetAccessFlags(), method_it->GetAccessFlags()); + EXPECT_EQ(method.GetCodeItem(), method_it->GetCodeItem()); + ++method_it; + }); + ASSERT_TRUE(field_it == fields.end()); + ASSERT_TRUE(method_it == methods.end()); + } + EXPECT_EQ(class_def_idx, dex_file->NumClassDefs()); + } +} + +} // namespace art diff --git a/profman/profman.cc b/profman/profman.cc index 661132d94f..096e5dc3bd 100644 --- a/profman/profman.cc +++ b/profman/profman.cc @@ -931,12 +931,12 @@ class ProfMan FINAL { std::vector<ProfileMethodInfo> methods; if (method_str == kClassAllMethods) { ClassAccessor accessor(*dex_file, *dex_file->FindClassDef(class_ref.TypeIndex())); - accessor.VisitMethods([&](const ClassAccessor::Method& method) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { if (method.GetCodeItemOffset() != 0) { // Add all of the methods that have code to the profile. methods.push_back(ProfileMethodInfo(method.GetReference())); } - }); + } } // TODO: Check return values? profile->AddMethods(methods, static_cast<ProfileCompilationInfo::MethodHotness::Flag>(flags)); diff --git a/tools/dexanalyze/dexanalyze_experiments.cc b/tools/dexanalyze/dexanalyze_experiments.cc index 82cb0d28ab..0f20a99f05 100644 --- a/tools/dexanalyze/dexanalyze_experiments.cc +++ b/tools/dexanalyze/dexanalyze_experiments.cc @@ -130,7 +130,7 @@ void CountDexIndices::ProcessDexFile(const DexFile& dex_file) { for (ClassAccessor accessor : dex_file.GetClasses()) { std::set<size_t> unique_method_ids; std::set<size_t> unique_string_ids; - accessor.VisitMethods([&](const ClassAccessor::Method& method) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { dex_code_bytes_ += method.GetInstructions().InsnsSizeInBytes(); unique_code_items.insert(method.GetCodeItemOffset()); for (const DexInstructionPcPair& inst : method.GetInstructions()) { @@ -188,7 +188,7 @@ void CountDexIndices::ProcessDexFile(const DexFile& dex_file) { break; } } - }); + } total_unique_method_idx_ += unique_method_ids.size(); total_unique_string_ids_ += unique_string_ids.size(); } |